Lower x86-interrupt byval first arg as pointer

Author: Dnreikronos

src/abi.rs

@@ -16,8 +16,6 @@ use rustc_target::callconv::{ArgAttributes, CastTarget, FnAbi, PassMode};
 use rustc_target::spec::Arch;
 
 use crate::builder::Builder;
-#[cfg(feature = "master")]
-use crate::common::type_is_pointer;
 use crate::context::CodegenCx;
 use crate::type_of::LayoutGccExt;
 
@@ -153,7 +151,10 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
         #[cfg(not(feature = "master"))]
         let apply_attrs = |ty: Type<'gcc>, _attrs: &ArgAttributes, _arg_index: usize| ty;
 
-        for arg in self.args.iter() {
+        for (source_arg_index, arg) in self.args.iter().enumerate() {
+            #[cfg(not(feature = "master"))]
+            let _ = source_arg_index;
+
             let arg_ty = match arg.mode {
                 PassMode::Ignore => continue,
                 PassMode::Pair(a, b) => {
@@ -179,9 +180,28 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
                     apply_attrs(ty, &cast.attrs, argument_tys.len())
                 }
                 PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: true } => {
-                    // This is a "byval" argument, so we don't apply the `restrict` attribute on it.
-                    on_stack_param_indices.insert(argument_tys.len());
-                    arg.layout.gcc_type(cx)
+                    let x86_interrupt_first_arg = {
+                        #[cfg(feature = "master")]
+                        {
+                            source_arg_index == 0
+                                && matches!(self.conv, CanonAbi::Interrupt(InterruptKind::X86))
+                        }
+                        #[cfg(not(feature = "master"))]
+                        {
+                            false
+                        }
+                    };
+
+                    if x86_interrupt_first_arg {
+                        // Rust lowers the first `x86-interrupt` argument as a byval stack slot.
+                        // LLVM represents that as a pointer parameter with `byval`; GCC's
+                        // interrupt attribute likewise requires a pointer-shaped first parameter.
+                        cx.type_ptr_to(arg.layout.gcc_type(cx))
+                    } else {
+                        // This is a "byval" argument, so we don't apply the `restrict` attribute on it.
+                        on_stack_param_indices.insert(argument_tys.len());
+                        arg.layout.gcc_type(cx)
+                    }
                 }
                 PassMode::Direct(attrs) => {
                     apply_attrs(arg.layout.immediate_gcc_type(cx), &attrs, argument_tys.len())
@@ -279,33 +299,3 @@ pub fn conv_to_fn_attribute<'gcc>(conv: CanonAbi, arch: &Arch) -> Option<FnAttri
     };
     Some(attribute)
 }
-
-/// GCC's `interrupt` attribute requires the first parameter of an `x86-interrupt`
-/// function to be a pointer. When it is not, libgccjit aborts codegen with an internal
-/// error and leaves no object file behind. Detect that case so the backend can emit a
-/// clean diagnostic and skip the attribute instead of crashing (issue #833).
-///
-/// This works on the already-lowered GCC argument types so callers that have them (e.g.
-/// `declare_fn`) avoid a redundant ABI lowering. `x86-interrupt` functions return `()`,
-/// so there is no struct-return pointer prepended and `arguments_type[0]` is the frame
-/// argument.
-#[cfg(feature = "master")]
-pub fn x86_interrupt_first_arg_is_invalid<'gcc>(
-    conv: CanonAbi,
-    arguments_type: &[Type<'gcc>],
-) -> bool {
-    matches!(conv, CanonAbi::Interrupt(InterruptKind::X86))
-        && arguments_type.first().is_none_or(|ty| !type_is_pointer(*ty))
-}
-
-/// Convenience wrapper around [`x86_interrupt_first_arg_is_invalid`] for callers that only
-/// hold the `FnAbi` and must lower it themselves. Short-circuits before lowering for the
-/// common non-`x86-interrupt` case.
-#[cfg(feature = "master")]
-pub fn x86_interrupt_has_invalid_first_arg<'gcc, 'tcx>(
-    cx: &CodegenCx<'gcc, 'tcx>,
-    fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
-) -> bool {
-    matches!(fn_abi.conv, CanonAbi::Interrupt(InterruptKind::X86))
-        && x86_interrupt_first_arg_is_invalid(fn_abi.conv, &fn_abi.gcc_type(cx).arguments_type)
-}

src/declare.rs

@@ -7,8 +7,6 @@ use rustc_span::Symbol;
 use rustc_target::callconv::FnAbi;
 
 use crate::abi::FnAbiGccExt;
-#[cfg(feature = "master")]
-use crate::abi::x86_interrupt_first_arg_is_invalid;
 use crate::context::CodegenCx;
 
 impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
@@ -112,18 +110,9 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
     }
 
     pub fn declare_fn(&self, name: &str, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> Function<'gcc> {
-        // Lower the ABI once and reuse it below, including for the `x86-interrupt` check,
-        // instead of lowering it a second time inside a helper.
         let fn_abi_gcc = fn_abi.gcc_type(self);
         #[cfg(feature = "master")]
-        let conv = if x86_interrupt_first_arg_is_invalid(fn_abi.conv, &fn_abi_gcc.arguments_type) {
-            // GCC rejects an `x86-interrupt` function whose first argument is not a
-            // pointer. Drop the calling-convention attribute so libgccjit does not abort;
-            // a clean error is emitted in `predefine_fn` instead (issue #833).
-            None
-        } else {
-            fn_abi.gcc_cconv(self)
-        };
+        let conv = fn_abi.gcc_cconv(self);
         #[cfg(not(feature = "master"))]
         let conv = None;
         let func = declare_raw_fn(

src/errors.rs

@@ -30,13 +30,3 @@ pub(crate) struct NulBytesInAsm {
     #[primary_span]
     pub span: Span,
 }
-
-#[cfg(feature = "master")]
-#[derive(Diagnostic)]
-#[diag(
-    "the GCC backend requires the first argument of an `x86-interrupt` function to be a pointer"
-)]
-pub(crate) struct X86InterruptBadFirstArg {
-    #[primary_span]
-    pub span: Span,
-}

src/mono_item.rs

@@ -10,11 +10,7 @@ use rustc_middle::mono::Visibility;
 use rustc_middle::ty::layout::{FnAbiOf, HasTypingEnv, LayoutOf};
 use rustc_middle::ty::{self, Instance, TypeVisitableExt};
 
-#[cfg(feature = "master")]
-use crate::abi::x86_interrupt_has_invalid_first_arg;
 use crate::context::CodegenCx;
-#[cfg(feature = "master")]
-use crate::errors::X86InterruptBadFirstArg;
 use crate::type_of::LayoutGccExt;
 use crate::{attributes, base};
 
@@ -55,12 +51,6 @@ impl<'gcc, 'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
         assert!(!instance.args.has_infer());
 
         let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty());
-        #[cfg(feature = "master")]
-        if x86_interrupt_has_invalid_first_arg(self, fn_abi) {
-            self.tcx
-                .dcx()
-                .emit_err(X86InterruptBadFirstArg { span: self.tcx.def_span(instance.def_id()) });
-        }
         self.linkage.set(base::linkage_to_gcc(linkage));
         let decl = self.declare_fn(symbol_name, fn_abi);
         //let attrs = self.tcx.codegen_instance_attrs(instance.def);

tests/compile/x86_interrupt_bad_first_arg.rs

@@ -0,0 +1,21 @@
+// Compiler:
+
+// Test that an `x86-interrupt` function whose first argument is passed by value
+// emits a pointer-shaped GCC parameter instead of tripping libgccjit (issue #833).
+
+#![feature(abi_x86_interrupt)]
+
+#[repr(C)]
+pub struct Frame {
+    ip: u64,
+}
+
+pub extern "x86-interrupt" fn scalar(_a: i64) {}
+
+pub extern "x86-interrupt" fn aggregate(_frame: Frame) {}
+
+fn main() {
+    // Take the functions' addresses so that they are codegened.
+    let _scalar: extern "x86-interrupt" fn(i64) = scalar;
+    let _aggregate: extern "x86-interrupt" fn(Frame) = aggregate;
+}

tests/compile/x86_interrupt_first_arg_byval.rs

@@ -216,7 +216,7 @@ fn compile_tests(tempdir: PathBuf, current_dir: String) {
             "asm_nul_byte.rs",
             "global_asm_nul_byte.rs",
             "naked_asm_nul_byte.rs",
-            "x86_interrupt_bad_first_arg.rs",
+            "x86_interrupt_first_arg_byval.rs",
         ],
     );
 }