Commit 1ff9a18cd3

Andrew Kelley <andrew@ziglang.org>
2022-05-11 01:43:51
stage1: back out the broken visibility changes
``` $ valgrind ./zig test ../test/behavior.zig -target powerpc-linux-musl -lc -I../test ==2828778== Invalid read of size 1 ==2828778== at 0x6EA0265: LLVMSetVisibility (in /home/andy/Downloads/zig/build/zig) ==2828778== by 0x1BCE60B: do_code_gen(CodeGen*) (codegen.cpp:9031) ==2828778== by 0x1BD51E2: codegen_build_object(CodeGen*) (codegen.cpp:10610) ==2828778== by 0x1BA5C17: zig_stage1_build_object (stage1.cpp:132) ==2828778== by 0xE61E24: Module.build_object (stage1.zig:149) ==2828778== by 0xC3D4CE: Compilation.updateStage1Module (Compilation.zig:5025) ==2828778== by 0xC3117E: Compilation.performAllTheWork (Compilation.zig:2691) ==2828778== by 0xC2A3ED: Compilation.update (Compilation.zig:2098) ==2828778== by 0xBB9D1F: main.updateModule (main.zig:3104) ==2828778== by 0xB16B75: main.buildOutputType (main.zig:2793) ==2828778== by 0xAD0526: main.mainArgs (main.zig:225) ==2828778== by 0xACFCB9: main (stage1.zig:48) ``` Since the plan is to ship stage3 for Zig 0.10.0, the stage1 implementation of this hardly matters.
1 parent c4c5020
src/stage1/all_types.hpp
@@ -571,12 +571,6 @@ enum GlobalLinkageId {
     GlobalLinkageIdLinkOnce,
 };
 
