From: Nathan Chancellor <nathan@kernel.org>
To: Jian Cai <jiancai@google.com>,
gregkh@linuxfoundation.org, sashal@kernel.org, will@kernel.org,
catalin.marinas@arm.com
Cc: stable@vger.kernel.org, ndesaulniers@google.com,
manojgupta@google.com, llozano@google.com,
clang-built-linux@googlegroups.com,
Will Deacon <will.deacon@arm.com>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4.19 ONLY v4] arm64: vdso: remove commas between macro name and arguments
Date: Thu, 6 May 2021 10:43:33 -0700 [thread overview]
Message-ID: <fd08dce2-71c0-3414-d661-d065480c04ff@kernel.org> (raw)
In-Reply-To: <20210506012508.3822221-1-jiancai@google.com>
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:
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-05-06 17:45 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-06 1:25 [PATCH 4.19 ONLY v4] arm64: vdso: remove commas between macro name and arguments Jian Cai
2021-05-06 17:43 ` Nathan Chancellor [this message]
2021-05-10 8:03 ` Greg KH
2021-05-10 8:28 ` Patch "arm64: vdso: remove commas between macro name and arguments" has been added to the 4.19-stable tree gregkh
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=fd08dce2-71c0-3414-d661-d065480c04ff@kernel.org \
--to=nathan@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=clang-built-linux@googlegroups.com \
--cc=gregkh@linuxfoundation.org \
--cc=jiancai@google.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=llozano@google.com \
--cc=manojgupta@google.com \
--cc=ndesaulniers@google.com \
--cc=sashal@kernel.org \
--cc=stable@vger.kernel.org \
--cc=will.deacon@arm.com \
--cc=will@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).