Re: [PATCH 4.19 ONLY v4] arm64: vdso: remove commas between macro name and arguments

[Nathan Chancellor <nathan@kernel.org>]

On 5/5/2021 6:25 PM, Jian Cai wrote:
> LLVM's integrated assembler appears to assume an argument with default
> value is passed whenever it sees a comma right after the macro name.
> It will be fine if the number of following arguments is one less than
> the number of parameters specified in the macro definition. Otherwise,
> it fails. For example, the following code works:
> 
> $ cat foo.s
> .macro  foo arg1=2, arg2=4
>          ldr r0, [r1, #\arg1]
>          ldr r0, [r1, #\arg2]
> .endm
> 
> foo, arg2=8
> 
> $ llvm-mc -triple=armv7a -filetype=obj foo.s -o ias.o
> arm-linux-gnueabihf-objdump -dr ias.o
> 
> ias.o:     file format elf32-littlearm
> 
> Disassembly of section .text:
> 
> 00000000 <.text>:
>     0: e5910001 ldr r0, [r1, #2]
>     4: e5910003 ldr r0, [r1, #8]
> 
> While the the following code would fail:
> 
> $ cat foo.s
> .macro  foo arg1=2, arg2=4
>          ldr r0, [r1, #\arg1]
>          ldr r0, [r1, #\arg2]
> .endm
> 
> foo, arg1=2, arg2=8
> 
> $ llvm-mc -triple=armv7a -filetype=obj foo.s -o ias.o
> foo.s:6:14: error: too many positional arguments
> foo, arg1=2, arg2=8
> 
> This causes build failures as follows:
> 
> arch/arm64/kernel/vdso/gettimeofday.S:230:24: error: too many positional
> arguments
>   clock_gettime_return, shift=1
>                         ^
> arch/arm64/kernel/vdso/gettimeofday.S:253:24: error: too many positional
> arguments
>   clock_gettime_return, shift=1
>                         ^
> arch/arm64/kernel/vdso/gettimeofday.S:274:24: error: too many positional
> arguments
>   clock_gettime_return, shift=1
> 
> This error is not in mainline because commit 28b1a824a4f4 ("arm64: vdso:
> Substitute gettimeofday() with C implementation") rewrote this assembler
> file in C as part of a 25 patch series that is unsuitable for stable.
> Just remove the comma in the clock_gettime_return invocations in 4.19 so
> that GNU as and LLVM's integrated assembler work the same.
> 
> Link:
> https://github.com/ClangBuiltLinux/linux/issues/1349
> 
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Jian Cai <jiancai@google.com>

Thanks for the updated example and explanation, this looks good to me now.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> Changes v1 -> v2:
>    Keep the comma in the macro definition to be consistent with other
>    definitions.
> 
> Changes v2 -> v3:
>    Edit tags.
> 
> Changes v3 -> v4:
>    Update the commit message based on Nathan's comments.
> 
>   arch/arm64/kernel/vdso/gettimeofday.S | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S
> index 856fee6d3512..b6faf8b5d1fe 100644
> --- a/arch/arm64/kernel/vdso/gettimeofday.S
> +++ b/arch/arm64/kernel/vdso/gettimeofday.S
> @@ -227,7 +227,7 @@ realtime:
>   	seqcnt_check fail=realtime
>   	get_ts_realtime res_sec=x10, res_nsec=x11, \
>   		clock_nsec=x15, xtime_sec=x13, xtime_nsec=x14, nsec_to_sec=x9
> -	clock_gettime_return, shift=1
> +	clock_gettime_return shift=1
>   
>   	ALIGN
>   monotonic:
> @@ -250,7 +250,7 @@ monotonic:
>   		clock_nsec=x15, xtime_sec=x13, xtime_nsec=x14, nsec_to_sec=x9
>   
>   	add_ts sec=x10, nsec=x11, ts_sec=x3, ts_nsec=x4, nsec_to_sec=x9
> -	clock_gettime_return, shift=1
> +	clock_gettime_return shift=1
>   
>   	ALIGN
>   monotonic_raw:
> @@ -271,7 +271,7 @@ monotonic_raw:
>   		clock_nsec=x15, nsec_to_sec=x9
>   
>   	add_ts sec=x10, nsec=x11, ts_sec=x13, ts_nsec=x14, nsec_to_sec=x9
> -	clock_gettime_return, shift=1
> +	clock_gettime_return shift=1
>   
>   	ALIGN
>   realtime_coarse:
>