This is an archive of the discontinued LLVM Phabricator instance.

[NewPM] Port tsan
ClosedPublic

Authored by philip.pfaffe on Jan 8 2019, 4:40 AM.

Details

Summary

A straightforward port of tsan to the new PM, following the same path
as D55647.

Diff Detail

Repository
rL LLVM

Event Timeline

philip.pfaffe created this revision.Jan 8 2019, 4:40 AM

Only one real question... Worried a little about the total cost here, but the code looks fine.

llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
88 ↗(On Diff #180641)

Similar documentation update as for msan?

179 ↗(On Diff #180641)

There are a lot of functions here, including numerous loops... Is this really ok to do every function?

chandlerc accepted this revision.Jan 8 2019, 9:42 AM

On further thought, what is maybe best is to land this and see how it goes. LGTM overall. We can try again if something blows up.

This revision is now accepted and ready to land.Jan 8 2019, 9:42 AM
This revision was automatically updated to reflect the committed changes.
delcypher added a subscriber: delcypher.EditedJan 9 2019, 6:25 AM

@philip.pfaffe This patch got reverted (not by me r350719) because it broke TSan on macOS.

If I diff the LLVM IR between a working AppleClang and the important thing I see is

 @.str = private unnamed_addr constant [7 x i8] c"hello\0A\00", align 1
-@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @tsan.module_ctor, i8* null }]
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @__tsan_init, i8* null }]
 
 ; Function Attrs: noinline norecurse optnone sanitize_thread ssp uwtable
 define i32 @main() #0 personality i32 (...)* @__gcc_personality_v0 {
@@ -30,11 +30,6 @@
 
 declare void @__tsan_init()
 
-define internal void @tsan.module_ctor() {
-  call void @__tsan_init()
-  ret void
-}
-

On macOS it seems dyld doesn't like __tsan_init being called directly from llvm.global_ctors. I get

dyld: initializer function 0x100129cd0 not in mapped image for <path_to_binary>

__tsan_init is external to generated TSan code (it lives in the TSan runtime), where as tsan.module_ctor() was in the same module. Perhaps dyld doesn't like calling functions external to the module directly.

@philip.pfaffe

__tsan_init is external to generated TSan code (it lives in the TSan runtime), where as tsan.module_ctor() was in the same module. Perhaps dyld doesn't like calling functions external to the module directly.

I've confirmed this with the authors of dyld. The indirection via tsan.module_ctor() is necessary for Darwin platforms because dyld requires that the global constructors for an image (module) point to functions in the same .TEXT segment as that image.

philip.pfaffe marked an inline comment as done.Jan 16 2019, 3:19 AM

@delcypher Thanks for pointing it out! Just landed a revised version in rL351314.