[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 set to
the instruction address space because the logic is dealing with functions
which must be placed there, or 2) 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

@@ -18,7 +18,7 @@ use rustc_data_structures::small_c_str::SmallCStr;
 use rustc_hir::def_id::DefId;
 use rustc_middle::ty::layout::TyAndLayout;
 use rustc_middle::ty::{self, Ty, TyCtxt};
-use rustc_target::abi::{self, Align, Size};
+use rustc_target::abi::{self, AddressSpace, Align, Size};
 use rustc_target::spec::{HasTargetSpec, Target};
 use std::borrow::Cow;
 use std::ffi::CStr;
@@ -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(AddressSpace::DATA));
+        let src = self.pointercast(src, self.type_i8p(AddressSpace::DATA));
         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(AddressSpace::DATA));
+        let src = self.pointercast(src, self.type_i8p(AddressSpace::DATA));
         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(AddressSpace::DATA));
         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(AddressSpace::DATA));
         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;
@@ -244,7 +244,7 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
                 }
             }
             Scalar::Ptr(ptr) => {
-                let base_addr = match self.tcx.global_alloc(ptr.alloc_id) {
+                let (base_addr, base_addr_space) = match self.tcx.global_alloc(ptr.alloc_id) {
                     GlobalAlloc::Memory(alloc) => {
                         let init = const_alloc_to_llvm(self, alloc);
                         let value = match alloc.mutability {
@@ -254,18 +254,21 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
                         if !self.sess().fewer_names() {
                             llvm::set_value_name(value, format!("{:?}", ptr.alloc_id).as_bytes());
                         }
-                        value
+                        (value, AddressSpace::DATA)
                     }
-                    GlobalAlloc::Function(fn_instance) => self.get_fn_addr(fn_instance),
+                    GlobalAlloc::Function(fn_instance) => (
+                        self.get_fn_addr(fn_instance),
+                        self.data_layout().instruction_address_space,
+                    ),
                     GlobalAlloc::Static(def_id) => {
                         assert!(self.tcx.is_static(def_id));
                         assert!(!self.tcx.is_thread_local_static(def_id));
-                        self.get_static(def_id)
+                        (self.get_static(def_id), AddressSpace::DATA)
                     }
                 };
                 let llval = unsafe {
                     llvm::LLVMConstInBoundsGEP(
-                        self.const_bitcast(base_addr, self.type_i8p()),
+                        self.const_bitcast(base_addr, self.type_i8p(base_addr_space)),
                         &self.const_usize(ptr.offset.bytes()),
                         1,
                     )
@@ -296,7 +299,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::DATA)),
                     &self.const_usize(offset.bytes()),
                     1,
                 )

src/librustc_codegen_llvm/consts.rs

@@ -13,14 +13,14 @@ use rustc_hir::def_id::DefId;
 use rustc_hir::Node;
 use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
 use rustc_middle::mir::interpret::{
-    read_target_uint, Allocation, ConstValue, ErrorHandled, Pointer,
+    read_target_uint, Allocation, ConstValue, ErrorHandled, GlobalAlloc, Pointer,
 };
 use rustc_middle::mir::mono::MonoItem;
 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;
 
@@ -53,10 +53,16 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll
         )
         .expect("const_alloc_to_llvm: could not read relocation pointer")
             as u64;
+
+        let address_space = match cx.tcx.global_alloc(alloc_id) {
+            GlobalAlloc::Function(..) => cx.data_layout().instruction_address_space,
+            GlobalAlloc::Static(..) | GlobalAlloc::Memory(..) => AddressSpace::DATA,
+        };
+
         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(address_space),
         ));
         next_offset = offset + pointer_size;
     }
@@ -496,7 +502,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::DATA));
                 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::DATA);
         let void = self.type_void();
         let i1 = self.type_i1();
         let t_i8 = self.type_i8();

src/librustc_codegen_llvm/intrinsic.rs

@@ -23,7 +23,7 @@ use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt};
 use rustc_middle::ty::{self, Ty};
 use rustc_middle::{bug, span_bug};
 use rustc_span::Span;
-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;
@@ -930,7 +930,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::DATA), ptr_align);
         bx.invoke(try_func, &[data], normal.llbb(), catchswitch.llbb(), None);
 
         normal.ret(bx.const_i32(0));
@@ -952,10 +952,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::DATA));
         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::DATA)), type_name],
+            false,
+        );
         let tydesc = bx.declare_global("__rust_panic_type_info", bx.val_ty(type_info));
         unsafe {
             llvm::LLVMRustSetLinkage(tydesc, llvm::Linkage::LinkOnceODRLinkage);
@@ -1035,14 +1038,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::DATA), 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::DATA))
             }
-            None => bx.const_null(bx.type_i8p()),
+            None => bx.const_null(bx.type_i8p(AddressSpace::DATA)),
         };
         catch.add_clause(vals, tydesc);
         let ptr = catch.extract_value(vals, 0);

src/librustc_codegen_llvm/llvm/ffi.rs

@@ -1334,12 +1334,6 @@ extern "C" {
         DestTy: &'a Type,
         Name: *const c_char,
     ) -> &'a Value;
-    pub fn LLVMBuildPointerCast(
-        B: &Builder<'a>,
-        Val: &'a Value,
-        DestTy: &'a Type,
-        Name: *const c_char,
-    ) -> &'a Value;
     pub fn LLVMRustBuildIntCast(
         B: &Builder<'a>,
         Val: &'a Value,

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,13 @@ 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::DATA)
+    }
+
+    fn type_ptr_to_ext(&self, ty: &'ll Type, address_space: AddressSpace) -> &'ll Type {
+        ty.ptr_to(address_space)
     }
 
     fn element_type(&self, ty: &'ll Type) -> &'ll Type {
@@ -241,11 +245,11 @@ impl Type {
     }
 
     pub fn i8p_llcx(llcx: &llvm::Context) -> &Type {
-        Type::i8_llcx(llcx).ptr_to()
+        Type::i8_llcx(llcx).ptr_to(AddressSpace::DATA)
     }
 
-    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::DATA));
     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,7 @@ 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::DATA)), 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, LayoutOf, Scalar, VariantIdx};
 
 use std::cmp;
 use std::ops::{Deref, DerefMut};
@@ -423,7 +423,10 @@ 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(AddressSpace::DATA))],
+                cx.type_int(),
+            )
         } else {
             cx.type_func(&[], cx.type_int())
         };
@@ -471,7 +474,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::DATA))),
+                    arg_argc,
+                    arg_argv,
+                ],
             )
         } else {
             debug!("using user-defined start fn");
@@ -501,7 +508,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::DATA)));
         (arg_argc, arg_argv)
     }
 }

src/librustc_codegen_ssa/meth.rs

@@ -75,7 +75,7 @@ pub fn get_vtable<'tcx, Cx: CodegenMethods<'tcx>>(
     }
 
     // Not in the cache; build it.
-    let nullptr = cx.const_null(cx.type_i8p());
+    let nullptr = cx.const_null(cx.type_i8p(cx.data_layout().instruction_address_space));
 
     let methods_root;
     let methods = if let Some(trait_ref) = trait_ref {

src/librustc_codegen_ssa/mir/block.rs

@@ -19,7 +19,7 @@ use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt};
 use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
 use rustc_span::{source_map::Span, symbol::Symbol};
 use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode};
-use rustc_target::abi::{self, LayoutOf};
+use rustc_target::abi::{self, AddressSpace, LayoutOf};
 use rustc_target::spec::abi::Abi;
 
 use std::borrow::Cow;
@@ -1247,7 +1247,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
 
     fn landing_pad_type(&self) -> Bx::Type {
         let cx = self.cx;
-        cx.type_struct(&[cx.type_i8p(), cx.type_i32()], false)
+        cx.type_struct(&[cx.type_i8p(AddressSpace::DATA), cx.type_i32()], false)
     }
 
     fn unreachable_block(&mut self) -> Bx::BasicBlock {

src/librustc_codegen_ssa/mir/mod.rs

@@ -6,6 +6,7 @@ use rustc_middle::mir::interpret::ErrorHandled;
 use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt, TyAndLayout};
 use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
 use rustc_target::abi::call::{FnAbi, PassMode};
+use rustc_target::abi::HasDataLayout;
 
 use std::iter;
 
@@ -323,7 +324,8 @@ fn create_funclets<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
                     // C++ personality function, but `catch (...)` has no type so
                     // it's null. The 64 here is actually a bitfield which
                     // represents that this is a catch-all block.
-                    let null = bx.const_null(bx.type_i8p());
+                    let null =
+                        bx.const_null(bx.type_i8p(bx.cx().data_layout().instruction_address_space));
                     let sixty_four = bx.const_i32(64);
                     funclet = cp_bx.catch_pad(cs, &[null, sixty_four, null]);
                     cp_bx.br(llbb);