[Lldb-commits] [PATCH] D108817: [LLDB] Fix 'std::out_of_range' crashing bug when file name completion using file path.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

Thanks for the patch! As Jonas pointed out, tests are usually expected for these kind of patches. But the IOHandler logic has some really quirky way to write tests, so just went ahead and wrote one that you can just apply on top:

  diff --git a/lldb/test/API/iohandler/completion/Makefile b/lldb/test/API/iohandler/completion/Makefile
  new file mode 100644
  index 000000000000..10495940055b
  --- /dev/null
  +++ b/lldb/test/API/iohandler/completion/Makefile
  @@ -0,0 +1,3 @@
  +C_SOURCES := main.c
  +
  +include Makefile.rules
  diff --git a/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py b/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py
  index 2f49b810fb4c..42cc00b4f9e8 100644
  --- a/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py
  +++ b/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py
  @@ -20,7 +20,8 @@ class IOHandlerCompletionTest(PExpectTest):
       @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr49408')
       @skipIf(oslist=["linux"], archs=["arm", "aarch64"])
       def test_completion(self):
  -        self.launch(dimensions=(100,500))
  +        self.build()
  +        self.launch(dimensions=(100,500), executable=self.getBuildArtifact("a.out"))
   
           # Start tab completion, go to the next page and then display all with 'a'.
           self.child.send("\t\ta")
  @@ -51,6 +52,11 @@ class IOHandlerCompletionTest(PExpectTest):
           self.child.send("\n")
           self.expect_prompt()
   
  +        # Complete a file path.
  +        # FIXME: This should complete to './main.c' and not 'main.c'
  +        self.child.send("breakpoint set --file ./main\t")
  +        self.child.expect_exact("main.c")
  +        self.child.send("\n")
  +        self.expect_prompt()
  +
           # Start tab completion and abort showing more commands with 'n'.
           self.child.send("\t")
           self.child.expect_exact("More (Y/n/a)")

FWIW, I think this is just fixing the symptom but not the cause of the underlying LLDB bug. The file completion logic completes `./hello` to `hello.c` instead of `./hello.c`. It also completes `/hello` to `hello.c` apparently. This doesn't seem right. I think fixing this crash is fine, but we probably should also add an assert somewhere that the completion of a token is actually completing the provided token (and not something else). If people want to rewrite the current token they should do it explicitly via a different (new) completion mode. But this seems out-of-scope for this fix. So LGTM with the test applies on top.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108817/new/

https://reviews.llvm.org/D108817