[PATCH] x86_64: Work around old gas bug

[PATCH] x86_64: Work around old gas bug

[Tao Guo]

gas in binutils(2.16.91) could not parse parentheses within macro
parameters unless fully parenthesized, and this is a workaround to
make old gas work without generating below errors:
arch/x86/kernel/entry_64.S: Assembler messages:
arch/x86/kernel/entry_64.S:387: Error: too many positional arguments
arch/x86/kernel/entry_64.S:389: Error: too many positional arguments
arch/x86/kernel/entry_64.S:390: Error: too many positional arguments
arch/x86/kernel/entry_64.S:391: Error: too many positional arguments
arch/x86/kernel/entry_64.S:392: Error: too many positional arguments
arch/x86/kernel/entry_64.S:393: Error: too many positional arguments
arch/x86/kernel/entry_64.S:394: Error: too many positional arguments

Signed-off-by: Tao Guo <glorioustao@gmail.com>
---
 arch/x86/include/asm/calling.h |   48 +++++++++++++++++++--------------------
 arch/x86/kernel/entry_64.S     |   20 ++++++++--------
 2 files changed, 33 insertions(+), 35 deletions(-)

diff --git a/arch/x86/include/asm/calling.h b/arch/x86/include/asm/calling.h
index a9e3a74..7f8422a 100644
--- a/arch/x86/include/asm/calling.h
+++ b/arch/x86/include/asm/calling.h
@@ -49,38 +49,36 @@ For 32-bit we have the following conventions - kernel is built with
 #include "dwarf2.h"
 
 /*
- * 64-bit system call stack frame layout defines and helpers, for
- * assembly code (note that the seemingly unnecessary parentheses
- * are to prevent cpp from inserting spaces in expressions that get
- * passed to macros):
+ * 64-bit system call stack frame layout defines and helpers,
+ * for assembly code:
  */
 
-#define R15		  (0)
-#define R14		  (8)
-#define R13		 (16)
-#define R12		 (24)
-#define RBP		 (32)
-#define RBX		 (40)
+#define R15		  0
+#define R14		  8
+#define R13		 16
+#define R12		 24
+#define RBP		 32
+#define RBX		 40
 
 /* arguments: interrupts/non tracing syscalls only save up to here: */
-#define R11		 (48)
-#define R10		 (56)
-#define R9		 (64)
-#define R8		 (72)
-#define RAX		 (80)
-#define RCX		 (88)
-#define RDX		 (96)
-#define RSI		(104)
-#define RDI		(112)
-#define ORIG_RAX	(120)       /* + error_code */
+#define R11		 48
+#define R10		 56
+#define R9		 64
+#define R8		 72
+#define RAX		 80
+#define RCX		 88
+#define RDX		 96
+#define RSI		104
+#define RDI		112
+#define ORIG_RAX	120       /* + error_code */
 /* end of arguments */
 
 /* cpu exception frame or undefined in case of fast syscall: */
-#define RIP		(128)
-#define CS		(136)
-#define EFLAGS		(144)
-#define RSP		(152)
-#define SS		(160)
+#define RIP		128
+#define CS		136
+#define EFLAGS		144
+#define RSP		152
+#define SS		160
 
 #define ARGOFFSET	R11
 #define SWFRAME		ORIG_RAX
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 69babd8..bf77e70 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -342,15 +342,15 @@ ENDPROC(native_usergs_sysret64)
 	.macro SAVE_ARGS_IRQ
 	cld
 	/* start from rbp in pt_regs and jump over */
-	movq_cfi rdi, RDI-RBP
-	movq_cfi rsi, RSI-RBP
-	movq_cfi rdx, RDX-RBP
-	movq_cfi rcx, RCX-RBP
-	movq_cfi rax, RAX-RBP
-	movq_cfi  r8,  R8-RBP
-	movq_cfi  r9,  R9-RBP
-	movq_cfi r10, R10-RBP
-	movq_cfi r11, R11-RBP
+	movq_cfi rdi, (RDI-RBP)
+	movq_cfi rsi, (RSI-RBP)
+	movq_cfi rdx, (RDX-RBP)
+	movq_cfi rcx, (RCX-RBP)
+	movq_cfi rax, (RAX-RBP)
+	movq_cfi  r8,  (R8-RBP)
+	movq_cfi  r9,  (R9-RBP)
+	movq_cfi r10, (R10-RBP)
+	movq_cfi r11, (R11-RBP)
 
 	/* Save rbp so that we can unwind from get_irq_regs() */
 	movq_cfi rbp, 0
@@ -384,7 +384,7 @@ ENDPROC(native_usergs_sysret64)
 	.endm
 
 ENTRY(save_rest)
-	PARTIAL_FRAME 1 REST_SKIP+8
+	PARTIAL_FRAME 1 (REST_SKIP+8)
 	movq 5*8+16(%rsp), %r11	/* save return address */
 	movq_cfi rbx, RBX+16
 	movq_cfi rbp, RBP+16
-- 
1.7.7.6

Re: [PATCH] x86_64: Work around old gas bug

[Jan Beulich]

