[PATCH] D25916: Modules: emit an error instead of a random crash (or a misleading error) due to use-after-free.

Manman Ren via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 14 12:08:15 PST 2016


manmanren added a comment.

@ Ben,

We are hitting this issue when building large projects, but the reproducibility is quite low.

This proposed patch is currently a little too complicated. I am thinking about just fixing the testing case for now, and adding the check later when we start to share some data structures between threads (the idea of keeping MemoryBuffer consistent for threads within a single clang invocation).

For this testing case, we ignore the diagnostic options when a module is imported by a system module (see the code snippet below):

  ModuleFile *TopImport = *ModuleMgr.rbegin();
  while (!TopImport->ImportedBy.empty())
    TopImport = TopImport->ImportedBy[0];
  if (TopImport->Kind != MK_ImplicitModule)
    return false;
  
  StringRef ModuleName = TopImport->ModuleName;
  assert(!ModuleName.empty() && "diagnostic options read before module name");
  
  Module *M = PP.getHeaderSearchInfo().lookupModule(ModuleName);
  assert(M && "missing module");
  
  // FIXME: if the diagnostics are incompatible, save a DiagnosticOptions that
  // contains the union of their flags.
  return checkDiagnosticMappings(*Diags, ExistingDiags, M->IsSystem, Complain);

And here

  // If we're reading the first module for this group, check its options
  // are compatible with ours. For modules it imports, no further checking
  // is required, because we checked them when we built it.
  if (Listener && !ImportedBy) {

Does it mean that a system module should only import system modules? If a system module is allowed to import non-system modules, for a non-system module, we will validate diagnostic options differently depending on whether a system module or a non-system module imports it. This will cause a non-system module that was validated earlier to be invalidated by a child thread.

Thanks,
Manman


https://reviews.llvm.org/D25916





More information about the cfe-commits mailing list