Skip to content

Handle extended const segment offsets in the fuzzer #6382

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.

Sign up for GitHub

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

Merged
tlively merged 3 commits into from
Mar 7, 2024
Merged

Handle extended const segment offsets in the fuzzer #6382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ed0d77
Handle extended const segment offsets in the fuzzer
tlively Mar 7, 2024
8c02b22
check for import
tlively Mar 7, 2024
979b615
assertions
tlively Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions src/tools/fuzzing/fuzzing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,15 +465,12 @@ void TranslateToFuzzReader::finalizeMemory() {
// definition to what used to be an imported global in initial contents.
// To fix that, replace such invalid offsets with a constant.
for (auto* get : FindAll<GlobalGet>(segment->offset).list) {
// N.B: We never currently encounter imported globals here, but we do
// the check for robustness.
if (!wasm.getGlobal(get->name)->imported()) {
// TODO: It would be better to avoid segment overlap so that
// MemoryPacking can run.
segment->offset =
builder.makeConst(Literal::makeFromInt32(0, Type::i32));
break;
}
// No imported globals should remain.
assert(!wasm.getGlobal(get->name)->imported());
// TODO: It would be better to avoid segment overlap so that
// MemoryPacking can run.
segment->offset =
builder.makeConst(Literal::makeFromInt32(0, Type::i32));
}
}
if (auto* offset = segment->offset->dynCast<Const>()) {
Expand Down Expand Up @@ -507,10 +504,13 @@ void TranslateToFuzzReader::finalizeTable() {
for (auto& table : wasm.tables) {
ModuleUtils::iterTableSegments(
wasm, table->name, [&](ElementSegment* segment) {
// If the offset is a global that was imported (which is ok) but no
// longer is (not ok) we need to change that.
if (auto* offset = segment->offset->dynCast<GlobalGet>()) {
if (!wasm.getGlobal(offset->name)->imported()) {
Copy link
Member

@kripken kripken Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did the check for ->imported() go away? I think we only need to zero it out of there is a global.get that is not imported.

Copy link
Member Author

@tlively tlively Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I see that finalizeMemory does do this check, but with a note that imported globals are never encountered. Indeed, setupGlobals removes all imports. I guess I can follow finalizeMemory's lead here.

// If the offset contains a global that was imported (which is ok) but
// no longer is (not ok unless GC is enabled), we may need to change
// that.
if (!wasm.features.hasGC()) {
for (auto* get : FindAll<GlobalGet>(segment->offset).list) {
// No imported globals should remain.
assert(!wasm.getGlobal(get->name)->imported());
// TODO: the segments must not overlap...
segment->offset =
builder.makeConst(Literal::makeFromInt32(0, Type::i32));
Expand Down
1 change: 1 addition & 0 deletions test/lit/validation/extended-const.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

;; NO-EXTENDED: unexpected false: global init must be constant
;; NO-EXTENDED: unexpected false: memory segment offset should be constant
;; NO-EXTENDED: unexpected false: table segment offset should be constant

;; EXTENDED: (import "env" "global" (global $gimport$0 i32))
;; EXTENDED: (global $1 i32 (i32.add
Expand Down