[PATCH] D32724: [Modules] Include the enabled sanitizers in the module hash
Sean Callanan via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon May 8 10:47:46 PDT 2017
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).
+ // These options do not affect AST generation.
> 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 . 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.
>  CompilerInstance.cpp
> 1095 assert(ImportingInstance.getInvocation().getModuleHash() ==
> 1096 Invocation->getModuleHash() && "Module hash mismatch!");
Okay, I probably just didn't understand the role of `resetNonModularOptions()`.
More information about the cfe-commits