Re: [PATCH v4] [aarch64] Fix removal of non-address bits for PAuth

[Luis Machado <luis.machado@arm.com>]

Hi Simon,

On 12/12/22 18:54, Simon Marchi wrote:
> 
> Hi Luis,
> 
> I noted some nits, but the patch LGTM in any case:
> 
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
> 
>> +CORE_ADDR
>> +aarch64_mask_from_pac_registers (const CORE_ADDR cmask, const CORE_ADDR dmask)
>> +{
>> +  if (dmask != cmask)
>> +    {
>> +      /* Warn if the masks are different.  */
>> +      aarch64_pauth_mask_warning ();
> 
> We don't really need aarch64_pauth_mask_warning anymore, you can inline
> the warning here.
> 

Done now.

>> +# Cycle through the tag and pac bit ranges and check how GDB
>> +# behaves when trying to access these addresses.
>> +foreach upper_bits {"0x0" "0x1" "0x2" "0x4" "0x8" "0x10" "0x20" "0x40" "0x80"} {
>> +    foreach lower_bits {"0x0" "0x1" "0x2" "0x4" "0x8" "0x10" "0x20" "0x40"} {
> 
> I would perhaps suggest using foreach_with_prefix for these two, and
> then you can simplify the test names below.  I like using prefixes,
> because it standardizes how the test names look when multiple variants
> are tested or iterations are executed.

For a moment I forgot the prefix variable gets printed alongside the value, which made me print it
by hand so it would be more meaningful. Adjusted now. Thanks!

I'll send a v5 of what I plan to push.

> 
> Simon