[PostEmscripten] Fix calcSegmentOffsets for large offsets #6260
Conversation
|
I'm not sure it worth writing a test for this.. maybe you can suggest a way if you feel like it needs one. This bug is currently preventing some of the "2gb" tests in emscripten from running (where static data segments are >2Gb in the 32-bit space) |
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
In wasm64 don't these need to be 64-bit?
The Address being assigned to here is 64-bit or 32-bit accordingly, that is not the problem.. The bug being fixed here only occurs when on wasm32 when the value being read is is > 2&32. Prior to this change the |
| // The first operand is the function pointer index, which must be | ||
| // constant if we are to optimize it statically. | ||
| if (auto* index = curr->operands[0]->dynCast<Const>()) { | ||
| size_t indexValue = index->value.getInteger(); |
There was a problem hiding this comment.
This is just a drive by fix, that I noticed while fixing the line above.
Function pointer are also unsigned values.
sbc100
force-pushed
the
post_emscripten_high_offsets
branch
from
c5a7893 to
512738f
Compare
|
I added a test! |
sbc100
force-pushed
the
post_emscripten_high_offsets
branch
from
512738f to
94c15eb
Compare
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
I see now, thanks. lgtm % question about the test.
Specifically offsets larger than 2^32 which were being interpreted misinterpreted here as very large int64_t values.
sbc100
force-pushed
the
post_emscripten_high_offsets
branch
from
94c15eb to
715513b
Compare
…y#6260) Specifically offsets larger than 2^32 which were being interpreted misinterpreted here as very large int64_t values.
Specifically offsets larger than 2^32 which were being interpreted misinterpreted here as very large int64_t values.