rust-lang
Fix 32-bit overflows in LLVM composite constants
Inspired by #121868. Fixes unsoundness created when constructing constant arrays, strings, and structs with 2^32 or more elements on x86_64. This introduces copies of a few LLVM functions that have their signatures updated to use size_t in place of unsigned int. Alternatively we could just add overflow checks and just disallow huge composite constants. That introduces less code, but maybe a huge static block of memory is useful in embedded/no-os situations?
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nnethercote (or someone else) some time within the next two weeks.
Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:
-
@rustbot author: the review is finished, PR author should check the comments and take action accordingly -
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue
Would you be interested in upstreaming the new APIs to LLVM? If you do that, then we can directly start using them by providing shims like this:
// Or whatever version it was added
#if LLVM_VERSION_LE(17, 0)
LLVMValueRef LLVMConstArray2(LLVMTypeRef ElementTy, LLVMValueRef *ConstantVals,
uint64_t Length) {
ArrayRef<Constant *> V(unwrap<Constant>(ConstantVals, Length), Length);
return wrap(ConstantArray::get(ArrayType::get(unwrap(ElementTy), Length), V));
}
#endif
That way we'll automatically switch to using the upstream APIs as soon as we can.
@nikic Yeah, I was planning on doing that. I'm not sure how long it would take to get through, so I'm not sure which version we would put in the version macro.
Having 64bit APIs for structs and vectors did not make sense, because LLVM stores 32bit lengths for them anyway. Now we just assert for overflows before calling those functions. I made a pr to LLVM for 64bit ConstantString constructors and used your idea @nikic to create shims to delete later. If the LLVM patch gets through quickly we can update the LLVM_VERSION_LT check to be 19.
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
#13 3.641 Building wheels for collected packages: reuse
#13 3.642 Building wheel for reuse (pyproject.toml): started
#13 3.970 Building wheel for reuse (pyproject.toml): finished with status 'done'
#13 3.970 Created wheel for reuse: filename=reuse-1.1.0-cp310-cp310-manylinux_2_35_x86_64.whl size=181117 sha256=f5f58750481f69515c2c0d1d503daf565e2565c370d07fc6aeb95fe3498b4269
#13 3.971 Stored in directory: /tmp/pip-ephem-wheel-cache-y_7etvq_/wheels/c2/3c/b9/1120c2ab4bd82694f7e6f0537dc5b9a085c13e2c69a8d0c76d
#13 3.973 Installing collected packages: boolean-py, binaryornot, setuptools, reuse, python-debian, markupsafe, license-expression, jinja2, chardet
#13 3.995 Attempting uninstall: setuptools
#13 3.995 Found existing installation: setuptools 59.6.0
#13 3.996 Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
---
Checking rustc_ast_lowering v0.0.0 (/checkout/compiler/rustc_ast_lowering)
error[E0308]: mismatched types
--> compiler/rustc_codegen_llvm/src/common.rs:98:59
|
98 | unsafe { llvm::LLVMConstArray2(ty, elts.as_ptr(), elts.len()) }
| --------------------- ^^^^^^^^^^ expected `u64`, found `usize`
| arguments to this function are incorrect
|
note: function defined here
--> compiler/rustc_codegen_llvm/src/llvm/ffi.rs:939:12
--> compiler/rustc_codegen_llvm/src/llvm/ffi.rs:939:12
|
939 | pub fn LLVMConstArray2<'a>(
| ^^^^^^^^^^^^^^^
help: you can convert a `usize` to a `u64` and panic if the converted value doesn't fit
|
98 | unsafe { llvm::LLVMConstArray2(ty, elts.as_ptr(), elts.len().try_into().unwrap()) }
For more information about this error, try `rustc --explain E0308`.
error: could not compile `rustc_codegen_llvm` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
