Re: [PATCH] Add some error checking to DWARF assembler

[Tom Tromey <tromey@adacore.com>]

Andrew> Maybe I'm over thinking this, but you could change this to:

Here's the definitive (IMNSHO) over-thinking approach.  It works by
having the helper proc set the necessary variables for the caller.

Tom

commit f223a278829665589f47cd39724148fa0a4ca5ce
Author: Tom Tromey <tromey@adacore.com>
Date:   Thu Mar 18 09:54:27 2021 -0600

    Add some error checking to DWARF assembler
    
    I had written a DWARF location expression like
    
        DW_OP_const1u
        DW_OP_stack_value
    
    ... and was surprised to see that the DW_OP_stack_value didn't appear
    in the "readelf" output.
    
    The problem here is that DW_OP_const1u requires an operand, but
    neither the DWARF assembler nor gas diagnosed this problem.
    
    This patch adds some checking to Dwarf::_location to try to avoid this
    in the future.  The checking is done via a helper proc that also
    dissects the argument list and sets local variables in the caller's
    frame.
    
    gdb/testsuite/ChangeLog
    2021-03-18  Tom Tromey  <tromey@adacore.com>
    
            * lib/dwarf.exp (Dwarf::_get_args): New proc.
            (Dwarf::_location): Use it.

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index c1d7fec6cc4..f76bb167502 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-03-18  Tom Tromey  <tromey@adacore.com>
+
+	* lib/dwarf.exp (Dwarf::_get_args): New proc.
+	(Dwarf::_location): Use it.
+
 2021-03-17  Simon Marchi  <simon.marchi@polymtl.ca>
 	    Pedro Alves  <pedro@palves.net>
 
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index f8fbd381810..1c672351dd7 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -890,6 +890,19 @@ namespace eval Dwarf {
 	}
     }
 
+    # Assign elements from LINE to the variables named in ARGS.
+    # If the wrong number of elements appear in LINE, error.
+    proc _get_args {line op args} {
+	if {[llength $line] != [llength $args] + 1} {
+	    error "usage: $op [string toupper $args]"
+	}
+
+	foreach var $args value [lreplace $line 0 0] {
+	    upvar $var __$var
+	    set __$var $value
+	}
+    }
+
     # This is a miniature assembler for location expressions.  It is
     # suitable for use in the attributes to a DIE.  Its output is
     # prefixed with "=" to make it automatically use DW_FORM_block.
@@ -926,57 +939,69 @@ namespace eval Dwarf {
 
 	    switch -exact -- $opcode {
 		DW_OP_addr {
-		    _op .${addr_size}byte [lindex $line 1]
+		    _get_args $line $opcode size
+		    _op .${addr_size}byte $size
 		}
 
 		DW_OP_regx {
-		    _op .uleb128 [lindex $line 1]
+		    _get_args $line $opcode register
+		    _op .uleb128 $register
 		}
 
 		DW_OP_pick -
 		DW_OP_const1u -
 		DW_OP_const1s {
-		    _op .byte [lindex $line 1]
+		    _get_args $line $opcode const
+		    _op .byte $const
 		}
 
 		DW_OP_const2u -
 		DW_OP_const2s {
-		    _op .2byte [lindex $line 1]
+		    _get_args $line $opcode const
+		    _op .2byte $const
 		}
 
 		DW_OP_const4u -
 		DW_OP_const4s {
-		    _op .4byte [lindex $line 1]
+		    _get_args $line $opcode const
+		    _op .4byte $const
 		}
 
 		DW_OP_const8u -
 		DW_OP_const8s {
-		    _op .8byte [lindex $line 1]
+		    _get_args $line $opcode const
+		    _op .8byte $const
 		}
 
 		DW_OP_constu {
-		    _op .uleb128 [lindex $line 1]
+		    _get_args $line $opcode const
+		    _op .uleb128 $const
 		}
 		DW_OP_consts {
-		    _op .sleb128 [lindex $line 1]
+		    _get_args $line $opcode const
+		    _op .sleb128 $const
 		}
 
 		DW_OP_plus_uconst {
-		    _op .uleb128 [lindex $line 1]
+		    _get_args $line $opcode const
+		    _op .uleb128 $const
 		}
 
 		DW_OP_piece {
-		    _op .uleb128 [lindex $line 1]
+		    _get_args $line $opcode size
+		    _op .uleb128 $size
 		}
 
 		DW_OP_bit_piece {
-		    _op .uleb128 [lindex $line 1]
-		    _op .uleb128 [lindex $line 2]
+		    _get_args $line $opcode size offset
+		    _op .uleb128 $size
+		    _op .uleb128 $offset
 		}
 
 		DW_OP_skip -
 		DW_OP_bra {
-		    _op .2byte [lindex $line 1]
+		    _get_args $line $opcode label
+		    _op .2byte $label
 		}
 
 		DW_OP_implicit_value {
@@ -1000,27 +1025,21 @@ namespace eval Dwarf {
 
 		DW_OP_implicit_pointer -
 		DW_OP_GNU_implicit_pointer {
-		    if {[llength $line] != 3} {
-			error "usage: $opcode LABEL OFFSET"
-		    }
+		    _get_args $line $opcode label offset
 
 		    # Here label is a section offset.
-		    set label [lindex $line 1]
 		    if { $dwarf_version == 2 } {
 			_op .${addr_size}byte $label
 		    } else {
 			_op .${offset_size}byte $label
 		    }
-		    _op .sleb128 [lindex $line 2]
+		    _op .sleb128 $offset
 		}
 
 		DW_OP_GNU_variable_value {
-		    if {[llength $line] != 2} {
-			error "usage: $opcode LABEL"
-		    }
+		    _get_args $line $opcode label
 
 		    # Here label is a section offset.
-		    set label [lindex $line 1]
 		    if { $dwarf_version == 2 } {
 			_op .${addr_size}byte $label
 		    } else {
@@ -1029,16 +1048,14 @@ namespace eval Dwarf {
 		}
 
 		DW_OP_deref_size {
-		    if {[llength $line] != 2} {
-			error "usage: DW_OP_deref_size SIZE"
-		    }
-
-		    _op .byte [lindex $line 1]
+		    _get_args $line $opcode size
+		    _op .byte $size
 		}
 
 		DW_OP_bregx {
-		    _op .uleb128 [lindex $line 1]
-		    _op .sleb128 [lindex $line 2]
+		    _get_args $line $opcode register offset
+		    _op .uleb128 $register
+		    _op .sleb128 $offset
 		}
 
 		default {