Fix OP_DEBUG operand type and add NULL check for debug_op_hook #6714
Conversation
1. Change CASE(OP_DEBUG, Z) to CASE(OP_DEBUG, BBB) to match the definition in include/mruby/ops.h. The previous code declared Z (no operands) but then manually called FETCH_BBB(), which caused incorrect behavior with extended opcodes (OP_EXT1/2/3). 2. Add NULL check before calling debug_op_hook, consistent with how code_fetch_hook is handled. This prevents crashes when MRB_USE_DEBUG_HOOK is enabled but no hook function is set. Fixes: mruby#5686
Summary of ChangesHello @khasinski, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the stability and correctness of the mruby virtual machine by addressing two critical issues related to the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two important fixes to the mruby VM's debugging functionality. First, it corrects the operand type for OP_DEBUG to BBB, aligning it with its definition and ensuring correct behavior with extended opcodes. This change also simplifies the implementation by removing the now-redundant manual operand fetching. Second, a crucial NULL check is added before invoking mrb->debug_op_hook, preventing a potential crash if the hook is not set. Both changes are well-implemented, improving the robustness and correctness of the debugger.
2 participants
Summary
CASE(OP_DEBUG, Z)toCASE(OP_DEBUG, BBB)to match the definition ininclude/mruby/ops.hdebug_op_hookDetails
Operand type mismatch:
OP_DEBUGis defined asBBBinops.hbut was declared asZin the VM switch. The old code manually calledFETCH_BBB()after theCASEmacro, which breaks extended opcode handling (OP_EXT1/2/3).Missing NULL check: When
MRB_USE_DEBUG_HOOKis enabled,mrb->debug_op_hook()was called without checking if it's NULL first, which would crash if no hook is set.Fixes #5686