All of lore.kernel.org
 help / color / mirror / Atom feed
From: tixy@linaro.org (Jon Medhurst (Tixy))
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/3] [RESEND] ARM: kprobes: use "I" constraint for inline assembly offsets
Date: Thu, 18 Feb 2016 18:12:15 +0000	[thread overview]
Message-ID: <1455819135.2958.6.camel@linaro.org> (raw)
In-Reply-To: <1455815113-2562966-4-git-send-email-arnd@arndb.de>

On Thu, 2016-02-18 at 18:05 +0100, Arnd Bergmann wrote:
> build-testing with clang showed that the "J" constraint does not take
> positive arguments on clang when building in for Thumb-2:
> 
> core.c:540:3: error: invalid operand for inline asm constraint 'J'
> 
> This has been reported as llvm bug https://llvm.org/bugs/show_bug.cgi?id=26061
> 
> However, looking at the source code in depth, I found that the
> kernel is also wrong, and it should not use "J" at all, but should
> use "I" to pass an immediate argument to the inline assembly when that
> is used as an offset to an 'ldr' instruction rather than the 'sub'
> argument.

This patch doesn't seem correct to me.

The ARM ARM says the immediate offset to an ARM ldr instructions is "any
value in the range 0-4095" and offsets may be added or subtracted,
leading to values from ?4095 to 4095".

And GCC machine constraints [1] says

I
    Integer that is valid as an immediate operand in a data processing
    instruction. That is, an integer in the range 0 to 255 rotated by a
    multiple of 2
J
    Integer in the range ?4095 to 4095 

So the current use of 'J' seems correct to me.

[1] https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints


> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/probes/kprobes/core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index a4ec240ee7ba..4b34b40ca917 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -570,10 +570,10 @@ void __kprobes jprobe_return(void)
>  		:
>  		: "r" (kcb->jprobe_saved_regs.ARM_sp),
>  		  "I" (sizeof(struct pt_regs) * 2),
> -		  "J" (offsetof(struct pt_regs, ARM_sp)),
> -		  "J" (offsetof(struct pt_regs, ARM_pc)),
> -		  "J" (offsetof(struct pt_regs, ARM_cpsr)),
> -		  "J" (offsetof(struct pt_regs, ARM_lr))
> +		  "I" (offsetof(struct pt_regs, ARM_sp)),
> +		  "I" (offsetof(struct pt_regs, ARM_pc)),
> +		  "I" (offsetof(struct pt_regs, ARM_cpsr)),
> +		  "I" (offsetof(struct pt_regs, ARM_lr))
>  		: "memory", "cc");
>  }
>  

WARNING: multiple messages have this Message-ID (diff)
From: "Jon Medhurst (Tixy)" <tixy@linaro.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Russell King <linux@arm.linux.org.uk>,
	linux-arm-kernel@lists.infradead.org,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Nicolas Pitre <nico@linaro.org>,
	Marc Zyngier <marc.zyngier@arm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] [RESEND] ARM: kprobes: use "I" constraint for inline assembly offsets
Date: Thu, 18 Feb 2016 18:12:15 +0000	[thread overview]
Message-ID: <1455819135.2958.6.camel@linaro.org> (raw)
In-Reply-To: <1455815113-2562966-4-git-send-email-arnd@arndb.de>

On Thu, 2016-02-18 at 18:05 +0100, Arnd Bergmann wrote:
> build-testing with clang showed that the "J" constraint does not take
> positive arguments on clang when building in for Thumb-2:
> 
> core.c:540:3: error: invalid operand for inline asm constraint 'J'
> 
> This has been reported as llvm bug https://llvm.org/bugs/show_bug.cgi?id=26061
> 
> However, looking at the source code in depth, I found that the
> kernel is also wrong, and it should not use "J" at all, but should
> use "I" to pass an immediate argument to the inline assembly when that
> is used as an offset to an 'ldr' instruction rather than the 'sub'
> argument.

This patch doesn't seem correct to me.

The ARM ARM says the immediate offset to an ARM ldr instructions is "any
value in the range 0-4095" and offsets may be added or subtracted,
leading to values from −4095 to 4095".

And GCC machine constraints [1] says

I
    Integer that is valid as an immediate operand in a data processing
    instruction. That is, an integer in the range 0 to 255 rotated by a
    multiple of 2
J
    Integer in the range −4095 to 4095 

So the current use of 'J' seems correct to me.

[1] https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints


> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/probes/kprobes/core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index a4ec240ee7ba..4b34b40ca917 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -570,10 +570,10 @@ void __kprobes jprobe_return(void)
>  		:
>  		: "r" (kcb->jprobe_saved_regs.ARM_sp),
>  		  "I" (sizeof(struct pt_regs) * 2),
> -		  "J" (offsetof(struct pt_regs, ARM_sp)),
> -		  "J" (offsetof(struct pt_regs, ARM_pc)),
> -		  "J" (offsetof(struct pt_regs, ARM_cpsr)),
> -		  "J" (offsetof(struct pt_regs, ARM_lr))
> +		  "I" (offsetof(struct pt_regs, ARM_sp)),
> +		  "I" (offsetof(struct pt_regs, ARM_pc)),
> +		  "I" (offsetof(struct pt_regs, ARM_cpsr)),
> +		  "I" (offsetof(struct pt_regs, ARM_lr))
>  		: "memory", "cc");
>  }
>  

  reply	other threads:[~2016-02-18 18:12 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-18 17:05 [PATCH 0/3] ARM llvmlinux/clang build errors Arnd Bergmann
2016-02-18 17:05 ` [PATCH 1/3] [RESEND] ARM: pass -march=armv7-a when building NEON files with clang Arnd Bergmann
2016-02-18 17:05   ` Arnd Bergmann
2016-02-18 17:31   ` Nicolas Pitre
2016-02-18 17:31     ` Nicolas Pitre
2016-02-19 14:23     ` Arnd Bergmann
2016-02-19 14:23       ` Arnd Bergmann
2016-02-19 17:08       ` Nicolas Pitre
2016-02-19 17:08         ` Nicolas Pitre
2016-02-18 17:05 ` [PATCH 2/3] [RESEND] ARM: fix copypage-*.c building " Arnd Bergmann
2016-02-18 17:05   ` Arnd Bergmann
2016-02-18 17:34   ` Nicolas Pitre
2016-02-18 17:34     ` Nicolas Pitre
2016-02-18 17:05 ` [PATCH 3/3] [RESEND] ARM: kprobes: use "I" constraint for inline assembly offsets Arnd Bergmann
2016-02-18 17:05   ` Arnd Bergmann
2016-02-18 18:12   ` Jon Medhurst (Tixy) [this message]
2016-02-18 18:12     ` Jon Medhurst (Tixy)
2016-02-18 18:59     ` Robin Murphy
2016-02-18 18:59       ` Robin Murphy
2016-02-19  9:34       ` Jon Medhurst (Tixy)
2016-02-19  9:34         ` Jon Medhurst (Tixy)
2016-02-18 18:24   ` Nicolas Pitre
2016-02-18 18:24     ` Nicolas Pitre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1455819135.2958.6.camel@linaro.org \
    --to=tixy@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.