Details

We now check relocations offsets are within range, and the relocation index is valid.

Also updated tests which contained invalid Wasm files that were previously not checked.

Could this be controversial?!? I know some people advocate for "minimal" correctness checks in LLVM, since the linker reads in hundreds of object files and we don't want to do expensive validation for each one.

These checks are pretty lightweight though, and should handily catch any bugs we might have.

It's adding to the test since the test contains a file with relocations but no symbol table (so it's an invalid file).

You're right, I could add tests for exercising our handling of bad Wasm files... but we currently have very very few tests for bad Wasm files. What's the policy - is it our intention to have tests for all the various cases of illegal Wasm files?

I guess we would ideally, I'll add a test for the new error messages I've put in.

Oh I see. You mean that added these validations caused these tests to start failing? In that case yes, I think these test changes makes sense to be part of this CL. Perhaps mention that in the description.