On Tue, Dec 16, 2014 at 11:23 AM, Jakub Jelinek <ja...@redhat.com> wrote: > On Tue, Dec 16, 2014 at 10:47:06AM +0100, Richard Biener wrote: >> On Mon, Dec 15, 2014 at 7:50 PM, Jakub Jelinek <ja...@redhat.com> wrote: >> > Hi! >> > >> > As discussed in the PR, to support exceptions in -fsanitize=thread code, >> > it is desirable to call __tsan_func_exit also when leaving functions by >> > means of exceptions. >> > >> > Adding EH too late sounds too hard to me, so this patch instead adds an >> > internal call during gimplification, makes sure the inliner removes the >> > internal calls from the inline functions >> > (we don't care about inlines, only about functions we emit), and >> > for functions that didn't go through gimplify_function_tree (e.g. omp/tm >> > etc. functions) just keeps using the old __tsan_func_exit additions. >> > >> > Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk? >> >> So the issue is externally throwing EH, right? I wonder if we can teach >> the unwinder to do __tsan_func_exit instead, or have hooks in it that can >> be used by tsan? At least the issue seems generic enough for >> code instrumentation to consider a more general solution? How does >> -finstrument-functions work with externally throwing EH? > > -finstrument-functions works the way I wrote the patch, in fact the > gimplify_function_tree bits I've added were right after the > -finstrument-functions handling of the same. > > Anyway, the alternative would be to wrap the various personality functions > like > __gcc_personality_v0 > __gxx_personality_v0 > __gcj_personality_v0 > __gccgo_personality_v0 > __gnu_objc_personality_v0 > call the dlsym (, RTLD_NEXT) version from there and if it returns > _URC_INSTALL_CONTEXT , try to figure out what frame it will be in. > We can query the IP (_Unwind_GetIP), or the CFA (_Unwind_GetCFA), but then > map it through supposedly target dependent code to the actual frame pointers > __tsan_func_* store (do they?).
I suppose we could annotate the CFA with appropriate information? > The problem with wrapping those personality functions is that I'm not sure > how could it work with the various -static* options. Say if you > -static-libstdc++ (and link libtsan dynamically), then the > __gxx_personality_v0 in the binary will not be wrapped by what is in > libtsan.so. > If you > -static-libstdc++ -static-libtsan, then __gxx_personality_v0 linked in > will be very likely from libstdc++ and thus again not overloaded, or if > -ltsan would come first (right now it doesn't), then you still couldn't > use dlsym to get at the overloaded symbol. Yeah, which is why I suggested that one might want to have a generic (list of) callbacks that can be registered (like malloc hooks). It would be all extensions but GCC controls the unwinding ABI, right? > So, while the __tsan_func_exit in cleanup is more expensive at runtime, > it will work with all the wierdo options people are using. True. As it follows existing practice with -finstrument-functions the patch is probably the way to go for GCC 5. I was just wondering of a less heavy-weight solution piggy-backing on the unwinder. I suppose that idea wouldn't work for SJLJ exceptions so you'd need both approaches anyway. Richard. > Jakub
