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

Windows Builds Fail when clang-cl Flags are added to CXXFLAGS #435

Open
Redfire75369 opened this issue Dec 6, 2023 · 8 comments · May be fixed by #437
Open

Windows Builds Fail when clang-cl Flags are added to CXXFLAGS #435

Redfire75369 opened this issue Dec 6, 2023 · 8 comments · May be fixed by #437

Comments

@Redfire75369
Copy link
Contributor

Redfire75369 commented Dec 6, 2023

The root issue here is that bindgen uses clang (not clang-cl) on windows, and on unrecognised flags, it fails. Upon adding CXXFLAGS="/clang:-flto=thin /clang:-fuse-ld=lld-link", the below code automatically adds it to bindgen's flags as well. As /clang: flags are only recognised by clang-cl, there is no way for cc and the makefile.cargo to receive the CXXFLAGS but not bindgen.

mozjs/mozjs-sys/build.rs

Lines 334 to 338 in c7fb1b8

if let Ok(flags) = env::var("CXXFLAGS") {
for flag in flags.split_whitespace() {
builder = builder.clang_arg(flag);
}
}

There are a few possible approaches:

  1. If a flag starts with /clang: on windows, do not add it to bindgen.
  2. Add a separate BINDGEN_CXXFLAGS variable for bindgen's flags.

As said earlier, the root issue is the usage of clang, which means this error may not be just from /clang: flags but almost any cl flag. Hence, I would suggest the latter approach.

For a concrete example of the failure, see https://github.com/Redfire75369/spiderfire/actions/runs/7112793006/job/19363443030#step:6:18053, where a libclang error is caused.

In the event it matters for updating whatever depends on it, #136 was the original PR where this behaviour was introduced.

@jschwe
Copy link
Member

jschwe commented May 29, 2024

Does setting the CLANG_PATH environment variable to clang-cl fix this issue?

@Redfire75369
Copy link
Contributor Author

I've just tested that, and that doesn't do anything. Neither bindgen, nor libclang have any handling for such an environment variable as far I have seen.

@jschwe
Copy link
Member

jschwe commented Jul 29, 2024

bindgen uses libclang-sys, which documents CLANG_PATH here: https://github.com/KyleMayes/clang-sys?tab=readme-ov-file#environment-variables

The usage area is here in bindgen, which calls this function in clang-sys.

But I guess this maybe only affects the clang executable which is used to detect the include paths, and perhaps does not affect the parsing of arguments, which might be done directly via libclang.

@Redfire75369
Copy link
Contributor Author

Redfire75369 commented Jul 29, 2024

Looking at the libclang source, it directly adds clang to the args.
https://github.com/llvm/llvm-project/blob/release/18.x/clang/tools/libclang/CIndex.cpp#L4097

There's also no codepath to switch it to clang-cl reasonably.

Stack is along the lines of:
bindgen::ir::context::BindgenContext::new -> bindgen::clang::TranslationUnit::parse -> clang_sys::clang_parseTranslationUnit (C++) -> clang_sys::clang_parseTranslationUnit2 (C++, adds the clang) -> clang_sys::clang_parseTranslationUnit2FullArgv (C++) -> clang_sys::clang_parseTranslationUnit_Impl (C++)

It would be possible to use clang_parseTranslationUnit2FullArgv directly, but I can't really think of an API that makes sense for that.

@mukilan
Copy link
Member

mukilan commented Jul 29, 2024

https://llvm.org/devmtg/2014-04/PDFs/Talks/clang-cl.pdf mentions clang-cl.exe == clang.exe --driver-mode=cl

Does adding --driver-mode=cl as a clang argument to the builder help?

@Redfire75369
Copy link
Contributor Author

Redfire75369 commented Jul 29, 2024

That seems to half-work at least. It at least needs /clang:-fuse-ld=lld, /link /SUBSYSTEM:WINDOWS, /TP (instead of -x c++), /FI (instead of -include). Haven't tested it in a full build, currently at this:

lld-link: error: undefined symbol: WinMain
>>> referenced by D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:102
>>>               libcmt.lib(exe_winmain.obj):(int __cdecl invoke_main(void))
>>> referenced by D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
>>>               libcmt.lib(exe_winmain.obj):(int __cdecl __scrt_common_main_seh(void))
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Command:

clang "./src/jsapi.hpp" "-I" "<omitted>/dist/include" "-I" "<omitted>/js/src" "/TP" -fms-compatibility --driver-mode=cl /clang:-flto=thin /clang:-fuse-ld=lld "-DRUST_BINDGEN" "-DSTATIC_JS_API" "-std:c++17" "-DWIN32" "-D_CRT_USE_BUILTIN_OFFSETOF" "/FI" "<omitted>/js/src/js-confdefs.h" /link /SUBSYSTEM:WINDOWS 

@Redfire75369
Copy link
Contributor Author

Redfire75369 commented Jul 29, 2024

The /link causes problems though as it's a catch-all, so no inputs are detected, causing stacker for example to fail to build.

Command "clang-cl" "-nologo" "-MD" "-O2" "-Brepro" "-m64" "--driver-mode=cl" "/clang:-flto=thin" "/clang:-fuse-ld=lld" "/link" "/SUBSYSTEM:WINDOWS" "-I" "src/arch" "-DWINDOWS" "-FoC:\\Users\\Redfire\\spiderfire\\target\\x86_64-pc-windows-msvc\\release\\build\\stacker-a0dccd7a5f938667\\out\\src/arch/windows.o" "-c" "--" "src/arch/windows.c" with args "clang-cl" did not execute successfully

@Redfire75369
Copy link
Contributor Author

Given that BINDGEN_EXTRA_CLANG_ARGS exists, it might be best to just stop adding CFLAGS and CXXFLAGS to bindgen.

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 a pull request may close this issue.

3 participants