[PATCHv2 4/7, GCC, ARM, V8M] ARMv8-M Security Extension's cmse_nonsecure_entry: clear registers

[Kyrill Tkachov]

Hi Andre,

On 25/10/16 17:29, Andre Vieira (lists) wrote:
> On 24/08/16 12:01, Andre Vieira (lists) wrote:
>> On 25/07/16 14:23, Andre Vieira (lists) wrote:
>>> This patch extends support for the ARMv8-M Security Extensions
>>> 'cmse_nonsecure_entry' attribute to safeguard against leak of
>>> information through unbanked registers.
>>>
>>> When returning from a nonsecure entry function we clear all caller-saved
>>> registers that are not used to pass return values, by writing either the
>>> LR, in case of general purpose registers, or the value 0, in case of FP
>>> registers. We use the LR to write to APSR and FPSCR too. We currently do
>>> not support entry functions that pass arguments or return variables on
>>> the stack and we diagnose this. This patch relies on the existing code
>>> to make sure callee-saved registers used in cmse_nonsecure_entry
>>> functions are saved and restored thus retaining their nonsecure mode
>>> value, this should be happening already as it is required by AAPCS.
>>>
>>> This patch also clears padding bits for cmse_nonsecure_entry functions
>>> with struct and union return types. For unions a bit is only considered
>>> a padding bit if it is an unused bit in every field of that union. The
>>> function that calculates these is used in a later patch to do the same
>>> for arguments of cmse_nonsecure_call's.
>>>
>>> *** gcc/ChangeLog ***
>>> 2016-07-25  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>>>              Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>
>>>          * config/arm/arm.c (output_return_instruction): Clear
>>>          registers.
>>>          (thumb2_expand_return): Likewise.
>>>          (thumb1_expand_epilogue): Likewise.
>>>          (thumb_exit): Likewise.
>>>          (arm_expand_epilogue): Likewise.
>>>          (cmse_nonsecure_entry_clear_before_return): New.
>>>          (comp_not_to_clear_mask_str_un): New.
>>>          (compute_not_to_clear_mask): New.
>>>          * config/arm/thumb1.md (*epilogue_insns): Change length attribute.
>>>          * config/arm/thumb2.md (*thumb2_return): Likewise.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>> 2016-07-25  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>>>              Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>
>>>          * gcc.target/arm/cmse/cmse.exp: Test different multilibs separate.
>>>          * gcc.target/arm/cmse/struct-1.c: New.
>>>          * gcc.target/arm/cmse/bitfield-1.c: New.
>>>          * gcc.target/arm/cmse/bitfield-2.c: New.
>>>          * gcc.target/arm/cmse/bitfield-3.c: New.
>>>          * gcc.target/arm/cmse/baseline/cmse-2.c: Test that registers are
>>> cleared.
>>>          * gcc.target/arm/cmse/mainline/soft/cmse-5.c: New.
>>>          * gcc.target/arm/cmse/mainline/hard/cmse-5.c: New.
>>>          * gcc.target/arm/cmse/mainline/hard-sp/cmse-5.c: New.
>>>          * gcc.target/arm/cmse/mainline/softfp/cmse-5.c: New.
>>>          * gcc.target/arm/cmse/mainline/softfp-sp/cmse-5.c: New.
>>>
>> Updated this patch to correctly clear only the cumulative
>> exception-status (0-4,7) and the condition code bits (28-31) of the
>> FPSCR. I also adapted the code to be handle the bigger floating point
>> register files.
>>
>> ----
>>
>> This patch extends support for the ARMv8-M Security Extensions
>> 'cmse_nonsecure_entry' attribute to safeguard against leak of
>> information through unbanked registers.
>>
>> When returning from a nonsecure entry function we clear all caller-saved
>> registers that are not used to pass return values, by writing either the
>> LR, in case of general purpose registers, or the value 0, in case of FP
>> registers. We use the LR to write to APSR. For FPSCR we clear only the
>> cumulative exception-status (0-4, 7) and the condition code bits
>> (28-31). We currently do not support entry functions that pass arguments
>> or return variables on the stack and we diagnose this. This patch relies
>> on the existing code to make sure callee-saved registers used in
>> cmse_nonsecure_entry functions are saved and restored thus retaining
>> their nonsecure mode value, this should be happening already as it is
>> required by AAPCS.
>>
>> This patch also clears padding bits for cmse_nonsecure_entry functions
>> with struct and union return types. For unions a bit is only considered
>> a padding bit if it is an unused bit in every field of that union. The
>> function that calculates these is used in a later patch to do the same
>> for arguments of cmse_nonsecure_call's.
>>
>> *** gcc/ChangeLog ***
>> 2016-07-xx  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>>              Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>          * config/arm/arm.c (output_return_instruction): Clear
>>          registers.
>>          (thumb2_expand_return): Likewise.
>>          (thumb1_expand_epilogue): Likewise.
>>          (thumb_exit): Likewise.
>>          (arm_expand_epilogue): Likewise.
>>          (cmse_nonsecure_entry_clear_before_return): New.
>>          (comp_not_to_clear_mask_str_un): New.
>>          (compute_not_to_clear_mask): New.
>>          * config/arm/thumb1.md (*epilogue_insns): Change length attribute.
>>          * config/arm/thumb2.md (*thumb2_return): Duplicate pattern for
>>          cmse_nonsecure_entry functions.
>>
>> *** gcc/testsuite/ChangeLog ***
>> 2016-07-xx  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>>              Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>          * gcc.target/arm/cmse/cmse.exp: Test different multilibs separate.
>>          * gcc.target/arm/cmse/struct-1.c: New.
>>          * gcc.target/arm/cmse/bitfield-1.c: New.
>>          * gcc.target/arm/cmse/bitfield-2.c: New.
>>          * gcc.target/arm/cmse/bitfield-3.c: New.
>>          * gcc.target/arm/cmse/baseline/cmse-2.c: Test that registers are
>> cleared.
>>          * gcc.target/arm/cmse/mainline/soft/cmse-5.c: New.
>>          * gcc.target/arm/cmse/mainline/hard/cmse-5.c: New.
>>          * gcc.target/arm/cmse/mainline/hard-sp/cmse-5.c: New.
>>          * gcc.target/arm/cmse/mainline/softfp/cmse-5.c: New.
>>          * gcc.target/arm/cmse/mainline/softfp-sp/cmse-5.c: New.
>>
> Hi,
>
> Rebased previous patch on top of trunk as requested. No changes to
> ChangeLog.
>
> Cheers,
> Andre

+	  for (i = *regno; i < regno_t; i++)
+	    {
+	      /* For all but the last register used by this field only keep the
+		 padding bits that were padding bits in this field.  */
+	      padding_bits_to_clear_res[i] &= padding_bits_to_clear_t[i];
+	    }
+
+	    /* For the last register, keep all padding bits that were padding
+	       bits in this field and any padding bits that are still valid
+	       as padding bits but fall outside of this field's size.  */
+	    mask = (UINT32_MAX - ((uint32_t) 1 << last_used_bit_t)) + 1;
+	    padding_bits_to_clear_res[regno_t]
+	      &= padding_bits_to_clear_t[regno_t] | mask;

I tried this on a clean trunk (without any of my patches) and I'm getting a build error
about UINT32_MAX not being defined. Did you have some other patches in your tree that masked this?
I think avoiding UINT32_MAX would be sensible here. I don't see it used anywhere else in gcc.
Perhaps just using (uint32_t)-1 would work? Or reuse the 64-bit HOST_WIDE_INT_M1U to get a mask of all ones.

Kyrill