RE: [PATCH] Instrument bit field and unaligned accesses for TSAN

[Bernd Edlinger]

On Fri, 2 Jan 2015 20:01:02, Jakub Jelinke wrote:
>
> On Mon, Dec 29, 2014 at 09:20:57PM +0100, Bernd Edlinger wrote:
>
>> --- gcc/sanitizer.def (revision 218963)
>> +++ gcc/sanitizer.def (working copy)
>> @@ -188,6 +188,10 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_WRITE8, "__tsa
>> BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST)
>> DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_WRITE16, "__tsan_write16",
>> BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST)
>> +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_READ_RANGE, "__tsan_read_range",
>> + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST)
>> +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_WRITE_RANGE, "__tsan_write_range",
>> + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST)
>>
>> DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_ATOMIC8_LOAD,
>> "__tsan_atomic8_load",
>
> For the BUILT_IN_VPTR_UPDATE builtin you also need to change
> the prototype, to BT_FN_VOID_PTR_PTR. Do you have a testcase for the
> __tsan_vptr_update bug? Can you submit it separately, because it
> probably is desirable also for the 4.9 and 4.8 branches.
>

OK, I will do that.
I removed the __tsan_vptr_update stuff from the patch, for now.

It will probably be difficult for me to find a test case for this,
because I am not really sure what __tsan_vptr_update is actually good for,
(i.e. the use case).


>> @@ -173,13 +227,21 @@ instrument_expr (gimple_stmt_iterator gsi, tree ex
>> gimple_set_location (g, loc);
>> gimple_seq_add_stmt_without_update (&seq, g);
>> }
>> - if (rhs == NULL)
>> + if ((size & -size) != size || size> 16
>
> Isn't (size & (size - 1)) == 0 a better check?
>

Yes, that's of course better, Thanks.

I think I should also change the function type of BUILT_IN_READ_RANGE
and BUILT_IN_WRITE_RANGE to BT_FN_VOID_PTR_SIZE?

I attached the updated version of the patch.

Is that OK too?

Thanks
Bernd.

> Otherwise LGTM.
>
> Jakub
                                          
gcc/ChangeLog:
2015-01-02  Bernd Edlinger  <bernd.edlin...@hotmail.de>

        Instrument bit field and unaligned accesses for TSAN.
        * asan.c (initialize_sanitizer_builtins): New function type
        BT_FN_VOID_PTR_SIZE.
        * sanitizer.def (BUILT_IN_TSAN_READ_RANGE): New built-in function.
        (BUILT_IN_TSAN_WRITE_RANGE): New built-in function.
        * tsan.c (instrument_expr): Handle COMPONENT_REF and BIT_FIELD_REF.
        Use BUILT_IN_TSAN_READ_RANGE and BUILT_IN_TSAN_WRITE_RANGE for
        unaligned memory regions.

testsuite/ChangeLog:
2015-01-02  Bernd Edlinger  <bernd.edlin...@hotmail.de>

        * c-c++-common/tsan/bitfield_race.c: New testcase.
        * g++.dg/tsan/aligned_vs_unaligned_race.C: Fixed.

patch-tsan-bitfields.diff
Description: Binary data