gc_fuzz: Struct fields

[khagankhan]

Structs with fields

• StructNew: Consumes ExternRef values from the operand stack for ref-typed fields instead of always synthesizing ref.null extern. POD fields still get default values. The consumed count is tracked in externref_from_stack and used during encoding via scratch locals to interleave stack values with defaults in field declaration order. POD constants do not need to come from the stack since we can always synthesize them, but externref (and concrete struct ref types in the future) should be consumed from the stack when available. This is implemented as a special case in the fixup loop rather than changing the #[operands()] attribute or introducing new stack value types, because the number of ref fields a struct will consume is not known statically since it depends on the struct type's field layout. The rest of the ops use static operand and result types so making them dynamic would be a larger refactor with little benefit. When the top of the abstract stack has consecutive ExternRef values, fixup pops them and records the count in externref_from_stack, letting the encoding know how many live values to retrieve via local.set/local.get instead of synthesizing ref.null extern.

• StructSet: Consumes one ExternRef from the stack when the target mutable field is a ref type, using it as the field value instead of ref.null extern. During fixup, it finds the first mutable field starting from field_index (scanning forward, wrapping around) and sets it. if no mutable field exists, the struct ref is discarded.

• StructGet/StructSet null guards: Both now check for null refs before accessing fields (ref.is_null + if/else), preventing null reference traps.

Other changes

• Added new tests and StructField mutations.
  +cc @fitzgen @eeide

[khagankhan]

@fitzgen I know this maybe a lot here but what I need the most is to know whether this solution makes sense or do we need something else like maybe adding d PODs as GcOps and to Stack or something else.

[github-actions]

Subscribe to Label Action

cc @fitzgen

Details

This issue or pull request has been labeled: "fuzzing"

Thus the following users have been cc'd because of the following labels:

• fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[fitzgen]

Thanks! Sorry that it took a little while to review this one.

[fitzgen]

This can just .unwrap(): usize::try_from(my_u32).unwrap() is common enough that it doesn't need a diagnostic message and it also shouldn't ever fail on our supported architectures anyway. Same with the other instances of this here.

[fitzgen]

Also this pattern is probably a little more straight forward as:

let idx = ctx.rng().gen_index(FieldType::ALL.len());
let idx = u32::try_from(idx).unwrap();

And maybe even better deduplicated as a fn FieldType::random(rng: &mut mutatis::Rng) -> u32 helper function.

[fitzgen]

You should be able to just collect the fields into a Box<[_]> in the first place rather than collect and then call into_boxed_slice().

[fitzgen]

I'm a bit confused here: why are we special-casing externref-typed fields here? Why are they different from i32-typed fields, for example? In either case, we need an operand of each field's type so I don't understand why we're not handling them uniformly.

[khagankhan]

My thinking was that reusing an externref or concretely typed references already on the stack may be more interesting. For example values from table.set or table.get. Since externref is GC related I wanted to distinguish it from something like i32.

P.S. I do not like special cases. If this is not useful we can synthesize externref too. We can also add them as StackTypes or GcOps. But this may just generate and discard values and waste mutation and execution cycles.

[khagankhan]

I will choose the first one and address the feedback. I can make PRs in the future if this turns out to be non-ideal.

[khagankhan]

Thanks @fitzgen ! Ready for the next round of reviews