[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-15 Thread Aaron Ballman via cfe-commits

https://github.com/AaronBallman closed 
https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-15 Thread Naveen Seth Hanig via cfe-commits

naveen-seth wrote:

Yes, please.

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-15 Thread Aaron Ballman via cfe-commits

AaronBallman wrote:

> Thank you for reviewing! (And apologies for the formatting and 
> coding-standard issues on my end.)

No need to apologize, that sort of thing comes up! :-)

Do you need one of us to merge the changes on your behalf?

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-15 Thread Naveen Seth Hanig via cfe-commits

naveen-seth wrote:

Thank you for reviewing!
(And apologies for the formatting and coding-standard issues on my end!)

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-15 Thread Naveen Seth Hanig via cfe-commits

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 
Date: Sun, 11 May 2025 14:24:00 +
Subject: [PATCH 1/8] [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 0..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 
Date: Tue, 13 May 2025 20:00:34 +
Subject: [PATCH 2/8] 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 
Date: Thu, 15 May 2025 01:58:15 +
Subject: [PATCH 3/8] 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 0..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 inde

[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-15 Thread Aaron Ballman via cfe-commits

https://github.com/AaronBallman approved this pull request.

Other than a nit, LGTM!

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-15 Thread Aaron Ballman via cfe-commits


@@ -0,0 +1,17 @@
+// Regression test for #139375
+// 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 
:: or ::-: format, where 
 and  are integers greater than zero.

AaronBallman wrote:

Can you add a newline to the end of the file?

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Naveen Seth Hanig via cfe-commits

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 
Date: Sun, 11 May 2025 14:24:00 +
Subject: [PATCH 1/7] [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 0..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 
Date: Tue, 13 May 2025 20:00:34 +
Subject: [PATCH 2/7] 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 
Date: Thu, 15 May 2025 01:58:15 +
Subject: [PATCH 3/7] 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 0..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 inde

[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Naveen Seth Hanig via cfe-commits

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 
Date: Sun, 11 May 2025 14:24:00 +
Subject: [PATCH 1/7] [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 0..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 
Date: Tue, 13 May 2025 20:00:34 +
Subject: [PATCH 2/7] 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 
Date: Thu, 15 May 2025 01:58:15 +
Subject: [PATCH 3/7] 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 0..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 inde

[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Fangrui Song via cfe-commits

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Fangrui Song via cfe-commits

https://github.com/MaskRay approved this pull request.

LGTM with a few nits. Please give other reviewers some time to chime in 

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Fangrui Song via cfe-commits


@@ -0,0 +1,19 @@
+// 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 
::

MaskRay wrote:

If the diagnostic is on one line, use a single CHECK.

Multiple CHECK accepts output that we don't expect.

We don't rigidly require line wrapping for tests.

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Fangrui Song via cfe-commits

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Fangrui Song via cfe-commits


@@ -0,0 +1,12 @@
+// Related to #139375

MaskRay wrote:

I don't why some regression tests are named `crash-*`, but I would not name 
this `crash-*`.

I'd just name this source-loc-zero.cpp and change this comment to `// 
Regression test for #139375`

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Fangrui Song via cfe-commits


@@ -89,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

MaskRay wrote:

Ensure that full sentences in commends end with `.`

Remove brace for this single-line-simple-body case per 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Naveen Seth Hanig via cfe-commits

https://github.com/naveen-seth updated 
https://github.com/llvm/llvm-project/pull/139457



  



Rate limit · GitHub


  body {
background-color: #f6f8fa;
color: #24292e;
font-family: -apple-system,BlinkMacSystemFont,Segoe 
UI,Helvetica,Arial,sans-serif,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol;
font-size: 14px;
line-height: 1.5;
margin: 0;
  }

  .container { margin: 50px auto; max-width: 600px; text-align: center; 
padding: 0 24px; }

  a { color: #0366d6; text-decoration: none; }
  a:hover { text-decoration: underline; }

  h1 { line-height: 60px; font-size: 48px; font-weight: 300; margin: 0px; 
text-shadow: 0 1px 0 #fff; }
  p { color: rgba(0, 0, 0, 0.5); margin: 20px 0 40px; }

  ul { list-style: none; margin: 25px 0; padding: 0; }
  li { display: table-cell; font-weight: bold; width: 1%; }

  .logo { display: inline-block; margin-top: 35px; }
  .logo-img-2x { display: none; }
  @media
  only screen and (-webkit-min-device-pixel-ratio: 2),
  only screen and (   min--moz-device-pixel-ratio: 2),
  only screen and ( -o-min-device-pixel-ratio: 2/1),
  only screen and (min-device-pixel-ratio: 2),
  only screen and (min-resolution: 192dpi),
  only screen and (min-resolution: 2dppx) {
.logo-img-1x { display: none; }
.logo-img-2x { display: inline-block; }
  }

  #suggestions {
margin-top: 35px;
color: #ccc;
  }
  #suggestions a {
color: #66;
font-weight: 200;
font-size: 14px;
margin: 0 10px;
  }


  
  



  Whoa there!
  You have exceeded a secondary rate limit.
Please wait a few minutes before you try again;
in some cases this may take up to an hour.
  
  
https://support.github.com/contact";>Contact Support —
https://githubstatus.com";>GitHub Status —
https://twitter.com/githubstatus";>@githubstatus
  

  

  

  

  

  


___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Naveen Seth Hanig via cfe-commits

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 
Date: Sun, 11 May 2025 14:24:00 +
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 0..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 
Date: Tue, 13 May 2025 20:00:34 +
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 
Date: Thu, 15 May 2025 01:58:15 +
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 0..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 inde

[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Naveen Seth Hanig via cfe-commits

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 
Date: Sun, 11 May 2025 14:24:00 +
Subject: [PATCH 1/3] [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 0..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 
Date: Tue, 13 May 2025 20:00:34 +
Subject: [PATCH 2/3] 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 38ca85e5c2aff418dfc90aaa845908b574f3ba7a Mon Sep 17 00:00:00 2001
From: naveen-seth 
Date: Wed, 14 May 2025 20:00:31 +
Subject: [PATCH 3/3] 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..b0150fa3fe4be 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 a column or line number in 1-based command-line source 
locations.
+  Fixes crash caused by `0` input in 
`-selection=::[-:]`. (#GH139457)
+
 libclang
 
 - Fixed a bug in ``clang_File_isEqual`` that sometimes led to different
@@ -857,6 +862,8 @@ libclang
 
 Code Completion
 ---
+- Reject `0` as a column or line number in 1-based command-line source 
locations.
+  Fixes crash caused by `0` input in 
`-code-completion-at=::`. (#GH139457)
 
 Static Analyzer
 ---

_

[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-14 Thread Aaron Ballman via cfe-commits

https://github.com/AaronBallman commented:

Thank you for the fix! The change should come with a release note in 
`clang/docs/ReleaseNotes.rst` so users know about the improvement.

Otherwise, I think this is going in a reasonable direction.

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-13 Thread Naveen Seth Hanig via cfe-commits


@@ -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.

naveen-seth wrote:

Added.

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-13 Thread Naveen Seth Hanig via cfe-commits

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 
Date: Sun, 11 May 2025 14:24:00 +
Subject: [PATCH 1/2] [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 0..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 
Date: Tue, 13 May 2025 20:00:34 +
Subject: [PATCH 2/2] 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={{.*}}'

___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-11 Thread Naveen Seth Hanig via cfe-commits

naveen-seth wrote:

> Thanks for working on that. I think it might better to do that check where 
> `ParsedSourceLocation::FromString` is called, so that we can have a proper 
> front-end diagnostics for it (search for `err_fe_invalid_code_complete_file` 
> and `OPT_code_completion_at`)

Thank you for the quick review! :)

I added the check in `ParsedSourceLocation::FromString` because it also helps 
catch the same crash for callers of `ParsedSourceRange::FromString`, which also 
uses `ParsedSourceLocation::FromString`.

For example, the following would also crash (hitting the same assert as the one 
this PR fixes)

```bash
touch empty.cpp
clang-refactor local-rename -selection=input.cpp:1:1-1:0 -new-name=test -v 
input.cpp 
```

 Full crash log

```
Error while trying to load a compilation database:
Could not auto-detect compilation database for file "input.cpp"
No compilation database found in 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-debug-env/debugging-139375 
or any parent directory
fixed-compilation-database: Error while opening fixed database: No such file or 
directory
json-compilation-database: Error while opening JSON database: No such file or 
directory
Running without flags.
clang-refactor: 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Basic/SourceManager.cpp:1666:
 SourceLocation clang::SourceManager::translateLineCol(FileID, unsigned int, 
unsigned int) const: Assertion `Line && Col && "Line and column should start 
from 1!"' failed.
 #0 0x562e7ef658cd llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/llvm/lib/Support/Unix/Signals.inc:804:11
 #1 0x562e7ef65e0b PrintStackTraceSignalHandler(void*) 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/llvm/lib/Support/Unix/Signals.inc:888:1
 #2 0x562e7ef63f2f llvm::sys::RunSignalHandlers() 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/llvm/lib/Support/Signals.cpp:105:5
 #3 0x562e7ef664e9 SignalHandler(int, siginfo_t*, void*) 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/llvm/lib/Support/Unix/Signals.inc:418:7
 #4 0x7f6311a5e250 (/lib/x86_64-linux-gnu/libc.so.6+0x45250)
 #5 0x7f6311abcf1c pthread_kill (/lib/x86_64-linux-gnu/libc.so.6+0xa3f1c)
 #6 0x7f6311a5e19e raise (/lib/x86_64-linux-gnu/libc.so.6+0x4519e)
 #7 0x7f6311a41902 abort (/lib/x86_64-linux-gnu/libc.so.6+0x28902)
 #8 0x7f6311a4181e (/lib/x86_64-linux-gnu/libc.so.6+0x2881e)
 #9 0x7f6311a547c7 (/lib/x86_64-linux-gnu/libc.so.6+0x3b7c7)
#10 0x562e7fbee816 clang::SourceManager::translateLineCol(clang::FileID, 
unsigned int, unsigned int) const 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Basic/SourceManager.cpp:1668:11
#11 0x562e7ed5dc34 (anonymous 
namespace)::SourceRangeSelectionArgument::forAllRanges(clang::SourceManager 
const&, llvm::function_ref) 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/tools/clang-refactor/ClangRefactor.cpp:131:12
#12 0x562e7ed5eb2b (anonymous 
namespace)::ClangRefactorTool::callback(clang::ASTContext&) 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/tools/clang-refactor/ClangRefactor.cpp:418:11
#13 0x562e7ed5e840 (anonymous 
namespace)::ClangRefactorTool::getFrontendActionFactory()::'lambda'(clang::ASTContext&)::operator()(clang::ASTContext&)
 const 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/tools/clang-refactor/ClangRefactor.cpp:475:35
#14 0x562e7ed5e80d void std::__invoke_impl(std::__invoke_other, (anonymous 
namespace)::ClangRefactorTool::getFrontendActionFactory()::'lambda'(clang::ASTContext&)&,
 clang::ASTContext&) 
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/invoke.h:61:7
#15 0x562e7ed5e7ad std::enable_if, void>::type std::__invoke_r((anonymous 
namespace)::ClangRefactorTool::getFrontendActionFactory()::'lambda'(clang::ASTContext&)&,
 clang::ASTContext&) 
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/invoke.h:117:5
#16 0x562e7ed5e6f5 std::_Function_handler::_M_invoke(std::_Any_data
 const&, clang::ASTContext&) 
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/std_function.h:290:2
#17 0x562e7ed774c6 std::function::operator()(clang::ASTContext&) const 
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/std_function.h:591:2
#18 0x562e7ed5f3b1 (anonymous 
namespace)::ClangRefactorTool::getFrontendActionFactory()::ToolASTConsumer::HandleTranslationUnit(clang::ASTContext&)
 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/tools/clang-refactor/ClangRefactor.cpp:441:7
#19 0x562e80984ffb clang::ParseAST(clang::Sema&, bool, bool) 
/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Parse/ParseAST.cpp:191:12
#20 0x562e800193d9 clang::ASTFrontendAction::ExecuteAction() 
/home/nav/.distrobox/ubuntu-24-10-compiler-de

[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-11 Thread via cfe-commits

cor3ntin wrote:

Thanks for working on that.
I think it might better to do that check where 
`ParsedSourceLocation::FromString` is called, so that we can have a proper
front-end diagnostics for it (search for `err_fe_invalid_code_complete_file` 
and `OPT_code_completion_at`)

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-11 Thread via cfe-commits


@@ -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.

cor3ntin wrote:

Can you add a test using FileCheck that we produce a diagnostic?

https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-11 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Naveen Seth Hanig (naveen-seth)


Changes

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/139457.diff


2 Files Affected:

- (modified) clang/include/clang/Frontend/CommandLineSourceLoc.h (+4-1) 
- (added) clang/test/CodeCompletion/crash-if-zero-index.cpp (+6) 


``diff
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 0..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.

``




https://github.com/llvm/llvm-project/pull/139457
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

2025-05-11 Thread Naveen Seth Hanig via cfe-commits

https://github.com/naveen-seth created 
https://github.com/llvm/llvm-project/pull/139457

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.



  



Rate limit · GitHub


  body {
background-color: #f6f8fa;
color: #24292e;
font-family: -apple-system,BlinkMacSystemFont,Segoe 
UI,Helvetica,Arial,sans-serif,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol;
font-size: 14px;
line-height: 1.5;
margin: 0;
  }

  .container { margin: 50px auto; max-width: 600px; text-align: center; 
padding: 0 24px; }

  a { color: #0366d6; text-decoration: none; }
  a:hover { text-decoration: underline; }

  h1 { line-height: 60px; font-size: 48px; font-weight: 300; margin: 0px; 
text-shadow: 0 1px 0 #fff; }
  p { color: rgba(0, 0, 0, 0.5); margin: 20px 0 40px; }

  ul { list-style: none; margin: 25px 0; padding: 0; }
  li { display: table-cell; font-weight: bold; width: 1%; }

  .logo { display: inline-block; margin-top: 35px; }
  .logo-img-2x { display: none; }
  @media
  only screen and (-webkit-min-device-pixel-ratio: 2),
  only screen and (   min--moz-device-pixel-ratio: 2),
  only screen and ( -o-min-device-pixel-ratio: 2/1),
  only screen and (min-device-pixel-ratio: 2),
  only screen and (min-resolution: 192dpi),
  only screen and (min-resolution: 2dppx) {
.logo-img-1x { display: none; }
.logo-img-2x { display: inline-block; }
  }

  #suggestions {
margin-top: 35px;
color: #ccc;
  }
  #suggestions a {
color: #66;
font-weight: 200;
font-size: 14px;
margin: 0 10px;
  }


  
  



  Whoa there!
  You have exceeded a secondary rate limit.
Please wait a few minutes before you try again;
in some cases this may take up to an hour.
  
  
https://support.github.com/contact";>Contact Support —
https://githubstatus.com";>GitHub Status —
https://twitter.com/githubstatus";>@githubstatus
  

  

  

  

  

  


___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits