[llvm-branch-commits] [llvm] [Support] Reject out-of-range line offsets in verify directives (PR #233305)

[via llvm-branch-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: Alexander Richardson (arichardson)

<details>
<summary>Changes</summary>

`@+N` and `@-N` offsets were parsed with radix auto-detection into an
`int` without checking for errors, so `@+010` meant 8, `@+08` used an
uninitialized value, and offsets pointing outside of the file silently
wrapped around. Parse them as decimal and report invalid and
out-of-range offsets. Line 0 is still allowed since MLIR uses it for
the implicit top-level module.

Assisted-by: AI tooling

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


3 Files Affected:

- (modified) llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp (+21-4) 
- (modified) llvm/test/tools/llvm-mc/verify/basic.s (+10-1) 
- (modified) llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp (+36) 


``````````diff
diff --git a/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp b/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
index 75b0e27abe612..95b053967eca4 100644
--- a/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
+++ b/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
@@ -189,13 +189,30 @@ SourceMgrDiagnosticVerifier::computeExpectedDiags(const MemoryBuffer *Buf) {
 
       // Get the integer value without the @ and +/- prefix.
       if (OffsetMatch[0] == '+' || OffsetMatch[0] == '-') {
-        int Offset;
-        OffsetMatch.drop_front().getAsInteger(0, Offset);
+        // Parse as decimal so that e.g. '@+08' is not treated as octal.
+        unsigned Offset;
+        if (OffsetMatch.drop_front().getAsInteger(10, Offset)) {
+          emitError(Record, "invalid line offset '" + OffsetMatch + "'");
+          continue;
+        }
 
-        if (OffsetMatch.front() == '+')
+        if (OffsetMatch.front() == '+') {
+          if (Offset > E - Record.LineNo) {
+            emitError(Record, "line offset '" + OffsetMatch +
+                                  "' is past the end of the file");
+            continue;
+          }
           Record.LineNo += Offset;
-        else
+        } else {
+          // Line 0 is allowed since MLIR uses it for the implicit top-level
+          // module.
+          if (Offset > Record.LineNo) {
+            emitError(Record, "line offset '" + OffsetMatch +
+                                  "' is before the start of the file");
+            continue;
+          }
           Record.LineNo -= Offset;
+        }
       } else if (OffsetMatch.consume_front("unknown")) {
         if (Record.ColNo != 0) {
           emitError(Record, "cannot specify a column with '@unknown'");
diff --git a/llvm/test/tools/llvm-mc/verify/basic.s b/llvm/test/tools/llvm-mc/verify/basic.s
index e38d76fcc9777..82fd0a1f70bcf 100644
--- a/llvm/test/tools/llvm-mc/verify/basic.s
+++ b/llvm/test/tools/llvm-mc/verify/basic.s
@@ -18,9 +18,11 @@
 ## of a longer word such as 'uncheck'.
 # RUN: llvm-mc --triple=riscv32 --verify=check,alt custom-prefix.s
 
-## Malformed '-re' directives fail verification.
+## Malformed '-re' directives and out-of-range line offsets fail verification.
 # RUN: not llvm-mc --triple=riscv32 --verify invalid-regex.s 2>&1 \
 # RUN:   | FileCheck invalid-regex.s
+# RUN: not llvm-mc --triple=riscv32 --verify invalid-offset.s 2>&1 \
+# RUN:   | FileCheck invalid-offset.s
 
 ## A diagnostic that matches the text but not the kind of a directive.
 # RUN: not llvm-mc --triple=riscv32 --verify near-miss.s 2>&1 \
@@ -118,6 +120,13 @@ addi a0, a0, 1
 # CHECK: error: found start of regex with no end '}}'
 # CHECK: error: invalid regex:
 
+#--- invalid-offset.s
+addi a0, a0, 1
+# expected-error at -99 {{before start}}
+# expected-error at +99 {{past end}}
+# CHECK: error: line offset '-99' is before the start of the file
+# CHECK: error: line offset '+99' is past the end of the file
+
 #--- near-miss.s
 .foo_directive
 # expected-warning at -1 {{unknown directive}}
diff --git a/llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp b/llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp
index 433c4d9de0291..58df0a59bf2c1 100644
--- a/llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp
+++ b/llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp
@@ -159,3 +159,39 @@ TEST_F(SourceMgrDiagnosticVerifierTest, ColumnChecking) {
       MatchResult::Matched);
   EXPECT_FALSE(Verifier.verify());
 }
+
+TEST_F(SourceMgrDiagnosticVerifierTest, LineZeroAndLeadingZeroOffsets) {
+  const MemoryBuffer *Buf =
+      addBuffer("// expected-error at +02 {{decimal leading zero}}\n"
+                "// expected-remark at -2 {{implicit module at line 0}}\n"
+                "op\n");
+  SourceMgrDiagnosticVerifier Verifier(SM, OS);
+
+  using Location = SourceMgrDiagnosticVerifier::Location;
+  EXPECT_EQ(Verifier.process(SourceMgr::DK_Remark, Location{Buf, 0, 0},
+                             "implicit module at line 0"),
+            MatchResult::Matched);
+  EXPECT_EQ(Verifier.process(SourceMgr::DK_Error, Location{Buf, 3, 1},
+                             "decimal leading zero"),
+            MatchResult::Matched);
+  EXPECT_TRUE(Verifier.verify());
+  EXPECT_EQ(Output, "");
+}
+
+TEST_F(SourceMgrDiagnosticVerifierTest, OutOfBoundsLineOffsets) {
+  addBuffer("op\n"
+            "# expected-error at -3 {{before start}}\n"
+            "# expected-error at +5 {{past end}}\n"
+            "# expected-error at +99999999999 {{overflow}}\n");
+  SourceMgrDiagnosticVerifier Verifier(SM, OS);
+
+  EXPECT_FALSE(Verifier.verify());
+  EXPECT_NE(StringRef(Output).find(
+                "error: line offset '-3' is before the start of the file"),
+            StringRef::npos);
+  EXPECT_NE(StringRef(Output).find(
+                "error: line offset '+5' is past the end of the file"),
+            StringRef::npos);
+  EXPECT_NE(StringRef(Output).find("error: invalid line offset '+99999999999'"),
+            StringRef::npos);
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/233305