Skip to content

Commit 77a7a4d

Browse files
fix(cc): move user provided -isystem paths to front of command
Problem: Currently if you pass a `-isystem` argument to `zig cc`, the include path will be searched after all of the libc, libcpp, and native system paths. However if you run the same command against `clang` and `gcc` you'll see that any user provided `-isystem` paths are at the very front. Solution: Push all user provided arguments to the arg list at the start of `addCCArgs()`. This makes it so that any compiler arguments that rely on the order of arguments passed will prioritize the user provided values first. Fixes: #24243
1 parent c9ce1de commit 77a7a4d

File tree

1 file changed

+169
-2
lines changed

1 file changed

+169
-2
lines changed

src/Compilation.zig

Lines changed: 169 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6317,6 +6317,30 @@ pub fn addCCArgs(
63176317
) !void {
63186318
const target = &mod.resolved_target.result;
63196319

6320+
var user_isystem_paths: std.ArrayList([]const u8) = .init(arena);
6321+
var other_cc_argv: std.ArrayList([]const u8) = .init(arena);
6322+
6323+
var isystem_path_next = false;
6324+
for ([_][]const []const u8{ comp.global_cc_argv, mod.cc_argv }) |cc_argv| {
6325+
for (cc_argv) |arg| {
6326+
if (isystem_path_next) {
6327+
try user_isystem_paths.append(arg);
6328+
isystem_path_next = false;
6329+
continue;
6330+
}
6331+
const isystem_flag = "-isystem";
6332+
if (mem.eql(u8, arg, isystem_flag)) {
6333+
isystem_path_next = true;
6334+
continue;
6335+
}
6336+
if (mem.startsWith(u8, arg, isystem_flag)) {
6337+
try user_isystem_paths.append(arg[isystem_flag.len..]);
6338+
continue;
6339+
}
6340+
try other_cc_argv.append(arg);
6341+
}
6342+
}
6343+
63206344
// As of Clang 16.x, it will by default read extra flags from /etc/clang.
63216345
// I'm sure the person who implemented this means well, but they have a lot
63226346
// to learn about abstractions and where the appropriate boundaries between
@@ -6531,6 +6555,12 @@ pub fn addCCArgs(
65316555
}
65326556
}
65336557

