Page MenuHomePhabricator

[lldb-mi] Implement -gdb-set breakpoint pending on/off
ClosedPublic

Authored by malaperle on Oct 5 2018, 3:47 PM.

Details

Summary

This allows creating pending breakpoint automatically when a location is not found.
This is used by some front-ends instead of doing "-break-insert -f" every time.

See also https://sourceware.org/gdb/onlinedocs/gdb/Set-Breaks.html

Signed-off-by: Marc-Andre Laperle <malaperle@gmail.com>

Diff Detail

Repository
rL LLVM

Event Timeline

malaperle created this revision.Oct 5 2018, 3:47 PM
malaperle set the repository for this revision to rL LLVM.Oct 5 2018, 3:49 PM
malaperle added a project: Restricted Project.
malaperle added a subscriber: lldb-commits.
MaskRay added inline comments.
tools/lldb-mi/MICmdCmdGdbSet.cpp
447 ↗(On Diff #168549)

clang-format

And there is no reviewers set :)

And there is no reviewers set :)

I don't know who to add.

jingham added a subscriber: jingham.

This looks okay to me, there's a trivial comment nit... But Alexander has been doing the most work on MI recently, he might want to give this a look-over.

malaperle updated this revision to Diff 170301.Oct 19 2018, 9:38 PM

Clang-format it

malaperle marked an inline comment as done.Oct 19 2018, 9:38 PM

I think that it's worth it to rewrite the test with LIT and FileCheck because the python approach has some limitations, e.g. timeouts. You can find examples in lldb/lit/tools/lldb-mi/.