>>> On 26.09.12 at 10:28, Tao Guo <glorioustao@gmail.com> wrote:
> gas in binutils(2.16.91) could not parse parentheses within macro
> parameters unless fully parenthesized, and this is a workaround to
> make old gas work without generating below errors:
> arch/x86/kernel/entry_64.S: Assembler messages:
> arch/x86/kernel/entry_64.S:387: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:389: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:390: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:391: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:392: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:393: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:394: Error: too many positional arguments
> 
> Signed-off-by: Tao Guo <glorioustao@gmail.com>

This looks okay now to me, but I'm somewhat reluctant to formally
ack it given that we know how broken those particular gas
versions are.

Jan

> ---
>  arch/x86/include/asm/calling.h |   48 +++++++++++++++++++--------------------
>  arch/x86/kernel/entry_64.S     |   20 ++++++++--------
>  2 files changed, 33 insertions(+), 35 deletions(-)
> 
> diff --git a/arch/x86/include/asm/calling.h b/arch/x86/include/asm/calling.h
> index a9e3a74..7f8422a 100644
> --- a/arch/x86/include/asm/calling.h
> +++ b/arch/x86/include/asm/calling.h
> @@ -49,38 +49,36 @@ For 32-bit we have the following conventions - kernel is 
> built with
>  #include "dwarf2.h"
>  
>  /*
> - * 64-bit system call stack frame layout defines and helpers, for
> - * assembly code (note that the seemingly unnecessary parentheses
> - * are to prevent cpp from inserting spaces in expressions that get
> - * passed to macros):
> + * 64-bit system call stack frame layout defines and helpers,
> + * for assembly code:
>   */
>  
> -#define R15		  (0)
> -#define R14		  (8)
> -#define R13		 (16)
> -#define R12		 (24)
> -#define RBP		 (32)
> -#define RBX		 (40)
> +#define R15		  0
> +#define R14		  8
> +#define R13		 16
> +#define R12		 24
> +#define RBP		 32
> +#define RBX		 40
>  
>  /* arguments: interrupts/non tracing syscalls only save up to here: */
> -#define R11		 (48)
> -#define R10		 (56)
> -#define R9		 (64)
> -#define R8		 (72)
> -#define RAX		 (80)
> -#define RCX		 (88)
> -#define RDX		 (96)
> -#define RSI		(104)
> -#define RDI		(112)
> -#define ORIG_RAX	(120)       /* + error_code */
> +#define R11		 48
> +#define R10		 56
> +#define R9		 64
> +#define R8		 72
> +#define RAX		 80
> +#define RCX		 88
> +#define RDX		 96
> +#define RSI		104
> +#define RDI		112
> +#define ORIG_RAX	120       /* + error_code */
>  /* end of arguments */
>  
>  /* cpu exception frame or undefined in case of fast syscall: */
> -#define RIP		(128)
> -#define CS		(136)
> -#define EFLAGS		(144)
> -#define RSP		(152)
> -#define SS		(160)
> +#define RIP		128
> +#define CS		136
> +#define EFLAGS		144
> +#define RSP		152
> +#define SS		160
>  
>  #define ARGOFFSET	R11
>  #define SWFRAME		ORIG_RAX
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index 69babd8..bf77e70 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -342,15 +342,15 @@ ENDPROC(native_usergs_sysret64)
>  	.macro SAVE_ARGS_IRQ
>  	cld
>  	/* start from rbp in pt_regs and jump over */
> -	movq_cfi rdi, RDI-RBP
> -	movq_cfi rsi, RSI-RBP
> -	movq_cfi rdx, RDX-RBP
> -	movq_cfi rcx, RCX-RBP
> -	movq_cfi rax, RAX-RBP
> -	movq_cfi  r8,  R8-RBP
> -	movq_cfi  r9,  R9-RBP
> -	movq_cfi r10, R10-RBP
> -	movq_cfi r11, R11-RBP
> +	movq_cfi rdi, (RDI-RBP)
> +	movq_cfi rsi, (RSI-RBP)
> +	movq_cfi rdx, (RDX-RBP)
> +	movq_cfi rcx, (RCX-RBP)
> +	movq_cfi rax, (RAX-RBP)
> +	movq_cfi  r8,  (R8-RBP)
> +	movq_cfi  r9,  (R9-RBP)
> +	movq_cfi r10, (R10-RBP)
> +	movq_cfi r11, (R11-RBP)
>  
>  	/* Save rbp so that we can unwind from get_irq_regs() */
>  	movq_cfi rbp, 0
> @@ -384,7 +384,7 @@ ENDPROC(native_usergs_sysret64)
>  	.endm
>  
>  ENTRY(save_rest)
> -	PARTIAL_FRAME 1 REST_SKIP+8
> +	PARTIAL_FRAME 1 (REST_SKIP+8)
>  	movq 5*8+16(%rsp), %r11	/* save return address */
>  	movq_cfi rbx, RBX+16
>  	movq_cfi rbp, RBP+16
> -- 
> 1.7.7.6

Re: [PATCH] x86_64: Work around old gas bug

