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

Reply via email to