Merge pull request #21056 from alexrp/start-fix-precedence

andrewrk · web-flow · commit b917d778c6e8 · 2024-08-15T22:10:59.000-07:00
`start`: Avoid string concatenation in inline asm.
diff --git a/lib/std/start.zig b/lib/std/start.zig
@@ -232,6 +232,16 @@ fn _start() callconv(.Naked) noreturn {
         );
     }
 
+    // Move this to the riscv prong below when this is resolved: https://github.com/ziglang/zig/issues/20918
+    if (builtin.cpu.arch.isRISCV() and builtin.zig_backend != .stage2_riscv64) asm volatile (
+        \\ .weak __global_pointer$
+        \\ .hidden __global_pointer$
+        \\ .option push
+        \\ .option norelax
+        \\ lla gp, __global_pointer$
+        \\ .option pop
+    );
+
     // Note that we maintain a very low level of trust with regards to ABI guarantees at this point.
     // We will redundantly align the stack, clear the link register, etc. While e.g. the Linux
     // kernel is usually good about upholding the ABI guarantees, the same cannot be said of dynamic
@@ -275,24 +285,19 @@ fn _start() callconv(.Naked) noreturn {
             \\ and sp, #-16
             \\ b %[posixCallMainAndExit]
             ,
-            // zig fmt: off
             .csky =>
-            if (builtin.position_independent_code)
-                // The CSKY ABI assumes that `gb` is set to the address of the GOT in order for
-                // position-independent code to work. We depend on this in `std.os.linux.start_pie`
-                // to locate `_DYNAMIC` as well.
-                \\ grs t0, 1f
-                \\ 1:
-                \\ lrw gb, 1b@GOTPC
-                \\ addu gb, t0
-            else ""
-            ++
+            // The CSKY ABI assumes that `gb` is set to the address of the GOT in order for
+            // position-independent code to work. We depend on this in `std.os.linux.start_pie`
+            // to locate `_DYNAMIC` as well.
+            \\ grs t0, 1f
+            \\ 1:
+            \\ lrw gb, 1b@GOTPC
+            \\ addu gb, t0
             \\ movi lr, 0
             \\ mov a0, sp
             \\ andi sp, sp, -8
             \\ jmpi %[posixCallMainAndExit]
             ,
-            // zig fmt: on
             .hexagon =>
             // r29 = SP, r30 = FP
             \\ r30 = #0
@@ -308,27 +313,13 @@ fn _start() callconv(.Naked) noreturn {
             \\ bstrins.d $sp, $zero, 3, 0
             \\ b %[posixCallMainAndExit]
             ,
-            // zig fmt: off
             .riscv32, .riscv64 =>
-            // The self-hosted riscv64 backend is not able to assemble this yet.
-            if (builtin.zig_backend != .stage2_riscv64)
-                // The RISC-V ELF ABI assumes that `gp` is set to the value of `__global_pointer$` at
-                // startup in order for GP relaxation to work, even in static builds.
-                \\ .weak __global_pointer$
-                \\ .hidden __global_pointer$
-                \\ .option push
-                \\ .option norelax
-                \\ lla gp, __global_pointer$
-                \\ .option pop
-            else ""
-            ++
             \\ li s0, 0
             \\ li ra, 0
             \\ mv a0, sp
             \\ andi sp, sp, -16
             \\ tail %[posixCallMainAndExit]@plt
             ,
-            // zig fmt: on
             .m68k =>
             // Note that the - 8 is needed because pc in the jsr instruction points into the middle
             // of the jsr instruction. (The lea is 6 bytes, the jsr is 4 bytes.)
diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig
@@ -2909,7 +2909,17 @@ pub const Object = struct {
             function_index.setAlignment(resolved.alignment.toLlvm(), &o.builder);
 
         // Function attributes that are independent of analysis results of the function body.
-        try o.addCommonFnAttributes(&attributes, owner_mod);
+        try o.addCommonFnAttributes(
+            &attributes,
+            owner_mod,
+            // Some backends don't respect the `naked` attribute in `TargetFrameLowering::hasFP()`,
+            // so for these backends, LLVM will happily emit code that accesses the stack through
+            // the frame pointer. This is nonsensical since what the `naked` attribute does is
+            // suppress generation of the prologue and epilogue, and the prologue is where the
+            // frame pointer normally gets set up. At time of writing, this is the case for at
+            // least x86 and RISC-V.
+            owner_mod.omit_frame_pointer or fn_info.cc == .Naked,
+        );
 
         if (fn_info.return_type == .noreturn_type) try attributes.addFnAttr(.noreturn, &o.builder);
 
@@ -2956,13 +2966,14 @@ pub const Object = struct {
         o: *Object,
         attributes: *Builder.FunctionAttributes.Wip,
         owner_mod: *Package.Module,
+        omit_frame_pointer: bool,
     ) Allocator.Error!void {
         const comp = o.pt.zcu.comp;
 
         if (!owner_mod.red_zone) {
             try attributes.addFnAttr(.noredzone, &o.builder);
         }
-        if (owner_mod.omit_frame_pointer) {
+        if (omit_frame_pointer) {
             try attributes.addFnAttr(.{ .string = .{
                 .kind = try o.builder.string("frame-pointer"),
                 .value = try o.builder.string("none"),
@@ -4528,7 +4539,7 @@ pub const Object = struct {
 
         var attributes: Builder.FunctionAttributes.Wip = .{};
         defer attributes.deinit(&o.builder);
-        try o.addCommonFnAttributes(&attributes, zcu.root_mod);
+        try o.addCommonFnAttributes(&attributes, zcu.root_mod, zcu.root_mod.omit_frame_pointer);
 
         function_index.setLinkage(.internal, &o.builder);
         function_index.setCallConv(.fastcc, &o.builder);
@@ -4557,7 +4568,7 @@ pub const Object = struct {
 
         var attributes: Builder.FunctionAttributes.Wip = .{};
         defer attributes.deinit(&o.builder);
-        try o.addCommonFnAttributes(&attributes, zcu.root_mod);
+        try o.addCommonFnAttributes(&attributes, zcu.root_mod, zcu.root_mod.omit_frame_pointer);
 
         function_index.setLinkage(.internal, &o.builder);
         function_index.setCallConv(.fastcc, &o.builder);
@@ -6709,8 +6720,6 @@ pub const FuncGen = struct {
         const operand_ty = self.typeOf(pl_op.operand);
         const name = self.air.nullTerminatedString(pl_op.payload);
 
-        if (needDbgVarWorkaround(o)) return .none;
-
         const debug_local_var = try o.builder.debugLocalVar(
             try o.builder.metadataString(name),
             self.file,
@@ -6734,7 +6743,10 @@ pub const FuncGen = struct {
                 },
                 "",
             );
-        } else if (owner_mod.optimize_mode == .Debug) {
+        } else if (owner_mod.optimize_mode == .Debug and !self.is_naked) {
+            // We avoid taking this path for naked functions because there's no guarantee that such
+            // functions even have a valid stack pointer, making the `alloca` + `store` unsafe.
+
             const alignment = operand_ty.abiAlignment(pt).toLlvm();
             const alloca = try self.buildAlloca(operand.typeOfWip(&self.wip), alignment);
             _ = try self.wip.store(.normal, operand, alloca, alignment);
@@ -8815,7 +8827,6 @@ pub const FuncGen = struct {
         if (self.is_naked) return arg_val;
 
         const inst_ty = self.typeOfIndex(inst);
-        if (needDbgVarWorkaround(o)) return arg_val;
 
         const name = self.air.instructions.items(.data)[@intFromEnum(inst)].arg.name;
         if (name == .none) return arg_val;
@@ -9676,7 +9687,7 @@ pub const FuncGen = struct {
 
         var attributes: Builder.FunctionAttributes.Wip = .{};
         defer attributes.deinit(&o.builder);
-        try o.addCommonFnAttributes(&attributes, zcu.root_mod);
+        try o.addCommonFnAttributes(&attributes, zcu.root_mod, zcu.root_mod.omit_frame_pointer);
 
         function_index.setLinkage(.internal, &o.builder);
         function_index.setCallConv(.fastcc, &o.builder);
@@ -11820,16 +11831,6 @@ const optional_layout_version = 3;
 
 const lt_errors_fn_name = "__zig_lt_errors_len";
 
-/// Without this workaround, LLVM crashes with "unknown codeview register H1"
-/// https://github.com/llvm/llvm-project/issues/56484
-fn needDbgVarWorkaround(o: *Object) bool {
-    const target = o.pt.zcu.getTarget();
-    if (target.os.tag == .windows and target.cpu.arch == .aarch64) {
-        return true;
-    }
-    return false;
-}
-
 fn compilerRtIntBits(bits: u16) u16 {
     inline for (.{ 32, 64, 128 }) |b| {
         if (bits <= b) {
