https://github.com/naveen-seth updated
https://github.com/llvm/llvm-project/pull/139457
>From 5f246435b9784113ada3f82328433487391f40ab Mon Sep 17 00:00:00 2001
From: naveen-seth <naveen.ha...@outlook.com>
Date: Sun, 11 May 2025 14:24:00 +0000
Subject: [PATCH 1/6] [clang] Enforce 1-based indexing for command line source
locations
Fixes #139375
Clang expects command line source locations to be provided using
1-based indexing.
Currently, Clang does not reject zero as invalid argument for column
or line number, which can cause Clang to crash.
This commit extends validation in `ParsedSourceLocation::FromString`
to only accept (unsinged) non-zero integers.
---
clang/include/clang/Frontend/CommandLineSourceLoc.h | 5 ++++-
clang/test/CodeCompletion/crash-if-zero-index.cpp | 6 ++++++
2 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 clang/test/CodeCompletion/crash-if-zero-index.cpp
diff --git a/clang/include/clang/Frontend/CommandLineSourceLoc.h
b/clang/include/clang/Frontend/CommandLineSourceLoc.h
index 074800a881a89..a412d41dbb023 100644
--- a/clang/include/clang/Frontend/CommandLineSourceLoc.h
+++ b/clang/include/clang/Frontend/CommandLineSourceLoc.h
@@ -24,7 +24,9 @@ namespace clang {
/// A source location that has been parsed on the command line.
struct ParsedSourceLocation {
std::string FileName;
+ // The 1-based line number
unsigned Line;
+ // The 1-based column number
unsigned Column;
public:
@@ -38,7 +40,8 @@ struct ParsedSourceLocation {
// If both tail splits were valid integers, return success.
if (!ColSplit.second.getAsInteger(10, PSL.Column) &&
- !LineSplit.second.getAsInteger(10, PSL.Line)) {
+ !LineSplit.second.getAsInteger(10, PSL.Line) &&
+ !(PSL.Column == 0 || PSL.Line == 0)) {
PSL.FileName = std::string(LineSplit.first);
// On the command-line, stdin may be specified via "-". Inside the
diff --git a/clang/test/CodeCompletion/crash-if-zero-index.cpp
b/clang/test/CodeCompletion/crash-if-zero-index.cpp
new file mode 100644
index 0000000000000..2f0eae35738e6
--- /dev/null
+++ b/clang/test/CodeCompletion/crash-if-zero-index.cpp
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:0:1 %s -o -
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:1:0 %s -o -
+
+// Related to #139375
+// Clang uses 1-based indexing for source locations given from the
command-line.
+// Verify Clang doesn’t crash when 0 is given as line or column number.
>From 90a3bebcebf92e754ea8b8791ab104fb7db54316 Mon Sep 17 00:00:00 2001
From: naveen-seth <naveen.ha...@outlook.com>
Date: Tue, 13 May 2025 20:00:34 +0000
Subject: [PATCH 2/6] Test for diagnostic using FileCheck
---
clang/test/CodeCompletion/crash-if-zero-index.cpp | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/clang/test/CodeCompletion/crash-if-zero-index.cpp
b/clang/test/CodeCompletion/crash-if-zero-index.cpp
index 2f0eae35738e6..dec45f3048c9b 100644
--- a/clang/test/CodeCompletion/crash-if-zero-index.cpp
+++ b/clang/test/CodeCompletion/crash-if-zero-index.cpp
@@ -1,6 +1,10 @@
-// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:0:1 %s -o -
-// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:1:0 %s -o -
-
// Related to #139375
// Clang uses 1-based indexing for source locations given from the
command-line.
-// Verify Clang doesn’t crash when 0 is given as line or column number.
+// Verify that Clang rejects 0 as an invalid value for line or column number.
+
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:0:1 %s -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-DIAG %s
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:1:0 %s -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-DIAG %s
+
+// CHECK-DIAG: error: invalid value '{{.*}}' in '-code-completion-at={{.*}}'
>From e925cf7c11f73bf26d5040dd1cf2864439b953bb Mon Sep 17 00:00:00 2001
From: naveen-seth <naveen.ha...@outlook.com>
Date: Thu, 15 May 2025 01:58:15 +0000
Subject: [PATCH 3/6] Add tests for clang-refactor
---
... => crash-command-line-source-loc-zero.cpp} | 0
.../crash-command-line-source-loc-zero.cpp | 18 ++++++++++++++++++
2 files changed, 18 insertions(+)
rename clang/test/CodeCompletion/{crash-if-zero-index.cpp =>
crash-command-line-source-loc-zero.cpp} (100%)
create mode 100644 clang/test/Refactor/crash-command-line-source-loc-zero.cpp
diff --git a/clang/test/CodeCompletion/crash-if-zero-index.cpp
b/clang/test/CodeCompletion/crash-command-line-source-loc-zero.cpp
similarity index 100%
rename from clang/test/CodeCompletion/crash-if-zero-index.cpp
rename to clang/test/CodeCompletion/crash-command-line-source-loc-zero.cpp
diff --git a/clang/test/Refactor/crash-command-line-source-loc-zero.cpp
b/clang/test/Refactor/crash-command-line-source-loc-zero.cpp
new file mode 100644
index 0000000000000..d6a35f7312526
--- /dev/null
+++ b/clang/test/Refactor/crash-command-line-source-loc-zero.cpp
@@ -0,0 +1,18 @@
+// Related to #139457
+// Clang uses 1-based indexing for source locations given from the
command-line.
+// Verify that `clang-refactor` rejects 0 as an invalid value for line or
column number.
+
+// For range start:
+// RUN: not clang-refactor local-rename -selection=%s:0:1-1:1 -new-name=test
%s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-DIAG %s
+// RUN: not clang-refactor local-rename -selection=%s:1:0-1:1 -new-name=test
%s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-DIAG %s
+
+// For range end:
+// RUN: not clang-refactor local-rename -selection=%s:1:1-0:1 -new-name=test
%s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-DIAG %s
+// RUN: not clang-refactor local-rename -selection=%s:1:1-1:0 -new-name=test
%s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-DIAG %s
+
+// CHECK-DIAG: error: '-selection' option must be specified using
<file>:<line>:<column>
+// CHECK-DIAG: or <file>:<line>:<column>-<line>:<column> format
>From 5126e5ba65c7c2373b2f8647b39f15654834a981 Mon Sep 17 00:00:00 2001
From: naveen-seth <naveen.ha...@outlook.com>
Date: Thu, 15 May 2025 02:00:37 +0000
Subject: [PATCH 4/6] Ensure 1-based indexing for end of ParsedSourceRange as
well
---
clang/include/clang/Frontend/CommandLineSourceLoc.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Frontend/CommandLineSourceLoc.h
b/clang/include/clang/Frontend/CommandLineSourceLoc.h
index a412d41dbb023..74143a75d3001 100644
--- a/clang/include/clang/Frontend/CommandLineSourceLoc.h
+++ b/clang/include/clang/Frontend/CommandLineSourceLoc.h
@@ -92,8 +92,13 @@ struct ParsedSourceRange {
// probably belongs to the filename which menas the whole
// string should be parsed.
RangeSplit.first = Str;
- } else
+ } else {
+ // Column and line numbers are 1-based
+ if (EndLine == 0 || EndColumn == 0) {
+ return std::nullopt;
+ }
HasEndLoc = true;
+ }
}
auto Begin = ParsedSourceLocation::FromString(RangeSplit.first);
if (Begin.FileName.empty())
>From 68d85ba2524438a48eadd1d941d88b9d6084c7ca Mon Sep 17 00:00:00 2001
From: naveen-seth <naveen.ha...@outlook.com>
Date: Thu, 15 May 2025 02:19:07 +0000
Subject: [PATCH 5/6] Add release notes
---
clang/docs/ReleaseNotes.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e362ec595a3bb..372be5596767b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -839,6 +839,11 @@ clang-format
- Add ``OneLineFormatOffRegex`` option for turning formatting off for one line.
- Add ``SpaceAfterOperatorKeyword`` option.
+clang-refactor
+------------
+- Reject `0` as column or line number in 1-based command-line source locations.
+ Fixes crash caused by `0` input in
`-selection=<file>:<line>:<column>[-<line>:<column>]`. (#GH139457)
+
libclang
--------
- Fixed a bug in ``clang_File_isEqual`` that sometimes led to different
@@ -857,6 +862,8 @@ libclang
Code Completion
---------------
+- Reject `0` as column or line number in 1-based command-line source locations.
+ Fixes crash caused by `0` input in
`-code-completion-at=<file>:<line>:<column>`. (#GH139457)
Static Analyzer
---------------
>From 41847821469d40e8a7cf0e34bef8dcd317fe3921 Mon Sep 17 00:00:00 2001
From: naveen-seth <naveen.ha...@outlook.com>
Date: Thu, 15 May 2025 03:48:55 +0000
Subject: [PATCH 6/6] Improve diagnostics
---
clang/include/clang/Basic/DiagnosticDriverKinds.td | 4 ++++
clang/lib/Frontend/CompilerInvocation.cpp | 6 ++++--
.../CodeCompletion/crash-command-line-source-loc-zero.cpp | 2 ++
clang/test/Refactor/crash-command-line-source-loc-zero.cpp | 3 ++-
clang/tools/clang-refactor/ClangRefactor.cpp | 3 ++-
5 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index b15cba698030c..4da8f80345ddc 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -670,6 +670,10 @@ def note_drv_verify_prefix_spelling : Note<
"-verify prefixes must start with a letter and contain only alphanumeric"
" characters, hyphens, and underscores">;
+def note_command_line_code_loc_requirement
+ : Note<"-code-completion-at=<file>:<line>:<column> requires <line> and "
+ "<column> to be integers greater than zero">;
+
def warn_drv_global_isel_incomplete : Warning<
"-fglobal-isel support for the '%0' architecture is incomplete">,
InGroup<GlobalISel>;
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp
b/clang/lib/Frontend/CompilerInvocation.cpp
index 394512978b521..fd48e425a5c21 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3112,9 +3112,11 @@ static bool ParseFrontendArgs(FrontendOptions &Opts,
ArgList &Args,
if (const Arg *A = Args.getLastArg(OPT_code_completion_at)) {
Opts.CodeCompletionAt =
ParsedSourceLocation::FromString(A->getValue());
- if (Opts.CodeCompletionAt.FileName.empty())
+ if (Opts.CodeCompletionAt.FileName.empty()) {
Diags.Report(diag::err_drv_invalid_value)
- << A->getAsString(Args) << A->getValue();
+ << A->getAsString(Args) << A->getValue();
+ Diags.Report(diag::note_command_line_code_loc_requirement);
+ }
}
Opts.Plugins = Args.getAllArgValues(OPT_load);
diff --git a/clang/test/CodeCompletion/crash-command-line-source-loc-zero.cpp
b/clang/test/CodeCompletion/crash-command-line-source-loc-zero.cpp
index dec45f3048c9b..c500c668afbb3 100644
--- a/clang/test/CodeCompletion/crash-command-line-source-loc-zero.cpp
+++ b/clang/test/CodeCompletion/crash-command-line-source-loc-zero.cpp
@@ -8,3 +8,5 @@
// RUN: | FileCheck -check-prefix=CHECK-DIAG %s
// CHECK-DIAG: error: invalid value '{{.*}}' in '-code-completion-at={{.*}}'
+// CHECK-DIAG: -code-completion-at=<file>:<line>:<column> requires <line>
+// CHECK-DIAG: and <column> to be integers greater than zero
diff --git a/clang/test/Refactor/crash-command-line-source-loc-zero.cpp
b/clang/test/Refactor/crash-command-line-source-loc-zero.cpp
index d6a35f7312526..dc0f5cf9d30a6 100644
--- a/clang/test/Refactor/crash-command-line-source-loc-zero.cpp
+++ b/clang/test/Refactor/crash-command-line-source-loc-zero.cpp
@@ -15,4 +15,5 @@
// RUN: | FileCheck -check-prefix=CHECK-DIAG %s
// CHECK-DIAG: error: '-selection' option must be specified using
<file>:<line>:<column>
-// CHECK-DIAG: or <file>:<line>:<column>-<line>:<column> format
+// CHECK-DIAG: or <file>:<line>:<column>-<line>:<column> format,
+// CHECK-DIAG: where <line> and <column> are integers greater than zero.
diff --git a/clang/tools/clang-refactor/ClangRefactor.cpp
b/clang/tools/clang-refactor/ClangRefactor.cpp
index 968f0594085d4..a92b3f91beaed 100644
--- a/clang/tools/clang-refactor/ClangRefactor.cpp
+++ b/clang/tools/clang-refactor/ClangRefactor.cpp
@@ -160,7 +160,8 @@ SourceSelectionArgument::fromString(StringRef Value) {
return std::make_unique<SourceRangeSelectionArgument>(std::move(*Range));
llvm::errs() << "error: '-selection' option must be specified using "
"<file>:<line>:<column> or "
- "<file>:<line>:<column>-<line>:<column> format\n";
+ "<file>:<line>:<column>-<line>:<column> format, "
+ "where <line> and <column> are integers greater than
zero.\n";
return nullptr;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
