Handle bytecode without IC state#617
Merged
bnoordhuis merged 3 commits intoquickjs-ng:masterfrom Oct 24, 2024
Merged
Conversation
Contributor
Author
|
Interesting...
I do believe ubsan is unearthing a (pre-existing) bug here. |
saghul
approved these changes
Oct 24, 2024
Contributor
saghul
left a comment
There was a problem hiding this comment.
LGTM! I also like you added those flags, I was planning on adding them myself to implement standalone executables in JS :-P
Contributor
Author
|
I'll squash the 2nd and 3rd commit (don't want to break |
UBSan is right to complain that `s->ptr_last == NULL` when tracing is disabled.
Don't raise a "invalid tag 12" exception when encountering bytecode and JS_READ_OBJ_BYTECODE is not set, because no one knows what "tag 12" means without looking it up, not even quickjs maintainers.
Deserialized bytecode does not have IC state, i.e., `bc->ic == NULL`. That may or may not be bug (IMO, it is and we should rebuild the IC state during deserialization) but, either way, don't segfault. DRY add_ic_slot() and its call sites in a hopefully NFC manner.
bba5cec to
553dee3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I split this into three commits. The second one segfaults, the third one fixes that.
Interestingly, starting qjs in REPL mode segfaulted but the other bytecode tests we have don't. Not quite sure what's up with that but
bc->icnot pointing to anything was definitely the issue though.I added two (for now undocumented) options to
std.evalScriptthat may be useful in their own right.