From c650559a452948ced2c054f55844fcb3e4ab9aa5 Mon Sep 17 00:00:00 2001
From: wilson0x4d <mrshaunwilson@msn.com>
Date: Tue, 13 Aug 2024 12:58:39 -0500
Subject: [PATCH] fix: incorrect error message in `ASTree::BuildFromCode(...)`

* `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.
---
 ASTree.cpp   |  9 +++++----
 bytecode.cpp | 23 +++++++++++++++++------
 bytecode.h   |  2 +-
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/ASTree.cpp b/ASTree.cpp
index 050eebfaf..f799b0aa5 100644
--- a/ASTree.cpp
+++ b/ASTree.cpp
@@ -85,6 +85,7 @@ PycRef<ASTNode> BuildFromCode(PycRef<PycCode> code, PycModule* mod)
     PycRef<ASTBlock> curblock = defblock;
     blocks.push(defblock);
 
+    unsigned char bytecode;
     int opcode, operand;
     int curpos = 0;
     int pos = 0;
@@ -108,7 +109,7 @@ PycRef<ASTNode> BuildFromCode(PycRef<PycCode> code, PycModule* mod)
 #endif
 
         curpos = pos;
-        bc_next(source, mod, opcode, operand, pos);
+        bc_next(source, mod, bytecode, opcode, operand, pos);
 
         if (need_try && opcode != Pyc::SETUP_EXCEPT_A) {
             need_try = false;
@@ -1788,7 +1789,7 @@ PycRef<ASTNode> BuildFromCode(PycRef<PycCode> code, PycModule* mod)
                     curblock = blocks.top();
                     curblock->append(prev.cast<ASTNode>());
 
-                    bc_next(source, mod, opcode, operand, pos);
+                    bc_next(source, mod, bytecode, opcode, operand, pos);
                 }
             }
             break;
@@ -1811,7 +1812,7 @@ PycRef<ASTNode> BuildFromCode(PycRef<PycCode> code, PycModule* mod)
                     curblock = blocks.top();
                     curblock->append(prev.cast<ASTNode>());
 
-                    bc_next(source, mod, opcode, operand, pos);
+                    bc_next(source, mod, bytecode, opcode, operand, pos);
                 }
             }
             break;
@@ -2474,7 +2475,7 @@ PycRef<ASTNode> BuildFromCode(PycRef<PycCode> code, PycModule* mod)
             }
             break;
         default:
-            fprintf(stderr, "Unsupported opcode: %s\n", Pyc::OpcodeName(opcode & 0xFF));
+            fprintf(stderr, "Unsupported opcode: %s (bytecode=%02Xh) at position %d.\n", Pyc::OpcodeName(opcode), bytecode, curpos);
             cleanBuild = false;
             return new ASTNodeList(defblock->nodes());
         }
diff --git a/bytecode.cpp b/bytecode.cpp
index 782145952..b77691ffa 100644
--- a/bytecode.cpp
+++ b/bytecode.cpp
@@ -276,14 +276,16 @@ void print_const(std::ostream& pyc_output, PycRef<PycObject> obj, PycModule* mod
     }
 }
 
-void bc_next(PycBuffer& source, PycModule* mod, int& opcode, int& operand, int& pos)
+void bc_next(PycBuffer& source, PycModule* mod, unsigned char& bytecode, int& opcode, int& operand, int& pos)
 {
-    opcode = Pyc::ByteToOpcode(mod->majorVer(), mod->minorVer(), source.getByte());
+    bytecode = source.getByte();
+    opcode = Pyc::ByteToOpcode(mod->majorVer(), mod->minorVer(), bytecode);
     if (mod->verCompare(3, 6) >= 0) {
         operand = source.getByte();
         pos += 2;
         if (opcode == Pyc::EXTENDED_ARG_A) {
-            opcode = Pyc::ByteToOpcode(mod->majorVer(), mod->minorVer(), source.getByte());
+            bytecode = source.getByte();
+            opcode = Pyc::ByteToOpcode(mod->majorVer(), mod->minorVer(), bytecode);
             operand = (operand << 8) | source.getByte();
             pos += 2;
         }
@@ -292,7 +294,8 @@ void bc_next(PycBuffer& source, PycModule* mod, int& opcode, int& operand, int&
         pos += 1;
         if (opcode == Pyc::EXTENDED_ARG_A) {
             operand = source.get16() << 16;
-            opcode = Pyc::ByteToOpcode(mod->majorVer(), mod->minorVer(), source.getByte());
+            bytecode = source.getByte();
+            opcode = Pyc::ByteToOpcode(mod->majorVer(), mod->minorVer(), bytecode);
             pos += 3;
         }
         if (opcode >= Pyc::PYC_HAVE_ARG) {
@@ -340,17 +343,25 @@ void bc_disasm(std::ostream& pyc_output, PycRef<PycCode> code, PycModule* mod,
 
     PycBuffer source(code->code()->value(), code->code()->length());
 
+    unsigned char bytecode;
     int opcode, operand;
     int pos = 0;
     while (!source.atEof()) {
         int start_pos = pos;
-        bc_next(source, mod, opcode, operand, pos);
+        bc_next(source, mod, bytecode, opcode, operand, pos);
         if (opcode == Pyc::CACHE && (flags & Pyc::DISASM_SHOW_CACHES) == 0)
             continue;
 
         for (int i=0; i<indent; i++)
             pyc_output << "    ";
-        formatted_print(pyc_output, "%-7d %-30s  ", start_pos, Pyc::OpcodeName(opcode));
+
+        auto opcode_name = Pyc::OpcodeName(opcode);
+        if (opcode == Pyc::Opcode::PYC_INVALID_OPCODE) {
+            // attempt to generate a more informative output for anyone doing copy-pasta Issue creation
+            formatted_print(pyc_output, "%-7d %-30s  (bytecode=%02Xh)", start_pos, opcode_name, bytecode);
+        } else {
+            formatted_print(pyc_output, "%-7d %-30s  ", start_pos, opcode_name);
+        }
 
         if (opcode >= Pyc::PYC_HAVE_ARG) {
             switch (opcode) {
diff --git a/bytecode.h b/bytecode.h
index 7e4179e8f..3a1de656d 100644
--- a/bytecode.h
+++ b/bytecode.h
@@ -29,6 +29,6 @@ int ByteToOpcode(int maj, int min, int opcode);
 
 void print_const(std::ostream& pyc_output, PycRef<PycObject> obj, PycModule* mod,
                  const char* parent_f_string_quote = nullptr);
-void bc_next(PycBuffer& source, PycModule* mod, int& opcode, int& operand, int& pos);
+void bc_next(PycBuffer& source, PycModule* mod, unsigned char& bytecode, int& opcode, int& operand, int& pos);
 void bc_disasm(std::ostream& pyc_output, PycRef<PycCode> code, PycModule* mod,
                int indent, unsigned flags);