Patchwork [v2,09/10] Avoid undefined behavior in expression dumping

login
register
mail settings
Submitter Tom Tromey
Date Oct. 2, 2018, 4:44 a.m.
Message ID <20181002044420.17628-10-tom@tromey.com>
Download mbox | patch
Permalink /patch/29617/
State New
Headers show

Comments

Tom Tromey - Oct. 2, 2018, 4:44 a.m.
-fsanitize=undefined pointed out undefined behavior in
dump_raw_expression like:

    runtime error: load of value 2887952, which is not a valid value for type 'exp_opcode'

dump_raw_expression will try to print the opcode for each element of
the expression, even when it is not valid.  To allow this, but have it
avoid undefined behavior, this patch sets the underlying type of enum
exp_opcode, and arranges for op_name to handle invalid opcodes more
nicely.

Before this patch, debug-expr.exp shows:

Dump of expression @ 0x60f000007750, before conversion to prefix form:
	Language c, 8 elements, 16 bytes each.
	Index                Opcode         Hex Value  String Value
	    0               OP_TYPE  89  Y...............
   <unknown 3851920>  107820862850704  ..:..b..........
	    2               OP_TYPE  89  Y...............
	    3          OP_VAR_VALUE  40  (...............
	    4     <unknown 2807568>  107820861806352  ..*..b..........
	    5     <unknown 2806368>  107820861805152  `.*..b..........
	    6          OP_VAR_VALUE  40  (...............
	    7      UNOP_MEMVAL_TYPE  57  9...............

Afterward, the output is:

Dump of expression @ 0x4820f90, before conversion to prefix form:
	Language c, 8 elements, 16 bytes each.
	Index                Opcode         Hex Value  String Value
	    0               OP_TYPE  89  Y...............
	    1   unknown opcode: 176  75444400  .0..............
	    2               OP_TYPE  89  Y...............
	    3          OP_VAR_VALUE  40  (...............
	    4               OP_BOOL  74616912  P.r.............
	    5   unknown opcode: 128  74615680  ..r.............
	    6          OP_VAR_VALUE  40  (...............
	    7      UNOP_MEMVAL_TYPE  57  9...............

gdb/ChangeLog
2018-10-01  Tom Tromey  <tom@tromey.com>

	* expression.h (enum exp_opcode): Use uint8_t as base type.
	* expprint.c (op_name): Handle invalid opcodes.
---
 gdb/ChangeLog    | 5 +++++
 gdb/expprint.c   | 7 +++++++
 gdb/expression.h | 2 +-
 3 files changed, 13 insertions(+), 1 deletion(-)
Pedro Alves - Oct. 3, 2018, 5:48 p.m.
On 10/02/2018 05:44 AM, Tom Tromey wrote:
> -fsanitize=undefined pointed out undefined behavior in
> dump_raw_expression like:
> 
>     runtime error: load of value 2887952, which is not a valid value for type 'exp_opcode'
> 
> dump_raw_expression will try to print the opcode for each element of
> the expression, even when it is not valid.  To allow this, but have it
> avoid undefined behavior, this patch sets the underlying type of enum
> exp_opcode, and arranges for op_name to handle invalid opcodes more
> nicely.
> 
> Before this patch, debug-expr.exp shows:
> 
> Dump of expression @ 0x60f000007750, before conversion to prefix form:
> 	Language c, 8 elements, 16 bytes each.
> 	Index                Opcode         Hex Value  String Value
> 	    0               OP_TYPE  89  Y...............
>    <unknown 3851920>  107820862850704  ..:..b..........
> 	    2               OP_TYPE  89  Y...............
> 	    3          OP_VAR_VALUE  40  (...............
> 	    4     <unknown 2807568>  107820861806352  ..*..b..........
> 	    5     <unknown 2806368>  107820861805152  `.*..b..........
> 	    6          OP_VAR_VALUE  40  (...............
> 	    7      UNOP_MEMVAL_TYPE  57  9...............
> 
> Afterward, the output is:
> 
> Dump of expression @ 0x4820f90, before conversion to prefix form:
> 	Language c, 8 elements, 16 bytes each.
> 	Index                Opcode         Hex Value  String Value
> 	    0               OP_TYPE  89  Y...............
> 	    1   unknown opcode: 176  75444400  .0..............
> 	    2               OP_TYPE  89  Y...............
> 	    3          OP_VAR_VALUE  40  (...............
> 	    4               OP_BOOL  74616912  P.r.............
> 	    5   unknown opcode: 128  74615680  ..r.............
> 	    6          OP_VAR_VALUE  40  (...............
> 	    7      UNOP_MEMVAL_TYPE  57  9...............
> 

LGTM.  Thanks for adding the example.

Thanks,
Pedro Alves

Patch

diff --git a/gdb/expprint.c b/gdb/expprint.c
index d6ed41253e..e87b3b709b 100644
--- a/gdb/expprint.c
+++ b/gdb/expprint.c
@@ -687,6 +687,13 @@  static int dump_subexp_body (struct expression *exp, struct ui_file *, int);
 const char *
 op_name (struct expression *exp, enum exp_opcode opcode)
 {
+  if (opcode >= OP_UNUSED_LAST)
+    {
+      char *cell = get_print_cell ();
+      xsnprintf (cell, PRINT_CELL_SIZE, "unknown opcode: %u",
+		 unsigned (opcode));
+      return cell;
+    }
   return exp->language_defn->la_exp_desc->op_name (opcode);
 }
 
diff --git a/gdb/expression.h b/gdb/expression.h
index bc7625f984..a5cb4c678e 100644
--- a/gdb/expression.h
+++ b/gdb/expression.h
@@ -39,7 +39,7 @@ 
    and skip that many.  Strings, like numbers, are indicated
    by the preceding opcode.  */
 
-enum exp_opcode
+enum exp_opcode : uint8_t
   {
 #define OP(name) name ,