spyffe added inline comments.
================ Comment at: lib/Basic/LangOptions.cpp:32 - // FIXME: This should not be reset; modules can be different with different - // sanitizer options (this affects __has_feature(address_sanitizer) etc). - Sanitize.clear(); + // These options do not affect AST generation. SanitizerBlacklistFiles.clear(); ---------------- vsk wrote: > spyffe wrote: > > I'd replace this with > > ``` > > Sanitize.clear(SanitizerKind::CFI | SanitizerKind::Integer | > > SanitizerKind::Nullability | SanitizerKind::Undefined); > > ``` > > We know those options don't affect modules, as demonstrated by you clearing > > them anyway in CompilerInvocation... > I don't think this would work, and don't quite see a way to make it work. The > problem is that when importing a module into a CU, the CU's hash is required > to match the to-be-imported module's hash [1]. If we clear some sanitizer > options in resetNonModularOptions(), then the "matching hashes" check would > break, because you can't reset the non-modular options in a CU that you're > importing a module into. You'd end up disabling the sanitizers for the CU > you're building. > > [1] CompilerInstance.cpp > ``` > 1095 assert(ImportingInstance.getInvocation().getModuleHash() == > > > > > 1096 Invocation->getModuleHash() && "Module hash mismatch!"); > ``` Okay, I probably just didn't understand the role of `resetNonModularOptions()`. https://reviews.llvm.org/D32724 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
