Repository: huff-language/huff-rs
Pull request #177 (2022-07-28 15:48:55)

fix: Jump Table UsizeConversion Error
## Overview

Packed Jump Tables are being incorrectly generated.

Bytes were being correctly converted to a hexadecimal string using the utilities `bytes32_to_string` function, but then parsed into a usize using `.parse::<usize>()`. By default, this parse impl expects a base 10 decimal string.

The fix is to use `from_str_radix` which is implemented for rust's `usize`. Thus we can do:
```rust
usize::from_str_radix(bytes32_to_string(&jt.size, false).as_str(), 16)
```
where `jt.size` is the bytes32 jump table 32 bytes.



Patch:
diff --git a/huff_cli/src/huffc.rs b/huff_cli/src/huffc.rs
index cf9a596..12ed0f9 100644
--- a/huff_cli/src/huffc.rs
+++ b/huff_cli/src/huffc.rs
@@ -199,7 +199,7 @@ fn main() {
                     ),
                     token: None,
                 });
-                tracing::error!(target: "cli", "COMPILER ERRORED: {:?}", e);
+                tracing::error!(target: "cli", "COMPILER ERRORED: {}", e);
                 eprintln!("{}", Paint::red(format!("{}", e)));
                 std::process::exit(1);
             }
@@ -296,7 +296,7 @@ fn main() {
             }
         }
         Err(e) => {
-            tracing::error!(target: "cli", "COMPILER ERRORED: {:?}", e);
+            tracing::error!(target: "cli", "COMPILER ERRORED: {}", e);
             eprintln!("{}", Paint::red(format!("{}", e)));
             std::process::exit(1);
         }
diff --git a/huff_codegen/src/lib.rs b/huff_codegen/src/lib.rs
index f93131d..6603887 100644
--- a/huff_codegen/src/lib.rs
+++ b/huff_codegen/src/lib.rs
@@ -9,11 +9,10 @@ use huff_utils::{
     artifact::*,
     ast::*,
     bytecode::*,
+    bytes_util,
     error::CodegenError,
     evm::Opcode,
-    prelude::{
-        bytes32_to_string, format_even_bytes, pad_n_bytes, CodegenErrorKind, FileSource, Span,
-    },
+    prelude::{format_even_bytes, pad_n_bytes, CodegenErrorKind, FileSource, Span},
     types::EToken,
 };
 use std::{collections::HashMap, fs, path::Path, sync::Arc};
@@ -68,6 +67,8 @@ impl Codegen {
             &mut Vec::default(),
         )?;
 
+        tracing::debug!(target: "codegen", "Generated main bytecode. Appending table bytecode...");
+
         // Generate the fully baked bytecode
         Codegen::gen_table_bytecode(bytecode_res)
     }
