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

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

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

Attachment: changelog-tsan-bitfields.txt
Description: Text document

Attachment: patch-tsan-bitfields.diff
Description: Binary data