-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Properly fail on type size overflows in LLVM #18041
Conversation
ebfbbba
to
73f1c7d
Compare
73f1c7d
to
93a56ee
Compare
3a95cef
to
943ecbb
Compare
Does it pass on 32-bit as well? ( |
943ecbb
to
1ee345a
Compare
Use the integer sizes LLVM uses, rather than having random projections laying around. Sizes are u64, Alignments are u32, C_*int is target-dependent but 64-bit is fine (the int -> C_int conversion is non-precision-losing, but it can be preceded by `as int` conversions which are, so it is somewhat ugly. However, being able to suffix a `u` to properly infer integer types is nice).
LLVM generates wrong code (which may be an instance of compile-time UB) when faced with types that take lots of memory - bigger than the address space. Make using such types a trans error. While trans errors are bad, overbig types are expected to be very rare.
The tests should all pass on 32-bit as well. Actually currently I have a compile-time-object size limit of 2GiB on both 32 and 64-bit architectures (because I was too lazy to make it work properly). |
I'll review this. @alexcrichton any idea why high-five didn't kick in here? |
OK, so I looked it over; it looks close, I had a few questions. |
Oh, also, because I didn't say it explicitly: thanks a lot for tackling this so promptly! I'm excited to see this hole get closed. |
@nikomatsakis I think that @nick29581 hasn't set up high-5 to work on all PRs just yet, it's just new contributors and those with |
70211f6
to
bf36f35
Compare
@arielb1 Thanks. I'd still like to see the logic in the coment that rounds up fieldsize etc spelled out a bit more (presuming you don't want to use checked adds) -- also, can you add a comment to the |
bf36f35
to
61ab2ea
Compare
If const calculations don't truncate integers before they get here divisions would get the wrong result – so that would be a bug. |
On 32-bit architectures, the size calculations on two of the tests wrap-around in typeck, which gives the relevant arrays a size of 0, which is (correctly) successfully allocated.
Should fix #17913. Also clean-up u64/u32-ness. I really should split this commit and add tests (I have no idea how to add them).
feat: better name suggestions for fn fix rust-lang#17631. Better name suggestions for fn-calls / method-calls in the form of `from()`, `from_xxx()`, `into()`, etc.
Should fix #17913.
Also clean-up u64/u32-ness. I really should split this commit and add tests (I have no idea how to add them).