[PATCH] D28434: [Sanitizer Coverage] Fix Instrumentation to work on Windows.

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: lib/Transforms/Instrumentation/SanitizerCoverage.cpp:372-387
+      GlobalVariable *SectionStart, *SectionStop;
+      SectionStart = new GlobalVariable(M, Int32PtrTy, false,
+          GlobalVariable::ExternalLinkage, nullptr,
+          SanCovTracePCGuardSectionStart);
+      SectionStart->setVisibility(GlobalValue::HiddenVisibility);
+      SectionStop = new GlobalVariable(M, Int32PtrTy, false,
+          GlobalVariable::ExternalLinkage, nullptr,
----------------
mpividori wrote:
> mpividori wrote:
> > rnk wrote:
> > > I don't think we need this dynamic initialization in every TU on Windows. If ___start___sancov_guard is linked statically into every DLL, we can add code to that object file to call __sanitizer_cov_trace_pc_guard_init on Windows.
> > @rnk, do you mean register this function in the section ".CRT$XCU" , like you do with `register_dso_globals`? Yes, I agree.
> > Here, I just updated the code to do the same than on linux. I think we can improve this as you suggest, also for linux.
> @rnk I think this approach is ok for now, so we do the same than for linux, and we simplify the code.
Yeah, definitely OK for now. Eventually we should do the .CRT$XCU thing, though. It's nicer.


Repository:
  rL LLVM

https://reviews.llvm.org/D28434