Re: [PATCH] Forbid section anchors for ASan build (PR sanitizer/81697)

[Maxim Ostapenko <m.ostapenko@samsung.com>]

+ Jakub

On 09/08/17 19:21, Jeff Law wrote:
> On 08/08/2017 12:46 AM, Vyacheslav Barinov wrote:
>> Hello,
>>
>> Andrew Pinski <pinskia@gmail.com> writes:
>>
>>> On Mon, Aug 7, 2017 at 11:15 PM, Slava Barinov <v.barinov@samsung.com> wrote:
>>>>         gcc/
>>>>         * varasm.c (use_object_blocks_p): Forbid section anchors for ASan
>>>>
>>>>         gcc/testsuite/
>>>>         * g++.dg/asan/global-alignment.cc: New test to test global
>>>>         variables alignment.
>>>
>>> Can you describe this a little bit more?  What is going wrong here?
>>> Is it because there is no red zone between the variables?
>> I've described situation in bugzilla PR 81697 with compiled files dumps, but
>> briefly yes, red zone size is incorrect between global variables.
>>
>> On certain platforms (we checked at least arm and ppc) the flow is following:
>> make_decl_rtl() calls create_block_symbol() on decl tree with ASan global
>> variable describing a string. But then get_variable_section() returns wrong
>> section (.rodata.str1.4 in my case) because check in asan_protect_global()
>> returns false due to !DECL_RTL_SET_P check.
>>
>> When variable is placed not into .rodata, but into .rodata.str ld treats it as
>> string and just shrinks the large part of red zone silently (gold at least
>> prints warning about wrong string alignment). But in run time libasan expects
>> that red zone is still there and reports false positives.
>>
>> In order to prevent the setting of RTL before ASan handling I tried to
>> reproduce the x86_64 behavior and found that use_object_blocks_p() returns
>> false for that platform. Accordingly to the function comment it reports if
>> 'current compilation mode benefits from grouping', and just switching it off
>> for any ASan builds resolves the issue.
>>
>>> Also I noticed you are using .cc as the testcase file name, why don't
>>> you use .C instead and then you won't need the other patch which you
>>> just posted.
>> Okay, attaching rebased version.
> In many ways it'd be better if asan could be made to work with section
> anchors.  Doing so means the code we're running for asan more closely
> matches what we're running in the normal case.

I'm not very familiar with this code, but as far as I can understand, 
the problem is that we have a circular dependency here -- 
categorize_decl_for_section calls asan_protect_global that returns false 
because DECL_RTL is not set at this point and assigns decl to 
SECCAT_RODATA_MERGE_CONST section. But when make_decl_rtl sets up 
SET_DECL_RTL (decl, x) and following asan_protect_global calls will 
return true and we end up with mismatch between section and alignment 
requirements. Perhaps we can add an additional flag to 
asan_protect_global to change its behavior in such cases (e.g. don't 
account for DECL_RTL_SET_P)? In this case we will change the section for 
SECCAT_RODATA without affecting global section anchors flag.

-Maxim

>
> But I'll defer to Jakub and Marek as they know the sanitizers far better
> than I do and are more familiar with the expected tradeoffs.
>
> jeff
>
>
>