On Fri, Jul 5, 2024 at 9:00 AM Nathan Chancellor nathan@kernel.org wrote:
On Thu, Jul 04, 2024 at 02:23:46PM -0700, Fangrui Song wrote:
On Thu, Jul 4, 2024 at 9:19 AM Nathan Chancellor nathan@kernel.org wrote:
After [1] in upstream LLVM, ld.lld's version output is slightly different when the cmake configuration option LLVM_APPEND_VC_REV is disabled.
Before:
Debian LLD 19.0.0 (compatible with GNU linkers)
After:
Debian LLD 19.0.0, compatible with GNU linkers
This results in ld-version.sh failing with
scripts/ld-version.sh: 19: arithmetic expression: expecting EOF: "10000 * 19 + 100 * 0 + 0,"
because the trailing comma is included in the patch level part of the expression. Remove the trailing comma when assigning the version variable in the LLD block to resolve the error, resulting in the proper output:
LLD 190000
With LLVM_APPEND_VC_REV enabled, there is no issue with the new output because it is treated the same as the prior LLVM_APPEND_VC_REV=OFF version string was.
ClangBuiltLinux LLD 19.0.0 (https://github.com/llvm/llvm-project a3c5c83273358a85a4e02f5f76379b1a276e7714), compatible with GNU linkers
Cc: stable@vger.kernel.org Fixes: 02aff8592204 ("kbuild: check the minimum linker version in Kconfig") Link: https://github.com/llvm/llvm-project/commit/0f9fbbb63cfcd2069441aa2ebef622c9... [1] Signed-off-by: Nathan Chancellor nathan@kernel.org
scripts/ld-version.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh index a78b804b680c..f2f425322524 100755 --- a/scripts/ld-version.sh +++ b/scripts/ld-version.sh @@ -47,7 +47,9 @@ else done
if [ "$1" = LLD ]; then
version=$2
# LLD after https://github.com/llvm/llvm-project/commit/0f9fbbb63cfcd2069441aa2ebef622c9716f8dbb# may have a trailing comma on the patch version with LLVM_APPEND_VC_REV=off.version=${2%,} min_version=$($min_tool_version llvm) name=LLD disp_name=LLD
base-commit: 22a40d14b572deb80c0648557f4bd502d7e83826 change-id: 20240704-update-ld-version-for-new-lld-ver-str-b7a4afbbd5f1
Best regards,
Nathan Chancellor nathan@kernel.org
Thanks for catching the issue. If we want to minimize the number of special cases, perhaps we can adjust `version=${version%-*}` below to
version=${version%%[^0-9.]*}
Thanks for the suggestion! I think this wants to be
version=${version%%[!0-9.]*}
because of "If an open bracket introduces a bracket expression as in XBD RE Bracket Expression, except that the <exclamation-mark> character ('!') shall replace the <circumflex> character ('^') in its role in a non-matching list in the regular expression notation, it shall introduce a pattern bracket expression." from the link that you have below.
Yes! ! should be used instead.
That does work for me with all the different linker versions that I can easily access (Arch, Debian, Fedora) along with my own self built toolchains, so it seems like it should be pretty robust.
Masahiro, would you be okay with me sending a v2 with that change or do you foresee any issues where it would not be sufficient? I would probably change the comment to:
# There may be something after the version, such as a distribution's # package release number (2.34-4.fc32) or a comma (like LLD adds # before the "compatible with GNU linkers" string), so remove anything # that is not a number or a period.
The v2 change LGTM :)
Reviewed-by: Fangrui Song maskray@google.com
(POSIX shell doc: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#:~...)
${version%%[^0-9.]*} is a simpler form than what glibc uses:
"LLD"*) # Accept LLD 13.0.0 or higher AC_CHECK_PROG_VER(LD, $LD, --version, [LLD.* ([0-9][0-9]*.[0-9.]*)], [1[3-9].*|[2-9][0-9].*],
Cheers, Nathan