Correct outdated object size limit #127546
Conversation
This comment has been minimized.
This comment has been minimized.
rustbot
added
A-testsuite
tests/ui/limits/huge-static.stderr
Outdated
| | | ||
| LL | static MY_TOO_BIG_ARRAY_2: [u8; HUGE_SIZE] = [0x00; HUGE_SIZE]; | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ values of the type `[u8; 2305843009213693951]` are too big for the current architecture | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ values of the type `[u8; 2305843009213693952]` are too big for the current architecture |
There was a problem hiding this comment.
yes, it was literally 1 byte too small to be oversized (at least, according to the relevant check).
This comment has been minimized.
This comment has been minimized.
|
I don't know how to fix the gcc errors? |
|
wait these are the 32-bit errors... good, actually, I was wondering. |
The comment here about 48 bit addresses being enough was written in 2016 but was made incorrect in 2019 by 5-level paging, and then persisted for another 5 years before being noticed and corrected.
These tests depend on the internal logic of rustc regarding handling very large objects. Fix them to reflect rustc_abi::obj_size_bound diffs.
workingjubilee
force-pushed
the
5-level-paging-exists
branch
2 times, most recently
from
98c3e20
to
f04e444
Compare
This comment has been minimized.
This comment has been minimized.
workingjubilee
force-pushed
the
5-level-paging-exists
branch
from
f04e444
to
2b99a34
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
@bors try |
…, r=<try> Correct outdated object size limit The comment here about 48 bit addresses being enough was written in 2016 but was made incorrect in 2019 by 5-level paging, and then persisted for another 5 years before being noticed and corrected. The bolding of the "exclusive" part is merely to call attention to something I missed when reading it and doublechecking the math. try-job: i686-msvc try-job: test-various
|
☀️ Try build successful - checks-actions |
|
Having read the error text here a zillion times, I now want it to say something like "are too big for the target architecture (x86_64)" because "current" could mean the host. If you agree, you can do that here. Otherwise, r=me. |
|
It is done. Adding the architecture as a param is a cool idea, but requires unifying the source of this error message. There are at least 4 separate instances of it, and the moment I attempted such a unification, a herd of yaks appeared, so I will not be doing that. |
This comment has been minimized.
This comment has been minimized.
|
The Miri subtree was changed cc @rust-lang/miri |
|
@rustbot ready |
rustbot
added
the
S-waiting-on-review
|
@bors r+ rollup=iffy |
workingjubilee
force-pushed
the
5-level-paging-exists
branch
from
a3e661f
to
d93d2f1
Compare
|
@bors r=saethlin |
…, r=saethlin Correct outdated object size limit The comment here about 48 bit addresses being enough was written in 2016 but was made incorrect in 2019 by 5-level paging, and then persisted for another 5 years before being noticed and corrected. The bolding of the "exclusive" part is merely to call attention to something I missed when reading it and doublechecking the math. try-job: i686-msvc try-job: test-various
This comment has been minimized.
This comment has been minimized.
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
| @@ -337,23 +337,21 @@ impl TargetDataLayout { | |||
| Ok(dl) | |||
| } | |||
|
|
|||
| /// Returns exclusive upper bound on object size. | |||
| /// Returns **exclusive** upper bound on object size. | |||
| /// | |||
| /// The theoretical maximum object size is defined as the maximum positive `isize` value. | |||
There was a problem hiding this comment.
I am not entirely sure what is even meant by "object". In general we are saying that "allocated objects" are limited by isize::MAX, and we do not publicly document anything else even for 64bit targets. So we do currently allow people to create heap allocations larger than obj_size_bound, and we rely on LLVM supporting that. It is only Rust types that can't be any bigger, and therefore static and let-bound variables and Box.
So either we should clarify the comment to say that this does not apply to all heap allocations, or we need to fix our docs.
@nikic will LLVM miscompile things if a heap allocation gets bigger than 2^61 bytes because that overflows u64 when counting in bits? Or does the 2^64 bits limit only apply to LLVM "typed objects"? Or is this too theoretical because nothing can be that big anyway? ;)
There was a problem hiding this comment.
I think it would only be a problem if it's a typed allocation/access, but I'm not particularly confident in that :)
There was a problem hiding this comment.
anyone got a 64-bit address space computer lying around to find out?
There was a problem hiding this comment.
I am not entirely sure what is even meant by "object".
Honestly I'm also not completely confident... I think allocation is what is meant?
I was pretty much just rolling with it because "we have a slightly wibbly definition of 'object' deep in the codegen backend" feels kinda low on our list of concerns. Relatively speaking.
|
I have no idea what's up with the failing test and it looks like nobody else knows eithe.r |
Co-authored-by: Ralf Jung <[email protected]>
|
I'm going to give this one retry before I sit down with that test and have a nice long chat with it. @bors retry |
bors
added
S-waiting-on-bors
|
hm wait that might not be the right commit. @bors r=saethlin |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (1d68e6d): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -2.6%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary 0.8%, secondary 2.5%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 767.545s -> 769.527s (0.26%) |
The comment here about 48 bit addresses being enough was written in 2016 but was made incorrect in 2019 by 5-level paging, and then persisted for another 5 years before being noticed and corrected.
The bolding of the "exclusive" part is merely to call attention to something I missed when reading it and doublechecking the math.
r? @saethlin
try-job: i686-msvc
try-job: test-various