Skip to content

Commit 6047243

Browse files
Rollup merge of #145867 - Zalathar:range-attr, r=nikic
cg_llvm: Assert that LLVM range-attribute values don't exceed 128 bits The underlying implementation of `LLVMCreateConstantRangeAttribute` assumes that each of `LowerWords` and `UpperWords` points to enough u64 values to define an integer of the specified bit-length, and will encounter UB if that is not the case. Our safe wrapper function always passes pointers to `[u64; 2]` arrays, regardless of the bit-length specified. That's fine in practice, because scalar primitives never exceed 128 bits, but it is technically a soundness hole in a safe function. We can close the soundness hole by explicitly asserting `size_bits <= 128`. This is effectively just a stricter version of the existing check that the value must be small enough to fit in `c_uint`. --- This is a narrower version of the fix in #145846.
2 parents 5d95ec0 + fcff8f7 commit 6047243

File tree

3 files changed

+30
-11
lines changed

3 files changed

+30
-11
lines changed

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1929,11 +1929,17 @@ unsafe extern "C" {
19291929
C: &Context,
19301930
effects: MemoryEffects,
19311931
) -> &Attribute;
1932+
/// ## Safety
1933+
/// - Each of `LowerWords` and `UpperWords` must point to an array that is
1934+
/// long enough to fully define an integer of size `NumBits`, i.e. each
1935+
/// pointer must point to `NumBits.div_ceil(64)` elements or more.
1936+
/// - The implementation will make its own copy of the pointed-to `u64`
1937+
/// values, so the pointers only need to outlive this function call.
19321938
pub(crate) fn LLVMRustCreateRangeAttribute(
19331939
C: &Context,
1934-
num_bits: c_uint,
1935-
lower_words: *const u64,
1936-
upper_words: *const u64,
1940+
NumBits: c_uint,
1941+
LowerWords: *const u64,
1942+
UpperWords: *const u64,
19371943
) -> &Attribute;
19381944

19391945
// Operations on functions

compiler/rustc_codegen_llvm/src/llvm/mod.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,16 +112,26 @@ pub(crate) fn CreateAllocKindAttr(llcx: &Context, kind_arg: AllocKindFlags) -> &
112112

113113
pub(crate) fn CreateRangeAttr(llcx: &Context, size: Size, range: WrappingRange) -> &Attribute {
114114
let lower = range.start;
115+
// LLVM treats the upper bound as exclusive, but allows wrapping.
115116
let upper = range.end.wrapping_add(1);
116-
let lower_words = [lower as u64, (lower >> 64) as u64];
117-
let upper_words = [upper as u64, (upper >> 64) as u64];
117+
118+
// Pass each `u128` endpoint value as a `[u64; 2]` array, least-significant part first.
119+
let as_u64_array = |x: u128| [x as u64, (x >> 64) as u64];
120+
let lower_words: [u64; 2] = as_u64_array(lower);
121+
let upper_words: [u64; 2] = as_u64_array(upper);
122+
123+
// To ensure that LLVM doesn't try to read beyond the `[u64; 2]` arrays,
124+
// we must explicitly check that `size_bits` does not exceed 128.
125+
let size_bits = size.bits();
126+
assert!(size_bits <= 128);
127+
// More robust assertions that are redundant with `size_bits <= 128` and
128+
// should be optimized away.
129+
assert!(size_bits.div_ceil(64) <= u64::try_from(lower_words.len()).unwrap());
130+
assert!(size_bits.div_ceil(64) <= u64::try_from(upper_words.len()).unwrap());
131+
let size_bits = c_uint::try_from(size_bits).unwrap();
132+
118133
unsafe {
119-
LLVMRustCreateRangeAttribute(
120-
llcx,
121-
size.bits().try_into().unwrap(),
122-
lower_words.as_ptr(),
123-
upper_words.as_ptr(),
124-
)
134+
LLVMRustCreateRangeAttribute(llcx, size_bits, lower_words.as_ptr(), upper_words.as_ptr())
125135
}
126136
}
127137

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,9 @@ extern "C" LLVMAttributeRef
488488
LLVMRustCreateRangeAttribute(LLVMContextRef C, unsigned NumBits,
489489
const uint64_t LowerWords[],
490490
const uint64_t UpperWords[]) {
491+
// FIXME(Zalathar): There appears to be no stable guarantee that C++
492+
// `AttrKind` values correspond directly to the `unsigned KindID` values
493+
// accepted by LLVM-C API functions, though in practice they currently do.
491494
return LLVMCreateConstantRangeAttribute(C, Attribute::Range, NumBits,
492495
LowerWords, UpperWords);
493496
}

0 commit comments

Comments
 (0)