[AVR] Correctly set the pointer address space when constructing pointers to functions

This patch extends the existing `type_i8p` method so that it requires an
explicit address space to be specified. Before this patch, the
`type_i8p` method implcitily assumed the default address space, which is
not a safe transformation on all targets, namely AVR.

The Rust compiler already has support for tracking the "instruction
address space" on a per-target basis. This patch extends the code
generation routines so that an address space must always be specified.

In my estimation, around 15% of the callers of `type_i8p` produced
invalid code on AVR due to the loss of address space prior to LLVM final
code generation. This would lead to unavoidable assertion errors
relating to invalid bitcasts.

With this patch, the address space is always either 1) explicitly preserved
from the input type, or 2) explicitly set to the instruction address
space because the logic is dealing with functions which must be placed
there, or 3) explicitly set to the default address space 0 because the
logic can only operate on data space pointers and thus we keep the
existing semantics of assuming the default, "data" address space.

Author: dylanmckay

src/librustc_codegen_llvm/abi.rs

@@ -366,7 +366,7 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> {
         unsafe {
             llvm::LLVMPointerType(
                 self.llvm_type(cx),
-                cx.data_layout().instruction_address_space as c_uint,
+                cx.data_layout().instruction_address_space.0 as c_uint,
             )
         }
     }

src/librustc_codegen_llvm/builder.rs

@@ -725,8 +725,8 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
         }
         let size = self.intcast(size, self.type_isize(), false);
         let is_volatile = flags.contains(MemFlags::VOLATILE);
