llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-format
Author: mygitljf
<details>
<summary>Changes</summary>
## Summary
Fix a clang-format assertion crash when an ObjC `@<!-- -->interface` or
`@<!-- -->implementation` body contains a `-` or `+` that is not followed by `(`
or an identifier.
## Problem
`clang-format` aborts on inputs like:
```objc
@<!-- -->interface;
-
```
with:
```
Assertion `FormatTok->isOneOf(tok::l_paren, tok::identifier) &&
"'(' or identifier expected."' failed.
```
`UnwrappedLineParser::parseObjCUntilAtEnd()` consumes the `-`/`+` and
unconditionally calls `parseObjCMethod()`, whose entry `assert` requires
the next token to be `(` or an identifier. Malformed input (EOF, `;`,
`@<!-- -->end`, ...) violates that contract and aborts the process.
This is reachable from clangd's `textDocument/formatting` path while a
user is mid-typing a method declaration, so it shows up in real
editing sessions, not just hand-crafted reproducers.
## Solution
Guard the call site so `parseObjCMethod()` is only entered when its
entry contract is satisfied. The entry assertion stays as-is — the
contract is enforced by the caller, matching how the other ObjC
sub-parsers in this file (`parseObjCProtocolList`,
`parseObjCLightweightGenerics`, ...) gate their own entry conditions.
```diff
} else if (FormatTok->isOneOf(tok::minus, tok::plus)) {
nextToken();
- parseObjCMethod();
+ if (FormatTok->isOneOf(tok::l_paren, tok::identifier))
+ parseObjCMethod();
}
```
Malformed `-`/`+` tokens fall through to the next iteration of
`parseObjCUntilAtEnd`, which terminates naturally on `@<!-- -->end` or EOF.
## Testing
- New `FormatTestObjC.NoCrashOnStrayMethodSign` covers all 6 variants
reported in the issue.
- `FormatTests` — 1265/1265 pass (incl. 27 `FormatTestObjC`).
- `check-clang-format` (lit) — 33/33 pass.
- Manual reproduction of every variant in the issue now exits 0
with no abort.
Fixes #<!-- -->199075
---
Full diff: https://github.com/llvm/llvm-project/pull/199104.diff
2 Files Affected:
- (modified) clang/lib/Format/UnwrappedLineParser.cpp (+5-1)
- (modified) clang/unittests/Format/FormatTestObjC.cpp (+12)
``````````diff
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp
b/clang/lib/Format/UnwrappedLineParser.cpp
index 9536a233def58..3fb719c2a67a2 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -4262,7 +4262,11 @@ void UnwrappedLineParser::parseObjCUntilAtEnd() {
addUnwrappedLine();
} else if (FormatTok->isOneOf(tok::minus, tok::plus)) {
nextToken();
- parseObjCMethod();
+ // Guard against malformed input where '-'/'+' is not followed by a
+ // method declaration. parseObjCMethod() asserts the next token is '('
+ // or an identifier; silently skip the stray sign token otherwise.
+ if (FormatTok->isOneOf(tok::l_paren, tok::identifier))
+ parseObjCMethod();
} else {
parseStructuralElement();
}
diff --git a/clang/unittests/Format/FormatTestObjC.cpp
b/clang/unittests/Format/FormatTestObjC.cpp
index 09a9687d6f87a..b6aa96f5aea79 100644
--- a/clang/unittests/Format/FormatTestObjC.cpp
+++ b/clang/unittests/Format/FormatTestObjC.cpp
@@ -1803,6 +1803,18 @@ TEST_F(FormatTestObjC, AttributesOnObjCProperty) {
"@property(weak) id delegate ATTRIBUTE_MACRO(X) __attribute__((X));");
}
+TEST_F(FormatTestObjC, NoCrashOnStrayMethodSign) {
+ // Issue #199075: clang-format used to assert in parseObjCMethod() when an
+ // ObjC interface/implementation contained a '-' or '+' that was not
+ // followed by '(' or an identifier.
+ verifyNoCrash("@interface;\n-");
+ verifyNoCrash("@interface Foo\n-");
+ verifyNoCrash("@interface Foo\n+");
+ verifyNoCrash("@implementation Foo\n-");
+ verifyNoCrash("@interface Foo\n-\n@end");
+ verifyNoCrash("@interface Foo\n- ;");
+}
+
} // end namespace
} // namespace test
} // end namespace format
``````````
</details>
https://github.com/llvm/llvm-project/pull/199104
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