[Ingo Molnar]

* Jan Beulich <JBeulich@suse.com> wrote:

> >>> On 26.09.12 at 10:28, Tao Guo <glorioustao@gmail.com> wrote:
> > gas in binutils(2.16.91) could not parse parentheses within macro
> > parameters unless fully parenthesized, and this is a workaround to
> > make old gas work without generating below errors:
> > arch/x86/kernel/entry_64.S: Assembler messages:
> > arch/x86/kernel/entry_64.S:387: Error: too many positional arguments
> > arch/x86/kernel/entry_64.S:389: Error: too many positional arguments
> > arch/x86/kernel/entry_64.S:390: Error: too many positional arguments
> > arch/x86/kernel/entry_64.S:391: Error: too many positional arguments
> > arch/x86/kernel/entry_64.S:392: Error: too many positional arguments
> > arch/x86/kernel/entry_64.S:393: Error: too many positional arguments
> > arch/x86/kernel/entry_64.S:394: Error: too many positional arguments
> > 
> > Signed-off-by: Tao Guo <glorioustao@gmail.com>
> 
> This looks okay now to me, but I'm somewhat reluctant to 
> formally ack it given that we know how broken those particular 
> gas versions are.

Well, assuming Tao Guo test-booted a kernel build with this old 
GAS version, we could apply this patch on a best-effort basis - 
the changes don't look particularly ugly. If other, unacceptable 
uglies or fragilities come up then we might balk.

Agreed?

Thanks,

	Ingo

Re: [PATCH] x86_64: Work around old gas bug

[Jan Beulich]

>>> On 26.09.12 at 13:16, Ingo Molnar <mingo@kernel.org> wrote:

> * Jan Beulich <JBeulich@suse.com> wrote:
> 
>> >>> On 26.09.12 at 10:28, Tao Guo <glorioustao@gmail.com> wrote:
>> > gas in binutils(2.16.91) could not parse parentheses within macro
>> > parameters unless fully parenthesized, and this is a workaround to
>> > make old gas work without generating below errors:
>> > arch/x86/kernel/entry_64.S: Assembler messages:
>> > arch/x86/kernel/entry_64.S:387: Error: too many positional arguments
>> > arch/x86/kernel/entry_64.S:389: Error: too many positional arguments
>> > arch/x86/kernel/entry_64.S:390: Error: too many positional arguments
>> > arch/x86/kernel/entry_64.S:391: Error: too many positional arguments
>> > arch/x86/kernel/entry_64.S:392: Error: too many positional arguments
>> > arch/x86/kernel/entry_64.S:393: Error: too many positional arguments
>> > arch/x86/kernel/entry_64.S:394: Error: too many positional arguments
>> > 
>> > Signed-off-by: Tao Guo <glorioustao@gmail.com>
>> 
>> This looks okay now to me, but I'm somewhat reluctant to 
>> formally ack it given that we know how broken those particular 
>> gas versions are.
> 
> Well, assuming Tao Guo test-booted a kernel build with this old 
> GAS version, we could apply this patch on a best-effort basis - 
> the changes don't look particularly ugly. If other, unacceptable 
> uglies or fragilities come up then we might balk.
> 
> Agreed?

Fine with me.

Jan

Re: [PATCH] x86_64: Work around old gas bug

[Greg KH]

On Wed, Sep 26, 2012 at 04:28:22AM -0400, Tao Guo wrote:
> gas in binutils(2.16.91) could not parse parentheses within macro
> parameters unless fully parenthesized, and this is a workaround to
> make old gas work without generating below errors:
> arch/x86/kernel/entry_64.S: Assembler messages:
> arch/x86/kernel/entry_64.S:387: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:389: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:390: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:391: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:392: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:393: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:394: Error: too many positional arguments
> 
> Signed-off-by: Tao Guo <glorioustao@gmail.com>
> ---
>  arch/x86/include/asm/calling.h |   48 +++++++++++++++++++--------------------
>  arch/x86/kernel/entry_64.S     |   20 ++++++++--------
>  2 files changed, 33 insertions(+), 35 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

Re: [PATCH] x86_64: Work around old gas bug

[Tao Guo]

On Wed, Sep 26, 2012 at 9:25 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Wed, Sep 26, 2012 at 04:28:22AM -0400, Tao Guo wrote:
>> gas in binutils(2.16.91) could not parse parentheses within macro
>> parameters unless fully parenthesized, and this is a workaround to
>> make old gas work without generating below errors:
>> arch/x86/kernel/entry_64.S: Assembler messages:
>> arch/x86/kernel/entry_64.S:387: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:389: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:390: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:391: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:392: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:393: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:394: Error: too many positional arguments
>>
>> Signed-off-by: Tao Guo <glorioustao@gmail.com>
>> ---
>>  arch/x86/include/asm/calling.h |   48 +++++++++++++++++++--------------------
>>  arch/x86/kernel/entry_64.S     |   20 ++++++++--------
>>  2 files changed, 33 insertions(+), 35 deletions(-)
>
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
> for how to do this properly.
>
> </formletter>

