linux-kernel - [PATCH] MIPS: Add option to pass return address location to _mcount. Was: [PATCH -v4 4/9] tracing: add static function tracer support for MIPS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 23 Oct 2009 15:48:13 -0700
From:	David Daney <ddaney@...iumnetworks.com>
To:	rdsandiford@...glemail.com, GCC Patches <gcc-patches@....gnu.org>
CC:	wuzhangjin@...il.com, Adam Nemet <anemet@...iumnetworks.com>,
	rostedt@...dmis.org, linux-kernel@...r.kernel.org,
	linux-mips@...ux-mips.org, Thomas Gleixner <tglx@...utronix.de>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Nicholas Mc Guire <der.herr@...r.at>
Subject: [PATCH] MIPS: Add option to pass return address location to _mcount.
  Was: [PATCH -v4 4/9] tracing: add static function tracer support for MIPS

Richard Sandiford wrote:
> David Daney <ddaney@...iumnetworks.com> writes:
>> Wu Zhangjin wrote:
>>> On Wed, 2009-10-21 at 11:24 -0400, Steven Rostedt wrote:
>> [...]
>>>>> +
>>>>> +NESTED(_mcount, PT_SIZE, ra)
>>>>> +	RESTORE_SP_FOR_32BIT
>>>>> +	PTR_LA	t0, ftrace_stub
>>>>> +	PTR_L	t1, ftrace_trace_function /* please don't use t1 later, safe? */
>>>> Is t0 and t1 safe for mcount to use? Remember, mcount does not follow
>>>> the dynamics of C function ABI.
>>> So, perhaps we can use the saved registers(a0,a1...) instead.
>>>
>> a0..a7 may not always be saved.
>>
>> You can use at, v0, v1 and all the temporary registers.  Note that for 
>> the 64-bit ABIs sometimes the names t0-t4 shadow a4-a7.  So for a 64-bit 
>> kernel, you can use: $1, $2, $3, $12, $13, $14, $15, $24, $25, noting 
>> that at == $1 and contains the callers ra.  For a 32-bit kernel you can 
>> add $8, $9, $10, and $11
>>
>> This whole thing seems a little fragile.
>>
>> I think it might be a good idea to get input from Richard Sandiford, 
>> and/or Adam Nemet about this approach (so I add them to the CC).
>>
>> This e-mail thread starts here:
>>
>> http://www.linux-mips.org/archives/linux-mips/2009-10/msg00286.html
>>
>> and here:
>>
>> http://www.linux-mips.org/archives/linux-mips/2009-10/msg00290.html
> 
> I'm not sure that the "search for a save of RA" thing is really a good idea.
> The last version of that seemed to be "assume that any register stores
> will be in a block that immediately precedes the move into RA", but even
> if that's true now, it might not be in future.  And as Wu Zhangjin says,
> it doesn't cope with long calls, where the target address is loaded
> into a temporary register before the call.
> 
> FWIW, I'd certainly be happy to make GCC pass an additional parameter
> to _mcount.  The parameter could give the address of the return slot,
> or null for leaf functions.  In almost all cases[*], there would be
> no overhead, since the move would go in the delay slot of the call.
> 
> [*] Meaning when the frame is <=32k. ;)  I'm guessing you never
>     get anywhere near that, and if you did, the scan thing wouldn't
>     work anyway.
> 
> The new behaviour could be controlled by a command-line option,
> which would also give linux a cheap way of checking whether the
> feature is available.
> 

How about this patch, I think it does what you suggest.

When we pass -pg -mmcount-raloc, the address of the return address 
relative to sp is passed in $12 to _mcount.  If the return address is 
not saved, $12 will be zero.  I think this will work as registers are 
never saved with an offset of zero.  $12 is a temporary register that is 
not part of the ABI.

$12 is also used by libffi closure support, but I think its use there 
will not interfere with _mcount.

It is very lightly tested, I would bootstrap and regression test with 
some new test cases if it were deemed acceptable.

2009-10-23  David Daney  <ddaney@...iumnetworks.com>

	* doc/invoke.texi (mmcount-raloc): Document new command line option.
	* config/mips/mips.opt (config/mips/mips.opt): New option.
	* config/mips/mips-protos.h (mips_function_profiler): Declare new
	function.
	* config/mips/mips.c (struct mips_frame_info): Add ra_fp_offset member.
	(mips_for_each_saved_gpr_and_fpr): Set ra_fp_offset.
	(mips_raloc_in_delay_slot_p): New function.
	(mips_function_profiler): Moved from FUNCTION_PROFILER, and rewritten.
	* config/mips/mips.h (FUNCTION_PROFILER): Body of macro moved to
	mips_function_profiler.


View attachment "mcount.patch" of type "text/plain" (8423 bytes)