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/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 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/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 <naveen.ha...@outlook.com> Date: Thu, 15 May 2025 01:58:15 +0000 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 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/7] 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 8da04dc924ffbfe3df1039b7c25587149b315940 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/7] 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..df49b5c4241ee 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 6b8ef5605d390f591dd3c5aeef5adc2490f6e684 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/7] 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; } >From 4c7fca7471078175b4b6722e22c4ee3c1f09c2e4 Mon Sep 17 00:00:00 2001 From: naveen-seth <naveen.ha...@outlook.com> Date: Thu, 15 May 2025 05:51:36 +0000 Subject: [PATCH 7/7] Address comments --- clang/include/clang/Frontend/CommandLineSourceLoc.h | 5 ++--- ...command-line-source-loc-zero.cpp => source-loc-zero.cpp} | 5 ++--- ...ommand-line-source-loc-zero.cpp => source-loc-zero.cpp } | 6 ++---- 3 files changed, 6 insertions(+), 10 deletions(-) rename clang/test/CodeCompletion/{crash-command-line-source-loc-zero.cpp => source-loc-zero.cpp} (76%) rename clang/test/Refactor/{crash-command-line-source-loc-zero.cpp => source-loc-zero.cpp } (81%) diff --git a/clang/include/clang/Frontend/CommandLineSourceLoc.h b/clang/include/clang/Frontend/CommandLineSourceLoc.h index 74143a75d3001..b07ffcb65c067 100644 --- a/clang/include/clang/Frontend/CommandLineSourceLoc.h +++ b/clang/include/clang/Frontend/CommandLineSourceLoc.h @@ -93,10 +93,9 @@ struct ParsedSourceRange { // string should be parsed. RangeSplit.first = Str; } else { - // Column and line numbers are 1-based - if (EndLine == 0 || EndColumn == 0) { + // Column and line numbers are 1-based. + if (EndLine == 0 || EndColumn == 0) return std::nullopt; - } HasEndLoc = true; } } diff --git a/clang/test/CodeCompletion/crash-command-line-source-loc-zero.cpp b/clang/test/CodeCompletion/source-loc-zero.cpp similarity index 76% rename from clang/test/CodeCompletion/crash-command-line-source-loc-zero.cpp rename to clang/test/CodeCompletion/source-loc-zero.cpp index c500c668afbb3..a428c1534ffde 100644 --- a/clang/test/CodeCompletion/crash-command-line-source-loc-zero.cpp +++ b/clang/test/CodeCompletion/source-loc-zero.cpp @@ -1,4 +1,4 @@ -// Related to #139375 +// Regression test for #139375 // Clang uses 1-based indexing for source locations given from the command-line. // Verify that Clang rejects 0 as an invalid value for line or column number. @@ -8,5 +8,4 @@ // 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 +// CHECK-NEXT: hint: -code-completion-at=<file>:<line>:<column> requires <line> 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/source-loc-zero.cpp similarity index 81% rename from clang/test/Refactor/crash-command-line-source-loc-zero.cpp rename to clang/test/Refactor/source-loc-zero.cpp index dc0f5cf9d30a6..2f55780809729 100644 --- a/clang/test/Refactor/crash-command-line-source-loc-zero.cpp +++ b/clang/test/Refactor/source-loc-zero.cpp @@ -1,4 +1,4 @@ -// Related to #139457 +// 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. @@ -14,6 +14,4 @@ // 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, -// CHECK-DIAG: where <line> and <column> are integers greater than zero. +// CHECK-DIAG: error: '-selection' option must be specified using <file>:<line>:<column> or <file>:<line>:<column>-<line>:<column> format, where <line> and <column> are integers greater than zero. \ No newline at end of file _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits