llvm::Module::getOrInsertGlobal returns a Constant*, not always GlobalValue*

[cuviper]

xref: https://bugzilla.redhat.com/show_bug.cgi?id=1990657

In Fedora, one particular crate has been seeing non-deterministic errors or crashes in the compiler, and I was able to reproduce it with a rustup-installed compiler as well. Valgrind gave the following error report, even on runs that otherwise appeared to succeed.

Valgrind error

==325== Invalid read of size 1
==325==    at 0x93E6CF4: getVisibility (GlobalValue.h:229)
==325==    by 0x93E6CF4: LLVMGetVisibility (Core.cpp:1992)
==325==    by 0x4F6D05C: LLVMRustGetVisibility (RustWrapper.cpp:1602)
==325==    by 0x51AE144: rustc_codegen_llvm::mono_item::<impl rustc_codegen_llvm::context::CodegenCx>::should_assume_dso_local (mono_item.rs:106)
==325==    by 0x51A41DF: rustc_codegen_llvm::consts::<impl rustc_codegen_llvm::context::CodegenCx>::get_static (consts.rs:289)
==325==    by 0x51A1E05: rustc_codegen_llvm::common::<impl rustc_codegen_ssa::traits::consts::ConstMethods for rustc_codegen_llvm::context::CodegenCx>::scalar_to_backend (common.rs:267)
==325==    by 0x521D477: rustc_codegen_ssa::mir::operand::OperandRef<V>::from_const (operand.rs:85)
==325==    by 0x523D07A: eval_mir_constant_to_operand<rustc_codegen_llvm::builder::Builder> (constant.rs:20)
==325==    by 0x523D07A: rustc_codegen_ssa::mir::operand::<impl rustc_codegen_ssa::mir::FunctionCx<Bx>>::codegen_operand (operand.rs:450)
==325==    by 0x5238B33: rustc_codegen_ssa::mir::rvalue::<impl rustc_codegen_ssa::mir::FunctionCx<Bx>>::codegen_rvalue_operand (rvalue.rs:546)
==325==    by 0x522DB68: codegen_statement<rustc_codegen_llvm::builder::Builder> (statement.rs:24)
==325==    by 0x522DB68: codegen_block<rustc_codegen_llvm::builder::Builder> (block.rs:901)
==325==    by 0x522DB68: rustc_codegen_ssa::mir::codegen_mir (mod.rs:258)
==325==    by 0x51B6E09: rustc_codegen_ssa::base::codegen_instance (base.rs:342)
==325==    by 0x51E249C: <rustc_middle::mir::mono::MonoItem as rustc_codegen_ssa::mono_item::MonoItemExt>::define (mono_item.rs:70)
==325==    by 0x51F713E: rustc_codegen_llvm::base::compile_codegen_unit::module_codegen (base.rs:141)
==325==  Address 0x1734a400 is 8 bytes after a block of size 56 alloc'd
==325==    at 0x4840FF5: operator new(unsigned long) (vg_replace_malloc.c:417)
==325==    by 0x94DDFE4: allocateFixedOperandUser (User.cpp:127)
==325==    by 0x94DDFE4: llvm::User::operator new(unsigned long, unsigned int) (User.cpp:146)
==325==    by 0x93CE0C6: operator new (ConstantsContext.h:55)
==325==    by 0x93CE0C6: llvm::ConstantExprKeyType::create(llvm::Type*) const (ConstantsContext.h:612)
==325==    by 0x93DA482: create (ConstantsContext.h:715)
==325==    by 0x93DA482: llvm::ConstantUniqueMap<llvm::ConstantExpr>::getOrCreate(llvm::Type*, llvm::ConstantExprKeyType) (ConstantsContext.h:734)
==325==    by 0x93E02F2: getFoldedCast (Constants.cpp:1937)
==325==    by 0x93E02F2: getBitCast (Constants.cpp:2194)
==325==    by 0x93E02F2: llvm::ConstantExpr::getBitCast(llvm::Constant*, llvm::Type*, bool) (Constants.cpp:2185)
==325==    by 0x94AA7E0: llvm::Module::getOrInsertGlobal(llvm::StringRef, llvm::Type*) (Module.cpp:226)
==325==    by 0x51C67D5: declare_global (declare.rs:60)
==325==    by 0x51C67D5: rustc_codegen_llvm::consts::check_and_apply_linkage (consts.rs:157)
==325==    by 0x51A34BC: rustc_codegen_llvm::consts::<impl rustc_codegen_llvm::context::CodegenCx>::get_static (consts.rs:234)
==325==    by 0x51A1E05: rustc_codegen_llvm::common::<impl rustc_codegen_ssa::traits::consts::ConstMethods for rustc_codegen_llvm::context::CodegenCx>::scalar_to_backend (common.rs:267)
==325==    by 0x521D477: rustc_codegen_ssa::mir::operand::OperandRef<V>::from_const (operand.rs:85)
==325==    by 0x523D07A: eval_mir_constant_to_operand<rustc_codegen_llvm::builder::Builder> (constant.rs:20)
==325==    by 0x523D07A: rustc_codegen_ssa::mir::operand::<impl rustc_codegen_ssa::mir::FunctionCx<Bx>>::codegen_operand (operand.rs:450)
==325==    by 0x5238B33: rustc_codegen_ssa::mir::rvalue::<impl rustc_codegen_ssa::mir::FunctionCx<Bx>>::codegen_rvalue_operand (rvalue.rs:546)

