-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
Validation rules? #30
Comments
I agree that the language used could be more specific. As you said, clearly any "failure" is not a module failure (and this is also specified in the spec document above), but what should happen on a "custom section failure" is not specified. The current V8 behavior is the result of some discussions in the CG and I think it is reasonable. If the section is malformed it should not be trusted to have any useful info. That said, some form of warning would be useful for development. There is some logging in V8 IIRC but it's not visible by default, let alone shown in the console. |
ah and about the bogus entries: this was explicitly discussed in the CG, and the conclusion was indeed that checking their validity pro-actively is not useful and potentially expensive. That also informed the implementation in V8. I do think that the vagueness here could be improved (for example by suggesting/mandating that the section should be discarded if an error is encountered during parsing), but at the same time since the effect of applying the section is not observable, I don't see a problem with leaving the engines some room to decide on their own. Discarding the section for any reason is always a valid option after all. |
The Overview contains some normative language such as these examples:
which sounds like the module should fail to validate if these requirements aren't met; but OTOH we're talking about a custom section which never causes validation errors. So what should implementations do?
Should this be a spec-level question, i.e. is this a reason to introduce a kind of section that in terms of validation rules sits somewhere between regular core sections and arbitrary custom sections? (I'm not saying it should be; just that this would be one option.)
Should these rules be understood to apply to the validity of the section (rather than the module), so if one rule is violated, the entire branch hints section is disregarded?
Should implementations devise their own "warning" scheme, such as printing a message to a browser's developer console, or to the process'
stderr
, if a branch hints section contains invalid contents? That might work well for some scenarios, but e.g. doesn't lend itself well to automated tests.Should implementations just be lax and quietly disregard individual bits they don't like?
FWIW, the current state of things in V8 is:
1
) or hint (anything other than0
or1
), the branch hints section is entirely disregardedThe latter is obviously not very strict. Algorithmically it wouldn't be difficult to change it. However, it would have a runtime cost: checking whether the branch instruction offsets are valid is work that currently simply isn't being done; the way the current implementation works is: it parses the branch hints section into a 2-level map
func_index -> instruction_offset -> hint
, and when the optimizing compiler encounters a branch instruction it queries this map, so any bogus/useless entries in the section get put in the map but then are simply never retrieved from it. Before spending any time on changing anything there, I wanted to ask what the behavior even should be.The text was updated successfully, but these errors were encountered: