Re: [PATCH 8/9] Avoid undefined behavior in expression dumping

[Pedro Alves <palves@redhat.com>]

On 08/27/2018 03:56 PM, 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.

Could you include before/after example output?

>  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: %d", int (opcode));

If the underlying type is unsigned, what is the rationale for
printing it as signed?

> +      return cell;
> +    }
>    return exp->language_defn->la_exp_desc->op_name (opcode);
>  }
>  
> diff --git a/gdb/expression.h b/gdb/expression.h
> index 9f26bb8d60b..db572efe2a3 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 ,
>  
> 

Thanks,
Pedro Alves