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

[Ilya Biryukov via cfe-commits]

https://github.com/ilya-biryukov updated https://github.com/llvm/llvm-project/pull/69908

>From 6b73e1fbbad1a485ce6266dbd8d3c499956b859a Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Thu, 19 Oct 2023 17:50:38 +0200
Subject: [PATCH 1/2] [SourceManager] Report an error and crash on source
 location exhaustion in macros

`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.
---
 clang/include/clang/Basic/DiagnosticCommonKinds.td |  2 ++
 clang/lib/Basic/SourceManager.cpp                  | 11 +++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index f2df283c74829f6..080748a73571690 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -356,6 +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_expansions_too_large : Error<
+  "sorry, the translation unit is too large: ran out of source locations while processing a macro expansion">, DefaultFatal;
 def err_include_too_large : Error<
   "sorry, this include generates a translation unit too large for"
   " Clang to process.">, DefaultFatal;
diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index c44ecacb3de3a10..dcf18c47cbaf9ae 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -663,10 +663,13 @@ 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(Info.getExpansionLocStart(), diag::err_expansions_too_large);
+    Diag.Report(Info.getExpansionLocStart(), diag::remark_sloc_usage);
+    noteSLocAddressSpaceUsage(Diag);
+    return SourceLocation();
+  }
   // See createFileID for that +1.
   NextLocalOffset += Length + 1;
   return SourceLocation::getMacroLoc(NextLocalOffset - (Length + 1));

>From 2180aa94253647f6541c6311224c253be0c0de25 Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Mon, 23 Oct 2023 12:12:50 +0200
Subject: [PATCH 2/2] fixup! [SourceManager] Report an error and crash on
 source location exhaustion in macros

---
 clang/lib/Basic/SourceManager.cpp | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index dcf18c47cbaf9ae..415f0a33358703a 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -665,10 +665,12 @@ SourceManager::createExpansionLocImpl(const ExpansionInfo &Info,
   LocalSLocEntryTable.push_back(SLocEntry::get(NextLocalOffset, Info));
   if (NextLocalOffset + Length + 1 <= NextLocalOffset ||
       NextLocalOffset + Length + 1 > CurrentLoadedOffset) {
-    Diag.Report(Info.getExpansionLocStart(), diag::err_expansions_too_large);
-    Diag.Report(Info.getExpansionLocStart(), diag::remark_sloc_usage);
-    noteSLocAddressSpaceUsage(Diag);
-    return SourceLocation();
+    Diag.Report(Info.getSpellingLoc(), diag::err_expansions_too_large);
+    // FIXME: call `noteSLocAddressSpaceUsage` to report details to users.
+    // Currently, the call runs indefinitely, so we would first need to fix it.
+    // 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;