This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer_common] Rename OnPrint to __sanitizer_on_print.
ClosedPublic

Authored by morehouse on Sep 24 2019, 2:31 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

morehouse created this revision.Sep 24 2019, 2:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 24 2019, 2:31 PM

For the Go version we have a non-weak declaration without definition. How are we going to roll out the change? We would need to update llvm and the hook implementation atomically. But then I am confused how it works today. We have a non-extern "C" hook definition, so build should be broken for the last year...

morehouse added a comment.EditedSep 26 2019, 10:39 AM

I don't see TSAN_EXTERNAL_HOOKS defined anywhere internally. So presumably the non-weak declaration was never used.

Hmm.. Actually it is defined. I'll have to look into it more...

morehouse updated this revision to Diff 222679.Oct 1 2019, 1:06 PM
  • Keep Go's OnPrint declaration for now.

For the Go version we have a non-weak declaration without definition. How are we going to roll out the change? We would need to update llvm and the hook implementation atomically. But then I am confused how it works today. We have a non-extern "C" hook definition, so build should be broken for the last year...

I'm also not sure exactly why it's working internally. Apparently the extern "C" doesn't matter when compiling the race detector stuff, but I do get a linking error if I change the symbol name.

Let's temporarily keep the OnPrint symbol for Go. This will give us time to migrate to __sanitizer_on_print internally and then remove OnPrint here.

dvyukov accepted this revision.Oct 1 2019, 7:54 PM
This revision is now accepted and ready to land.Oct 1 2019, 7:54 PM
vitalybuka accepted this revision.Oct 2 2019, 11:03 AM
This revision was automatically updated to reflect the committed changes.