Fix #95: Use parenthesis instead of None as delimiter to work around …

…rust issue 67062 - Also some general refactoring around where delimiter is used, and the fact that it now is only useful for expressions. - Also added a testcase.
CensoredUsername · May 3, 2024 · 6ad3988 · 6ad3988
1 parent 907251b
commit 6ad3988
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 82 deletions.
diff --git a/plugin/src/arch/x64/compiler.rs b/plugin/src/arch/x64/compiler.rs
@@ -1,5 +1,5 @@
 use syn::spanned::Spanned;
-use proc_macro2::{Span, TokenTree};
+use proc_macro2::{Span, TokenTree, Literal};
 use quote::{quote_spanned};
 use proc_macro_error::emit_error;
 
@@ -260,7 +260,7 @@ pub(super) fn compile_instruction(ctx: &mut Context, instruction: Instruction, a
         };
 
         if let RegKind::Dynamic(_, expr) = rm_k {
-            let last: TokenTree = proc_macro2::Literal::u8_suffixed(*last).into();
+            let last: TokenTree = Literal::u8_suffixed(*last).into();
             buffer.push(Stmt::ExprUnsigned(serialize::expr_mask_shift_or(&last, &delimited(expr), 7, 0), Size::BYTE));
         } else {
             buffer.push(Stmt::u8(last + (rm_k.encode() & 7)));
@@ -455,7 +455,7 @@ pub(super) fn compile_instruction(ctx: &mut Context, instruction: Instruction, a
         let ireg = ireg.kind;
         let byte = ireg.encode() << 4;
 
-        let mut byte: TokenTree = proc_macro2::Literal::u8_suffixed(byte).into();
+        let mut byte: TokenTree = Literal::u8_suffixed(byte).into();
         if let RegKind::Dynamic(_, expr) = ireg {
             byte = serialize::expr_mask_shift_or(&byte, &delimited(expr), 0xF, 4);
         }
@@ -1510,7 +1510,7 @@ fn compile_rex(buffer: &mut Vec<Stmt>, rex_w: bool, reg: &Option<SizedArg>, rm:
         return;
     }
 
-    let mut rex: TokenTree = proc_macro2::Literal::u8_suffixed(rex).into();
+    let mut rex: TokenTree = Literal::u8_suffixed(rex).into();
 
     if let RegKind::Dynamic(_, expr) = reg_k {
         rex = serialize::expr_mask_shift_or(&rex, &delimited(expr), 8, -1);
@@ -1578,7 +1578,7 @@ rm: &Option<SizedArg>, map_sel: u8, rex_w: bool, vvvv: &Option<SizedArg>, vex_l:
             return;
         }
 
-        let mut byte1: TokenTree = proc_macro2::Literal::u8_suffixed(byte1).into();
+        let mut byte1: TokenTree = Literal::u8_suffixed(byte1).into();
         if let RegKind::Dynamic(_, expr) = reg_k {
             byte1 = serialize::expr_mask_shift_inverted_and(&byte1, &delimited(expr), 8, 4)
         }
@@ -1592,7 +1592,7 @@ rm: &Option<SizedArg>, map_sel: u8, rex_w: bool, vvvv: &Option<SizedArg>, vex_l:
     buffer.push(Stmt::u8(if data.flags.contains(Flags::VEX_OP) {0xC4} else {0x8F}));
 
     if mode == X86Mode::Long && (reg_k.is_dynamic() || index_k.is_dynamic() || base_k.is_dynamic()) {
-        let mut byte1: TokenTree = proc_macro2::Literal::u8_suffixed(byte1).into();
+        let mut byte1: TokenTree = Literal::u8_suffixed(byte1).into();
 
         if let RegKind::Dynamic(_, expr) = reg_k {
             byte1 = serialize::expr_mask_shift_inverted_and(&byte1, &delimited(expr), 8, 4);
@@ -1609,7 +1609,7 @@ rm: &Option<SizedArg>, map_sel: u8, rex_w: bool, vvvv: &Option<SizedArg>, vex_l:
     }
 
     if vvvv_k.is_dynamic() {
-        let mut byte2: TokenTree = proc_macro2::Literal::u8_suffixed(byte2).into();
+        let mut byte2: TokenTree = Literal::u8_suffixed(byte2).into();
 
         if let RegKind::Dynamic(_, expr) = vvvv_k {
             byte2 = serialize::expr_mask_shift_inverted_and(&byte2, &delimited(expr), 0xF, 3)
@@ -1630,7 +1630,7 @@ fn compile_modrm_sib(buffer: &mut Vec<Stmt>, mode: u8, reg1: RegKind, reg2: RegK
         return;
     }
 
-    let mut byte: TokenTree = proc_macro2::Literal::u8_suffixed(byte).into();
+    let mut byte: TokenTree = Literal::u8_suffixed(byte).into();
 
     if let RegKind::Dynamic(_, expr) = reg1 {
         byte = serialize::expr_mask_shift_or(&byte, &delimited(expr), 7, 3);
@@ -1645,8 +1645,8 @@ fn compile_sib_dynscale(buffer: &mut Vec<Stmt>, scale: u8, scale_expr: syn::Expr
     let byte = (reg1.encode()  & 7) << 3 |
                (reg2.encode()  & 7)      ;
 
-    let mut byte: TokenTree = proc_macro2::Literal::u8_suffixed(byte).into();
-    let scale: TokenTree = proc_macro2::Literal::u8_unsuffixed(scale).into();
+    let mut byte: TokenTree = Literal::u8_suffixed(byte).into();
+    let scale: TokenTree = Literal::u8_unsuffixed(scale).into();
 
     if let RegKind::Dynamic(_, expr) = reg1 {
         byte = serialize::expr_mask_shift_or(&byte, &delimited(expr), 7, 3);
@@ -1658,12 +1658,17 @@ fn compile_sib_dynscale(buffer: &mut Vec<Stmt>, scale: u8, scale_expr: syn::Expr
     let span = scale_expr.span();
     let scale_expr = delimited(scale_expr);
 
-    let (expr1, expr2) = serialize::expr_dynscale(
-        &delimited(quote_spanned!{ span=>
-            #scale_expr * #scale
-        }),
-        &byte
-    );
-    buffer.push(Stmt::PrefixStmt(expr1));
-    buffer.push(Stmt::ExprUnsigned(expr2, Size::BYTE));
+    let expr = delimited(quote_spanned!{ span=>
+        ((
+            match #scale_expr * #scale {
+                8 => 3,
+                4 => 2,
+                2 => 1,
+                1 => 0,
+                _ => panic!("Type size not representable as scale")
+            }
+        & 3) << 6) | #byte
+    });
+
+    buffer.push(Stmt::ExprUnsigned(expr, Size::BYTE));
 }
diff --git a/plugin/src/common.rs b/plugin/src/common.rs
@@ -1,7 +1,6 @@
 //! This module contains various infrastructure that is common across all assembler backends
-use proc_macro2::{Span, TokenTree};
+use proc_macro2::{Span, TokenTree, TokenStream, Literal, Group};
 use quote::ToTokens;
-use quote::quote;
 use syn::spanned::Spanned;
 use syn::parse;
 use syn::Token;
@@ -148,11 +147,11 @@ impl Jump {
     pub fn encode(self, field_offset: u8, ref_offset: u8, data: &[u8]) -> Stmt {
         let span = self.span();
 
-        let target_offset = delimited(if let Some(offset) = self.offset {
-            quote!(#offset)
+        let target_offset = if let Some(offset) = self.offset {
+            delimited(offset)
         } else {
-            quote!(0isize)
-        });
+            TokenTree::Literal(Literal::isize_suffixed(0))
+        };
 
         // Create a relocation descriptor, containing all information about the actual jump except for the target itself.
         let relocation = Relocation {
@@ -221,12 +220,8 @@ pub enum Stmt {
     DynamicJumpTarget(TokenTree, Relocation),
     BareJumpTarget(TokenTree, Relocation),
 
-    // a statement that provides some information for the next statement,
-    // and should therefore not be reordered with it
-    PrefixStmt(TokenTree),
-
     // a random statement that has to be inserted between assembly hunks
-    Stmt(TokenTree)
+    Stmt(TokenStream)
 }
 
 // convenience methods
@@ -251,19 +246,32 @@ impl Stmt {
 }
 
 
-// Makes a None-delimited TokenTree item out of anything that can be converted to tokens.
-// This is a useful shortcut to escape issues around not-properly delimited tokenstreams
-// because it is guaranteed to be parsed back properly to its source ast at type-level.
+// Takes an arbitrary tokenstream as input, and ensures it can be interpolated safely.
+// returns a tokentree representing either a single token, or a delimited group.
+//
+// If the given tokenstream contains multiple tokens, it will be parenthesized.
+//
+// this will panic if given an empty tokenstream.
+// this would use delimiter::None if not for https://github.com/rust-lang/rust/issues/67062
 pub fn delimited<T: ToTokens>(expr: T) -> TokenTree {
-    let span = expr.span();
-    let mut group = proc_macro2::Group::new(
-        proc_macro2::Delimiter::None, expr.into_token_stream()
+    let stream = expr.into_token_stream();
+
+    // the stream api is very limited, but cloning a stream is luckily cheap.
+    // so to check how many tokens are contained we can do this.
+    let mut iter = stream.clone().into_iter();
+    let first = iter.next().unwrap();
+    if iter.next().is_none() {
+        return first;
+    }
+
+    let span = stream.span();
+    let mut group = Group::new(
+        proc_macro2::Delimiter::Parenthesis, stream
     );
     group.set_span(span);
     proc_macro2::TokenTree::Group(group)
 }
 
-
 /// Create a bitmask with `scale` bits set
 pub fn bitmask(scale: u8) -> u32 {
     1u32.checked_shl(u32::from(scale)).unwrap_or(0).wrapping_sub(1)
@@ -274,4 +282,3 @@ pub fn bitmask(scale: u8) -> u32 {
 pub fn bitmask64(scale: u8) -> u64 {
     1u64.checked_shl(u32::from(scale)).unwrap_or(0).wrapping_sub(1)
 }
-
diff --git a/plugin/src/directive.rs b/plugin/src/directive.rs
@@ -2,8 +2,8 @@ use std::collections::hash_map::Entry;
 
 use syn::parse;
 use syn::Token;
-use quote::quote;
 use proc_macro_error::emit_error;
+use proc_macro2::{TokenTree, Literal};
 
 use crate::common::{Stmt, Size, delimited};
 use crate::arch;
@@ -64,7 +64,7 @@ pub(crate) fn evaluate_directive(invocation_context: &mut DynasmContext, stmts:
                 delimited(with)
             } else {
                 let with = invocation_context.current_arch.default_align();
-                delimited(quote!(#with))
+                TokenTree::Literal(Literal::u8_unsuffixed(with))
             };
 
             stmts.push(Stmt::Align(delimited(value), with));

diff --git a/plugin/src/lib.rs b/plugin/src/lib.rs
@@ -113,7 +113,7 @@ impl parse::Parse for Dynasm {
                 if !buffer.is_empty() {
                     // ensure that the statement is actually a proper statement and then emit it for serialization
                     let stmt: syn::Stmt = syn::parse2(buffer)?;
-                    stmts.push(common::Stmt::Stmt(common::delimited(stmt)));
+                    stmts.push(common::Stmt::Stmt(quote!{ #stmt }));
                 }
                 continue;
             }

diff --git a/plugin/src/serialize.rs b/plugin/src/serialize.rs
@@ -93,8 +93,7 @@ pub fn serialize(name: &TokenTree, stmts: Vec<Stmt>) -> TokenStream {
                 ("dynamic_reloc" , vec![expr, target_offset, Literal::u8_suffixed(field_offset).into(), Literal::u8_suffixed(ref_offset).into(), kind]),
             Stmt::BareJumpTarget(expr, Relocation { field_offset, ref_offset, kind, .. })    =>
                 ("bare_reloc"    , vec![expr, Literal::u8_suffixed(field_offset).into(), Literal::u8_suffixed(ref_offset).into(), kind]),
-            Stmt::PrefixStmt(s)
-            | Stmt::Stmt(s) => {
+            Stmt::Stmt(s) => {
                 output.extend(quote! {
                     #s ;
                 });
@@ -147,13 +146,6 @@ pub fn invert(stmts: Vec<Stmt>) -> Vec<Stmt> {
             _ => ()
         };
 
-        while let Some(Stmt::PrefixStmt(_)) = iter.peek() {
-            // ensure prefix statements still end up before their following emitting statement
-            let s = iter.next().unwrap();
-            let i = reversed.len() - 1;
-            reversed.insert(i, s);
-        }
-
         // otherwise, calculate the size of the current statement and add that to the counter
         let size = match &stmt {
             Stmt::Const(_, size)
@@ -173,7 +165,6 @@ pub fn invert(stmts: Vec<Stmt>) -> Vec<Stmt> {
             | Stmt::BackwardJumpTarget(_, _)
             | Stmt::DynamicJumpTarget(_, _)
             | Stmt::BareJumpTarget(_, _)
-            | Stmt::PrefixStmt(_)
             | Stmt::Stmt(_) => 0,
         };
 
@@ -225,16 +216,6 @@ pub fn expr_string_from_ident(i: &syn::Ident) -> TokenTree {
     proc_macro2::Literal::string(&name).into()
 }
 
-// Makes a dynamic scale expression. Useful for x64 generic addressing mode
-pub fn expr_dynscale(scale: &TokenTree, rest: &TokenTree) -> (TokenTree, TokenTree) {
-    let tempval = expr_encode_x64_sib_scale(&scale);
-    (delimited(quote_spanned! { Span::mixed_site()=>
-        let temp = #tempval
-    }), delimited(quote_spanned! { Span::mixed_site()=>
-         #rest | ((temp & 3) << 6)
-    }))
-}
-
 // makes (a, b)
 pub fn expr_tuple_of_u8s(span: Span, data: &[u8]) -> TokenTree {
     delimited(if data.len() == 1 {
@@ -332,28 +313,6 @@ pub fn expr_size_of(path: &syn::Path) -> TokenTree {
     })
 }
 
-// makes the following
-// match size {
-//    8 => 3,
-//    4 => 2,
-//    2 => 1,
-//    1 => 0,
-//    _ => panic!r("Type size not representable as scale")
-//}
-pub fn expr_encode_x64_sib_scale(size: &TokenTree) -> TokenTree {
-    let span = size.span();
-
-    delimited(quote_spanned! { span=>
-        match #size {
-            8 => 3,
-            4 => 2,
-            2 => 1,
-            1 => 0,
-            _ => panic!("Type size not representable as scale")
-        }
-    })
-}
-
 // Reparses a tokentree into an expression
 pub fn reparse(tt: &TokenTree) -> parse::Result<syn::Expr> {
     syn::parse2(tt.into_token_stream())

diff --git a/testing/tests/bugreports.rs b/testing/tests/bugreports.rs
@@ -78,3 +78,25 @@ fn bugreport_4() {
         ; and w0, w0, 255
     );
 }
+
+// Precedence issue around typemapped operands due to proc_macro2::Delimiter::None being broken.
+#[test]
+fn bugreport_5() {
+    #![allow(unused_parens)]
+    #[allow(dead_code)]
+    struct Test {
+        a: u32,
+        b: u32
+    }
+
+    let mut ops = dynasmrt::x64::Assembler::new().unwrap();
+    dynasm!(ops
+       ; .arch x64
+       ; mov rbx => Test[2 + 1].b, rax
+    );
+
+    let buf = ops.finalize().unwrap();
+    let hex: Vec<String> = buf.iter().map(|x| format!("0x{:02X}", *x)).collect();
+    let hex: String = hex.join(", ");
+    assert_eq!(hex, "0x48, 0x89, 0x83, 0x1C, 0x00, 0x00, 0x00", "bugreport_5");
+}