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

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 23 02:52:42 PDT 2023


https://github.com/ilya-biryukov created https://github.com/llvm/llvm-project/pull/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.

>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] [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));



More information about the cfe-commits mailing list