Re: [PATCH] kbuild: Update ld-version.sh for change in LLD version output

On Sat, Jul 6, 2024 at 1: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.

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.

Thanks. V2 seems much cleaner.