Skip to content

[clang-format] Fix an off-by-1 bug with -length option #143302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Jun 8, 2025

Also validate the argument value.

Fixes #56245

@llvmbot
Copy link
Member

llvmbot commented Jun 8, 2025

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Also validate the argument value.

Fixes #56245


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

3 Files Affected:

  • (modified) clang/test/Format/multiple-inputs-error.cpp (+1-1)
  • (modified) clang/test/Format/ranges.cpp (+11-2)
  • (modified) clang/tools/clang-format/ClangFormat.cpp (+7-3)
diff --git a/clang/test/Format/multiple-inputs-error.cpp b/clang/test/Format/multiple-inputs-error.cpp
index 1aa9c9f3e2fad..7cb835d39f23e 100644
--- a/clang/test/Format/multiple-inputs-error.cpp
+++ b/clang/test/Format/multiple-inputs-error.cpp
@@ -1,6 +1,6 @@
 // RUN: cp %s %t-1.cpp
 // RUN: cp %s %t-2.cpp
-// RUN: not clang-format 2>&1 >/dev/null -offset=1 -length=0 %t-1.cpp %t-2.cpp |FileCheck %s
+// RUN: not clang-format 2>&1 >/dev/null -offset=1 -length=1 %t-1.cpp %t-2.cpp |FileCheck %s
 // RUN: not clang-format 2>&1 >/dev/null -lines=1:1 %t-1.cpp %t-2.cpp |FileCheck %s -check-prefix=CHECK-LINE
 // CHECK: error: -offset, -length and -lines can only be used for single file.
 // CHECK-LINE: error: -offset, -length and -lines can only be used for single file.
diff --git a/clang/test/Format/ranges.cpp b/clang/test/Format/ranges.cpp
index 66b984e037b3c..6de407e7a20e4 100644
--- a/clang/test/Format/ranges.cpp
+++ b/clang/test/Format/ranges.cpp
@@ -1,5 +1,5 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style=LLVM -offset=2 -length=0 -offset=28 -length=0 \
+// RUN: grep -Ev "// *[A-Z0-9-]+:" %s \
+// RUN:   | clang-format -style=LLVM -offset=2 -length=1 -offset=28 -length=1 -offset=35 -length=8 \
 // RUN:   | FileCheck -strict-whitespace %s
 // CHECK: {{^int\ \*i;$}}
 int*i;
@@ -9,3 +9,12 @@ int  *  i;
 
 // CHECK: {{^int\ \*i;$}}
 int   *   i;
+
+// CHECK: int I;
+// CHECK-NEXT: int J ;
+int I ;
+int J ;
+
+// RUN: not clang-format -length=0 < %s 2>&1 \
+// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK0 %s
+// CHECK0: error: length should be at least 1
diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp
index ad12672fa89c1..67f49c4265cd9 100644
--- a/clang/tools/clang-format/ClangFormat.cpp
+++ b/clang/tools/clang-format/ClangFormat.cpp
@@ -283,7 +283,7 @@ static bool fillRanges(MemoryBuffer *Code,
     Lengths.push_back(Sources.getFileOffset(Sources.getLocForEndOfFile(ID)) -
                       Offsets[0]);
   } else if (Offsets.size() != Lengths.size()) {
-    errs() << "error: number of -offset and -length arguments must match.\n";
+    errs() << "error: number of -offset and -length arguments must match\n";
     return true;
   }
   for (unsigned I = 0, E = Offsets.size(); I < E; ++I) {
@@ -293,12 +293,16 @@ static bool fillRanges(MemoryBuffer *Code,
       return true;
     }
     const auto Length = Lengths[I];
+    if (Length == 0) {
+      errs() << "error: length should be at least 1\n";
+      return true;
+    }
     if (Offset + Length > Code->getBufferSize()) {
       errs() << "error: invalid length " << Length << ", offset + length ("
-             << Offset + Length << ") is outside the file.\n";
+             << Offset + Length << ") is outside the file\n";
       return true;
     }
-    Ranges.push_back(tooling::Range(Offset, Length));
+    Ranges.push_back(tooling::Range(Offset, Length - 1));
   }
   return false;
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang-format --length behavior
2 participants