commit e9fc58eab77d60dfb02155ff17178b496d75d035 (tree)
parent d9b0c984aaf4f5e738ca4d06f160a9110f9167ec
Author: Andrew Kelley <andrew@ziglang.org>
Date: Mon, 6 Jun 2022 18:21:24 -0700
LLVM: handle extern function name collisions
Zig allows multiple extern functions with the same name, and the
backends have to handle this possibility.
For LLVM, we keep a sparse map of collisions, and then resolve them in
flushModule(). This introduces some technical debt that will have to be
resolved when adding incremental compilation support to the LLVM
backend.
Diffstat:
2 files changed, 54 insertions(+), 12 deletions(-)
diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig
@@ -214,6 +214,10 @@ pub const Object = struct {
/// Note that the values are not added until flushModule, when all errors in
/// the compilation are known.
error_name_table: ?*const llvm.Value,
+ /// This map is usually very close to empty. It tracks only the cases when a
+ /// second extern Decl could not be emitted with the correct name due to a
+ /// name collision.
+ extern_collisions: std.AutoArrayHashMapUnmanaged(Module.Decl.Index, void),
pub const TypeMap = std.HashMapUnmanaged(
Type,
@@ -376,6 +380,7 @@ pub const Object = struct {
.type_map_arena = std.heap.ArenaAllocator.init(gpa),
.di_type_map = .{},
.error_name_table = null,
+ .extern_collisions = .{},
};
}
@@ -392,6 +397,7 @@ pub const Object = struct {
self.decl_map.deinit(gpa);
self.type_map.deinit(gpa);
self.type_map_arena.deinit();
+ self.extern_collisions.deinit(gpa);
self.* = undefined;
}
@@ -508,6 +514,22 @@ pub const Object = struct {
fn resolveExportExternCollisions(object: *Object) !void {
const mod = object.module;
+ // This map has externs with incorrect symbol names.
+ for (object.extern_collisions.keys()) |decl_index| {
+ const entry = object.decl_map.getEntry(decl_index) orelse continue;
+ const llvm_global = entry.value_ptr.*;
+ // Same logic as below but for externs instead of exports.
+ const decl = mod.declPtr(decl_index);
+ const other_global = object.getLlvmGlobal(decl.name) orelse continue;
+ if (other_global == llvm_global) continue;
+
+ const new_global_ptr = other_global.constBitCast(llvm_global.typeOf());
+ llvm_global.replaceAllUsesWith(new_global_ptr);
+ object.deleteLlvmGlobal(llvm_global);
+ entry.value_ptr.* = new_global_ptr;
+ }
+ object.extern_collisions.clearRetainingCapacity();
+
const export_keys = mod.decl_exports.keys();
for (mod.decl_exports.values()) |export_list, i| {
const decl_index = export_keys[i];
@@ -997,6 +1019,15 @@ pub const Object = struct {
return null;
}
+ /// TODO can this be done with simpler logic / different API binding?
+ fn deleteLlvmGlobal(o: Object, llvm_global: *const llvm.Value) void {
+ if (o.llvm_module.getNamedFunction(llvm_global.getValueName()) != null) {
+ llvm_global.deleteFunction();
+ return;
+ }
+ return llvm_global.deleteGlobal();
+ }
+
pub fn updateDeclExports(
self: *Object,
module: *Module,
@@ -1009,6 +1040,12 @@ pub const Object = struct {
const decl = module.declPtr(decl_index);
if (decl.isExtern()) {
llvm_global.setValueName(decl.name);
+ if (self.getLlvmGlobal(decl.name)) |other_global| {
+ if (other_global != llvm_global) {
+ log.debug("updateDeclExports isExtern()=true setValueName({s}) conflict", .{decl.name});
+ try self.extern_collisions.put(module.gpa, decl_index, {});
+ }
+ }
llvm_global.setUnnamedAddr(.False);
llvm_global.setLinkage(.External);
if (self.di_map.get(decl)) |di_node| {
@@ -2143,11 +2180,8 @@ pub const DeclGen = struct {
log.debug("gen: {s} type: {}, value: {}", .{
decl.name, decl.ty.fmtDebug(), decl.val.fmtDebug(),
});
-
- if (decl.val.castTag(.function)) |func_payload| {
- _ = func_payload;
- @panic("TODO llvm backend genDecl function pointer");
- } else if (decl.val.castTag(.extern_fn)) |extern_fn| {
+ assert(decl.val.tag() != .function);
+ if (decl.val.castTag(.extern_fn)) |extern_fn| {
_ = try dg.resolveLlvmFunction(extern_fn.data.owner_decl);
} else {
const target = dg.module.getTarget();
@@ -2246,12 +2280,14 @@ pub const DeclGen = struct {
if (!is_extern) {
llvm_fn.setLinkage(.Internal);
llvm_fn.setUnnamedAddr(.True);
- } else if (dg.module.getTarget().isWasm()) {
- dg.addFnAttrString(llvm_fn, "wasm-import-name", std.mem.sliceTo(decl.name, 0));
- if (decl.getExternFn().?.lib_name) |lib_name| {
- const module_name = std.mem.sliceTo(lib_name, 0);
- if (!std.mem.eql(u8, module_name, "c")) {
- dg.addFnAttrString(llvm_fn, "wasm-import-module", module_name);
+ } else {
+ if (dg.module.getTarget().isWasm()) {
+ dg.addFnAttrString(llvm_fn, "wasm-import-name", std.mem.sliceTo(decl.name, 0));
+ if (decl.getExternFn().?.lib_name) |lib_name| {
+ const module_name = std.mem.sliceTo(lib_name, 0);
+ if (!std.mem.eql(u8, module_name, "c")) {
+ dg.addFnAttrString(llvm_fn, "wasm-import-module", module_name);
+ }
}
}
}
diff --git a/test/behavior/bugs/529.zig b/test/behavior/bugs/529.zig
@@ -8,8 +8,14 @@ comptime {
_ = @import("529_other_file_2.zig");
}
+const builtin = @import("builtin");
+
test "issue 529 fixed" {
- if (@import("builtin").zig_backend != .stage1) return error.SkipZigTest; // TODO
+ if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO
+ if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO
+ if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
+ if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
+ if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
@import("529_other_file.zig").issue529(null);
issue529(null);