tools/lldb-mi/MICmdCmdGdbShow.cpp
369 ↗(On Diff #170301)

Following LLVM coding standarts, we should use early exits. In this case it might be like:

if (!CMIUtilString::Compare(strOption, "pending")) {
  // process error here
}
// success case here
...
malaperle updated this revision to Diff 171388.Oct 26 2018, 8:56 PM

Convert to lit test, convert some code to an early return, clang-format.

malaperle marked an inline comment as done.Oct 26 2018, 8:57 PM
apolyakov added inline comments.Oct 27 2018, 7:05 AM
lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
4 ↗(On Diff #171388)

As far as your first command is file-exec-and-symbols, the best way is to use a generic executable's name. Here it should be:

# RUN: %cc -o %t %p/inputs/break-insert-pending.c -g
# RUN: %lldbmi %t < %s | FileCheck %s
9 ↗(On Diff #171388)

It might be removed then.

20 ↗(On Diff #171388)

It should be # CHECK:, it will not work otherwise.

The same comment for other places.

Fixes to lit test

malaperle marked an inline comment as done.Oct 27 2018, 10:21 PM
malaperle added inline comments.
lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
4 ↗(On Diff #171388)

How can I use %t for the second file-exec-and-symbols? It doesn't seem to work

I ran the test and got a fail:

build/bin/llvm-lit -avv llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test 
-- Testing: 1 tests, 1 threads --
FAIL: lldb :: tools/lldb-mi/breakpoint/break-insert-enable-pending.test (1 of 1)
******************** TEST 'lldb :: tools/lldb-mi/breakpoint/break-insert-enable-pending.test' FAILED ********************
Script:
--
: 'RUN: at line 4';   /home/alexander/workspace/gsoc/build/./bin/clang -o b.exe /home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/inputs/break-insert-pending.c -g
: 'RUN: at line 5';   /home/alexander/workspace/gsoc/build/bin/lldb-mi --synchronous < /home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test | /home/alexander/workspace/gsoc/build/bin/FileCheck /home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
--
Exit Code: 1

Command Output (stderr):
--
/home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test:25:10: error: CHECK: expected string not found in input
# CHECK: =breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="{{0x[0-9a-f]*[^f][0-9a-f]*}}",func="??",file="??",fullname="??/??",line="0",pending=["printf"],times="{{0|1}}",original-location="printf"}
         ^
<stdin>:40:1: note: scanning from here
=thread-group-started,id="i1",pid="11020"
^

--

********************
Testing Time: 0.72s
********************
Failing Tests (1):
    lldb :: tools/lldb-mi/breakpoint/break-insert-enable-pending.test

  Unexpected Failures: 1
lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
4 ↗(On Diff #171388)

Should you use file-exec-and-symbols two times? AFAIK, after exiting a program, your target remains the same, so you don't need to load it again.

I ran the test and got a fail:

Can you paste the full input of the failure? You can add "--dump-input-on-failure" arg to FileCheck in the test. The test passes for me but it might be that a regex needs to be tweaked depending on the environment.

malaperle added inline comments.Oct 28 2018, 11:41 AM
lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
4 ↗(On Diff #171388)

I did that so that printf would not be resolved in order to test that it behaves properly after turning off pending breakpoints. But I can -break-insert printf on a different, non-existent symbol and it should be OK I think.

Remove need for second -file-exec-and-symbols

This comment was removed by apolyakov.

Here it is:

build/bin/llvm-lit -avv llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test -- Testing: 1 tests, 1 threads --
FAIL: lldb :: tools/lldb-mi/breakpoint/break-insert-enable-pending.test (1 of 1)
******************** TEST 'lldb :: tools/lldb-mi/breakpoint/break-insert-enable-pending.test' FAILED ********************
Script:
--
: 'RUN: at line 4';   /home/alexander/workspace/gsoc/build/./bin/clang -o /home/alexander/workspace/gsoc/build/tools/lldb/lit/tools/lldb-mi/breakpoint/Output/break-insert-enable-pending.test.tmp /home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/inputs/break-insert-pending.c -g
: 'RUN: at line 5';   /home/alexander/workspace/gsoc/build/bin/lldb-mi --synchronous /home/alexander/workspace/gsoc/build/tools/lldb/lit/tools/lldb-mi/breakpoint/Output/break-insert-enable-pending.test.tmp < /home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test | /home/alexander/workspace/gsoc/build/bin/FileCheck /home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test --dump-input-on-failure
--
Exit Code: 1

Command Output (stderr):
--
/home/alexander/workspace/gsoc/llvm/tools/lldb/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test:22:10: error: CHECK: expected string not found in input
# CHECK: =breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="{{0x[0-9a-f]*[^f][0-9a-f]*}}",func="??",file="??",fullname="??/??",line="0",pending=["printf"],times="{{0|1}}",original-location="printf"}
         ^
<stdin>:39:1: note: scanning from here
=thread-group-started,id="i1",pid="5022"
^
Full input was:
<<<<<<
(gdb)
-file-exec-and-symbols "/home/alexander/workspace/gsoc/build/tools/lldb/lit/tools/lldb-mi/breakpoint/Output/break-insert-enable-pending.test.tmp"
^done
(gdb)
^done
(gdb)
=library-loaded,id="/home/alexander/workspace/gsoc/build/tools/lldb/lit/tools/lldb-mi/breakpoint/Output/break-insert-enable-pending.test.tmp",target-name="/home/alexander/workspace/gsoc/build/tools/lldb/lit/tools/lldb-mi/breakpoint/Output/break-insert-enable-pending.test.tmp",host-name="/home/alexander/workspace/gsoc/build/tools/lldb/lit/tools/lldb-mi/breakpoint/Output/break-insert-enable-pending.test.tmp",symbols-loaded="0",loaded_addr="-",size="0"
^done
(gdb)
^done
(gdb)
^done
(gdb)
^done
(gdb)
^done
(gdb)
^error,msg="Command 'break-insert'. Breakpoint location 'printf' not found"
(gdb)
^done
(gdb)
^done
(gdb)
^done
(gdb)
^done,value="on"
(gdb)
^done
(gdb)
^done,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0xffffffffffffffff",func="??",file="??",fullname="??/??",line="0",pending=["printf"],times="0",original-location="printf"}
(gdb)
=breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0xffffffffffffffff",func="??",file="??",fullname="??/??",line="0",pending=["printf"],times="0",original-location="printf"}
(gdb)
^done
(gdb)
^done
(gdb)
^running
=thread-group-started,id="i1",pid="5022"
(gdb)
=thread-created,id="1",group-id="i1"
=thread-selected,id="1"
(gdb)
=library-loaded,id="/lib/x86_64-linux-gnu/ld-2.23.so",target-name="/lib/x86_64-linux-gnu/ld-2.23.so",host-name="/lib/x86_64-linux-gnu/ld-2.23.so",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/ld-2.23.so",loaded_addr="-",size="0"
(gdb)
=library-loaded,id="[vdso]",target-name="[vdso]",host-name="[vdso]",symbols-loaded="1",symbols-path="",loaded_addr="0x00007ffff7ffa000",size="0"
(gdb)
=library-loaded,id="/home/alexander/workspace/gsoc/build/tools/lldb/lit/tools/lldb-mi/breakpoint/Output/break-insert-enable-pending.test.tmp",target-name="/home/alexander/workspace/gsoc/build/tools/lldb/lit/tools/lldb-mi/breakpoint/Output/break-insert-enable-pending.test.tmp",host-name="/home/alexander/workspace/gsoc/build/tools/lldb/lit/tools/lldb-mi/breakpoint/Output/break-insert-enable-pending.test.tmp",symbols-loaded="0",loaded_addr="-",size="0"
1 location added to breakpoint 2
(gdb)
=library-loaded,id="/lib/x86_64-linux-gnu/libc.so.6",target-name="/lib/x86_64-linux-gnu/libc.so.6",host-name="/lib/x86_64-linux-gnu/libc.so.6",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/libc-2.23.so",loaded_addr="-",size="0"
=breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0x00007ffff7a6285b",func="__printf",file="printf.c",fullname="/build/glibc-bfm8X4/glibc-2.23/stdio-common/printf.c",line="32",pending=["printf"],times="1",original-location="printf"}
(gdb)
(gdb)
=library-loaded,id="/lib/x86_64-linux-gnu/libc.so.6",target-name="/lib/x86_64-linux-gnu/libc.so.6",host-name="/lib/x86_64-linux-gnu/libc.so.6",symbols-loaded="1",symbols-path="/usr/lib/debug/lib/x86_64-linux-gnu/libc-2.23.so",loaded_addr="-",size="0"
(gdb)
*stopped,reason="breakpoint-hit",disp="del",bkptno="2",frame={level="0",addr="0x00007ffff7a6285b",func="__printf",args=[{name="format",value="0x00000000004005d4 \"Print a formatted string so that GCC does not optimize this printf call: %s\\\\n\""}],file="printf.c",fullname="/build/glibc-bfm8X4/glibc-2.23/stdio-common/printf.c",line="32"},thread-id="1",stopped-threads="all"
(gdb)
^done
(gdb)
^done
(gdb)
^done
(gdb)
^done
(gdb)
=breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="n",addr="0x00007ffff7a6285b",func="__printf",file="printf.c",fullname="/build/glibc-bfm8X4/glibc-2.23/stdio-common/printf.c",line="32",pending=["printf"],times="1",original-location="printf"}
(gdb)
^done
(gdb)
^done
(gdb)
^running
(gdb)
@"Print a formatted string so that GCC does not optimize this printf call: /home/alexander/workspace/gsoc/build/tools/lldb/lit/tools/lldb-mi/breakpoint/Output/break-insert-enable-pending.test.tmp\r\n"
(gdb)
=thread-exited,id="1",group-id="i1"
=thread-group-exited,id="i1",exit-code="0"
*stopped,reason="exited-normally"
(gdb)
^done
(gdb)
^done
(gdb)
^done
(gdb)
^done,value="on"
(gdb)
^done
(gdb)
^done
(gdb)
^done
(gdb)
^done,value="off"
(gdb)
^done
(gdb)
^error,msg="Command 'break-insert'. Breakpoint location 'printf-non-existent' not found"
(gdb)
^done
(gdb)
^done
(gdb)
^done
(gdb)
^done
(gdb)
^done,value="on"
(gdb)
^done
(gdb)
^done
(gdb)
^done
(gdb)
^done,value="off"
(gdb)
^done
(gdb)
^error,msg="'breakpoint' expects "pending on" or "pending off""
(gdb)
^done
(gdb)

>>>>>>

--

********************
Testing Time: 0.70s
********************
Failing Tests (1):
    lldb :: tools/lldb-mi/breakpoint/break-insert-enable-pending.test

  Unexpected Failures: 1

Simply checks

I think those =breakpoint-modified checks are a bit overzealous. The important
check is whether of not we hit the breakpoint in a resolved location,
i.e. not 0xffffffff. So, at the *stopped. The presence and timing of

breakpoint-modified is orthogonal to this patch.

Simply checks

*Simplify

This revision is now accepted and ready to land.Oct 29 2018, 12:56 AM

LGTM.

Thanks a lot!

This revision was automatically updated to reflect the committed changes.