-        let dst = self.pointercast(dst, self.type_i8p());
-        let src = self.pointercast(src, self.type_i8p());
+        let dst = self.pointercast(dst, self.type_i8p(self.cx.address_space_of_value(dst)));
+        let src = self.pointercast(src, self.type_i8p(self.cx.address_space_of_value(src)));
         unsafe {
             llvm::LLVMRustBuildMemCpy(
                 self.llbuilder,
@@ -758,8 +758,8 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
         }
         let size = self.intcast(size, self.type_isize(), false);
         let is_volatile = flags.contains(MemFlags::VOLATILE);
-        let dst = self.pointercast(dst, self.type_i8p());
-        let src = self.pointercast(src, self.type_i8p());
+        let dst = self.pointercast(dst, self.type_i8p(self.cx.address_space_of_value(dst)));
+        let src = self.pointercast(src, self.type_i8p(self.cx.address_space_of_value(src)));
         unsafe {
             llvm::LLVMRustBuildMemMove(
                 self.llbuilder,
@@ -782,7 +782,7 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
         flags: MemFlags,
     ) {
         let is_volatile = flags.contains(MemFlags::VOLATILE);
-        let ptr = self.pointercast(ptr, self.type_i8p());
+        let ptr = self.pointercast(ptr, self.type_i8p(self.cx.address_space_of_value(ptr)));
         unsafe {
             llvm::LLVMRustBuildMemSet(
                 self.llbuilder,
@@ -1275,7 +1275,7 @@ impl Builder<'a, 'll, 'tcx> {
 
         let lifetime_intrinsic = self.cx.get_intrinsic(intrinsic);
 
-        let ptr = self.pointercast(ptr, self.cx.type_i8p());
+        let ptr = self.pointercast(ptr, self.cx.type_i8p(self.cx.address_space_of_value(ptr)));
         self.call(lifetime_intrinsic, &[self.cx.const_u64(size), ptr], None);
     }
 

src/librustc_codegen_llvm/common.rs

@@ -16,7 +16,7 @@ use rustc_middle::bug;
 use rustc_middle::mir::interpret::{Allocation, GlobalAlloc, Scalar};
 use rustc_middle::ty::layout::TyAndLayout;
 use rustc_span::symbol::Symbol;
-use rustc_target::abi::{self, HasDataLayout, LayoutOf, Pointer, Size};
+use rustc_target::abi::{self, AddressSpace, HasDataLayout, LayoutOf, Pointer, Size};
 
 use libc::{c_char, c_uint};
 use log::debug;
@@ -265,7 +265,7 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
                 };
                 let llval = unsafe {
                     llvm::LLVMConstInBoundsGEP(
-                        self.const_bitcast(base_addr, self.type_i8p()),
+                        self.const_bitcast(base_addr, self.type_i8p(AddressSpace::default())),
                         &self.const_usize(ptr.offset.bytes()),
                         1,
                     )
@@ -296,7 +296,7 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
 
             let llval = unsafe {
                 llvm::LLVMConstInBoundsGEP(
-                    self.const_bitcast(base_addr, self.type_i8p()),
+                    self.const_bitcast(base_addr, self.type_i8p(AddressSpace::default())),
                     &self.const_usize(offset.bytes()),
                     1,
                 )

src/librustc_codegen_llvm/consts.rs

@@ -20,7 +20,7 @@ use rustc_middle::ty::{self, Instance, Ty};
 use rustc_middle::{bug, span_bug};
 use rustc_span::symbol::{sym, Symbol};
 use rustc_span::Span;
-use rustc_target::abi::{Align, HasDataLayout, LayoutOf, Primitive, Scalar, Size};
+use rustc_target::abi::{AddressSpace, Align, HasDataLayout, LayoutOf, Primitive, Scalar, Size};
 
 use std::ffi::CStr;
 
@@ -56,7 +56,7 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll
         llvals.push(cx.scalar_to_backend(
             Pointer::new(alloc_id, Size::from_bytes(ptr_offset)).into(),
             &Scalar { value: Primitive::Pointer, valid_range: 0..=!0 },
-            cx.type_i8p(),
+            cx.type_i8p(AddressSpace::default()),
         ));
         next_offset = offset + pointer_size;
     }
@@ -496,7 +496,7 @@ impl StaticMethods for CodegenCx<'ll, 'tcx> {
 
             if attrs.flags.contains(CodegenFnAttrFlags::USED) {
                 // This static will be stored in the llvm.used variable which is an array of i8*
-                let cast = llvm::LLVMConstPointerCast(g, self.type_i8p());
+                let cast = llvm::LLVMConstPointerCast(g, self.type_i8p(AddressSpace::default()));
                 self.used_statics.borrow_mut().push(cast);
             }
         }

src/librustc_codegen_llvm/context.rs

@@ -20,7 +20,9 @@ use rustc_session::config::{CFGuard, CrateType, DebugInfo};
 use rustc_session::Session;
 use rustc_span::source_map::{Span, DUMMY_SP};
 use rustc_span::symbol::Symbol;
-use rustc_target::abi::{HasDataLayout, LayoutOf, PointeeInfo, Size, TargetDataLayout, VariantIdx};
+use rustc_target::abi::{
+    AddressSpace, HasDataLayout, LayoutOf, PointeeInfo, Size, TargetDataLayout, VariantIdx,
+};
 use rustc_target::spec::{HasTargetSpec, RelocModel, Target, TlsModel};
 
 use std::cell::{Cell, RefCell};
@@ -452,7 +454,7 @@ impl CodegenCx<'b, 'tcx> {
             ($($field_ty:expr),*) => (self.type_struct( &[$($field_ty),*], false))
         }
 
-        let i8p = self.type_i8p();
+        let i8p = self.type_i8p(AddressSpace::default());
         let void = self.type_void();
         let i1 = self.type_i1();
         let t_i8 = self.type_i8();

src/librustc_codegen_llvm/intrinsic.rs

@@ -24,7 +24,7 @@ use rustc_middle::ty::{self, Ty};
 use rustc_middle::{bug, span_bug};
 use rustc_span::Span;
 use rustc_span::Symbol;
-use rustc_target::abi::{self, HasDataLayout, LayoutOf, Primitive};
+use rustc_target::abi::{self, AddressSpace, HasDataLayout, LayoutOf, Primitive};
 use rustc_target::spec::PanicStrategy;
 
 use std::cmp::Ordering;
@@ -927,7 +927,7 @@ fn codegen_msvc_try(
         //
         // More information can be found in libstd's seh.rs implementation.
         let ptr_align = bx.tcx().data_layout.pointer_align.abi;
-        let slot = bx.alloca(bx.type_i8p(), ptr_align);
+        let slot = bx.alloca(bx.type_i8p(AddressSpace::default()), ptr_align);
         bx.invoke(try_func, &[data], normal.llbb(), catchswitch.llbb(), None);
 
         normal.ret(bx.const_i32(0));
@@ -949,10 +949,13 @@ fn codegen_msvc_try(
         //
         // When modifying, make sure that the type_name string exactly matches
         // the one used in src/libpanic_unwind/seh.rs.
-        let type_info_vtable = bx.declare_global("??_7type_info@@6B@", bx.type_i8p());
+        let type_info_vtable =
+            bx.declare_global("??_7type_info@@6B@", bx.type_i8p(AddressSpace::default()));
         let type_name = bx.const_bytes(b"rust_panic\0");
-        let type_info =
-            bx.const_struct(&[type_info_vtable, bx.const_null(bx.type_i8p()), type_name], false);
+        let type_info = bx.const_struct(
+            &[type_info_vtable, bx.const_null(bx.type_i8p(AddressSpace::default())), type_name],
+            false,
+        );
         let tydesc = bx.declare_global("__rust_panic_type_info", bx.val_ty(type_info));
         unsafe {
             llvm::LLVMRustSetLinkage(tydesc, llvm::Linkage::LinkOnceODRLinkage);
@@ -1032,14 +1035,14 @@ fn codegen_gnu_try(
         // being thrown.  The second value is a "selector" indicating which of
         // the landing pad clauses the exception's type had been matched to.
         // rust_try ignores the selector.
-        let lpad_ty = bx.type_struct(&[bx.type_i8p(), bx.type_i32()], false);
+        let lpad_ty = bx.type_struct(&[bx.type_i8p(AddressSpace::default()), bx.type_i32()], false);
         let vals = catch.landing_pad(lpad_ty, bx.eh_personality(), 1);
         let tydesc = match bx.tcx().lang_items().eh_catch_typeinfo() {
             Some(tydesc) => {
                 let tydesc = bx.get_static(tydesc);
-                bx.bitcast(tydesc, bx.type_i8p())
+                bx.bitcast(tydesc, bx.type_i8p(AddressSpace::default()))
             }
-            None => bx.const_null(bx.type_i8p()),
+            None => bx.const_null(bx.type_i8p(AddressSpace::default())),
         };
         catch.add_clause(vals, tydesc);
         let ptr = catch.extract_value(vals, 0);

src/librustc_codegen_llvm/llvm/ffi.rs

@@ -1920,6 +1920,8 @@ extern "C" {
     pub fn LLVMRustDIBuilderCreateOpDeref() -> i64;
     pub fn LLVMRustDIBuilderCreateOpPlusUconst() -> i64;
 
+    pub fn LLVMRustGetPointerTypeAddressSpace(ty: &Type) -> u32;
+
     #[allow(improper_ctypes)]
     pub fn LLVMRustWriteTypeToString(Type: &Type, s: &RustString);
     #[allow(improper_ctypes)]

src/librustc_codegen_llvm/type_.rs

@@ -15,7 +15,7 @@ use rustc_middle::bug;
 use rustc_middle::ty::layout::TyAndLayout;
 use rustc_middle::ty::Ty;
 use rustc_target::abi::call::{CastTarget, FnAbi, Reg};
-use rustc_target::abi::{Align, Integer, Size};
+use rustc_target::abi::{AddressSpace, Align, Integer, Size};
 
 use std::fmt;
 use std::ptr;
@@ -198,9 +198,17 @@ impl BaseTypeMethods<'tcx> for CodegenCx<'ll, 'tcx> {
         assert_ne!(
             self.type_kind(ty),
             TypeKind::Function,
-            "don't call ptr_to on function types, use ptr_to_llvm_type on FnAbi instead"
+            "don't call ptr_to on function types, use ptr_to_llvm_type on FnAbi instead or explicitly specify an address space if it makes sense"
         );
-        ty.ptr_to()
+        ty.ptr_to(AddressSpace::default())
+    }
+
+    fn type_ptr_to_ext(&self, ty: &'ll Type, address_space: AddressSpace) -> &'ll Type {
+        ty.ptr_to(address_space)
+    }
+
+    fn address_space_of_type(&self, ty: &'ll Type) -> AddressSpace {
+        ty.address_space()
     }
 
     fn element_type(&self, ty: &'ll Type) -> &'ll Type {
@@ -241,11 +249,16 @@ impl Type {
     }
 
     pub fn i8p_llcx(llcx: &llvm::Context) -> &Type {
-        Type::i8_llcx(llcx).ptr_to()
+        Type::i8_llcx(llcx).ptr_to(AddressSpace::default())
+    }
+
+    pub fn address_space(&self) -> AddressSpace {
+        let space_number = unsafe { llvm::LLVMRustGetPointerTypeAddressSpace(self) };
+        AddressSpace(space_number)
     }
 
-    fn ptr_to(&self) -> &Type {
-        unsafe { llvm::LLVMPointerType(&self, 0) }
+    fn ptr_to(&self, address_space: AddressSpace) -> &Type {
+        unsafe { llvm::LLVMPointerType(&self, address_space.0) }
     }
 }
 

src/librustc_codegen_llvm/va_arg.rs

@@ -8,7 +8,7 @@ use rustc_codegen_ssa::traits::{
 };
 use rustc_middle::ty::layout::HasTyCtxt;
 use rustc_middle::ty::Ty;
-use rustc_target::abi::{Align, HasDataLayout, LayoutOf, Size};
+use rustc_target::abi::{AddressSpace, Align, HasDataLayout, LayoutOf, Size};
 
 #[allow(dead_code)]
 fn round_pointer_up_to_alignment(
@@ -32,7 +32,7 @@ fn emit_direct_ptr_va_arg(
     slot_size: Align,
     allow_higher_align: bool,
 ) -> (&'ll Value, Align) {
-    let va_list_ptr_ty = bx.cx().type_ptr_to(bx.cx.type_i8p());
+    let va_list_ptr_ty = bx.cx().type_ptr_to(bx.cx.type_i8p(AddressSpace::default()));
     let va_list_addr = if list.layout.llvm_type(bx.cx) != va_list_ptr_ty {
         bx.bitcast(list.immediate(), va_list_ptr_ty)
     } else {
@@ -42,7 +42,15 @@ fn emit_direct_ptr_va_arg(
     let ptr = bx.load(va_list_addr, bx.tcx().data_layout.pointer_align.abi);
 
     let (addr, addr_align) = if allow_higher_align && align > slot_size {
-        (round_pointer_up_to_alignment(bx, ptr, align, bx.cx().type_i8p()), align)
+        (
+            round_pointer_up_to_alignment(
+                bx,
+                ptr,
+                align,
+                bx.cx().type_i8p(AddressSpace::default()),
+            ),
+            align,
+        )
     } else {
         (ptr, slot_size)
     };

src/librustc_codegen_ssa/base.rs

@@ -48,7 +48,7 @@ use rustc_session::utils::NativeLibKind;
 use rustc_session::Session;
 use rustc_span::Span;
 use rustc_symbol_mangling::test as symbol_names_test;
-use rustc_target::abi::{Abi, Align, LayoutOf, Scalar, VariantIdx};
+use rustc_target::abi::{Abi, AddressSpace, Align, HasDataLayout, LayoutOf, Scalar, VariantIdx};
 
 use std::cmp;
 use std::ops::{Deref, DerefMut};
@@ -423,7 +423,13 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
         // The entry function is either `int main(void)` or `int main(int argc, char **argv)`,
         // depending on whether the target needs `argc` and `argv` to be passed in.
         let llfty = if cx.sess().target.target.options.main_needs_argc_argv {
-            cx.type_func(&[cx.type_int(), cx.type_ptr_to(cx.type_i8p())], cx.type_int())
+            cx.type_func(
+                &[
+                    cx.type_int(),
+                    cx.type_ptr_to(cx.type_i8p(cx.data_layout().instruction_address_space)),
+                ],
+                cx.type_int(),
+            )
         } else {
             cx.type_func(&[], cx.type_int())
         };
@@ -471,7 +477,11 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             );
             (
                 start_fn,
-                vec![bx.pointercast(rust_main, cx.type_ptr_to(cx.type_i8p())), arg_argc, arg_argv],
+                vec![
+                    bx.pointercast(rust_main, cx.type_ptr_to(cx.type_i8p(AddressSpace::default()))),
+                    arg_argc,
+                    arg_argv,
+                ],
             )
         } else {
             debug!("using user-defined start fn");
@@ -501,7 +511,7 @@ fn get_argc_argv<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     } else {
         // The Rust start function doesn't need `argc` and `argv`, so just pass zeros.
         let arg_argc = bx.const_int(cx.type_int(), 0);
-        let arg_argv = bx.const_null(cx.type_ptr_to(cx.type_i8p()));
+        let arg_argv = bx.const_null(cx.type_ptr_to(cx.type_i8p(AddressSpace::default())));
         (arg_argc, arg_argv)
     }
 }