Skip to content
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

fix: incorrect error message in ASTree::BuildFromCode(...) #511

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wilson0x4d
Copy link

  • opcode was incorrectly clamped causing misleading output.

  • modified output with additional info about offending bytecode and position so end-user submissions can become more informative.

ASTree.cpp Outdated
@@ -2474,7 +2474,8 @@ PycRef<ASTNode> BuildFromCode(PycRef<PycCode> code, PycModule* mod)
}
break;
default:
fprintf(stderr, "Unsupported opcode: %s\n", Pyc::OpcodeName(opcode & 0xFF));
int bytecode = code->code()->value()[curpos] & 0xFF;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading directly from the code object doesn't handle the EXTENDED_ARG case properly... Perhaps just printing the position is sufficient, since it can be used to look up the context in disassembly.

* `opcode` was incorrectly clamped causing misleading output.

* modified output with additional info about offending bytecode and position so end-user submissions can become more informative.

* for `EXTENDED_ARG` bytecode the position will be off by a few bytes, but, at least the bytecode printed will be correct.
@wilson0x4d
Copy link
Author

wilson0x4d commented Aug 14, 2024

indeed, i overlooked the EXTENDED_ARG handling.

the update i've pushed instead relies on bc_next to pass bytecode back up to the caller, this seems cleaner (doesn't couple the code to code()->value()) and compensates for EXTENDED_ARG correctly. this does leave the position printed off by a few bytes (when EXTENDED_ARG), but the bytecode value becomes accurate (which is what I was really looking for in the output) and even being a few bytes off at least gets someone who understands the bytecode into the right area of the image (where they would see EXTENDED_ARG and could mentally compensate.)

i also updated the disasm code with a similar change (to include the offending bytecode in the output.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants