[clang] 324d1bb - [Clang] Report an error and crash on source location exhaustion in macros (#69908)

[via cfe-commits]

Author: Ilya Biryukov
Date: 2023-10-23T14:29:00+02:00
New Revision: 324d1bb35aefec737ef381d98211f7771c4b0ab5

URL: https://github.com/llvm/llvm-project/commit/324d1bb35aefec737ef381d98211f7771c4b0ab5
DIFF: https://github.com/llvm/llvm-project/commit/324d1bb35aefec737ef381d98211f7771c4b0ab5.diff

LOG: [Clang] Report an error and crash on source location exhaustion in macros (#69908)

`createExpansionLocImpl` has an assert that checks if we ran out of
source locations. We have observed this happening on real code and in
release builds the assertion does not fire and the compiler just keeps
running indefinitely without giving any indication that something went
wrong.

Diagnose this problem and reliably crash to make sure the problem is
easy to detect.

I have also tried:
- returning invalid source locations,
- reporting sloc address space usage on error.

Both caused the compiler to run indefinitely. It would be nice to dig
further why that happens, but until then crashing seems like a better
alternative.

Added: 
    

Modified: 
    clang/include/clang/Basic/DiagnosticCommonKinds.td
    clang/lib/Basic/SourceManager.cpp
    clang/test/Lexer/SourceLocationsOverflow.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index f2df283c74829f6..9f0ccd255a32148 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -356,9 +356,8 @@ def err_file_modified : Error<
   "file '%0' modified since it was first processed">, DefaultFatal;
 def err_file_too_large : Error<
   "sorry, unsupported: file '%0' is too large for Clang to process">;
-def err_include_too_large : Error<
-  "sorry, this include generates a translation unit too large for"
-  " Clang to process.">, DefaultFatal;
+def err_sloc_space_too_large : Error<
+  "sorry, the translation unit is too large for Clang to process: ran out of source locations">, DefaultFatal;
 def err_unsupported_bom : Error<"%0 byte order mark detected in '%1', but "
   "encoding is not supported">, DefaultFatal;
 def err_unable_to_rename_temp : Error<

diff  --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index c44ecacb3de3a10..d627c233a3aa094 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -605,7 +605,7 @@ FileID SourceManager::createFileIDImpl(ContentCache &File, StringRef Filename,
   unsigned FileSize = File.getSize();
   if (!(NextLocalOffset + FileSize + 1 > NextLocalOffset &&
         NextLocalOffset + FileSize + 1 <= CurrentLoadedOffset)) {
-    Diag.Report(IncludePos, diag::err_include_too_large);
+    Diag.Report(IncludePos, diag::err_sloc_space_too_large);
     noteSLocAddressSpaceUsage(Diag);
     return FileID();
   }
@@ -663,10 +663,16 @@ SourceManager::createExpansionLocImpl(const ExpansionInfo &Info,
     return SourceLocation::getMacroLoc(LoadedOffset);
   }
   LocalSLocEntryTable.push_back(SLocEntry::get(NextLocalOffset, Info));
-  // FIXME: Produce a proper diagnostic for this case.
-  assert(NextLocalOffset + Length + 1 > NextLocalOffset &&
-         NextLocalOffset + Length + 1 <= CurrentLoadedOffset &&
-         "Ran out of source locations!");
+  if (NextLocalOffset + Length + 1 <= NextLocalOffset ||
+      NextLocalOffset + Length + 1 > CurrentLoadedOffset) {
+    Diag.Report(SourceLocation(), diag::err_sloc_space_too_large);
+    // FIXME: call `noteSLocAddressSpaceUsage` to report details to users and
+    // use a source location from `Info` to point at an error.
+    // Currently, both cause Clang to run indefinitely, this needs to be fixed.
+    // FIXME: return an error instead of crashing. Returning invalid source
+    // locations causes compiler to run indefinitely.
+    llvm::report_fatal_error("ran out of source locations");
+  }
   // See createFileID for that +1.
   NextLocalOffset += Length + 1;
   return SourceLocation::getMacroLoc(NextLocalOffset - (Length + 1));

diff  --git a/clang/test/Lexer/SourceLocationsOverflow.c b/clang/test/Lexer/SourceLocationsOverflow.c
index 2b838ca4e861d08..c42f648fedf5018 100644
--- a/clang/test/Lexer/SourceLocationsOverflow.c
+++ b/clang/test/Lexer/SourceLocationsOverflow.c
@@ -1,6 +1,6 @@
 // RUN: not %clang %s -S -o - 2>&1 | FileCheck %s
 // CHECK: In file included from {{.*}}SourceLocationsOverflow.c
-// CHECK-NEXT: inc1.h{{.*}}: fatal error: sorry, this include generates a translation unit too large for Clang to process.
+// CHECK-NEXT: inc1.h{{.*}}: fatal error: sorry, the translation unit is too large for Clang to process: ran out of source locations
 // CHECK-NEXT: #include "inc2.h"
 // CHECK-NEXT:          ^
 // CHECK-NEXT: note: 214{{.......}}B in local locations, 0B in locations loaded from AST files, for a total of 214{{.......}}B (99% of available space)