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

[Simon Marchi <simon.marchi@polymtl.ca>]

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<U> 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<P> 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

Simon