HighCommander4 wrote:
Superseded by #78092.
Thanks for putting together this patch, @robozati. While we didn't go with the
fix approach taken in this patch, it did prod me to look at the issue in more
detail :laughing:
https://github.com/llvm/llvm-project/pull/69849
https://github.com/HighCommander4 closed
https://github.com/llvm/llvm-project/pull/69849
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
HighCommander4 wrote:
> What do you think about this as an alternative: if `spelledForExpanded`
> receives as input an expanded token range `[firstExpanded, lastExpanded]`
> where `lastExpanded` is the `eof` token, return the spelled tokens
> corresponding to `[firstExpanded, lastExpanded -
HighCommander4 wrote:
> I am slightly suspicious of source locations pointing at `eof` in the AST in
> the first place, even in invalid code. I wonder if we would be better off
> just having an invalid source location there instead of pointing at `eof`.
I think in this case it's deliberate.
ilya-biryukov wrote:
I second everything that @HighCommander4 said. It would be really useful to add
an assertion that we never get out-of-bounds here, but this indicates a logic
error somewhere in the code that should be fixed.
I am slightly suspicious of source locations pointing at `eof`
HighCommander4 wrote:
Anyways, in a case like this, this preferable behaviour would be for
`spelledForExpanded()` to return the spelled tokens covering `int main() {`
(not including the `eof` token but including everything else), **not** nullopt.
HighCommander4 wrote:
I poked at this a bit more. A few more notes:
* Since the test case currently in the patch does not trigger the crash
(without the fix), and it was not obvious to me what sort of call to
`spelledForExpanded()` to formulate to trigger the crash, I instead formulated
a
HighCommander4 wrote:
I spent a bit more time looking at this. I'd say I'm still quite far from
having a complete understanding of this code, but two things occur to me:
1. `TokenBuffer` operates at the level of the preprocessor.
"spelledForExpanded" is basically an operation which takes as
robozati wrote:
@HighCommander4, thank you for reviewing this PR!
Unfortunately, I’m currently in my finals and will only have time to look at
this next week. So, I’ll update this later.
https://github.com/llvm/llvm-project/pull/69849
___
@@ -585,6 +585,17 @@ TEST_F(TokenCollectorTest, DelayedParsing) {
EXPECT_THAT(collectAndDump(Code), StartsWith(ExpectedTokens));
}
+TEST_F(TokenCollectorTest, UnclosedToken) {
HighCommander4 wrote:
Indeed, it looks like `TokenBuffer::spelledForExpanded()`
@@ -585,6 +585,17 @@ TEST_F(TokenCollectorTest, DelayedParsing) {
EXPECT_THAT(collectAndDump(Code), StartsWith(ExpectedTokens));
}
+TEST_F(TokenCollectorTest, UnclosedToken) {
HighCommander4 wrote:
I started looking at this to try and make some progress
robozati wrote:
Rewrote the test and now it passes.
Sorry for pinging you @HighCommander4, but I don't know who to ping and you
participated in that issue.
https://github.com/llvm/llvm-project/pull/69849
___
cfe-commits mailing list
https://github.com/robozati ready_for_review
https://github.com/llvm/llvm-project/pull/69849
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/robozati updated
https://github.com/llvm/llvm-project/pull/69849
>From 239302d66dea74333e27288acbceffdf3f2d3b6b Mon Sep 17 00:00:00 2001
From: Gabriel Pezati
Date: Sat, 21 Oct 2023 19:28:57 -0300
Subject: [PATCH] Fix UB on unclosed parentheses, brackets or braces in Tokens
https://github.com/robozati converted_to_draft
https://github.com/llvm/llvm-project/pull/69849
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
robozati wrote:
Tests are failing because of the `findExpanded` function, but it didn’t result
in UB. Will fix the test later.
https://github.com/llvm/llvm-project/pull/69849
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/robozati updated
https://github.com/llvm/llvm-project/pull/69849
>From e57657cf296e1e93fa17f021bb8ce591071b2176 Mon Sep 17 00:00:00 2001
From: robozati <139823421+roboz...@users.noreply.github.com>
Date: Sat, 21 Oct 2023 12:04:14 -0300
Subject: [PATCH 1/2] Fix UB on unclosed
https://github.com/robozati created
https://github.com/llvm/llvm-project/pull/69849
When getting the AST from clangd, if there is an unclosed token, a segfault or
an UB can happen when accessing `File.SpelledTokens` because the supposed
closing token is not present, so its index is greater
18 matches
Mail list logo
