search

staticanalysis / data-race-test

Automatically exported from code.google.com/p/data-race-test
0 stars 0 forks source link

[v2] Ignore writes to vptr in dtors #90

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
subj

Original issue reported on code.google.com by dvyu...@google.com on 12 Mar 2012 at 12:56

GoogleCodeExporter commented 9 years ago 
Alex, please remind us how you implemented this in the previous LLVM variant?

Original comment by konstant...@gmail.com on 13 Mar 2012 at 3:09

GoogleCodeExporter commented 9 years ago 
http://code.google.com/p/data-race-test/source/browse/trunk/llvm/opt/ThreadSanit
izer/ThreadSanitizer.cpp, look for "WorkAroundVptrRace"

I looked for "D0" and "D1" in the mangled name, which is not generally correct, 
but sufficient for Chrome. A better idea is to check whether a memory access is 
writing into vptr -- I think it's easy to tell from the variable name.

Original comment by gli...@chromium.org on 13 Mar 2012 at 3:31

GoogleCodeExporter commented 9 years ago 
Yes, for example the vtable for class A is mangled to _ZTV1A. You can check 
that the store operation is reading that global (and probably doing that in the 
dtor)

Original comment by gli...@chromium.org on 13 Mar 2012 at 3:38

GoogleCodeExporter commented 9 years ago 
Problem statement: accesses to vptr have to be treated specially to avoid false 
negatives 
and to avoid reporting benign races.
http://code.google.com/p/data-race-test/wiki/PopularDataRaces#Data_race_on_vptr

Suggestion from our LLVM friends: 
- Clang codegen adds special metadata to a function if it is 
dtor-of-type-with-vptr
and to store-to-vptr expressions (load-from-vptr may be needed too for other 
reasons).
- LLVM checks if a function is dtor-of-type-with-vptr.
- If yes, it either treats store-to-vptr in a special way or 
(if metadata is lost, which may happen) treats all stores in the first BB in a 
special way

Original comment by konstant...@gmail.com on 14 Mar 2012 at 10:01

GoogleCodeExporter commented 9 years ago 
added __tsan_vptr_update callback to rt interface. 
Will need to use it instead of __tsan_write4/8 when instrumenting vptr updates.

Original comment by konstant...@gmail.com on 19 Mar 2012 at 8:59

GoogleCodeExporter commented 9 years ago 
This is (almost fixed) by LLVM 153448.
- clang adds TBAA metadata to vptr stores
- LLVM (tsan module) instruments vptr stores using __tsan_vptr_update
- tsan run-time (__tsan_vptr_update) checks if the value of vptr is the same as 
before. 

Currently, TBAA is not generated at O0, so not closing the bug yet.

Original comment by konstant...@gmail.com on 26 Mar 2012 at 5:39

GoogleCodeExporter commented 9 years ago 
I hope this is fully fixed by LLVM r155430.

Original comment by konstant...@gmail.com on 24 Apr 2012 at 6:59