Re: [RFC 2/3] Record function descriptor address instead of function address in value

[Yao Qi <qiyaoltc at gmail dot com>]

On Fri, Oct 14, 2016 at 6:35 PM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> On 2016-10-14 06:53, Yao Qi wrote:
>>
>> In this patch, I add a new gdbarch method convert_from_func_addr, which
>> converts function address back to function descriptor address or
>> function pointer address.  It is the reversed operation of
>> convert_from_func_ptr_addr.  We convert function address to function
>> descriptor address when,
>
>
> I think these could be better named, byt saying what it converts from _and_
> what it converts to.  With convert_from_func_addr, we know it takes as input
> a function address, but we don't know what it converts it to.  What about
> something like convert_func_address_to_descriptor (or
> convert_func_addr_to_desc if you prefer shorter names)?
>
> I think that it would also be clearer if we always used the same terminology
> (address and descriptor seem good).  It is not very intuitive what is the
> difference between convert_from_func_ptr_addr and convert_from_func_addr.
> "function pointer address" and "function address" sound too close to each
> other, so it's easy to confuse them...
>

If we want to know what these two methods convert to, I'd like to use
"func_addr" and "func_ptr_addr".  These two methods can be named as
convert_func_addr_to_func_ptr_addr and
convert_func_ptr_addr_to_func_addr, but they are a little bit long.  Maybe,
remove "convert_" from the names?

>> 3. User visible changes
>>
>> This patch brings several user visible changes, which look more
>> accurate, shown by this table below,
>>
>>  COMMAND           BEFORE                       AFTER
>>  p main            main function address        main function descriptor
>>                                                 address
>>  disass main       disassembly function main    not changed
>>  disass main+4,+4  disassembly 4 bytes from     disassembly 4 bytes from
>>                    function main address + 4    main's function descriptor
>> + 4
>>  x/i main          show one instruction on      show one instruction on
>> main's
>>                    function main address        function descriptor
>>
>> Although the latter looks inconvenient, that is consistent to the
>> meaning on C language level.  Due to these changes, test cases are
>> adjusted accordingly.
>
>
> Could you provide example of the actual output of those commands, before and
> after?  It is not clear to me what the difference will be.
>

"p incr" shows the function descriptor address instead of function address.

old ppc64 gdb:
(gdb) p incr
$2 = {int (int)} 0x1000069c <incr>
(gdb) p &incr
$3 = (int (*)(int)) 0x1000069c <incr>

old arm gdb:
(gdb) p incr
$2 = {int (int)} 0x104fe <incr>
(gdb) p &incr
$3 = (int (*)(int)) 0x104fe <incr>

new ppc64 gdb:
(gdb) p incr
$2 = {int (int)} 0x10020090 <incr>
(gdb) p &incr
$3 = (int (*)(int)) @0x10020090: 0x1000069c <incr>

new arm gdb:
(gdb) p incr
$1 = {int (int)} 0x104ff <incr>
(gdb) p &incr
$2 = (int (*)(int)) @0x104ff: 0x104fe <incr>

"disassemble incr" is not changed,

(gdb) disassemble incr
Dump of assembler code for function incr:
   0x000000001000069c <+0>: mflr    r0
   0x00000000100006a0 <+4>: std     r0,16(r1)

"disassemble incr+4,+4" is changed, "incr" means function address
in old gdb, but it means function descriptor address in new gdb.

old gdb:
(gdb) disassemble incr+4,+4
Dump of assembler code from 0x100006a0 to 0x100006a4:
   0x00000000100006a0 <incr+4>: std     r0,16(r1)

new gdb:
(gdb) disassemble incr+4,+4
Dump of assembler code from 0x10020094 to 0x10020098:
   0x0000000010020094 <incr+4>: ps_madds0 f0,f0,f26,f0

... so "disassemble incr+4,+4" makes no sense in new gdb.  However,
you can use address instead.

Similarly, in old gdb, "x/i incr" is to show the first instruction at function
incr, but in new gdb, it shows the first instruction at function descriptor.

(gdb) x/i incr
   0x1000069c <incr>: mflr    r0
(gdb) x/i incr
   0x10020090 <incr>: .long 0x0

-- 
Yao (齐尧)