Thanks Greg, I just realized that I followed the old rules in an old
Documentation/stable_kernel_rules.txt file. Since the patches for
other stable branches will be a little different from this one, I will
send them later to stable@vger.kernel.org after this patch gets
accepted.

Thanks,
-Tao

Re: [PATCH] x86_64: Work around old gas bug

[Ingo Molnar]

* Tao Guo <glorioustao@gmail.com> wrote:

> On Wed, Sep 26, 2012 at 9:25 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Wed, Sep 26, 2012 at 04:28:22AM -0400, Tao Guo wrote:
> >> gas in binutils(2.16.91) could not parse parentheses within macro
> >> parameters unless fully parenthesized, and this is a workaround to
> >> make old gas work without generating below errors:
> >> arch/x86/kernel/entry_64.S: Assembler messages:
> >> arch/x86/kernel/entry_64.S:387: Error: too many positional arguments
> >> arch/x86/kernel/entry_64.S:389: Error: too many positional arguments
> >> arch/x86/kernel/entry_64.S:390: Error: too many positional arguments
> >> arch/x86/kernel/entry_64.S:391: Error: too many positional arguments
> >> arch/x86/kernel/entry_64.S:392: Error: too many positional arguments
> >> arch/x86/kernel/entry_64.S:393: Error: too many positional arguments
> >> arch/x86/kernel/entry_64.S:394: Error: too many positional arguments
> >>
> >> Signed-off-by: Tao Guo <glorioustao@gmail.com>
> >> ---
> >>  arch/x86/include/asm/calling.h |   48 +++++++++++++++++++--------------------
> >>  arch/x86/kernel/entry_64.S     |   20 ++++++++--------
> >>  2 files changed, 33 insertions(+), 35 deletions(-)
> >
> > <formletter>
> >
> > This is not the correct way to submit patches for inclusion in the
> > stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
> > for how to do this properly.
> >
> > </formletter>
> 
> Thanks Greg, I just realized that I followed the old rules in 
> an old Documentation/stable_kernel_rules.txt file. Since the 
> patches for other stable branches will be a little different 
> from this one, I will send them later to 
> stable@vger.kernel.org after this patch gets accepted.

Please don't send this to -stale yet (or at all).

I have queued up the patch for v3.7, not even for v3.6. It needs 
more testing - old GAS users are probably better off having a 
non-building kernel than a subtly broken one. v3.7 and onwards 
stable kernels will have the fix - hopefully by the time it's 
released any more subtle bugs are found and fixed.

Thanks,

	Ingo

[tip:x86/asm] x86_64: Work around old GAS bug

[tip-bot for Tao Guo]

Commit-ID:  1b2b23d8573076a587ed2081e0d2b69691079e0e
Gitweb:     http://git.kernel.org/tip/1b2b23d8573076a587ed2081e0d2b69691079e0e
Author:     Tao Guo <glorioustao@gmail.com>
AuthorDate: Wed, 26 Sep 2012 04:28:22 -0400
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 26 Sep 2012 13:35:32 +0200

x86_64: Work around old GAS bug

GAS in binutils(2.16.91) could not parse parentheses within
macro parameters unless fully parenthesized, and this is a
workaround to make old gas work without generating below errors:

 arch/x86/kernel/entry_64.S: Assembler messages:
 arch/x86/kernel/entry_64.S:387: Error: too many positional arguments
 arch/x86/kernel/entry_64.S:389: Error: too many positional arguments
 [...]

