Skip to content

Commit

Permalink
Rollup merge of #91070 - cuviper:insert-global, r=nagisa
Browse files Browse the repository at this point in the history
Make `LLVMRustGetOrInsertGlobal` always return a `GlobalVariable`

`Module::getOrInsertGlobal` returns a `Constant*`, which is a super
class of `GlobalVariable`, but if the given type doesn't match an
existing declaration, it returns a bitcast of that global instead.
This causes UB when we pass that to `LLVMGetVisibility` which
unconditionally casts the opaque argument to a `GlobalValue*`.

Instead, we can do our own get-or-insert without worrying whether
existing types match exactly. It's not relevant when we're just trying
to get/set the linkage and visibility, and if types are needed we can
bitcast or error nicely from `rustc_codegen_llvm` instead.

Fixes #91050, fixes #87933, fixes #87813.
  • Loading branch information
matthiaskrgr authored Nov 21, 2021
2 parents 789d168 + 3aa1954 commit df552b3
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 1 deletion.
12 changes: 11 additions & 1 deletion compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,18 @@ extern "C" LLVMValueRef LLVMRustGetOrInsertFunction(LLVMModuleRef M,

extern "C" LLVMValueRef
LLVMRustGetOrInsertGlobal(LLVMModuleRef M, const char *Name, size_t NameLen, LLVMTypeRef Ty) {
Module *Mod = unwrap(M);
StringRef NameRef(Name, NameLen);
return wrap(unwrap(M)->getOrInsertGlobal(NameRef, unwrap(Ty)));

// We don't use Module::getOrInsertGlobal because that returns a Constant*,
// which may either be the real GlobalVariable*, or a constant bitcast of it
// if our type doesn't match the original declaration. We always want the
// GlobalVariable* so we can access linkage, visibility, etc.
GlobalVariable *GV = Mod->getGlobalVariable(NameRef, true);
if (!GV)
GV = new GlobalVariable(*Mod, unwrap(Ty), false,
GlobalValue::ExternalLinkage, nullptr, NameRef);
return wrap(GV);
}

extern "C" LLVMValueRef
Expand Down
34 changes: 34 additions & 0 deletions src/test/ui/statics/issue-91050-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// build-pass
// compile-flags: --crate-type=rlib --emit=llvm-ir -Cno-prepopulate-passes

// This test declares globals by the same name with different types, which
// caused problems because Module::getOrInsertGlobal would return a Constant*
// bitcast instead of a GlobalVariable* that could access linkage/visibility.
// In alt builds with LLVM assertions this would fail:
//
// rustc: /checkout/src/llvm-project/llvm/include/llvm/Support/Casting.h:269:
// typename cast_retty<X, Y *>::ret_type llvm::cast(Y *) [X = llvm::GlobalValue, Y = llvm::Value]:
// Assertion `isa<X>(Val) && "cast<Ty>() argument of incompatible type!"' failed.
//
// In regular builds, the bad cast was UB, like "Invalid LLVMRustVisibility value!"

pub mod before {
#[no_mangle]
pub static GLOBAL1: [u8; 1] = [1];
}

pub mod inner {
extern "C" {
pub static GLOBAL1: u8;
pub static GLOBAL2: u8;
}

pub fn call() {
drop(unsafe { (GLOBAL1, GLOBAL2) });
}
}

pub mod after {
#[no_mangle]
pub static GLOBAL2: [u8; 1] = [2];
}
24 changes: 24 additions & 0 deletions src/test/ui/statics/issue-91050-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// build-pass
// compile-flags: --crate-type=rlib --emit=llvm-ir -Cno-prepopulate-passes

// This is a variant of issue-91050-1.rs -- see there for an explanation.

pub mod before {
extern "C" {
pub static GLOBAL1: [u8; 1];
}

pub unsafe fn do_something_with_array() -> u8 {
GLOBAL1[0]
}
}

pub mod inner {
extern "C" {
pub static GLOBAL1: u8;
}

pub unsafe fn call() -> u8 {
GLOBAL1 + 42
}
}

0 comments on commit df552b3

Please sign in to comment.