-enum SymbolVisibilityId {
-    SymbolVisibilityIdDefault,
-    SymbolVisibilityIdHidden,
-    SymbolVisibilityIdProtected,
-};
-
 enum TldId {
     TldIdVar,
     TldIdFn,
@@ -1660,7 +1654,6 @@ enum FnAnalState {
 struct GlobalExport {
     Buf name;
     GlobalLinkageId linkage;
-    SymbolVisibilityId visibility;
 };
 
 struct ZigFn {
src/stage1/analyze.cpp
@@ -3717,15 +3717,14 @@ ZigType *get_test_fn_type(CodeGen *g) {
     return g->test_fn_type;
 }
 
-void add_var_export(CodeGen *g, ZigVar *var, const char *symbol_name, GlobalLinkageId linkage, SymbolVisibilityId visibility) {
+void add_var_export(CodeGen *g, ZigVar *var, const char *symbol_name, GlobalLinkageId linkage) {
     GlobalExport *global_export = var->export_list.add_one();
     memset(global_export, 0, sizeof(GlobalExport));
     buf_init_from_str(&global_export->name, symbol_name);
     global_export->linkage = linkage;
-    global_export->visibility = visibility;
 }
 
-void add_fn_export(CodeGen *g, ZigFn *fn_table_entry, const char *symbol_name, GlobalLinkageId linkage, SymbolVisibilityId visibility, CallingConvention cc) {
+void add_fn_export(CodeGen *g, ZigFn *fn_table_entry, const char *symbol_name, GlobalLinkageId linkage, CallingConvention cc) {
     CallingConvention winapi_cc = g->zig_target->arch == ZigLLVM_x86
         ? CallingConventionStdcall
         : CallingConventionC;
@@ -3750,7 +3749,6 @@ void add_fn_export(CodeGen *g, ZigFn *fn_table_entry, const char *symbol_name, G
     memset(fn_export, 0, sizeof(GlobalExport));
     buf_init_from_str(&fn_export->name, symbol_name);
     fn_export->linkage = linkage;
-    fn_export->visibility = visibility;
 }
 
 static void resolve_decl_fn(CodeGen *g, TldFn *tld_fn) {
@@ -3854,13 +3852,13 @@ static void resolve_decl_fn(CodeGen *g, TldFn *tld_fn) {
                 case CallingConventionWin64:
                 case CallingConventionPtxKernel:
                     add_fn_export(g, fn_table_entry, buf_ptr(&fn_table_entry->symbol_name),
-                                  GlobalLinkageIdStrong, SymbolVisibilityIdDefault, fn_cc);
+                                  GlobalLinkageIdStrong, fn_cc);
                     break;
                 case CallingConventionUnspecified:
                     // An exported function without a specific calling
                     // convention defaults to C
                     add_fn_export(g, fn_table_entry, buf_ptr(&fn_table_entry->symbol_name),
-                                  GlobalLinkageIdStrong, SymbolVisibilityIdDefault, CallingConventionC);
+                                  GlobalLinkageIdStrong, CallingConventionC);
                     break;
             }
         }
@@ -4323,7 +4321,7 @@ static void resolve_decl_var(CodeGen *g, TldVar *tld_var, bool allow_lazy) {
 
     if (is_export) {
         validate_export_var_type(g, type, source_node);
-        add_var_export(g, tld_var->var, tld_var->var->name, GlobalLinkageIdStrong, SymbolVisibilityIdDefault);
+        add_var_export(g, tld_var->var, tld_var->var->name, GlobalLinkageIdStrong);
     }
 
     if (is_extern) {
src/stage1/analyze.hpp
@@ -221,8 +221,8 @@ ZigType *get_align_amt_type(CodeGen *g);
 ZigPackage *new_anonymous_package(void);
 
 Buf *const_value_to_buffer(ZigValue *const_val);
-void add_fn_export(CodeGen *g, ZigFn *fn_table_entry, const char *symbol_name, GlobalLinkageId linkage, SymbolVisibilityId visibility, CallingConvention cc);
-void add_var_export(CodeGen *g, ZigVar *fn_table_entry, const char *symbol_name, GlobalLinkageId linkage, SymbolVisibilityId visibility);
+void add_fn_export(CodeGen *g, ZigFn *fn_table_entry, const char *symbol_name, GlobalLinkageId linkage, CallingConvention cc);
+void add_var_export(CodeGen *g, ZigVar *fn_table_entry, const char *symbol_name, GlobalLinkageId linkage);
 
 
 ZigValue *get_builtin_value(CodeGen *codegen, const char *name);
src/stage1/codegen.cpp
@@ -242,18 +242,6 @@ static LLVMLinkage to_llvm_linkage(GlobalLinkageId id, bool is_extern) {
     zig_unreachable();
 }
 
-static LLVMVisibility to_llvm_visibility(SymbolVisibilityId id) {
-    switch (id) {
-        case SymbolVisibilityIdDefault:
-            return LLVMDefaultVisibility;
-        case SymbolVisibilityIdHidden:
-            return LLVMHiddenVisibility;
-        case SymbolVisibilityIdProtected:
-            return LLVMProtectedVisibility;
-    }
-    zig_unreachable();
-}
-
 struct CalcLLVMFieldIndex {
     uint32_t offset;
     uint32_t field_index;
@@ -412,7 +400,6 @@ static LLVMValueRef make_fn_llvm_value(CodeGen *g, ZigFn *fn) {
     const char *unmangled_name = buf_ptr(&fn->symbol_name);
     const char *symbol_name;
     GlobalLinkageId linkage;
-    SymbolVisibilityId visibility = SymbolVisibilityIdDefault;
     if (fn->body_node == nullptr) {
         symbol_name = unmangled_name;
         linkage = GlobalLinkageIdStrong;
@@ -423,7 +410,6 @@ static LLVMValueRef make_fn_llvm_value(CodeGen *g, ZigFn *fn) {
         GlobalExport *fn_export = &fn->export_list.items[0];
         symbol_name = buf_ptr(&fn_export->name);
         linkage = fn_export->linkage;
-        visibility = fn_export->visibility;
     }
 
     CallingConvention cc = fn->type_entry->data.fn.fn_type_id.cc;
@@ -546,8 +532,6 @@ static LLVMValueRef make_fn_llvm_value(CodeGen *g, ZigFn *fn) {
         LLVMSetUnnamedAddr(llvm_fn, true);
     }
 
-    LLVMSetVisibility(llvm_fn, to_llvm_visibility(visibility));
-
     ZigType *return_type = fn_type->data.fn.fn_type_id.return_type;
     if (return_type->id == ZigTypeIdUnreachable) {
         addLLVMFnAttr(llvm_fn, "noreturn");
@@ -8967,7 +8951,6 @@ static void do_code_gen(CodeGen *g) {
         assert(var->decl_node);
 
         GlobalLinkageId linkage;
-        SymbolVisibilityId visibility = SymbolVisibilityIdDefault;
         const char *unmangled_name = var->name;
         const char *symbol_name;
         if (var->export_list.length == 0) {
@@ -8982,7 +8965,6 @@ static void do_code_gen(CodeGen *g) {
             GlobalExport *global_export = &var->export_list.items[0];
             symbol_name = buf_ptr(&global_export->name);
             linkage = global_export->linkage;
-            visibility = global_export->visibility;
         }
 
         LLVMValueRef global_value;
@@ -9028,8 +9010,6 @@ static void do_code_gen(CodeGen *g) {
             set_global_tls(g, var, global_value);
         }
 
-        LLVMSetVisibility(global_value, to_llvm_visibility(visibility));
-
         var->value_ref = global_value;
 
         for (size_t export_i = 1; export_i < var->export_list.length; export_i += 1) {
src/stage1/ir.cpp
@@ -8635,25 +8635,6 @@ static bool ir_resolve_global_linkage(IrAnalyze *ira, Stage1AirInst *value, Glob
     return true;
 }
 
-static bool ir_resolve_global_visibility(IrAnalyze *ira, Stage1AirInst *value, SymbolVisibilityId *out) {
-    if (type_is_invalid(value->value->type))
-        return false;
-
-    ZigType *global_visibility_type = get_builtin_type(ira->codegen, "SymbolVisibility");
-
-    Stage1AirInst *casted_value = ir_implicit_cast(ira, value, global_visibility_type);
-    if (type_is_invalid(casted_value->value->type))
-        return false;
-
-    ZigValue *const_val = ir_resolve_const(ira, casted_value, UndefBad);
-    if (!const_val)
-        return false;
-
-    *out = (SymbolVisibilityId)bigint_as_u32(&const_val->data.x_enum_tag);
-    return true;
-}
-
-
 static bool ir_resolve_float_mode(IrAnalyze *ira, Stage1AirInst *value, FloatMode *out) {
     if (type_is_invalid(value->value->type))
         return false;
@@ -11680,12 +11661,6 @@ static Stage1AirInst *ir_analyze_instruction_export(IrAnalyze *ira, Stage1ZirIns
     if (type_is_invalid(section_inst->value->type))
         return ira->codegen->invalid_inst_gen;
 
-    TypeStructField *visibility_field = find_struct_type_field(options_type, buf_create_from_str("visibility"));
-    src_assert(visibility_field != nullptr, instruction->base.source_node);
-    Stage1AirInst *visibility_inst = ir_analyze_struct_value_field_value(ira, instruction->base.scope, instruction->base.source_node, options, visibility_field);
-    if (type_is_invalid(visibility_inst->value->type))
-        return ira->codegen->invalid_inst_gen;
-
     // The `section` field is optional, we have to unwrap it first
     Stage1AirInst *non_null_check = ir_analyze_test_non_null(ira, instruction->base.scope, instruction->base.source_node, section_inst);
     bool is_non_null;
@@ -11714,10 +11689,6 @@ static Stage1AirInst *ir_analyze_instruction_export(IrAnalyze *ira, Stage1ZirIns
     if (!ir_resolve_global_linkage(ira, linkage_inst, &global_linkage_id))
         return ira->codegen->invalid_inst_gen;
 
-    SymbolVisibilityId global_visibility_id;
-    if (!ir_resolve_global_visibility(ira, visibility_inst, &global_visibility_id))
-        return ira->codegen->invalid_inst_gen;
-
     Buf *section_name = nullptr;
     if (section_str_inst != nullptr && !(section_name = ir_resolve_str(ira, section_str_inst)))
         return ira->codegen->invalid_inst_gen;
@@ -11780,7 +11751,7 @@ static Stage1AirInst *ir_analyze_instruction_export(IrAnalyze *ira, Stage1ZirIns
                 case CallingConventionSysV:
                 case CallingConventionWin64:
                 case CallingConventionPtxKernel:
-                    add_fn_export(ira->codegen, fn_entry, buf_ptr(symbol_name), global_linkage_id, global_visibility_id, cc);
+                    add_fn_export(ira->codegen, fn_entry, buf_ptr(symbol_name), global_linkage_id, cc);
                     fn_entry->section_name = section_name;
                     break;
             }
@@ -11927,7 +11898,7 @@ static Stage1AirInst *ir_analyze_instruction_export(IrAnalyze *ira, Stage1ZirIns
         if (load_ptr->ptr->id == Stage1AirInstIdVarPtr) {
             Stage1AirInstVarPtr *var_ptr = reinterpret_cast<Stage1AirInstVarPtr *>(load_ptr->ptr);
             ZigVar *var = var_ptr->var;
-            add_var_export(ira->codegen, var, buf_ptr(symbol_name), global_linkage_id, global_visibility_id);
+            add_var_export(ira->codegen, var, buf_ptr(symbol_name), global_linkage_id);
             var->section_name = section_name;
         }
     }