melver added a comment. Thanks! PTAL.
================ Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:759 // do globals-gc. - UseCtorComdat(UseGlobalsGC && ClWithComdat) { - this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover; - this->CompileKernel = - ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan : CompileKernel; - + UseCtorComdat(UseGlobalsGC && ClWithComdat && !this->CompileKernel) { C = &(M.getContext()); ---------------- glider wrote: > `UseGlobalsGC` assumes `!this->CompileKernel` already, doesn't it? I experimented a bit and found that even if one of them is enabled things break (say if you remove UseGlobalsGC here). This is more explicit and less error-prone in case somebody removes UseGlobalsGC from UseCtorComdat. And there is no real penalty here for having it on both. ================ Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1796 + const GlobalAlias &GA) const { + if (CompileKernel) { + // When compiling the kernel, globals that are aliased by symbols prefixed ---------------- glider wrote: > Maybe it's better to call this function only if CompileKernel is true? > I don't think we need it except for the kernel. Done, though the previous version was cleaner. I guarded the call but now we need an assert and a comment to make sure the check is moved when people decide to use this for more than the kernel. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81390/new/ https://reviews.llvm.org/D81390 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits