-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
search path demotion for llvm headers when building self-hosted with custom llvm-dependency #12888
Comments
Note, that the cause of this specific issue can be isolated to zig code: set(ZIG_INSTALL_ARGS "build"
--zig-lib-dir "${CMAKE_SOURCE_DIR}/lib"
--prefix "${CMAKE_INSTALL_PREFIX}"
"-Dconfig_h=${ZIG_CONFIG_H_OUT}"
"-Denable-llvm"
"-Denable-stage1"
${ZIG_RELEASE_ARG}
${ZIG_STATIC_ARG}
${ZIG_SKIP_INSTALL_LIB_FILES_ARG}
${ZIG_SINGLE_THREADED_ARG}
"-Dtarget=${ZIG_TARGET_TRIPLE}"
"-Dcpu=${ZIG_TARGET_MCPU}"
"-Dversion-string=${ZIG_VERSION}"
)
add_custom_target(stage3 ALL
COMMAND zig2 ${ZIG_INSTALL_ARGS}
DEPENDS zig2
COMMENT STATUS "Building stage3"
WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}"
) Do you have any opinion on how one could eliminate this slippery slope from clang?
|
Response in chat by @mikdusan How include paths given by build.zig currently work
To prevent this (major) clang footgun we could:
other workaround: diff --git a/lib/std/build.zig b/lib/std/build.zig
index 2c141d540..50fcad9f8 100644
--- a/lib/std/build.zig
+++ b/lib/std/build.zig
@@ -3119,7 +3119,7 @@ pub const LibExeObjStep = struct {
try zig_args.append(builder.pathJoin(&.{
search_prefix, "lib",
}));
- try zig_args.append("-isystem");
+ try zig_args.append("-I");
try zig_args.append(builder.pathJoin(&.{
search_prefix, "include",
})); |
Hmm after thinking about this some more I think the behaviour may need to be more forgiving. Consider if you have a project that has all dependencies installed in separate dirs. Like so:
then there is generally never going to be conflict. But let's say a maintainer decides to fold all deps into one tree:
typical logic would be:
and if MODULE_A_ROOT == MODULE_B_ROOT, I think it would be too much friction to force people to check this.
|
Adding the patch above along along with a -DCMAKE_PREFIX_PATH=$HOME/local/llvm15 to the PKGBUILD for zig-git (Arch Linux) allows it to work with a private copy of LLVM. |
found this tidbit from gcc docs; which clang and the notes in this issue are consistent with:
|
build.zig: - use "-I" instead of "-isystem" for `b.addSearchPrefix()` main.zig: - silently ignore superfluous search dirs - warn when a dir is added to multiple searchlists - consolidate "expected paramter after {s}" fatal error messages closes ziglang#12888
build.zig: - use "-I" instead of "-isystem" for `b.addSearchPrefix()` main.zig: - silently ignore superfluous search dirs - warn when a dir is added to multiple searchlists - consolidate "expected paramter after {s}" fatal error messages - rename command-line switch `-dirafter` → `-idirafter` closes ziglang#12888
This is related to #11801 |
build.zig: - use "-I" instead of "-isystem" for `b.addSearchPrefix()` main.zig: - silently ignore superfluous search dirs - warn when a dir is added to multiple searchlists - consolidate "expected paramter after {s}" fatal error messages - rename command-line switch `-dirafter` → `-idirafter` closes #12888
Zig Version
0.10.0-dev.4110+dfcadd22b
Steps to Reproduce
/usr/include
) that are incompatible (eg. too old) to build zig against. This creates an interference with a custom llvm that you have built to build zig against.Expected Behavior
self-hosted build to succeed
Actual Behavior
self-hosted build fails because the wrong llvm header version is picked up and in this case zig's code is using a function that does not exist in
/usr/include
's version.To further drill down, after the failure, add
--verbose-cc
to the failedzig2 build-exe
command to get thezig2 clang
command where we add-v
to show header search path:The issue cause is
/opt/llvm-linux-x86_64-15.0.0-release/include
(my custom llvm location) comes after/usr/include
. The reasoning for this is a detail in clang command-line parsing. Here's what happens.-I /opt/llvm-linux-x86_64-15.0.0-release/include
. This is good.-isystem /usr/include
. This is good.-isystem /opt/llvm-linux-x86_64-15.0.0-release/include
. This bad as it interferes with previous-I
option. clang's beheaviour is to group-I
searches together, in order as first seen on command-line. It also groups-isystem
searches together as first seen on command line and in the end, the absolute search list first searches all-I
dirs, then it searches all-isystem
dirs. When clang gets a-isystem
dir that interferes with-I
it silently removes the dir from-I
and adds it to-isystem
, in effect demoting the search precedence of the dir.I think we've had this behaviour for quite some time, and it explains a lot of "fuzzy" build issue reports with self-hosted people have been having over the last(?) year when a different version of llvm is already installed in a standard OS location such as
/usr/include
.So what's the cause? This bit of code assumes search-prefix headers are
-isystem
. A simple fix may be to change this code to use-I
. The other annoyance is we shouldn't have llvm headers listed twice on the command line. AFAICT, duplicate search dirs, as long as they use the same command switch, are harmless:zig/lib/std/build.zig
Lines 3122 to 3125 in 2c3d87b
note: the workaround to this issue is to temporarily remove the interfering llvm package from system
The text was updated successfully, but these errors were encountered: