Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compilers: Do not pass -fuse-ld=lld via -Wl, #14012

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

lhmouse
Copy link
Contributor

@lhmouse lhmouse commented Dec 16, 2024

-fuse-ld= is a driver option for selection of a linker; it shall not be passed to a linker with -Wl,.

Previously, using Clang-CL and LLD-LINK, given:

cc = meson.get_compiler('c')
assert(cc.has_link_argument('/LTCG'))

This code failed to detect the /LTCG option, because -fuse-ld was passed to the linker, as an invalid option:

Command line: clang E:\lh_mouse\Desktop\t\build\meson-private\tmpg0221f ee\testfile.c -o E:\lh_mouse\Desktop\t\build\meson-private\tmpg0221fee\o utput.exe -D_FILE_OFFSET_BITS=64 -O0 -Werror=implicit-function-declarati on -Wl,-WX -Wl,/LTCG -Wl,-fuse-ld=lld -> 4044
stdout:
LINK : warning LNK4044: unrecognized option '/fuse-ld=lld'; ignored
LINK : error LNK1218: warning treated as error; no output file generated

@eli-schwartz
Copy link
Member

eli-schwartz commented Dec 16, 2024

I was initially trying to figure out why this was required at all. Then it hit me -- the linker_to_compiler_args function defines how to translate linker flags for microsoft platform linkers, which might expect to be passed to link.exe, such that e.g. clang when acting as the linker driver will forward those flags on to the real linker.

That's why we don't expect the flags to automatically come with -Wl, and correspondingly why we would add -Wl, ourselves. Which is therefore why we need to explicitly exclude -fuse-ld...

Can you mention that in the commit message?

P.S. When quoting "Command line: XXXX" etc. as snippets from a logfile, the 80-columns convention doesn't apply (actually, the convention itself says that this is an exception), just enclose it in triple-backticks and carry on. :)

@lhmouse
Copy link
Contributor Author

lhmouse commented Dec 16, 2024

I was initially trying to figure out why this was required at all. Then it hit me -- the linker_to_compiler_args function defines how to translate linker flags for microsoft platform linkers, which might expect to be passed to link.exe, such that e.g. clang when acting as the linker driver will forward those flags on to the real linker.

That's why we don't expect the flags to automatically come with -Wl, and correspondingly why we would add -Wl, ourselves. Which is therefore why we need to explicitly exclude -fuse-ld...

Can you mention that in the commit message?

Done now. A second paragraph has been added.

This commit doesn't solve all issues in this setup. For example, cc.has_link_argument('-subsystem:windows,6.1')) won't work because that option contains a comma and can't be passed with -Wl,. I would like to leave it for now.

P.S. When quoting "Command line: XXXX" etc. as snippets from a logfile, the 80-columns convention doesn't apply (actually, the convention itself says that this is an exception), just enclose it in triple-backticks and carry on. :)

Unwrapped now. I don't write commit message as Markdown, as I work with command-line Git which displays plaintext.

lhmouse added a commit to lhmouse/mcfgthread that referenced this pull request Dec 16, 2024
Clang-CL setups do not work with Meson 1.3 but they are already broken,
so let's wait.

Reference: mesonbuild/meson#13998
Reference: mesonbuild/meson#14012
`-fuse-ld=` is a driver option for selection of a linker; it shall not be
passed to a linker with `-Wl,`.

For the Microsoft compiler and linker, the options for the compiler and those
for the linker are separated by `/LINK`, which looks like `cl /cl-options ...
/link /link-options ...`. Formally, they are passed in the same command line.
When Clang is invoking the Microsoft linker or a Microsoft-style linker (that
is, LLD-LINK), every linker option has to prefixed by `-Wl,` or `-Xlink`.

Previously, using Clang-CL and LLD-LINK, given:

   cc = meson.get_compiler('c')
   assert(cc.has_link_argument('/LTCG'))

This code failed to detect the `/LTCG` option, because `-fuse-ld=lld` was
passed to the linker, as an invalid option:

   Command line: `clang E:\lh_mouse\Desktop\t\build\meson-private\tmpg0221fee\testfile.c -o E:\lh_mouse\Desktop\t\build\meson-private\tmpg0221fee\output.exe -D_FILE_OFFSET_BITS=64 -O0 -Werror=implicit-function-declaration -Wl,-WX -Wl,/LTCG -Wl,-fuse-ld=lld` -> 4044
   stdout:
   LINK : warning LNK4044: unrecognized option '/fuse-ld=lld'; ignored
   LINK : error LNK1218: warning treated as error; no output file generated

However, it should be noted that not all LINK options can be passed with
`-Wl,`. The `/subsystem:windows,6.1` option has a comma, which would be
converted to a space. Therefore, this option must be passed as
`-Xlinker -subsystem:windows,6.1`. This issue is not addressed in this commit.

Signed-off-by: LIU Hao <[email protected]>
@lhmouse
Copy link
Contributor Author

lhmouse commented Dec 18, 2024

Do those errors about macos-clang matter? I don't see these options in Meson source, and don't think these are caused by this commit:

[1/4] clang++ -ITestApplication.p -I. '-I../test cases/objcpp/3 objfw' -I/opt/homebrew/Cellar/objfw/1.2.3/include -I/opt/homebrew/include -fdiagnostics-color=always -Wall -Winvalid-pch -O0 -g -fexceptions -fobjc-exceptions -funwind-tables -fconstant-string-class=OFConstantString -Xclang -fno-constant-cfstrings -Xclang -fno-constant-nsnumber-literals -Xclang -fno-constant-nsarray-literals -Xclang -fno-constant-nsdictionary-literals -Xclang -fblocks -MD -MQ TestApplication.p/TestApplication.mm.o -MF TestApplication.p/TestApplication.mm.o.d -o TestApplication.p/TestApplication.mm.o -c '../test cases/objcpp/3 objfw/TestApplication.mm'
FAILED: TestApplication.p/TestApplication.mm.o 
clang++ -ITestApplication.p -I. '-I../test cases/objcpp/3 objfw' -I/opt/homebrew/Cellar/objfw/1.2.3/include -I/opt/homebrew/include -fdiagnostics-color=always -Wall -Winvalid-pch -O0 -g -fexceptions -fobjc-exceptions -funwind-tables -fconstant-string-class=OFConstantString -Xclang -fno-constant-cfstrings -Xclang -fno-constant-nsnumber-literals -Xclang -fno-constant-nsarray-literals -Xclang -fno-constant-nsdictionary-literals -Xclang -fblocks -MD -MQ TestApplication.p/TestApplication.mm.o -MF TestApplication.p/TestApplication.mm.o.d -o TestApplication.p/TestApplication.mm.o -c '../test cases/objcpp/3 objfw/TestApplication.mm'
error: unknown argument: '-fno-constant-nsnumber-literals'
error: unknown argument: '-fno-constant-nsarray-literals'
error: unknown argument: '-fno-constant-nsdictionary-literals'

@eli-schwartz
Copy link
Member

If you can't see any way for your PR to have produced the error, it's usually worth looking to see whether the same error occurs on git master -- in this case, they do, so it's a known problem. :)

See e.g. Homebrew/homebrew-core#201295 (comment)

@lhmouse
Copy link
Contributor Author

lhmouse commented Dec 18, 2024

If you can't see any way for your PR to have produced the error, it's usually worth looking to see whether the same error occurs on git master -- in this case, they do, so it's a known problem. :)

See e.g. Homebrew/homebrew-core#201295 (comment)

OK fair enough.

@eli-schwartz eli-schwartz merged commit 6eac015 into mesonbuild:master Dec 19, 2024
31 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants