Skip to content

Commit df552b3

Browse files
authored
Rollup merge of #91070 - cuviper:insert-global, r=nagisa
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.
2 parents 789d168 + 3aa1954 commit df552b3

File tree

3 files changed

+69
-1
lines changed

3 files changed

+69
-1
lines changed

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

+11-1
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,18 @@ extern "C" LLVMValueRef LLVMRustGetOrInsertFunction(LLVMModuleRef M,
124124

125125
extern "C" LLVMValueRef
126126
LLVMRustGetOrInsertGlobal(LLVMModuleRef M, const char *Name, size_t NameLen, LLVMTypeRef Ty) {
127+
Module *Mod = unwrap(M);
127128
StringRef NameRef(Name, NameLen);
128-
return wrap(unwrap(M)->getOrInsertGlobal(NameRef, unwrap(Ty)));
129+
130+
// We don't use Module::getOrInsertGlobal because that returns a Constant*,
131+
// which may either be the real GlobalVariable*, or a constant bitcast of it
132+
// if our type doesn't match the original declaration. We always want the
133+
// GlobalVariable* so we can access linkage, visibility, etc.
134+
GlobalVariable *GV = Mod->getGlobalVariable(NameRef, true);
135+
if (!GV)
136+
GV = new GlobalVariable(*Mod, unwrap(Ty), false,
137+
GlobalValue::ExternalLinkage, nullptr, NameRef);
138+
return wrap(GV);
129139
}
130140

131141
extern "C" LLVMValueRef

src/test/ui/statics/issue-91050-1.rs

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// build-pass
2+
// compile-flags: --crate-type=rlib --emit=llvm-ir -Cno-prepopulate-passes
3+
4+
// This test declares globals by the same name with different types, which
5+
// caused problems because Module::getOrInsertGlobal would return a Constant*
6+
// bitcast instead of a GlobalVariable* that could access linkage/visibility.
7+
// In alt builds with LLVM assertions this would fail:
8+
//
9+
// rustc: /checkout/src/llvm-project/llvm/include/llvm/Support/Casting.h:269:
10+
// typename cast_retty<X, Y *>::ret_type llvm::cast(Y *) [X = llvm::GlobalValue, Y = llvm::Value]:
11+
// Assertion `isa<X>(Val) && "cast<Ty>() argument of incompatible type!"' failed.
12+
//
13+
// In regular builds, the bad cast was UB, like "Invalid LLVMRustVisibility value!"
14+
15+
pub mod before {
16+
#[no_mangle]
17+
pub static GLOBAL1: [u8; 1] = [1];
18+
}
19+
20+
pub mod inner {
21+
extern "C" {
22+
pub static GLOBAL1: u8;
23+
pub static GLOBAL2: u8;
24+
}
25+
26+
pub fn call() {
27+
drop(unsafe { (GLOBAL1, GLOBAL2) });
28+
}
29+
}
30+
31+
pub mod after {
32+
#[no_mangle]
33+
pub static GLOBAL2: [u8; 1] = [2];
34+
}

src/test/ui/statics/issue-91050-2.rs

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// build-pass
2+
// compile-flags: --crate-type=rlib --emit=llvm-ir -Cno-prepopulate-passes
3+
4+
// This is a variant of issue-91050-1.rs -- see there for an explanation.
5+
6+
pub mod before {
7+
extern "C" {
8+
pub static GLOBAL1: [u8; 1];
9+
}
10+
11+
pub unsafe fn do_something_with_array() -> u8 {
12+
GLOBAL1[0]
13+
}
14+
}
15+
16+
pub mod inner {
17+
extern "C" {
18+
pub static GLOBAL1: u8;
19+
}
20+
21+
pub unsafe fn call() -> u8 {
22+
GLOBAL1 + 42
23+
}
24+
}

0 commit comments

Comments
 (0)