[tlively]

Uninhabitable heap types from the initial module are filtered out of interestingHeapTypes in setupHeapTypes, but inhabitable types in interestingHeapTypes (as well as initial module globals and functions) can still reference them. Track uninhabitableHeapTypes in TranslateToFuzzReader and check for them in makeCompoundRef, getSubType, and getSuperType so makeCompoundRef emits ref.null, local.get, or ref.as_non_null rather than infinitely recursing across non-nullable type cycles.

[gemini-code-assist]

Code Review

This pull request prevents infinite recursion in makeCompoundRef during fuzzing by tracking and handling uninhabitable heap types from the initial module contents. It populates a set of uninhabitable heap types and checks it when generating compound references, falling back to null references or non-null cast escape hatches. A helper method isUninhabitable is also introduced and used during sub/supertype generation. Feedback on the changes points out a potential assertion failure in isUninhabitable when called with non-reference types, recommending a check for type.isRef() before retrieving the heap type.

[gemini-code-assist]

In Binaryen, Type::isNonNullable() can return true for non-reference types (such as basic types like i32 or f64), but calling Type::getHeapType() on a non-reference type will trigger an assertion failure in debug builds. Since GCTypeUtils::isUninhabitable(type) internally calls type.getHeapType() without checking type.isRef(), calling TranslateToFuzzReader::isUninhabitable on a non-reference type would result in an assertion failure.\n\nTo make this method robust and defensively guard against non-reference types, we should check type.isRef() first before calling GCTypeUtils::isUninhabitable(type) or accessing its heap type.

bool TranslateToFuzzReader::isUninhabitable(Type type) {\n  return type.isRef() &&\n         (GCTypeUtils::isUninhabitable(type) ||\n          (type.isNonNullable() &&\n           uninhabitableHeapTypes.contains(type.getHeapType())));\n}

[tlively]

In Binaryen, Type::isNonNullable() can return true for non-reference types (such as basic types like i32 or f64)...

This is not true. Type::isNonNullable() is defined like this:

  bool isNonNullable() const { return isRef() && !(id & NullMask); }

[stevenfontanella]

On that note it looks like the check for isRef here is redundant, can we remove it? Separately it'd be nice to rename isNonNullable to isNonNullableRef in another PR.

[stevenfontanella]

LGTM after comments

[tlively]

@stevenfontanella, it turned out that there were some other ways for uninhabitable types to slip in. PTAL at the latest changes to fix that.

[stevenfontanella]

The CI failure looks real, can you take a look? fuzz-uninhabitable.wast shows a diff

[tlively]

Yep, fixed now.

[stevenfontanella]

I might be missing something, can we add a seed here to make it deterministic and then check the output here?

[tlively]

Kind of. The previous version of the test did this by using the test file contents itself as the seed. The problem was that on Windows the line endings were read differently, leading to different random choices and the CI failure you saw.

Another problem is that future fuzzer changes could also change the random choices, creating an ongoing maintenance burden if this test starts failing as a result.

[tlively]

Sorry, I don't think I meant to merge this :/ Looks like I had auto-merge enabled since a couple days ago and forgot to turn it off. Happy to continue addressing any comments in a follow-up.