[PATCH] D30963: Fix crash when an 'import a module' TypoCorrection has its CorrectionDecls replaced by visible decls.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe created this revision.

The TypoCorrection::RequiresImport flag is managed by
checkCorrectionVisibility() in SemaLookup.cpp. Currently, when none of the
declarations in the typo correction are visible RequiresImport is set to true,
and if there are no declarations, it's implicitly set to false by assigning a
default-constructed TypoCorrection. If all declarations are visible, nothing is
done.

The current logic assumes a TypoCorrection starts as a 'normal' correction and
gets converted into an 'import a module' correction when suggested fixes are not
visible. This is not always the case. The crash in the included test case is
caused by performQualifiedLookups() copying a TypoCorrection with
RequiresImport == true, clearing its CorrectionDecls, then finding another
visible declaration in a lookup. This results in an 'import a module' correction
for a visible declaration.

The fix makes checkCorrectionVisibility() set RequiresImport in all cases, so
the relationship between RequiresImport and suggestion visibility always holds
after returning.


https://reviews.llvm.org/D30963

Files:
  lib/Sema/SemaLookup.cpp
  test/Modules/Inputs/crash-typo-correction-visibility/module.h
  test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
  test/Modules/crash-typo-correction-visibility.cpp


Index: test/Modules/crash-typo-correction-visibility.cpp
===================================================================
--- /dev/null
+++ test/Modules/crash-typo-correction-visibility.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=module -o %T/module.pcm -emit-module %S/Inputs/crash-typo-correction-visibility/module.modulemap
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-file=%T/module.pcm %s -verify
+
+struct S {
+  int member; // expected-note {{declared here}}
+};
+
+int f(...);
+
+int b = sizeof(f(member)); // expected-error {{undeclared identifier 'member'}}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
@@ -0,0 +1,3 @@
+module "module" {
+  header "module.h"
+}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.h
@@ -0,0 +1 @@
+struct member;
Index: lib/Sema/SemaLookup.cpp
===================================================================
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3786,8 +3786,8 @@
                                       bool FindHidden);
 
 /// \brief Check whether the declarations found for a typo correction are
-/// visible, and if none of them are, convert the correction to an 'import
-/// a module' correction.
+/// visible. Set the correction's RequiresImport flag to true if none of the
+/// declarations are visible, false otherwise.
 static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC) {
   if (TC.begin() == TC.end())
     return;
@@ -3797,9 +3797,11 @@
   for (/**/; DI != DE; ++DI)
     if (!LookupResult::isVisible(SemaRef, *DI))
       break;
-  // Nothing to do if all decls are visible.
-  if (DI == DE)
+  // No filtering needed if all decls are visible.
+  if (DI == DE) {
+    TC.setRequiresImport(false);
     return;
+  }
 
   llvm::SmallVector<NamedDecl*, 4> NewDecls(TC.begin(), DI);
   bool AnyVisibleDecls = !NewDecls.empty();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30963.91803.patch
Type: text/x-patch
Size: 2232 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170315/62972a84/attachment-0001.bin>