Skip to content
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

SelectionTree crashes #999

Closed
hokein opened this issue Jan 26, 2022 · 1 comment
Closed

SelectionTree crashes #999

hokein opened this issue Jan 26, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@hokein
Copy link
Contributor

hokein commented Jan 26, 2022

void func(char*) {
  func("ab^c // no new line at the eof!

stack trace:

ClangdTests: llvm-project/clang-tools-extra/clangd/Selection.cpp:319: SelectionTree::Selection clang::clangd::(anonymous namespace)::SelectionTester::test(llvm::ArrayRef<syntax::Token>) const: Assertion `!Batch.empty()' failed.
llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) llvm-project/llvm/lib/Support/Unix/Signals.inc:565:13
llvm::sys::RunSignalHandlers() llvm-project/llvm/lib/Support/Signals.cpp:98:18
SignalHandler(int) llvm-project/llvm/lib/Support/Unix/Signals.inc:407:1
__restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x13200)
raise ./signal/../sysdeps/unix/sysv/linux/raise.c:50:1
abort ./stdlib/abort.c:81:7
get_sysdep_segment_value ./intl/loadmsgcat.c:509:8
_nl_load_domain ./intl/loadmsgcat.c:970:34
(/lib/x86_64-linux-gnu/libc.so.6+0x35212)
llvm::ArrayRef<clang::syntax::Token>::drop_front(unsigned long) const llvm-project/llvm/include/llvm/ADT/ArrayRef.h:203:7
clang::clangd::(anonymous namespace)::SelectionTester::test(llvm::ArrayRef<clang::syntax::Token>) const llvm-project/clang-tools-extra/clangd/Selection.cpp:320:39
clang::clangd::(anonymous namespace)::SelectionVisitor::claimRange(clang::SourceRange, clang::clangd::SelectionTree::Selection&) llvm-project/clang-tools-extra/clangd/Selection.cpp:919:33
clang::clangd::(anonymous namespace)::SelectionVisitor::pop() llvm-project/clang-tools-extra/clangd/Selection.cpp:816:11
llvm::SmallVectorBase<unsigned int>::empty 

Looks like we're leaving an eof token in the ExpandedTokens, and it never gets removed (T.location() < Limit doesn't meet)

@hokein hokein added the bug Something isn't working label Jan 26, 2022
@hokein
Copy link
Contributor Author

hokein commented Jan 27, 2022

A simpler case int f() {.

// `-CompoundStmt 0xa349148 <col:9, col:10>
int f() {
// vs the valid case
// `-CompoundStmt 0xa349148 <col:9, col:10>
int f() {}

Because of the error recovery, the source range of the compound stmt is the same for both cases. It is problematic for the broken case, as the location doesn't reflect the written source code -- the end-loc points to the end of file.

Fix options:

  • fix it in SelectionTree, by dropping the eof token in the ExpandedTokens if it presents
  • fix it in TokenBuffer::expandedTokens(SourceRange R) by not returning the eof token. This seems more aggressive. As this function is mainly used with a range provided by an AST node, this is probably ok (it might avoid the same issue in other clients?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant