Page MenuHomePhabricator

[lldb][Editline] Fix crash when navigating through empty command history.
ClosedPublic

Authored by rupprecht on Apr 7 2021, 10:01 AM.

Details

Summary

An empty history entry can happen by entering the expression evaluator an immediately hitting enter:

$ lldb
(lldb) e
Enter expressions, then terminate with an empty line to evaluate:
  1:  <hit enter>

The next time the user enters the expression evaluator, if they hit the up arrow to load the previous expression, lldb crashes. This patch treats empty history sessions as a single expression of zero length, instead of an empty list of expressions.

Fixes http://llvm.org/PR49845.

Diff Detail

Event Timeline

rupprecht requested review of this revision.Apr 7 2021, 10:01 AM
rupprecht created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2021, 10:01 AM
JDevlieghere accepted this revision.Apr 7 2021, 10:04 AM

LGTM. Thanks!

This revision is now accepted and ready to land.Apr 7 2021, 10:04 AM
This revision was landed with ongoing or failed builds.Apr 7 2021, 10:51 AM
This revision was automatically updated to reflect the committed changes.
iii added a subscriber: iii.Jul 15 2021, 4:13 AM

lldb-arm-ubuntu (https://lab.llvm.org/buildbot/#/builders/17/builds/9015) fails with:

FAIL: LLDB (/home/tcwg-buildslave/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_nav_arrow_up_empty_pr49845 (TestMultilineNavigation.TestCase)
======================================================================
ERROR: test_nav_arrow_up_empty_pr49845 (TestMultilineNavigation.TestCase)
   Tests that navigating with the up arrow doesn't crash.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildslave/worker/lldb-arm-ubuntu/llvm-project/lldb/third_party/Python/module/pexpect-4.6/pexpect/expect.py", line 111, in expect_loop
    incoming = spawn.read_nonblocking(spawn.maxread, timeout)
  File "/home/tcwg-buildslave/worker/lldb-arm-ubuntu/llvm-project/lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py", line 482, in read_nonblocking
    raise TIMEOUT('Timeout exceeded.')
pexpect.exceptions.TIMEOUT: Timeout exceeded.

The bot attributed this to my changes (https://reviews.llvm.org/D105629), but they have nothing to do with lldb or arm. I'm not sure whether it's a real issue or just a laggy builder, in which case increasing a timeout might be worth considering?

@rupprecht could you take a look please?

lldb-arm-ubuntu (https://lab.llvm.org/buildbot/#/builders/17/builds/9015) fails with:

FAIL: LLDB (/home/tcwg-buildslave/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_nav_arrow_up_empty_pr49845 (TestMultilineNavigation.TestCase)
======================================================================
ERROR: test_nav_arrow_up_empty_pr49845 (TestMultilineNavigation.TestCase)
   Tests that navigating with the up arrow doesn't crash.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildslave/worker/lldb-arm-ubuntu/llvm-project/lldb/third_party/Python/module/pexpect-4.6/pexpect/expect.py", line 111, in expect_loop
    incoming = spawn.read_nonblocking(spawn.maxread, timeout)
  File "/home/tcwg-buildslave/worker/lldb-arm-ubuntu/llvm-project/lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py", line 482, in read_nonblocking
    raise TIMEOUT('Timeout exceeded.')
pexpect.exceptions.TIMEOUT: Timeout exceeded.

The bot attributed this to my changes (https://reviews.llvm.org/D105629), but they have nothing to do with lldb or arm. I'm not sure whether it's a real issue or just a laggy builder, in which case increasing a timeout might be worth considering?

@rupprecht could you take a look please?

It passed on the next build attempt. I don't think this is a real issue. Maybe the arm buildbot is just not provisioned very well.

The timeout appears to be 30, per the failing test logs. I don't know if raising further it is the right call, because this seems to happen so rarely, and you probably want to keep the value low so you can iterate on tests faster.