[wasm-split] Validate output modules when custom-descriptor is enabled

[aheejin]

We are currently validating inputs but not outputs. This PR makes wasm-split validate output modules too unless --no-validation is given.

But this enables output validation only when custom-descriptor is enabled for now, and adds a TODO to enable it in all cases, because of this reason, from #8043's description:

Do not use exact imports when custom descriptors are not enabled. In principle this could cause validation errors because we allow e.g. exact locals even when custom descriptors are not enabled. This could be worked around in the future either by running a pass to remove exactness before splitting or by always using and allowing exact imports, but then emitting them as inexact imports when custom descriptors are disabled.

[tlively]

Won't this make the fuzzer unhappy when it runs the Split handler without custom descriptors enabled? And it will potentially start throwing validation errors for real users, too, forcing them to add the --no-validation flag.

I think it would be best to either add a pass to remove exactness before making this change, or possibly validate only if custom descriptors are enabled for now, with a TODO about removing that restriction in the future.

[aheejin]

Won't this make the fuzzer unhappy when it runs the Split handler without custom descriptors enabled? And it will potentially start throwing validation errors for real users, too, forcing them to add the --no-validation flag.

The rationale behind this PR was, I thought, "if this generated invalid modules so far in fuzzing, it would've errored out anyway because we ran the output modules using v8". But apparently we drop the exactness when we write binaries if custom-descriptors is not enabled, and the validation happens before that... Yeah, it would likely crash the fuzzer and the user errors.

I think it would be best to either add a pass to remove exactness before making this change, or possibly validate only if custom descriptors are enabled for now, with a TODO about removing that restriction in the future.

Will do. Speaking of fixing this, do you have any idea on which direction is better, in your comments in #8043?

This could be worked around in the future either by running a pass to remove exactness before splitting or by always using and allowing exact imports, but then emitting them as inexact imports when custom descriptors are disabled.

[tlively]

The rationale for allowing exactness in private types when custom descriptors is not enabled is that we want the optimizer to take advantage of exact reference even without custom descriptors. That means we must always allow .wast files to contain exact references so we can represent IR with exact references in tests.

But public type identities must be preserved, and we cannot write binaries containing exact references without custom descriptors, so it is a user error to ever have an exact reference in a public type in that case. This only matters when parsing text modules for tests; binaries without custom descriptors enabled should never have exact types to begin with.

Since users should typically use wasm-split only on binaries, and since we don't run any optimizations on the parsed IR before splitting it, users should never encounter this problem in practice, so it seems a waste to run an extra pass just to keep the validator happy when it should always be happy in practice.

I wonder if we can just fix the tests to avoid the validator failures instead? I admit I don't understand why the tests other than exact.wast are running into validation failures to begin with, though.

If we can't fix the tests, I'd lean toward adding the pass.