Note that llvm::Module::getOrInsertGlobal returns a Constant*, but LLVMGetVisibility casts its argument to a GlobalValue*, which is a subclass. Most of the time you do get a GlobalVariable* (a further subclass), except when getOrInsertGlobal is given different types it instead returns a constant bitcast expression, as you can see in the error backtrace with getBitCast.

I ran a new rustc with LLVM assertions, and it does fail there trying to cast GlobalValue*:

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.

So, casting the wrong pointer type is Undefined Behavior, and the non-reproducible aspect of this bug is just "luck" of whatever happens to be in memory there.

I'm not yet sure why rustc would have a type mismatch in what it's feeding getOrInsertGlobal, but my first suspicion is this real_name indirection in check_and_apply_linkage.

rust/compiler/rustc_codegen_llvm/src/consts.rs

Lines 180 to 200 in cc946fc

	// Declare a symbol `foo` with the desired linkage.
	let g1 = cx.declare_global(sym, llty2);
	llvm::LLVMRustSetLinkage(g1, base::linkage_to_llvm(linkage));
	// Declare an internal global `extern_with_linkage_foo` which
	// is initialized with the address of `foo`. If `foo` is
	// discarded during linking (for example, if `foo` has weak
	// linkage and there are no definitions), then
	// `extern_with_linkage_foo` will instead be initialized to
	// zero.
	let mut real_name = "_rust_extern_with_linkage_".to_string();
	real_name.push_str(sym);
	let g2 = cx.define_global(&real_name, llty).unwrap_or_else(|| {
	cx.sess().span_fatal(
	cx.tcx.def_span(span_def_id),
	&format!("symbol `{}` is already defined", &sym),
	)
	});
	llvm::LLVMRustSetLinkage(g2, llvm::Linkage::InternalLinkage);
	llvm::LLVMSetInitializer(g2, g1);
	g2

[tmiasko]

Related bug reports #87933 #87813. One case when casts are inserted is when a global is redeclared with a different type.

[cuviper]

One case when casts are inserted is when a global is redeclared with a different type.

Ah, that does seem to be the case in the Fedora bug with sha1collisiondetection.

https://gitlab.com/sequoia-pgp/sha1collisiondetection/-/blob/main/lib/ubc_check.rs#L105

#[no_mangle]
pub static mut sha1_dvs: [dv_info_t; 33] =

https://gitlab.com/sequoia-pgp/sha1collisiondetection/-/blob/main/lib/sha1.rs#L4

extern "C" {
    static mut sha1_dvs: [dv_info_t; 0];

[wgianopoulos]

Nice to know this is fixed, but will there be a 1.57.1 with the fix? I this fixed in 1.58.0-beta? do we need to wait for 1.59.0?

[cuviper]

We considered a backport in the discussion starting with #91070 (comment), but decided against it because it was a longstanding problem, not a new regression. That fix is in 1.58.0-beta though, yes.

[wgianopoulos]

Great! Having the fix in 1.58.0-beta is sufficient for my needs. Reason I asked is that the issue just started to show up in mozilla-central recently so although the problem existed in earlier versions of rust for a long time, for some reason it just started to show up in my mozilla-central builds.

…

On Tue, Dec 7, 2021 at 5:41 PM Josh Stone ***@***.***> wrote: We considered a backport in the discussion starting with #91070 (comment) <#91070 (comment)>, but decided against it because it was a longstanding problem, not a new regression. That fix is in 1.58.0-beta though, yes. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#91050 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABXPJUZXQQLSNONIBVHETGLUP2ESJANCNFSM5IM2EVBA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.