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

lldb crashes when using tab completion. #81536

Closed
ZequanWu opened this issue Feb 12, 2024 · 7 comments · Fixed by #83234
Closed

lldb crashes when using tab completion. #81536

ZequanWu opened this issue Feb 12, 2024 · 7 comments · Fixed by #83234
Labels
crash Prefer [crash-on-valid] or [crash-on-invalid] lldb

Comments

@ZequanWu
Copy link
Contributor

lldb version: trunk.

a.cpp

class B {
  int x;
};
class A {
  B t;
  int t2;
};

int main(int argc, char* argv[]) {
  A a;
  return 0;
}

command:
clang++ -g a.cpp && lldb a.out -o "b a.cpp:11" -o "r"

Enter a.t. and use tab completion causes lldb to crash:

(lldb) p a.t.terminate called after throwing an instance of 'std::out_of_range'
  what():  basic_string::substr: __pos (which is 4) > this->size() (which is 3)
LLDB diagnostics will be written to /tmp/diagnostics-eaf5b2
Please include the directory content when filing a bug report
[1]    2833408 IOT instruction  lldb a.out -o "b a.cpp:11" -o "r"
@ZequanWu ZequanWu added the lldb label Feb 12, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 12, 2024

@llvm/issue-subscribers-lldb

Author: Zequan Wu (ZequanWu)

lldb version: trunk.

a.cpp

class B {
  int x;
};
class A {
  B t;
  int t2;
};

int main(int argc, char* argv[]) {
  A a;
  return 0;
}

command:
clang++ -g a.cpp && lldb a.out -o "b a.cpp:11" -o "r"

Enter a.t. and use tab completion causes lldb to crash:

(lldb) p a.t.terminate called after throwing an instance of 'std::out_of_range'
  what():  basic_string::substr: __pos (which is 4) > this->size() (which is 3)
LLDB diagnostics will be written to /tmp/diagnostics-eaf5b2
Please include the directory content when filing a bug report
[1]    2833408 IOT instruction  lldb a.out -o "b a.cpp:11" -o "r"

@EugeneZelenko EugeneZelenko added the crash Prefer [crash-on-valid] or [crash-on-invalid] label Feb 12, 2024
@Michael137
Copy link
Member

Looks like the problem is that we're trying to determine the longest common prefix of the completions to display, but don't account for the possibility of a . (or ->) in the GetCursorArgumentPrefix. Interestingly, we also crash on anything that isn't a letter or number:

v a.t@
v a.t\
etc.

@Michael137
Copy link
Member

Actually looks like the list of possible completions we get for v a.t. is:

(lldb_private::StringList) completions = {
  m_strings = size=4 {                    
    [0] = "a.t.x"                         
    [1] = "a.t2"                          
    [2] = "a.t22"                         
    [3] = "a.t3"                          
  }                                       
}                                         

Which doesn't seem right.

@svs-quic
Copy link
Contributor

svs-quic commented Feb 14, 2024

This is happening since in the checks for completions we check to see if a member name starts with a partial_member_name and in this case both B t and int t2 start with t and we are getting a.t2 as a completion for a.t. which is incorrect. This happens in the function PrivateAutoCompleteMembers in Variable.cpp. The crash goes away if we change the variable t2 to something like f2.

if (partial_member_name.empty() ||
  llvm::StringRef(member_name).starts_with(partial_member_name)) {
  if (member_name == partial_member_name) {
  PrivateAutoComplete(
      frame, partial_path,
      prefix_path + member_name, // Anything that has been resolved
                                 // already will be in here
      member_compiler_type.GetCanonicalType(), request);
  } else {
  request.AddCompletion((prefix_path + member_name).str());
  }

Changing this to the block below seems to work and not crash.

if (partial_member_name.empty()) {
  request.AddCompletion((prefix_path + member_name).str());
  } else if (llvm::StringRef(member_name).starts_with(partial_member_name)) {
  if (member_name == partial_member_name) {
    PrivateAutoComplete(
        frame, partial_path,
        prefix_path + member_name, // Anything that has been resolved
                                   // already will be in here
        member_compiler_type.GetCanonicalType(), request);
  } else if (partial_path.empty()) {
    request.AddCompletion((prefix_path + member_name).str());
  }

I'll still need to figure out how to add a test for this since the existing test TestCompletion.py does not seem to cover this use case.

@Michael137
Copy link
Member

This is happening since in the checks for completions we check to see if a member name starts with a partial_member_name and in this case both B t and int t2 start with t and we are getting a.t2 as a completion for a.t. which is incorrect. This happens in the function PrivateAutoCompleteMembers in Variable.cpp. The crash goes away if we change the variable t2 to something like f2.

if (partial_member_name.empty() ||
  llvm::StringRef(member_name).starts_with(partial_member_name)) {
  if (member_name == partial_member_name) {
  PrivateAutoComplete(
      frame, partial_path,
      prefix_path + member_name, // Anything that has been resolved
                                 // already will be in here
      member_compiler_type.GetCanonicalType(), request);
  } else {
  request.AddCompletion((prefix_path + member_name).str());
  }

Changing this to the block below seems to work and not crash.

if (partial_member_name.empty()) {
  request.AddCompletion((prefix_path + member_name).str());
  } else if (llvm::StringRef(member_name).starts_with(partial_member_name)) {
  if (member_name == partial_member_name) {
    PrivateAutoComplete(
        frame, partial_path,
        prefix_path + member_name, // Anything that has been resolved
                                   // already will be in here
        member_compiler_type.GetCanonicalType(), request);
  } else if (partial_path.empty()) {
    request.AddCompletion((prefix_path + member_name).str());
  }

I'll still need to figure out how to add a test for this since the existing test TestCompletion.py does not seem to cover this use case.

Yea that seems about right. We don't want to add completions if there's still user input left to check

@ZequanWu
Copy link
Contributor Author

Is there an open pr to fix this?

@svs-quic
Copy link
Contributor

I haven't been able to get to this due to other work that I was busy. Shall try to push a patch this week.

Michael137 added a commit that referenced this issue Feb 29, 2024
We weren't checking to see if the partial_path was empty before adding
completions and this led to crashes when the class object and a variable
both start with the same substring.

Fixes [#81536](#81536)

---------

Co-authored-by: Michael Buch <michaelbuch12@gmail.com>
Michael137 pushed a commit to Michael137/llvm-project that referenced this issue Mar 7, 2024
…3234)

We weren't checking to see if the partial_path was empty before adding
completions and this led to crashes when the class object and a variable
both start with the same substring.

Fixes [llvm#81536](llvm#81536)

---------

Co-authored-by: Michael Buch <michaelbuch12@gmail.com>
(cherry picked from commit de55188)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crash Prefer [crash-on-valid] or [crash-on-invalid] lldb
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants