Re: [PATCH] AArch64 pauth: Indicate unmasked addresses in backtrace

Pedro Alves
2019-07-17T15:18:09Z

On 7/17/19 4:01 PM, Simon Marchi wrote:
> On 2019-07-17 09:35, Alan Hayward wrote:
>>> On 17 Jul 2019, at 12:15, Pedro Alves <palves@redhat.com> wrote:
>>>
>>> On 7/17/19 9:14 AM, Alan Hayward wrote:
>>>> Armv8.3-a Pointer Authentication causes the function return address to be
>>>> obfuscated on entry to some functions. GDB must unmask the link register in
>>>> order to produce a backtrace.
>>>>
>>>> The following patch adds markers of <unmasked> to the bracktrace, to indicate
>>>> which addresses needed unmasking.
>>>>
>>>> For example, consider the following backtrace:
>>>>
>>>> (gdb) bt
>>>> 0Â  0x0000000000400490 in puts@plt ()
>>>> 1Â  0x00000000004005dc in foo ("hello") at cbreak-lib.c:6
>>>> 2Â  0x0000000000400604<unmasked> in bar () at cbreak-lib.c:12
>>>> 3Â  0x0000000000400620<unmasked> in barbar () at cbreak.c:17
>>>> 4Â  0x00000000004005b4 in main () at cbreak-3.c:10
>>>>
>>>> The functions in the cbreak-lib use pointer auth, obfuscating the return address
>>>> to the previous function.Â  The caused the addresses of bar and barbar to require
>>>> unmasking in order to unwind the backtrace.
>>>>
>>>> Alternatively, I considered replacing <unmasked> with a single chracter, such
>>>> as * for brevity reasons, but felt this would be non obvious for the user.
>>>
>>> I don't have a particular suggestion, though my first reaction was that
>>> it seemed a bit verbose.
>>>
>>> IMHO, the marker doesn't have to stand out and be expressive, since users can
>>> always look at the manual.
>>
>> Reading the manual is an assumption I’m not sure is anywhere near the
>> common case.
>> Saying that, I agree we shouldn’t be designing the output for the non-readers.
>>
>> This comment has reminded me I need to add something to the manual as
>> part of this
>> patch.
>>
>>
>>> Â Once they learn something, often being concise
>>> helps -- or in other words, once you learn what "<unmasked>" or "U" or whatever
>>> is, and you're used to it, what would you rather see?Â  What's the main
>>> information you're looking for when staring at the backtrace?Â  Thoughts
>>> like that should guide the output too, IMO.
>>
>> PAC is the official abbreviation for the feature, so maybe :PAC works best.
>>
>> (gdb) bt
>> 0Â  0x0000000000400490 in puts@plt ()
>> 1Â  0x00000000004005dc in foo ("hello") at cbreak-lib.c:6
>> 2Â  0x0000000000400604:PAC in bar () at cbreak-lib.c:12
>> 3Â  0x0000000000400620:PAC in barbar () at cbreak.c:17
>> 4Â  0x00000000004005b4 in main () at cbreak-3.c:10
>>
>>
>> Some of my attempts at different representations:
>> 2Â  0x0000000000400604* in bar () at cbreak-lib.c:12
>> 2Â  0x0000000000400604! in bar () at cbreak-lib.c:12
>> 2Â  0x0000000000400604U in bar () at cbreak-lib.c:122
>> 2Â  0x0000000000400604:U in bar () at cbreak-lib.c:122
>> 2Â 0x0000000000400604 in bar () at cbreak-lib.c:12
>> 2Â  0x0000000000400604[U] in bar () at cbreak-lib.c:12
>> 2Â  0x0000000000400604<M> in bar () at cbreak-lib.c:12
>> 2Â 0x0000000000400604 in bar () at cbreak-lib.c:12
>> 2Â  0x0000000000400604<PAC> in bar () at cbreak-lib.c:12
>> 2Â  0x0000000000400604PAC in bar () at cbreak-lib.c:12
>> 2Â  0x0000000000400604:PAC in bar () at cbreak-lib.c:12
>> 2Â  0x0000000000400604,PAC in bar () at cbreak-lib.c:12
>>
>> I found a single character was too hidden. A single character or symbol was also
>> a little confusing - my brain read U as unsigned, * as pointer, [] as an array.
>>
>> I also like ,PAC as it might be easier to add future extensions.
>
> It might not be easily doable, but I think it would be nice if you could somehow make it so the function names stay aligned (regardless of which marker you end up choosing), like:
>
> 0Â  0x0000000000400490Â Â Â Â  in puts@plt ()
> 1Â  0x00000000004005dcÂ Â Â Â  in foo ("hello") at cbreak-lib.c:6
> 2Â  0x0000000000400604 [U] in bar () at cbreak-lib.c:12
> 3Â  0x0000000000400620 [U] in barbar () at cbreak.c:17
> 4Â  0x00000000004005b4Â Â Â Â  in main () at cbreak-3.c:10

I almost suggested the same, but didn't when I realized that we
don't always print the addresses:

(top-gdb) bt
#0 gdb_main (args=0x7fffffffd3a0) at src/gdb/main.c:1186
#1 0x0000000000469a7e in main (argc=1, argv=0x7fffffffd4a8) at src/gdb/gdb.c:32

But if you do want to align the addresses, you could do that by
specifying a width for the "addr" column. If "[U]" is rare, given no column
headers, the spaces may look a bit odd, though. Maybe you'd want to pre-compute
the max column width by looking at the max number of frames that fit on a
page, or something along those lines.

Thanks,
Pedro Alves