[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 05:16:38 PDT 2023
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/5] [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/5] 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;
>From 1344a7a687b1a3147820c1b4406f9e974924c676 Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Mon, 23 Oct 2023 13:52:15 +0200
Subject: [PATCH 3/5] fixup! [SourceManager] Report an error and crash on
source location exhaustion in macros
- Unify the error when running out of slocs in includes and macro
- Use `SourceLocation()` instead of `Info.getSpellingLoc()` to avoid printing
the location, which also causes Clang to run indefinitely.
---
clang/include/clang/Basic/DiagnosticCommonKinds.td | 7 ++-----
clang/lib/Basic/SourceManager.cpp | 4 ++--
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 080748a73571690..9f0ccd255a32148 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -356,11 +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;
+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 415f0a33358703a..805c2ff48bb3182 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();
}
@@ -665,7 +665,7 @@ SourceManager::createExpansionLocImpl(const ExpansionInfo &Info,
LocalSLocEntryTable.push_back(SLocEntry::get(NextLocalOffset, Info));
if (NextLocalOffset + Length + 1 <= NextLocalOffset ||
NextLocalOffset + Length + 1 > CurrentLoadedOffset) {
- Diag.Report(Info.getSpellingLoc(), diag::err_expansions_too_large);
+ Diag.Report(SourceLocation(), diag::err_sloc_space_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
>From c1bead751797c7463676ecb91f8e890c031bbef3 Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Mon, 23 Oct 2023 13:58:29 +0200
Subject: [PATCH 4/5] fixup! [SourceManager] Report an error and crash on
source location exhaustion in macros
- Add a comment about using SourceLocation()
---
clang/lib/Basic/SourceManager.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index 805c2ff48bb3182..d627c233a3aa094 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -666,8 +666,9 @@ SourceManager::createExpansionLocImpl(const ExpansionInfo &Info,
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.
- // Currently, the call runs indefinitely, so we would first need to fix it.
+ // 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");
>From b4ffc6af4d5bc8eef87d9df50ac8f19b7b0a8efa Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Mon, 23 Oct 2023 14:15:57 +0200
Subject: [PATCH 5/5] fixup! [SourceManager] Report an error and crash on
source location exhaustion in macros
- Update test for sloc overflow
---
clang/test/Lexer/SourceLocationsOverflow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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)
More information about the cfe-commits
mailing list