On Mon, Nov 26, 2012 at 9:07 PM, Xinliang David Li <davi...@google.com> wrote: >>> >>> Ok. A slight problem then is that where the tsan pass sits right now, >>> >>> there >>> >>> is no easy way to find out if the builtin call will be expanded inline >>> >>> or >>> >>> not, so (similar for asan), if we instrument them in the pass, it might >>> >>> be >>> >>> instrumented twice at runtime if the builtin is expanded as a library >>> >>> call >>> >>> (once the added instrumentation for the builtin, once in the intercepted >>> >>> library call). That isn't wrong, just might need slightly more >>> >>> resources >>> >>> than if we ensured we only instrument the builtin if it isn't expanded >>> >>> inline. >>> >>> >>> >> >>> >> Should inlining of those functions be disabled as if -fno-builtins is >>> >> specified? >>> > >>> > Yes, it sounds reasonable. Performance characteristics under tsan >>> > differ significantly, so most likely we don't care. >>> >>> >>> Do we still need range access functions then? >> >> Yes. I think whether to instrument builtins inline or not should be a user >> decision > > Why is that? Most users probably are not aware builtins can be inlined > or care about it.
I agree with David. Most likely it has negligible overhead under tsan. Even if gcc provides ability to choose between builtins and library functions, it does not have to provide it under tsan (which is a very special mode). On the other hand, I don't know what complexity is involved on gcc side. As for runtime I can easily implement it, it's just a matter of writing 2 thin wrappers for existing functionality. >>, -fsanitize-thread and either nothinng or -fno-builtin. Implying >> -fno-builtin might penalize some code unnecessarily (remember, builtin >> handling is not only about expanding some builtins inline, but also about >> folding them if they are used with constant arguments, performing all sorts >> of optimizations etc., > > Yes, in some cases, it can incur a little runtime overhead as > > int i; > memset(&i, 1, 4); > > but I think compared with tsan's runtime overhead, this is almost negligible. > > >> all of which are ok, just we don't have a knob to >> disable inline expansion of some builtins (which, some are certainly ok, >> e.g. those that don't have a library counterpart). > > There is an internal interface to call: > > disable_builtin_function (...) -- tsan option process can call it > selectively for the stringop related builtins. > >> Also note that many programs use __builtin_* calls directly and then you >> can't disable recognizing them as builtins. > > Those calls won't be affected, I think. Compiler can also directly > generate builtin calls. However that means the range checking is still > needed. > > David > > >> >> Jakub