@@ -135,13 +136,16 @@ impl Codegen {
 
         res.utilized_tables.iter().try_for_each(|jt| {
             table_offsets.insert(jt.name.to_string(), table_offset);
-            let size = match bytes32_to_string(&jt.size, false).as_str().parse::<usize>() {
+            let size = match bytes_util::hex_to_usize(bytes_util::bytes32_to_string(&jt.size, false).as_str()) {
                 Ok(s) => s,
-                Err(_) => return Err(CodegenError {
-                    kind: CodegenErrorKind::UsizeConversion(format!("{:?}", jt.size)),
-                    span: jt.span.clone(),
-                    token: None
-                })
+                Err(e) => {
+                    tracing::error!(target: "codegen", "Errored converting bytes32 to str. Bytes {:?} with error: {:?}", jt.size, e);
+                    return Err(CodegenError {
+                        kind: CodegenErrorKind::UsizeConversion(format!("{:?}", jt.size)),
+                        span: jt.span.clone(),
+                        token: None
+                    })
+                }
             };
             table_offset += size;
 
diff --git a/huff_core/src/lib.rs b/huff_core/src/lib.rs
index 4e595ae..57c75ba 100644
--- a/huff_core/src/lib.rs
+++ b/huff_core/src/lib.rs
@@ -252,7 +252,7 @@ impl<'a> Compiler<'a> {
         let main_bytecode = match Codegen::generate_main_bytecode(&contract) {
             Ok(mb) => mb,
             Err(mut e) => {
-                tracing::error!(target: "codegen", "FAILED TO GENERATE MAIN BYTECODE FOR CONTRACT");
+                tracing::error!(target: "core", "FAILED TO GENERATE MAIN BYTECODE FOR CONTRACT");
                 // Add File Source to Span
                 e.span = AstSpan(
                     e.span
@@ -264,7 +264,7 @@ impl<'a> Compiler<'a> {
                         })
                         .collect::<Vec<Span>>(),
                 );
-                tracing::error!(target: "codegen", "Roll Failed with CodegenError: {:?}", e);
+                tracing::error!(target: "core", "Roll Failed with CodegenError: {:?}", e.kind);
                 return Err(CompilerError::CodegenError(e))
             }
         };
diff --git a/huff_utils/src/ast.rs b/huff_utils/src/ast.rs
index 8491837..53c676d 100644
--- a/huff_utils/src/ast.rs
+++ b/huff_utils/src/ast.rs
@@ -143,7 +143,8 @@ impl Contract {
                 &mut last_assigned_free_pointer,
             ),
             None => {
-                tracing::error!(target: "ast", "'CONSTRUCTOR' MACRO NOT FOUND WHILE DERIVING STORAGE POINTERS!")
+                // The constructor is not required, so we can just warn
+                tracing::warn!(target: "ast", "'CONSTRUCTOR' MACRO NOT FOUND WHILE DERIVING STORAGE POINTERS!")
             }
         }
 
diff --git a/huff_utils/src/bytes_util.rs b/huff_utils/src/bytes_util.rs
index 2018ecd..9e901bf 100644
--- a/huff_utils/src/bytes_util.rs
+++ b/huff_utils/src/bytes_util.rs
@@ -1,3 +1,5 @@
+use std::num::ParseIntError;
+
 use tiny_keccak::{Hasher, Keccak};
 
 /// Convert a string slice to a `[u8; 32]`
@@ -23,12 +25,20 @@ pub fn str_to_bytes32(s: &str) -> [u8; 32] {
 pub fn bytes32_to_string(bytes: &[u8; 32], prefixed: bool) -> String {
     let mut s = String::default();
     let start = bytes.iter().position(|b| *b != 0).unwrap_or(bytes.len() - 1);
+    tracing::debug!(target: "bytes_util", "got start: {}", start);
     for b in &bytes[start..bytes.len()] {
+        tracing::debug!(target: "bytes_util", "Converting byte: {}", b);
         s = format!("{}{:02x}", s, *b);
+        tracing::debug!(target: "bytes_util", "Converted byte: {} to string {}", b, s);
     }
     format!("{}{}", if prefixed { "0x" } else { "" }, s)
 }
 
+/// Wrapper to convert a hex string to a usize.
+pub fn hex_to_usize(s: &str) -> Result<usize, ParseIntError> {
+    usize::from_str_radix(s, 16)
+}
+
 /// Pad a hex string with n 0 bytes to the left. Will not pad a hex string that has a length
 /// greater than or equal to `num_bytes * 2`
 pub fn pad_n_bytes(hex: &str, num_bytes: usize) -> String {


Test patch:
diff --git a/huff_core/tests/breaking_jumptable.rs b/huff_core/tests/breaking_jumptable.rs
new file mode 100644
index 0000000..efb976c
--- /dev/null
+++ b/huff_core/tests/breaking_jumptable.rs
@@ -0,0 +1,155 @@
+use huff_codegen::*;
+use huff_lexer::*;
+use huff_parser::*;
+use huff_utils::prelude::*;
+
+#[test]
+fn test_breaking_jump_table() {
+    let source: &str = r#"
+        #define jumptable__packed TEST_JUMPTABLE {
+            label_0_0 label_0_1 label_0_2 label_0_3 label_0_4 label_0_5 label_0_6 label_0_7 label_0_8 label_0_9 label_0_a label_0_b label_0_c label_0_d label_0_e label_0_f
+            label_1_0 label_1_1 label_1_2 label_1_3 label_1_4 label_1_5 label_1_6 label_1_7 label_1_8 label_1_9 label_1_a label_1_b label_1_c label_1_d label_1_e label_1_f
+            label_2_0 label_2_1 label_2_2 label_2_3 label_2_4 label_2_5 label_2_6 label_2_7 label_2_8 label_2_9 label_2_a label_2_b label_2_c label_2_d label_2_e label_2_f
+            label_3_0 label_3_1 label_3_2 label_3_3 label_3_4 label_3_5 label_3_6 label_3_7 label_3_8 label_3_9 label_3_a label_3_b label_3_c label_3_d label_3_e label_3_f
+            label_4_0 label_4_1 label_4_2 label_4_3 label_4_4 label_4_5 label_4_6 label_4_7 label_4_8 label_4_9 label_4_a label_4_b label_4_c label_4_d label_4_e label_4_f
+            label_5_0 label_5_1 label_5_2 label_5_3 label_5_4 label_5_5 label_5_6 label_5_7 label_5_8 label_5_9 label_5_a label_5_b label_5_c label_5_d label_5_e label_5_f
+            label_5_0 label_5_1 label_5_2 label_5_3 label_5_4 label_5_5 label_5_6 label_5_7 label_5_8 label_5_9 label_5_a label_5_b label_5_c label_5_d label_5_e label_5_f
+            label_5_0 label_5_1 label_5_2 label_5_3 label_5_4 label_5_5 label_5_6 label_5_7 label_5_8 label_5_9 label_5_a label_5_b label_5_c label_5_d label_5_e
+        }
+
+
+        // Program Entrypoint
+        #define macro MAIN() = takes(0) returns (0) {
+            // Copy the jumptable to the stack
+            __tablesize(TEST_JUMPTABLE) __tablestart(TEST_JUMPTABLE) 0x0 codecopy
+
+            // Revert here if the labels jump back
+            0x00 dup1 revert
+
+            // Define labels
+            label_0_0: dup1
+            label_0_1: dup1
+            label_0_2: dup1
+            label_0_3: dup1
+            label_0_4: dup1
+            label_0_5: dup1
+            label_0_6: dup1
+            label_0_7: dup1
+            label_0_8: dup1
+            label_0_9: dup1
+            label_0_a: dup1
+            label_0_b: dup1
+            label_0_c: dup1
+            label_0_d: dup1
+            label_0_e: dup1
+            label_0_f: dup1
+
+            label_1_0: dup1
+            label_1_1: dup1
+            label_1_2: dup1
+            label_1_3: dup1
+            label_1_4: dup1
+            label_1_5: dup1
+            label_1_6: dup1
+            label_1_7: dup1
+            label_1_8: dup1
+            label_1_9: dup1
+            label_1_a: dup1
+            label_1_b: dup1
+            label_1_c: dup1
+            label_1_d: dup1
+            label_1_e: dup1
+            label_1_f: dup1
+
+            label_2_0: dup1
+            label_2_1: dup1
+            label_2_2: dup1
+            label_2_3: dup1
+            label_2_4: dup1
+            label_2_5: dup1
+            label_2_6: dup1
+            label_2_7: dup1
+            label_2_8: dup1
+            label_2_9: dup1
+            label_2_a: dup1
+            label_2_b: dup1
+            label_2_c: dup1
+            label_2_d: dup1
+            label_2_e: dup1
+            label_2_f: dup1
+
+            label_3_0: dup1
+            label_3_1: dup1
+            label_3_2: dup1
+            label_3_3: dup1
+            label_3_4: dup1
+            label_3_5: dup1
+            label_3_6: dup1
+            label_3_7: dup1
+            label_3_8: dup1
+            label_3_9: dup1
+            label_3_a: dup1
+            label_3_b: dup1
+            label_3_c: dup1
+            label_3_d: dup1
+            label_3_e: dup1
+            label_3_f: dup1
+
+            label_4_0: dup1
+            label_4_1: dup1
+            label_4_2: dup1
+            label_4_3: dup1
+            label_4_4: dup1
+            label_4_5: dup1
+            label_4_6: dup1
+            label_4_7: dup1
+            label_4_8: dup1
+            label_4_9: dup1
+            label_4_a: dup1
+            label_4_b: dup1
+            label_4_c: dup1
+            label_4_d: dup1
+            label_4_e: dup1
+            label_4_f: dup1
+
+            label_5_0: dup1
+            label_5_1: dup1
+            label_5_2: dup1
+            label_5_3: dup1
+            label_5_4: dup1
+            label_5_5: dup1
+            label_5_6: dup1
+            label_5_7: dup1
+            label_5_8: dup1
+            label_5_9: dup1
+            label_5_a: dup1
+            label_5_b: dup1
+            label_5_c: dup1
+            label_5_d: dup1
+            label_5_e: dup1
+            label_5_f: dup1
+        }
+    "#;
+
+    // Parse tokens
+    let flattened_source = FullFileSource { source, file: None, spans: vec![] };
+    let lexer = Lexer::new(flattened_source);
+    let tokens = lexer.into_iter().map(|x| x.unwrap()).collect::<Vec<Token>>();
+    let mut parser = Parser::new(tokens, None);
+
+    // Parse the AST
+    let mut contract = parser.parse().unwrap();
+
+    // Derive storage pointers
+    contract.derive_storage_pointers();
+
+    // Instantiate Codegen
+    let cg = Codegen::new();
+
+    // The codegen instance should have no artifact
+    assert!(cg.artifact.is_none());
+
+    // Have the Codegen create the main macro bytecode
+    let mbytes = Codegen::generate_main_bytecode(&contract).unwrap();
+    assert_eq!(mbytes, String::from("60fe6100cc600039600080fd5b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b805b80000c000e00100012001400160018001a001c001e00200022002400260028002a002c002e00300032003400360038003a003c003e00400042004400460048004a004c004e00500052005400560058005a005c005e00600062006400660068006a006c006e00700072007400760078007a007c007e00800082008400860088008a008c008e00900092009400960098009a009c009e00a000a200a400a600a800aa00ac00ae00b000b200b400b600b800ba00bc00be00c000c200c400c600c800ca00ac00ae00b000b200b400b600b800ba00bc00be00c000c200c400c600c800ca00ac00ae00b000b200b400b600b800ba00bc00be00c000c200c400c600c8"));
+}
diff --git a/huff_utils/tests/bytes.rs b/huff_utils/tests/bytes.rs
new file mode 100644
index 0000000..76baa39
--- /dev/null
+++ b/huff_utils/tests/bytes.rs
@@ -0,0 +1,20 @@
+use huff_utils::bytes_util::*;
+
+#[test]
+fn test_bytes32_to_string() {
+    let byte_arr: [u8; 32] = [
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 254,
+    ];
+    let converted_string = bytes32_to_string(&byte_arr, false);
+    assert_eq!(converted_string, "fe");
+}
+
+#[test]
+fn test_hex_to_usize() {
+    for i in 0..255 {
+        let str = format!("{:02x}", i);
+        let converted_usize = hex_to_usize(&str).unwrap();
+        assert_eq!(converted_usize, i);
+    }
+}