Signed-off-by: Tao Guo <glorioustao@gmail.com>
Reluctantly-Acked-by: Jan Beulich <jbeulich@novell.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/1348648102-12653-1-git-send-email-glorioustao@gmail.com
[ Jan argues that these old GAS versions are fragile - which is so, but lets give them a chance. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/calling.h |   48 +++++++++++++++++++--------------------
 arch/x86/kernel/entry_64.S     |   20 ++++++++--------
 2 files changed, 33 insertions(+), 35 deletions(-)

diff --git a/arch/x86/include/asm/calling.h b/arch/x86/include/asm/calling.h
index a9e3a74..7f8422a 100644
--- a/arch/x86/include/asm/calling.h
+++ b/arch/x86/include/asm/calling.h
@@ -49,38 +49,36 @@ For 32-bit we have the following conventions - kernel is built with
 #include "dwarf2.h"
 
 /*
- * 64-bit system call stack frame layout defines and helpers, for
- * assembly code (note that the seemingly unnecessary parentheses
- * are to prevent cpp from inserting spaces in expressions that get
- * passed to macros):
+ * 64-bit system call stack frame layout defines and helpers,
+ * for assembly code:
  */
 
-#define R15		  (0)
-#define R14		  (8)
-#define R13		 (16)
-#define R12		 (24)
-#define RBP		 (32)
-#define RBX		 (40)
+#define R15		  0
+#define R14		  8
+#define R13		 16
+#define R12		 24
+#define RBP		 32
+#define RBX		 40
 
 /* arguments: interrupts/non tracing syscalls only save up to here: */
-#define R11		 (48)
-#define R10		 (56)
-#define R9		 (64)
-#define R8		 (72)
-#define RAX		 (80)
-#define RCX		 (88)
-#define RDX		 (96)
-#define RSI		(104)
-#define RDI		(112)
-#define ORIG_RAX	(120)       /* + error_code */
+#define R11		 48
+#define R10		 56
+#define R9		 64
+#define R8		 72
+#define RAX		 80
+#define RCX		 88
+#define RDX		 96
+#define RSI		104
+#define RDI		112
+#define ORIG_RAX	120       /* + error_code */
 /* end of arguments */
 
 /* cpu exception frame or undefined in case of fast syscall: */
-#define RIP		(128)
-#define CS		(136)
-#define EFLAGS		(144)
-#define RSP		(152)
-#define SS		(160)
+#define RIP		128
+#define CS		136
+#define EFLAGS		144
+#define RSP		152
+#define SS		160
 
 #define ARGOFFSET	R11
 #define SWFRAME		ORIG_RAX
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index b1dac12..2c67061 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -342,15 +342,15 @@ ENDPROC(native_usergs_sysret64)
 	.macro SAVE_ARGS_IRQ
 	cld
 	/* start from rbp in pt_regs and jump over */
-	movq_cfi rdi, RDI-RBP
-	movq_cfi rsi, RSI-RBP
-	movq_cfi rdx, RDX-RBP
-	movq_cfi rcx, RCX-RBP
-	movq_cfi rax, RAX-RBP
-	movq_cfi  r8,  R8-RBP
-	movq_cfi  r9,  R9-RBP
-	movq_cfi r10, R10-RBP
-	movq_cfi r11, R11-RBP
+	movq_cfi rdi, (RDI-RBP)
+	movq_cfi rsi, (RSI-RBP)
+	movq_cfi rdx, (RDX-RBP)
+	movq_cfi rcx, (RCX-RBP)
+	movq_cfi rax, (RAX-RBP)
+	movq_cfi  r8,  (R8-RBP)
+	movq_cfi  r9,  (R9-RBP)
+	movq_cfi r10, (R10-RBP)
+	movq_cfi r11, (R11-RBP)
 
 	/* Save rbp so that we can unwind from get_irq_regs() */
 	movq_cfi rbp, 0
@@ -384,7 +384,7 @@ ENDPROC(native_usergs_sysret64)
 	.endm
 
 ENTRY(save_rest)
-	PARTIAL_FRAME 1 REST_SKIP+8
+	PARTIAL_FRAME 1 (REST_SKIP+8)
 	movq 5*8+16(%rsp), %r11	/* save return address */
 	movq_cfi rbx, RBX+16
 	movq_cfi rbp, RBP+16

Re: [PATCH] x86_64: Work around old gas bug

[Tao Guo]

On 9/26/12, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 26.09.12 at 08:15, Tao Guo <glorioustao@gmail.com> wrote:
>> gas in binutils(2.16.91) could not parse parentheses within macro
>> parameters,
>
> This description of yours contradicts the last hunk of the patch -
> iirc the requirement for macro parameters in those old gas
> versions is to be fully parenthesized if there's any blank inside
> (which could possibly not even be present in source code, but
> get inserted by the C preprocessor).
Yes, the description is a little misleading and I have corrected in
the new patch.
>
>> and this is a workaround to make old gas work without
>> generating below errors:
>> arch/x86/kernel/entry_64.S: Assembler messages:
>> arch/x86/kernel/entry_64.S:387: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:389: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:390: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:391: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:392: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:393: Error: too many positional arguments
>> arch/x86/kernel/entry_64.S:394: Error: too many positional arguments
>>
>> Signed-off-by: Tao Guo <glorioustao@gmail.com>
>> ---
>>  arch/x86/include/asm/calling.h |   52
>> +++++++++++++++++++--------------------
>>  arch/x86/kernel/entry_64.S     |   18 +++++++-------
>>  2 files changed, 34 insertions(+), 36 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/calling.h
>> b/arch/x86/include/asm/calling.h
>> index a9e3a74..7627e26 100644
>> --- a/arch/x86/include/asm/calling.h
>> +++ b/arch/x86/include/asm/calling.h
>> @@ -49,38 +49,36 @@ For 32-bit we have the following conventions - kernel
>> is
>> built with
>>  #include "dwarf2.h"
>>
>>  /*
>> - * 64-bit system call stack frame layout defines and helpers, for
>> - * assembly code (note that the seemingly unnecessary parentheses
>> - * are to prevent cpp from inserting spaces in expressions that get
>> - * passed to macros):
>> + * 64-bit system call stack frame layout defines and helpers,
>> + * for assembly code:
>>   */
>>
>> -#define R15		  (0)
>> -#define R14		  (8)
>> -#define R13		 (16)
>> -#define R12		 (24)
>> -#define RBP		 (32)
>> -#define RBX		 (40)
>> +#define R15		  0
>> +#define R14		  8
>> +#define R13		 16
>> +#define R12		 24
>> +#define RBP		 32
>> +#define RBX		 40
>>
>>  /* arguments: interrupts/non tracing syscalls only save up to here: */
>> -#define R11		 (48)
>> -#define R10		 (56)
>> -#define R9		 (64)
>> -#define R8		 (72)
>> -#define RAX		 (80)
>> -#define RCX		 (88)
>> -#define RDX		 (96)
>> -#define RSI		(104)
>> -#define RDI		(112)
>> -#define ORIG_RAX	(120)       /* + error_code */
>> +#define R11		 48
>> +#define R10		 56
>> +#define R9		 64
>> +#define R8		 72
>> +#define RAX		 80
>> +#define RCX		 88
>> +#define RDX		 96
>> +#define RSI		104
>> +#define RDI		112
>> +#define ORIG_RAX	120       /* + error_code */
>>  /* end of arguments */
>>
>>  /* cpu exception frame or undefined in case of fast syscall: */
>> -#define RIP		(128)
>> -#define CS		(136)
>> -#define EFLAGS		(144)
>> -#define RSP		(152)
>> -#define SS		(160)
>> +#define RIP		128
>> +#define CS		136
>> +#define EFLAGS		144
>> +#define RSP		152
>> +#define SS		160
>>
>>  #define ARGOFFSET	R11
>>  #define SWFRAME		ORIG_RAX
>
> While up the here the changes are certainly acceptable, ...
>
>> @@ -107,7 +105,7 @@ For 32-bit we have the following conventions - kernel
>> is
>> built with
>>
>>  	.endm
>>
>> -#define ARG_SKIP	(9*8)
>> +#define ARG_SKIP	9*8
>>
>>  	.macro RESTORE_ARGS rstor_rax=1, addskip=0, rstor_rcx=1, rstor_r11=1, \
>>  			    rstor_r8910=1, rstor_rdx=1
>> @@ -157,7 +155,7 @@ For 32-bit we have the following conventions - kernel
>> is
>> built with
>>  	.endif
>>  	.endm
>>
>> -#define REST_SKIP	(6*8)
>> +#define REST_SKIP	6*8
>>
>>  	.macro SAVE_REST
>>  	subq $REST_SKIP, %rsp
>
> ... these two ones aren't - you can't simply remove parentheses
> from expressions without causing (potentially future) problems,
> up to and including someone re-introducing the parentheses to
> address such problems. You ought to parenthesize the macro
> parameters where needed instead, as you did below.
Thanks for the suggestion and I will correct it in the new patch.
>
> But that's all leaving aside that in the past it was already
> suggested (by hpa iirc) to simply declare these 2.16-based
> assemblers non-suitable for building the kernel.
Yes, that's another way to solve such compiling problems; People can
at least get some instructions to solve their problems.

Thanks,
-Tao
>
> Jan
>
>> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
>> index 69babd8..784a5b6 100644
>> --- a/arch/x86/kernel/entry_64.S
>> +++ b/arch/x86/kernel/entry_64.S
>> @@ -342,15 +342,15 @@ ENDPROC(native_usergs_sysret64)
>>  	.macro SAVE_ARGS_IRQ
>>  	cld
>>  	/* start from rbp in pt_regs and jump over */
>> -	movq_cfi rdi, RDI-RBP
>> -	movq_cfi rsi, RSI-RBP
>> -	movq_cfi rdx, RDX-RBP
>> -	movq_cfi rcx, RCX-RBP
>> -	movq_cfi rax, RAX-RBP
>> -	movq_cfi  r8,  R8-RBP
>> -	movq_cfi  r9,  R9-RBP
>> -	movq_cfi r10, R10-RBP
>> -	movq_cfi r11, R11-RBP
>> +	movq_cfi rdi, (RDI-RBP)
>> +	movq_cfi rsi, (RSI-RBP)
>> +	movq_cfi rdx, (RDX-RBP)
>> +	movq_cfi rcx, (RCX-RBP)
>> +	movq_cfi rax, (RAX-RBP)
>> +	movq_cfi  r8,  (R8-RBP)
>> +	movq_cfi  r9,  (R9-RBP)
>> +	movq_cfi r10, (R10-RBP)
>> +	movq_cfi r11, (R11-RBP)
>>
>>  	/* Save rbp so that we can unwind from get_irq_regs() */
>>  	movq_cfi rbp, 0
>> --
>> 1.7.7
>
>
>

Re: [PATCH] x86_64: Work around old gas bug

[Jan Beulich]

>>> On 26.09.12 at 08:15, Tao Guo <glorioustao@gmail.com> wrote:
> gas in binutils(2.16.91) could not parse parentheses within macro
> parameters,

This description of yours contradicts the last hunk of the patch -
iirc the requirement for macro parameters in those old gas
versions is to be fully parenthesized if there's any blank inside
(which could possibly not even be present in source code, but
get inserted by the C preprocessor).

> and this is a workaround to make old gas work without
> generating below errors:
> arch/x86/kernel/entry_64.S: Assembler messages:
> arch/x86/kernel/entry_64.S:387: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:389: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:390: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:391: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:392: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:393: Error: too many positional arguments
> arch/x86/kernel/entry_64.S:394: Error: too many positional arguments
> 
> Signed-off-by: Tao Guo <glorioustao@gmail.com>
> ---
>  arch/x86/include/asm/calling.h |   52 +++++++++++++++++++--------------------
>  arch/x86/kernel/entry_64.S     |   18 +++++++-------
>  2 files changed, 34 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/x86/include/asm/calling.h b/arch/x86/include/asm/calling.h
> index a9e3a74..7627e26 100644
> --- a/arch/x86/include/asm/calling.h
> +++ b/arch/x86/include/asm/calling.h
> @@ -49,38 +49,36 @@ For 32-bit we have the following conventions - kernel is 
> built with
>  #include "dwarf2.h"
>  
>  /*
> - * 64-bit system call stack frame layout defines and helpers, for
> - * assembly code (note that the seemingly unnecessary parentheses
> - * are to prevent cpp from inserting spaces in expressions that get
> - * passed to macros):
> + * 64-bit system call stack frame layout defines and helpers,
> + * for assembly code:
>   */
>  
> -#define R15		  (0)
> -#define R14		  (8)
> -#define R13		 (16)
> -#define R12		 (24)
> -#define RBP		 (32)
> -#define RBX		 (40)
> +#define R15		  0
> +#define R14		  8
> +#define R13		 16
> +#define R12		 24
> +#define RBP		 32
> +#define RBX		 40
>  
>  /* arguments: interrupts/non tracing syscalls only save up to here: */
> -#define R11		 (48)
> -#define R10		 (56)
> -#define R9		 (64)
> -#define R8		 (72)
> -#define RAX		 (80)
> -#define RCX		 (88)
> -#define RDX		 (96)
> -#define RSI		(104)
> -#define RDI		(112)
> -#define ORIG_RAX	(120)       /* + error_code */
> +#define R11		 48
> +#define R10		 56
> +#define R9		 64
> +#define R8		 72
> +#define RAX		 80
> +#define RCX		 88
> +#define RDX		 96
> +#define RSI		104
> +#define RDI		112
> +#define ORIG_RAX	120       /* + error_code */
>  /* end of arguments */
>  
>  /* cpu exception frame or undefined in case of fast syscall: */
> -#define RIP		(128)
> -#define CS		(136)
> -#define EFLAGS		(144)
> -#define RSP		(152)
> -#define SS		(160)
> +#define RIP		128
> +#define CS		136
> +#define EFLAGS		144
> +#define RSP		152
> +#define SS		160
>  
>  #define ARGOFFSET	R11
>  #define SWFRAME		ORIG_RAX

While up the here the changes are certainly acceptable, ...

> @@ -107,7 +105,7 @@ For 32-bit we have the following conventions - kernel is 
> built with
>  
>  	.endm
>  
> -#define ARG_SKIP	(9*8)
> +#define ARG_SKIP	9*8
>  
>  	.macro RESTORE_ARGS rstor_rax=1, addskip=0, rstor_rcx=1, rstor_r11=1, \
>  			    rstor_r8910=1, rstor_rdx=1
> @@ -157,7 +155,7 @@ For 32-bit we have the following conventions - kernel is 
> built with
>  	.endif
>  	.endm
>  
> -#define REST_SKIP	(6*8)
> +#define REST_SKIP	6*8
>  
>  	.macro SAVE_REST
>  	subq $REST_SKIP, %rsp

... these two ones aren't - you can't simply remove parentheses
from expressions without causing (potentially future) problems,
up to and including someone re-introducing the parentheses to
address such problems. You ought to parenthesize the macro
parameters where needed instead, as you did below.

But that's all leaving aside that in the past it was already
suggested (by hpa iirc) to simply declare these 2.16-based
assemblers non-suitable for building the kernel.

Jan

> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index 69babd8..784a5b6 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -342,15 +342,15 @@ ENDPROC(native_usergs_sysret64)
>  	.macro SAVE_ARGS_IRQ
>  	cld
>  	/* start from rbp in pt_regs and jump over */
> -	movq_cfi rdi, RDI-RBP
> -	movq_cfi rsi, RSI-RBP
> -	movq_cfi rdx, RDX-RBP
> -	movq_cfi rcx, RCX-RBP
> -	movq_cfi rax, RAX-RBP
> -	movq_cfi  r8,  R8-RBP
> -	movq_cfi  r9,  R9-RBP
> -	movq_cfi r10, R10-RBP
> -	movq_cfi r11, R11-RBP
> +	movq_cfi rdi, (RDI-RBP)
> +	movq_cfi rsi, (RSI-RBP)
> +	movq_cfi rdx, (RDX-RBP)
> +	movq_cfi rcx, (RCX-RBP)
> +	movq_cfi rax, (RAX-RBP)
> +	movq_cfi  r8,  (R8-RBP)
> +	movq_cfi  r9,  (R9-RBP)
> +	movq_cfi r10, (R10-RBP)
> +	movq_cfi r11, (R11-RBP)
>  
>  	/* Save rbp so that we can unwind from get_irq_regs() */
>  	movq_cfi rbp, 0
> -- 
> 1.7.7

[PATCH] x86_64: Work around old gas bug

[Tao Guo]

gas in binutils(2.16.91) could not parse parentheses within macro
parameters, and this is a workaround to make old gas work without
generating below errors:
arch/x86/kernel/entry_64.S: Assembler messages:
arch/x86/kernel/entry_64.S:387: Error: too many positional arguments
arch/x86/kernel/entry_64.S:389: Error: too many positional arguments
arch/x86/kernel/entry_64.S:390: Error: too many positional arguments
arch/x86/kernel/entry_64.S:391: Error: too many positional arguments
arch/x86/kernel/entry_64.S:392: Error: too many positional arguments
arch/x86/kernel/entry_64.S:393: Error: too many positional arguments
arch/x86/kernel/entry_64.S:394: Error: too many positional arguments

Signed-off-by: Tao Guo <glorioustao@gmail.com>
---
 arch/x86/include/asm/calling.h |   52 +++++++++++++++++++--------------------
 arch/x86/kernel/entry_64.S     |   18 +++++++-------
 2 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/arch/x86/include/asm/calling.h b/arch/x86/include/asm/calling.h
index a9e3a74..7627e26 100644
--- a/arch/x86/include/asm/calling.h
+++ b/arch/x86/include/asm/calling.h
@@ -49,38 +49,36 @@ For 32-bit we have the following conventions - kernel is built with
 #include "dwarf2.h"
 
 /*
- * 64-bit system call stack frame layout defines and helpers, for
- * assembly code (note that the seemingly unnecessary parentheses
- * are to prevent cpp from inserting spaces in expressions that get
- * passed to macros):
+ * 64-bit system call stack frame layout defines and helpers,
+ * for assembly code:
  */
 
-#define R15		  (0)
-#define R14		  (8)
-#define R13		 (16)
-#define R12		 (24)
-#define RBP		 (32)
-#define RBX		 (40)
+#define R15		  0
+#define R14		  8
+#define R13		 16
+#define R12		 24
+#define RBP		 32
+#define RBX		 40
 
 /* arguments: interrupts/non tracing syscalls only save up to here: */
-#define R11		 (48)
-#define R10		 (56)
-#define R9		 (64)
-#define R8		 (72)
-#define RAX		 (80)
-#define RCX		 (88)
-#define RDX		 (96)
-#define RSI		(104)
-#define RDI		(112)
-#define ORIG_RAX	(120)       /* + error_code */
+#define R11		 48
+#define R10		 56
+#define R9		 64
+#define R8		 72
+#define RAX		 80
+#define RCX		 88
+#define RDX		 96
+#define RSI		104
+#define RDI		112
+#define ORIG_RAX	120       /* + error_code */
 /* end of arguments */
 
 /* cpu exception frame or undefined in case of fast syscall: */
-#define RIP		(128)
-#define CS		(136)
-#define EFLAGS		(144)
-#define RSP		(152)
-#define SS		(160)
+#define RIP		128
+#define CS		136
+#define EFLAGS		144
+#define RSP		152
+#define SS		160
 
 #define ARGOFFSET	R11
 #define SWFRAME		ORIG_RAX
@@ -107,7 +105,7 @@ For 32-bit we have the following conventions - kernel is built with
 
 	.endm
 
-#define ARG_SKIP	(9*8)
+#define ARG_SKIP	9*8
 
 	.macro RESTORE_ARGS rstor_rax=1, addskip=0, rstor_rcx=1, rstor_r11=1, \
 			    rstor_r8910=1, rstor_rdx=1
@@ -157,7 +155,7 @@ For 32-bit we have the following conventions - kernel is built with
 	.endif
 	.endm
 
-#define REST_SKIP	(6*8)
+#define REST_SKIP	6*8
 
 	.macro SAVE_REST
 	subq $REST_SKIP, %rsp
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 69babd8..784a5b6 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -342,15 +342,15 @@ ENDPROC(native_usergs_sysret64)
 	.macro SAVE_ARGS_IRQ
 	cld
 	/* start from rbp in pt_regs and jump over */
-	movq_cfi rdi, RDI-RBP
-	movq_cfi rsi, RSI-RBP
-	movq_cfi rdx, RDX-RBP
-	movq_cfi rcx, RCX-RBP
-	movq_cfi rax, RAX-RBP
-	movq_cfi  r8,  R8-RBP
-	movq_cfi  r9,  R9-RBP
-	movq_cfi r10, R10-RBP
-	movq_cfi r11, R11-RBP
+	movq_cfi rdi, (RDI-RBP)
+	movq_cfi rsi, (RSI-RBP)
+	movq_cfi rdx, (RDX-RBP)
+	movq_cfi rcx, (RCX-RBP)
+	movq_cfi rax, (RAX-RBP)
+	movq_cfi  r8,  (R8-RBP)
+	movq_cfi  r9,  (R9-RBP)
+	movq_cfi r10, (R10-RBP)
+	movq_cfi r11, (R11-RBP)
 
 	/* Save rbp so that we can unwind from get_irq_regs() */
 	movq_cfi rbp, 0
-- 
1.7.7