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

Simon Marchi via Gdb-patches
2021-01-25T02:29:00Z

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, " ");
>
> So, when we do a ptype, we don't get an error, but instead, get
> some information about the symbol:
>
> | (gdb) ptype opaque
> | type =
>
>
> 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