Re: [PATCH 009/203] what is this code for

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-01-03 1:00 a.m., Joel Brobecker wrote:
> Hi Tom,
> 
>> @@ -1224,7 +1224,7 @@ eval_op_var_msym_value (struct type *expect_type, struct expression *exp,
>>  
>>    struct type *type = value_type (val);
>>    if (type->code () == TYPE_CODE_ERROR
>> -      && (noside != EVAL_AVOID_SIDE_EFFECTS || pc != 0))
>> +      && (noside != EVAL_AVOID_SIDE_EFFECTS))
>>      error_unknown_type (msymbol->print_name ());
>>    return val;
> 
> I tracked the origin of this code down to the following commit...
> 
>     commit 46a4882b3c7d9ec981568b8b13a3c9c39c8f8e61
>     Author: Pedro Alves <palves@redhat.com>
>     Date:   Mon Sep 4 20:21:15 2017 +0100
>     Subject: Stop assuming no-debug-info variables have type int
> 
> ... which was the second stage of a similar change for functions.
> The revision log did not really give an explicit explanation for
> this code, but after a bit of thinking, I think I was able to
> figure it out.
> 
> Consider the case where you're trying to investigate a variable
> called "opaque" which is declared in the symbol table but does not
> have any debug info.
> 
>   * If noside != EVAL_AVOID_SIDE_EFFECTS (typically, we're trying
>     to print the value, then we always error out with:
> 
>         | 'opaque' has unknown type; cast it to its declared type
> 
>     ... This is what the patch is about. That's not what you are
>     asking about, because it's easy to understand. OK.
> 
>   * So, if noside == EVAL_AVOID_SIDE_EFFECTS, what does it mean
>     to only print the error message above if pc != 0?
> 
>     Well, if pc == 0, it means OP_VAR_MSYM_VALUE node is first,
>     and since that operator doesn't have any "parameters", what
>     it means to me is that it must actually be the only operand
>     of the expression. Thus, we must be facing the following
>     situation:
> 
>         | (gdb) ptype opaque
> 
>     And what do we do in that scenario? Well, we have code in gdbtype.c
>     which gives default types for the various kinds of minimal symbols.
>     For data symbols, it looks like this:
> 
>         | objfile_type->nodebug_data_symbol
>         |   = init_nodebug_var_type (objfile, "<data variable, no debug info>");
> 
>     So, when we do a ptype, we don't get an error, but instead, get
>     some information about the symbol:
> 
>         | (gdb) ptype opaque
>         | type = <data variable, no debug info>
> 
> 
>     If, on the other hand pc != 0, it means our minsym is part
>     of a larger expression, such as, for instance "opaque + 1".
>     With today's GDB, a "ptype" of the expression above generates
>     the same error message when when in EVAL_NORMAL mode:
> 
>         | (gdb) ptype opaque + 1
>         | 'opaque' has unknown type; cast it to its declared type
> 
>     If you remove the check above, then we no longer trigger
>     the "has unknown type" error. So, instead, we just evaluate
>     the minsym as is, which means we get a value whose type is
>     the type of "objfile_type->nodebug_data_symbol", in other words
>     TYPE_CODE_ERROR, which leads to:
> 
>         | (gdb) ptype opaque + 1
>         | Argument to arithmetic operation not a number or boolean.
> 
>     For minimal symbols which are text symbols, these get a default
>     type of TYPE_CODE_FUNC, so removing the above breaks the check
>     which forces the user to specify the return type.
> 
> We should probably add a test or two in gdb.base/nodebug.exp for that...
> 

In addition to this good explanation: I tried removing the `|| pc == 0`
from the code (based on current master) and got these failures:

+FAIL: gdb.base/nodebug.exp: p sizeof(dataglobal + 1)
+FAIL: gdb.base/nodebug.exp: ptype *dataglobal
+FAIL: gdb.base/nodebug.exp: ptype dataglobal + 1
+FAIL: gdb.base/nodebug.exp: ptype sizeof(dataglobal + 1)
+FAIL: gdb.base/nodebug.exp: whatis *dataglobal
+FAIL: gdb.base/nodebug.exp: whatis dataglobal + 1
+FAIL: gdb.base/nodebug.exp: whatis sizeof(dataglobal + 1)

So it's already covered.  Tom, are these cases correctly handled with
this series applied?

Simon