6558+
// User provided isystem paths come first in clang.
6559+
for (user_isystem_paths.items) |path| {
6560+
try argv.append("-isystem");
6561+
try argv.append(path);
6562+
}
6563+
65346564
if (comp.config.link_libcpp) {
65356565
try argv.append("-isystem");
65366566
try argv.append(try fs.path.join(arena, &[_][]const u8{
@@ -6811,8 +6841,145 @@ pub fn addCCArgs(
68116841
else => {},
68126842
}
68136843

6814-
try argv.appendSlice(comp.global_cc_argv);
6815-
try argv.appendSlice(mod.cc_argv);
6844+
try argv.appendSlice(other_cc_argv.items);
6845+
}
6846+
6847+
test "addCCArgs() puts user provided `-isystem` paths before native paths" {
6848+
const expected_isys_path_order: []const []const u8 = &.{ "/user_provided/isystem/dir/", "/module/isystem/dir1/", "/module/isystem/dir2/", "/native/isystem/dir/" };
6849+
var expected_isys_path_queue: std.fifo.LinearFifo([]const u8, .{ .Static = expected_isys_path_order.len }) = .init();
6850+
try expected_isys_path_queue.write(expected_isys_path_order);
6851+
var alloc: std.heap.DebugAllocator(.{}) = .init;
6852+
var arena = std.heap.ArenaAllocator.init(alloc.allocator());
6853+
6854+
const resolved_target: Package.Module.ResolvedTarget = .{
6855+
.is_native_os = true,
6856+
.is_explicit_dynamic_linker = false,
6857+
.is_native_abi = true,
6858+
.result = .{
6859+
.abi = .musl,
6860+
.cpu = .{
6861+
.arch = .x86_64,
6862+
.features = .empty,
6863+
.model = Target.Cpu.Model.baseline(.x86_64, .{
6864+
.tag = .linux,
6865+
.version_range = .{ .none = {} },
6866+
}),
6867+
},
6868+
.os = .{
6869+
.tag = .linux,
6870+
.version_range = .default(.x86_64, .linux, .musl),
6871+
},
6872+
.ofmt = .c,
6873+
},
6874+
};
6875+
const conf = try Compilation.Config.resolve(.{
6876+
.emit_bin = true,
6877+
.have_zcu = false,
6878+
.is_test = true,
6879+
.output_mode = .Exe,
6880+
.resolved_target = resolved_target,
6881+
});
6882+
6883+
const cwd = try std.fs.cwd().realpathAlloc(arena.allocator(), ".");
6884+
const global_cache = try introspect.resolveGlobalCacheDir(arena.allocator());
6885+
const global_cache_handle = try std.fs.openDirAbsolute(global_cache, .{ .access_sub_paths = true });
6886+
const local_cache = (try introspect.resolveSuitableLocalCacheDir(arena.allocator(), cwd)).?;
6887+
const local_cache_handle = try std.fs.openDirAbsolute(local_cache, .{ .access_sub_paths = true });
6888+
var thread_pool: std.Thread.Pool = undefined;
6889+
try thread_pool.init(.{ .allocator = arena.allocator() });
6890+
6891+
const mod = try Package.Module.create(arena.allocator(), .{
6892+
.paths = .{
6893+
.root = .{
6894+
.root = .none,
6895+
.sub_path = cwd,
6896+
},
6897+
.root_src_path = "src",
6898+
},
6899+
.cc_argv = &.{ "-isystem", "/module/isystem/dir1/", "-isystem/module/isystem/dir2/" },
6900+
.fully_qualified_name = "mod",
6901+
.parent = null,
6902+
.global = conf,
6903+
.inherited = .{ .resolved_target = resolved_target },
6904+
});
6905+
6906+
const compilation = try Compilation.create(
6907+
alloc.allocator(),
6908+
arena.allocator(),
6909+
.{
6910+
.dirs = .{
6911+
.cwd = cwd,
6912+
.global_cache = .{ .path = global_cache, .handle = global_cache_handle },
6913+
.local_cache = .{ .path = local_cache, .handle = local_cache_handle },
6914+
.zig_lib = try introspect.findZigLibDir(arena.allocator()),
6915+
},
6916+
.cache_mode = .whole,
6917+
.config = conf,
6918+
.thread_pool = &thread_pool,
6919+
.root_mod = mod,
6920+
.root_name = "mod",
6921+
.emit_bin = .yes_cache,
6922+
.native_system_include_paths = &.{"/native/isystem/dir/"},
6923+
.global_cc_argv = &.{ "-isystem", "/user_provided/isystem/dir/" },
6924+
},
6925+
);
6926+
6927+
var argv: std.ArrayList([]const u8) = .init(arena.allocator());
6928+
6929+
try compilation.addCCArgs(arena.allocator(), &argv, .c, null, mod);
6930+
6931+
var isys_paths_result: std.ArrayList([]const u8) = .init(arena.allocator());
6932+
6933+
const isys_paths_in_order = check_path_order: {
6934+
var isystem_path_up_next = false;
6935+
for (argv.items) |arg| {
6936+
if (mem.eql(u8, arg, "-isystem")) {
6937+
isystem_path_up_next = true;
6938+
continue;
6939+
}
6940+
if (isystem_path_up_next) {
6941+
try isys_paths_result.append(arg);
6942+
if (mem.eql(u8, arg, expected_isys_path_queue.peekItem(0))) {
6943+
expected_isys_path_queue.discard(1);
6944+
if (expected_isys_path_queue.readableLength() == 0) {
6945+
break :check_path_order true;
6946+
}
6947+
}
6948+
}
6949+
isystem_path_up_next = false;
6950+
}
6951+
break :check_path_order false;
6952+
};
6953+
6954+
std.testing.expect(isys_paths_in_order) catch |e| {
6955+
var result_str: std.ArrayList(u8) = .init(arena.allocator());
6956+
var result_writer = result_str.writer();
6957+
var expect_str: std.ArrayList(u8) = .init(arena.allocator());
6958+
var expect_writer = expect_str.writer();
6959+
6960+
for (isys_paths_result.items, 0..) |path, index| {
6961+
_ = try result_writer.write(path);
6962+
if (index < isys_paths_result.items.len - 1)
6963+
_ = try result_writer.write(", ");
6964+
}
6965+
for (expected_isys_path_order, 0..) |path, index| {
6966+
_ = try expect_writer.write(path);
6967+
if (index < expected_isys_path_order.len - 1)
6968+
_ = try expect_writer.write(", ");
6969+
}
6970+
std.log.err(
6971+
\\Expected isystem paths were not found in the correct order.
6972+
\\
6973+
\\Expected:
6974+
\\ [{s}]
6975+
\\
6976+
\\Result:
6977+
\\ [{s}]
6978+
\\
6979+
\\Note: Expected and Result do not need to match 1:1, all items in expected just need to be found in result in the same order.
6980+
, .{ expect_str.items, result_str.items });
6981+
return e;
6982+
};
68166983
}
68176984

68186985
fn failCObj(

0 commit comments

Comments
 (0)