diff mbox

[asan] a small patch to fix bogus error about global buffer overflow

Message ID CA+4CFy4GhwkXLSn5JgsMRCcLgW4=T=Cx0MJh+M5Ub5jzbYRrWA@mail.gmail.com
State New
Headers show

Commit Message

Wei Mi Oct. 25, 2012, 10:48 p.m. UTC 
Hi,

Thanks for all the comments. Fixed. Ok to checkin?

2012-10-25  Wei Mi  <wmi@google.com>

        * varasm.c (assemble_variable): Set asan_protected even
        for decls that are already ASAN_RED_ZONE_SIZE or more
        bytes aligned.


On Thu, Oct 25, 2012 at 2:46 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Oct 25, 2012 at 02:32:33PM -0700, Wei Mi wrote:
>> A small patch to remove the bogus error reports exposed in the
>> spec2000 testing. In varasm.c, asan_protected should be equivalent
>> with asan_protect_global (decl) all the time, or else compiler will
>> not insert redzones for some globals planned to be protected.
>>
>> gcc/ChangeLog:
>> 2012-10-25   Wei Mi  <wmi@google.com>
>>
>> A small fix to remove bogus error report of global buffer overflow.
>> * varasm.c: correct the condition of asan_protected being true.
>
> The patch is almost ok, the ChangeLog entry is not.
> Two instead of 3 spaces between date and name, no need for introductory
> comment above * varasm.c line, name of modified function and capital
> letter after :.
> Perhaps
>         * varasm.c (assemble_variable): Set asan_protected even for decls
>         that are already ASAN_RED_ZONE_SIZE or more bytes aligned.
>
> Ok with that Change and:
>
>> --- varasm.c    (revision 192822)
>> +++ varasm.c    (working copy)
>> @@ -1991,11 +1991,10 @@ assemble_variable (tree decl, int top_le
>>    align_variable (decl, dont_output_data);
>>
>>    if (flag_asan
>> -      && asan_protect_global (decl)
>> -      && DECL_ALIGN (decl) < ASAN_RED_ZONE_SIZE * BITS_PER_UNIT)
>> +      && asan_protect_global (decl))
>>      {
>>        asan_protected = true;
>> -      DECL_ALIGN (decl) = ASAN_RED_ZONE_SIZE * BITS_PER_UNIT;
>> +      DECL_ALIGN (decl) = MAX (DECL_ALIGN (decl), ASAN_RED_ZONE_SIZE
>  * BITS_PER_UNIT);
>
> Too long line, put ASAN_RED_ZONE_SIZE * BITS_PER_UNIT on next
> line below the second DECL_ALIGN (decl).
>
>         Jakub

Comments

Jakub Jelinek Oct. 25, 2012, 10:58 p.m. UTC | #1 
On Thu, Oct 25, 2012 at 03:48:07PM -0700, Wei Mi wrote:
> Thanks for all the comments. Fixed. Ok to checkin?
> 
> 2012-10-25  Wei Mi  <wmi@google.com>
> 
>         * varasm.c (assemble_variable): Set asan_protected even
>         for decls that are already ASAN_RED_ZONE_SIZE or more
>         bytes aligned.

Yes, thanks.

	Jakub
diff mbox

Patch

Index: varasm.c
===================================================================
--- varasm.c	(revision 192822)
+++ varasm.c	(working copy)
@@ -1991,11 +1991,11 @@  assemble_variable (tree decl, int top_le
   align_variable (decl, dont_output_data);

   if (flag_asan
-      && asan_protect_global (decl)
-      && DECL_ALIGN (decl) < ASAN_RED_ZONE_SIZE * BITS_PER_UNIT)
+      && asan_protect_global (decl))
     {
       asan_protected = true;
-      DECL_ALIGN (decl) = ASAN_RED_ZONE_SIZE * BITS_PER_UNIT;
+      DECL_ALIGN (decl) = MAX (DECL_ALIGN (decl),
+                               ASAN_RED_ZONE_SIZE * BITS_PER_UNIT);
     }

   set_mem_align (decl_rtl, DECL_ALIGN (decl));