Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for __arch_hweight*()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 17, 2015 at 11:47:20AM -0500, Josh Poimboeuf wrote:
> If __arch_hweight32() or __arch_hweight64() is inlined at the beginning
> of a function, gcc can insert the call instruction before setting up a
> stack frame, which breaks frame pointer convention if
> CONFIG_FRAME_POINTER is enabled and can result in a bad stack trace.
> 
> Force a stack frame to be created if CONFIG_FRAME_POINTER is enabled by
> listing the stack pointer as an output operand for the inline asm
> statement.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> ---
>  arch/x86/include/asm/arch_hweight.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h
> index 9686c3d..e438a0d 100644
> --- a/arch/x86/include/asm/arch_hweight.h
> +++ b/arch/x86/include/asm/arch_hweight.h
> @@ -23,10 +23,11 @@
>   */
>  static inline unsigned int __arch_hweight32(unsigned int w)
>  {
> +	register void *__sp asm("esp");
>  	unsigned int res = 0;
>  
>  	asm (ALTERNATIVE("call __sw_hweight32", POPCNT32, X86_FEATURE_POPCNT)
> -		     : "="REG_OUT (res)
> +		     : "="REG_OUT (res), "+r" (__sp)
>  		     : REG_IN (w));
>  
>  	return res;
> @@ -44,6 +45,7 @@ static inline unsigned int __arch_hweight8(unsigned int w)
>  
>  static inline unsigned long __arch_hweight64(__u64 w)
>  {
> +	register void __maybe_unused *__sp asm("rsp");
>  	unsigned long res = 0;
>  
>  #ifdef CONFIG_X86_32
> @@ -51,7 +53,7 @@ static inline unsigned long __arch_hweight64(__u64 w)
>  		__arch_hweight32((u32)(w >> 32));
>  #else
>  	asm (ALTERNATIVE("call __sw_hweight64", POPCNT64, X86_FEATURE_POPCNT)
> -		     : "="REG_OUT (res)
> +		     : "="REG_OUT (res), "+r" (__sp)
>  		     : REG_IN (w));
>  #endif /* CONFIG_X86_32 */

Eeew, useless code so that some compile-time validation is done. Let's
not add this clutter please.

In this particular case, the majority of CPUs out there will get POPCNT
patched in and that CALL is gone. And for the remaining cases where we
do end up using the __sw_* variants, I'd prefer to rather not do the
validation instead of polluting the code with that fake rsp dependency.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux