Re: RFC: problems with minimal symbols (without a type)

[Pedro Alves <palves at redhat dot com>]

On 11/15/2017 01:35 AM, Joel Brobecker wrote:
>>> And unfortunately, the Ada equivalent of casting does not work either:
>>>
>>>     (gdb) print integer(constraint_error)
>>>     'constraint_error' has unknown type; cast it to its declared type
>>
>> Shouldn't we just make that work, like it works for C?
> 
> Of course. It's at the top of my list, and independent in my mind
> of the discussion about forcing the cast. It was a convenient
> work-around, however, allowing me to reduce the urgency to fix
> this issue with casting.

Sorry, it wasn't clear to me that this was intended as a
workaround as opposed to a proposal to make Ada always
assume int.

I'm not seeing this as independent -- if GDB assumes unknown
types are integers, then we won't need to support casting from
unknown types to anything else in the first place, it just
falls out of supporting conversion from integer to anything else.

Unless, gdb assumed int if there's no cast, and used the
cast-to type if there's a cast.  I don't think that's a good
idea, because it'd still lead to surprising results.
You could even get very hard-to-explain-by-users results,
like:

 int64_t global = 0xffffffff00000000;

 (gdb) p global
 -1                   

-1 looks reasonable, the program sets global to -1 some times.
I.e., there's no reason for a user to suspect gdb here, even
though -1 was actually the incorrect value.

And then with a cast, you'd get these conflicting results:

 (gdb) p /x global
 0xffffffff
 (gdb) p /x (int64_t) (int) global
 0xffffffffffffffff
 (gdb) p /x (int64_t) global
 0xffffffff00000000

Notice that this isn't just about printing the values
directly.  You can get hard-to-diagnose problems if
if you pass the value directly to some function.

  extern void function (unsigned long long);

 (gdb) p function (global) 

Above old GDB passes 0xffffffffffffffff, incorrectly.

> That being said...
> 
>> Let me quote a conversation that happened on IRC just last week
>> (user name anonymized):
> [...]
>>  <palves>	you have to cast malloc to the right function pointer type, and then call that.
>>  <palves>	something like: p ((void * (*) (size_t )) malloc) (128)
>>  <palves>	in master, the simpler 'p (void*)malloc (128)' does the right thing.
>>  <palves>	(it infers the prototype from the type of the passed in arguments + the cast-to type)
> 
> I understand why GDB now behaves more naturally when using the casting
> information to infer the symbol's type and how this is an improvement.
> But wouldn't that part be orthogonal to the question whether GDB should
> make this case mandatory or not?

I actually pasted that part just for completeness.  The point I was
really trying to convey is in the unquoted part.  I.e, that this is
really a FAQ, that frequently users show up on IRC confused
by GDB showing incorrect results they can't explain.

IMHO, it's also very common to have globals that are pointers,
which tend to be 64-bit nowadays, while integers are 32-bit,
leading to problems with incorrect slicing and sign extending,
similar to that user's issue with the return type of malloc.

> 
> This is probably a judgement call. And since I didn't comment on
> the patch when it was proposed, I suppose it's fair that I try it
> for Ada users as well, and see how they react. It'll keep the languages
> consistent too, which is an advantage.
> 
> Just to answer some of your questions (therefore not in the spirit
> of continuing a debate):
> 
>> At least an Ada conversion/cast is Ada syntax.
>> The "print {my_type} minsym'address" syntax above looks like
>> the GDB syntax/extension that works with C too?
> 
> Yes, this part is documented in the Ada section of the GDB manual.
> 

OK.  When involving "minsym" in a more complicated expression,
it seems to me that the Ada conversion syntax would be a bit
more convenient / natural:

(gdb) print foo + long_integer (minsym) + bar
(gdb) print foo + {long_integer} minsym'address + bar

But then again, I'm clueless on most things Ada.  :-)

>> How do users discover that that gdb syntax extension is available
>> and that they need to use it?
>> For non-integer types, users must already do some casting to get at
>> the real data.  Why treat integers differently?
> 
> For me, this is because those minimal symbols are typically integral
> types. So, for a non-trivial part of the times I use this feature,
> it actually gives me the right answer. Therefore, for me, GDB lost
> a small feature that didn't always work, but still did work for a good
> chunk of the scenarios I was involved in.
> 
> I'll work on fixing the casting whenever I find some time...

Thanks!  Hopefully it'll be a simple change to ada_evaluate_subexp's
UNOP_CAST handling, mirroring the evaluate_subexp_for_cast in
eval.c.  Let me know if you run into something odd.

> 
> That got me to one piece of code in evaluate_subexp_for_cast:
> 
>       /* Don't allow e.g. '&(int)var_with_no_debug_info'.  */
>       if (VALUE_LVAL (val) == lval_memory)
>         {
>           if (value_lazy (val))
>             value_fetch_lazy (val);
>           VALUE_LVAL (val) = not_lval;
>         }
> 
> I was wondering why do we not want to allow someone get its
> address? I checked the commit that introduced this change,
> and it doesn't say.

This is to follow usual language rules.  A cast expression isn't
an lvalue, so you can't take its address:

 int global;

 void foo ()
 {
   int *p = & (int) global;
 }

 $ gcc cast-address.c -o cast-address -g3 -O0
 cast-address.c: In function ‘foo’:
 cast-address.c:5:12: error: lvalue required as unary ‘&’ operand
    int *p = & (int) global;
             ^

So accordingly, taking the address of the lvalue in GDB
does work:

 (gdb) p &dataglobal
 $1 = (<data variable, no debug info> *) 0x601040 <dataglobal>

But taking the address of the rvalue doesn't:

 (gdb) p &(int) dataglobal
 Attempt to take address of value not located in memory.
 (gdb) p *& (int) dataglobal
 Attempt to take address of value not located in memory.

Just like what you'd get if you had debug info
for "dataglobal".

The correct way is to cast the resulting pointer instead:

 (gdb) p (int *) &dataglobal
 $2 = (int *) 0x601040 <dataglobal>
 (gdb) p *(int *) &dataglobal
 $3 = 3

which works the same whether you have debug info for
dataglobal or not.

This is covered by the tests in gdb.base/nodebug.exp, in the loop
right after "We can't rely on uintXX_t".

About the exception catchpoints, to answer my own question
about which expression is GDB trying to evaluate, I was able to 
reproduce the issue with catchpoints using the gdb.ada/mi_exc_info.exp 
testcase, btw, using 'strip -g' on the testcase's binary.

 (gdb) set language ada
 (gdb) catch exception const__aint_global_gdb_e
 warning: failed to reevaluate internal exception condition for catchpoint 0: 'const.aint_global_gdb_e' has unknown type; cast it to its declared type
 Catchpoint 2: `const__aint_global_gdb_e' Ada exception

Setting a breakpoint on 'warning' reveals:

 (top-gdb) p s
 $1 = 0x1ec6d00 "long_integer (e) = long_integer (&const__aint_global_gdb_e)"

Curious, I didn't know that "&" worked in Ada too.

Thanks,
Pedro Alves