On Tue, Jan 19, 2021 at 2:55 PM Jim Ingham <jing...@apple.com> wrote: > > > > > On Jan 19, 2021, at 11:40 AM, David Blaikie <dblai...@gmail.com> wrote: > > > > On Tue, Jan 19, 2021 at 10:17 AM Jim Ingham <jing...@apple.com> wrote: > >> > >> > >> > >>> On Jan 17, 2021, at 10:47 AM, David Blaikie <dblai...@gmail.com> wrote: > >>> > >>> On Fri, Jan 15, 2021 at 6:22 PM Jim Ingham <jing...@apple.com> wrote: > >>>> > >>>> If you set a breakpoint on source lines with no line table entries, lldb > >>>> slides the actual match forward to the nearest line table entry to the > >>>> given line number. That's necessary for instance because if you lay out > >>>> your function definitions over multiple lines they won't all get line > >>>> table entries, but we don't want people to have to guess which of them > >>>> was... Same is true of more complicated compound statements. > >>> > >>> Ah, sure - gdb seems to do that too, totally makes sense - as you say, > >>> it'd be pretty hard to know exactly which tokens end up with > >>> corresponding entries in the line table and which don't (especially > >>> under optimizations and complex compound statements). > >> > >> Yeah, I think you either have to have this heuristic, or have a debugger > >> that can display "valid breakpoint locations". The old Metrowerks > >> debugger used to do it that way. They only put up breakpoint affordances > >> in the UI for the lines that had line table entries and there was no > >> free-form way to set breakpoints, so you couldn't do an ambiguous thing. > >> But I don't think we have that option in lldb. > > > > Could be nice to have in source printing on the command line and could > > be done more fully in IDE integrations like XCode - but yeah, still > > would want all this sliding stuff for raw command line usage even if > > we had the rest. > > If we just cared about lines and not columns, it would be pretty easy to > annotate the source line output, maybe just a • before each breakable line. > Marking the column locations as well, while keeping the whole thing readable, > is a bit trickier. > > The problem with Xcode is that they don't do full compiles as you are > editing. They do run the front end for code completion, brace matching, > etc... But that doesn't generate line tables. So you would have to support > breakpoints in files where you do and don't know the valid line table > entries. If you didn't design for using the compiled code for UI in the IDE, > it's not easy to add it in after the fact. > > > > >>>> So people like that, but they really hate it when they have: > >>>> > >>>> > >>>> #ifdef SOMETHING_NOT_DEFINED > >>>> > >>>> int foo() { > >>>> > >>>> } > >>>> > >>>> #else > >>>> int bar() { > >>>> > >>>> } > >>>> #end > >>>> > >>>> but with lots more junk so you can't see the ifdef's and then they set a > >>>> breakpoint in the "int foo" part and the breakpoint gets moved to bar > >>>> instead and then doesn't get hit. > >>> > >>> Ah, indeed - that is a curious case that could be surprising to a > >>> user. Thanks for explaining it - though it seems gdb doesn't special > >>> case this & does produce the not-quite-what-the-user-intended > >>> behavior. > >>> > >>>> You might try to argue that they should have checked where the > >>>> breakpoint actually landed before coming into your office to yell at > >>>> you, but it's likely to leave you with fewer friends... > >>> > >>> FWIW: I don't have coworkers or friends come into my office to yell at > >>> me about anything like this, and I don't really think anyone should - > >>> that's not appropriate behavior for a workplace. > >>> > >>> Having a nuanced discussion about the tradeoff of features - sure. > >> > >> Yeah, IME people get cheesed off at debuggers in ways they don't with > >> compilers. I have some theories, but more of the after work quality than > >> the public mailing list quality. > >> > >>>> So I was trying to detect this case and not move the breakpoint if > >>>> sliding it crossed over the function start boundary. That way you'd see > >>>> that the breakpoint didn't work, and go figure out why. > >>> > >>> Neat idea! > >>> > >>>> The thinko in the original version was that we were still doing this > >>>> when we DIDN'T have to slide the breakpoint, when we got an exact match > >>>> in the line table. In that case, we shouldn't try to second guess the > >>>> line table at all. That's the patch in this fix. > >>> > >>> Might this still leave some confusing behavior. Now it'll slide > >>> forward into a function, but it won't slide forward into an > >>> initializer? (so now the user has to know exactly which lines of the > >>> initializer are attributed to the line table, making that somewhat > >>> difficult to use?) > >> > >> > >> Yes I knew this was a tradeoff of the implementation. If I did a more in > >> depth search in order to slide this over to function initializer, it would > >> IMO add way more complexity to an already overly fiddly part of the > >> debugger, and the trade-off didn't seem worth it. Most initializers have > >> a fairly simple structure, as opposed say to the before the body part of a > >> function definition (that's got to have a name, but I don't know it off > >> the top of my head.) So I think the payoff would be a lot less in this > >> area. I asked around here and the folks I polled agreed that it wasn't > >> worth the complexity. > >> > >>> > >>> Testing some things out: > >>> > >>> $ cat break.cpp > >>> __attribute__((optnone)) int f1() { return 1; } > >>> struct t1 { > >>> int // 3 > >>> i // 4 > >>> = // 5 > >>> f1 // 6 > >>> (); > >>> t1() { > >>> } > >>> }; > >>> > >>> t1 v1; > >>> int main() { > >>> } > >>> > >>> The line table: > >>> > >>> Address Line Column File ISA Discriminator Flags > >>> ------------------ ------ ------ ------ --- ------------- ------------- > >>> 0x0000000000401140 1 0 1 0 0 is_stmt > >>> 0x0000000000401144 1 37 1 0 0 is_stmt > >>> prologue_end > >>> 0x0000000000401150 13 0 1 0 0 is_stmt > >>> 0x0000000000401154 14 1 1 0 0 is_stmt > >>> prologue_end > >>> 0x0000000000401158 14 1 1 0 0 is_stmt > >>> end_sequence > >>> 0x0000000000401020 0 0 1 0 0 is_stmt > >>> 0x0000000000401024 12 4 1 0 0 is_stmt > >>> prologue_end > >>> 0x0000000000401040 0 0 1 0 0 is_stmt > >>> 0x000000000040104b 0 0 1 0 0 is_stmt > >>> end_sequence > >>> 0x0000000000401160 8 0 1 0 0 is_stmt > >>> 0x0000000000401174 6 7 1 0 0 is_stmt > >>> prologue_end > >>> 0x000000000040117f 4 7 1 0 0 is_stmt > >>> 0x0000000000401181 9 3 1 0 0 is_stmt > >>> 0x0000000000401187 9 3 1 0 0 is_stmt > >>> end_sequence > >>> > >>> Has entries for line 4 and 6 from the initializer (and 8 and 9 from > >>> the ctor), but gdb doesn't seem to want to break on any of them. So > >>> I'm not sure what gdb's doing, but it seems pretty unhelpful > >>> (apparently gdb breaks in to the ctor too late to even let you step > >>> into the initializers... guess that's because the initializers are > >>> called in the prologue according to the debug info): > >>> (gdb) b 1 > >>> Breakpoint 1 at 0x401144: file break.cpp, line 1. > >>> (gdb) b 2 > >>> Breakpoint 2 at 0x401181: file break.cpp, line 9. > >>> (gdb) b 3 > >>> Note: breakpoint 2 also set at pc 0x401181. > >>> Breakpoint 3 at 0x401181: file break.cpp, line 9. > >>> (gdb) b 4 > >>> Note: breakpoints 2 and 3 also set at pc 0x401181. > >>> Breakpoint 4 at 0x401181: file break.cpp, line 9. > >>> (gdb) b 5 > >>> Note: breakpoints 2, 3 and 4 also set at pc 0x401181. > >>> Breakpoint 5 at 0x401181: file break.cpp, line 9. > >>> (gdb) b 6 > >>> Note: breakpoints 2, 3, 4 and 5 also set at pc 0x401181. > >>> Breakpoint 6 at 0x401181: file break.cpp, line 9. > >>> (gdb) b 7 > >>> Note: breakpoints 2, 3, 4, 5 and 6 also set at pc 0x401181. > >>> Breakpoint 7 at 0x401181: file break.cpp, line 9. > >>> (gdb) b 8 > >>> Note: breakpoints 2, 3, 4, 5, 6 and 7 also set at pc 0x401181. > >>> Breakpoint 8 at 0x401181: file break.cpp, line 9. > >>> (gdb) b 9 > >>> Note: breakpoints 2, 3, 4, 5, 6, 7 and 8 also set at pc 0x401181. > >>> Breakpoint 9 at 0x401181: file break.cpp, line 9. > >>> > >>> lldb does already seem to let me break on the initializer without this > >>> patch - so any idea what part of this already works compared to what's > >>> being fixed here? (though lldb breaks on line 6 even when I asked to > >>> break on line 7 or 8, but it doesn't break on 6 when I ask to break on > >>> 6... ) > >> > >> > >> Yes. This is because the line table entry for the "first line" of a > >> function is also imprecise. For instance, if you do: > >> > >> int > >> foo() > >> { > >> > >> } > >> > >> the "int" line in the function definition pretty much never gets a line > >> table entry, yet that's a perfectly reasonable place to set a breakpoint > >> on the function. So the "am I sliding across function definitions" check > >> has a little slop in the comparison. Just put a handful of blank lines > >> between the "();" of the struct initialization and the definition of the > >> constructor, and then the breakpoint won't take. > > > > Ah, right right - I was still a bit confused by why the line was > > showing as line 6, but I get it - it's because that's where the first > > instruction for function is. Hmm, actually that doesn't quite make > > sense to me still - if I set a breakpoint at line 9, why wouldn't I > > get a breakpoint on that line? I guess maybe lldb detects that that is > > the line where a function starts according to the DW_AT_decl_line in > > the DW_TAG_subprogram, and instead of breaking on line 9 per the line > > table, it breaks on the start of the function which is line 6? Though > > when the program... huh, it does stop at 6, even though I said break 9 > > (well, I said break 13 in my new example where I've added the > > suggested handful of blank lines) > > > > Seems a bit surprising/not something I can readily explain: > > > > $ cat -n test.cpp > > 1 __attribute__((optnone)) int f1() { return 1; } > > 2 struct t1 { > > 3 int // 3 > > 4 i // 4 > > 5 = // 5 > > 6 f1 // 6 > > 7 (); > > 8 > > 9 > > 10 > > 11 > > 12 > > 13 t1() {} > > 14 }; > > 15 t1 v1; > > 16 int main() {} > > blaikie@blaikie-linux2:~/dev/scratch$ lldb ./a.out > > (lldb) target create "./a.out" > > Current executable set to > > '/usr/local/google/home/blaikie/dev/scratch/a.out' (x86_64). > > (lldb) b 13 > > Breakpoint 1: where = a.out`t1::t1() + 20 at test.cpp:6:7, address = > > 0x0000000000401174 > > (lldb) r > > Process 554103 launched: > > '/usr/local/google/home/blaikie/dev/scratch/a.out' (x86_64) > > Process 554103 stopped > > * thread #1, name = 'a.out', stop reason = breakpoint 1.1 > > frame #0: 0x0000000000401174 a.out`t1::t1(this=0x000000000040402c) > > at test.cpp:6:7 > > 3 int // 3 > > 4 i // 4 > > 5 = // 5 > > -> 6 f1 // 6 > > 7 (); > > 8 > > 9 > > (lldb) > > There's another "fiddly" bit of the breakpoint setting that likely explains > what you are seeing. If you set breakpoint (either by name or by file & > line) that happens to fall inside the prologue of a function, lldb moves the > breakpoint to the first instruction after the prologue. If we don't do this > then when you stop in the function at this breakpoint, argument values are > likely to be incorrect, since the debug information generally only gives > correct locations after the prologue. It also makes getting the backtrace > right trickier since you then have to understand where in the prologue you > are. > > Since not very many people actually need to debug the prologue sequence of a > function, this avoids a lot of confusion at the cost of a just a smidge more > mystery. gdb does this as well (or at least it did the last time I played > with it.) There is a setting to not skip the prologue, which you need to do > for instance if you want to ensure that you stop while the arguments are > still all in their ABI specified locations. But the default is to do the > slide. > > So if the first thing that happens after the prologue in t1() is the > initialization of i, then the sliding of the breakpoint past the prologue > would move it from line 13 to line 6, as you saw. And we don't let the > prologue slide cross function boundaries so we don't do a "cross function > boundaries" check in that move.
Hmm, yep, that seems to be it. Which just makes me more confused about what gdb's doing then (when I set a breakpoint at the function, it seems to be breaking after all the initializers have run - well beyond the prologue - perhaps that's what they've done to try to get the user to where the member initializers have finished running). Thanks for talking me through it. - Dave > > If somebody has some spare time on their hands it might be fun to try to > build up a little report describing these moves, and then store that in the > location and print it out with "break list -v". That would help when you are > puzzling over how your breakpoint got where it did... > > Jim > > > > > > >>> (lldb) target create "./a.out" > >>> Current executable set to > >>> '/usr/local/google/home/blaikie/dev/scratch/a.out' (x86_64). > >>> (lldb) b 1 > >>> Breakpoint 1: where = a.out`f1() + 4 at break.cpp:1:37, address = > >>> 0x0000000000401144 > >>> (lldb) b 2 > >>> Breakpoint 2: no locations (pending). > >>> WARNING: Unable to resolve breakpoint to any actual locations. > >>> (lldb) b 3 > >>> Breakpoint 3: no locations (pending). > >>> WARNING: Unable to resolve breakpoint to any actual locations. > >>> (lldb) b 4 > >>> Breakpoint 4: no locations (pending). > >>> WARNING: Unable to resolve breakpoint to any actual locations. > >>> (lldb) b 5 > >>> Breakpoint 5: no locations (pending). > >>> WARNING: Unable to resolve breakpoint to any actual locations. > >>> (lldb) b 6 > >>> Breakpoint 6: no locations (pending). > >>> WARNING: Unable to resolve breakpoint to any actual locations. > >>> (lldb) b 7 > >>> Breakpoint 7: where = a.out`t1::t1() + 20 at break.cpp:6:7, address = > >>> 0x0000000000401174 > >>> (lldb) b 8 > >>> Breakpoint 8: where = a.out`t1::t1() + 20 at break.cpp:6:7, address = > >>> 0x0000000000401174 > >>> (lldb) b 9 > >>> Breakpoint 9: where = a.out`t1::t1() + 33 at break.cpp:9:3, address = > >>> 0x0000000000401181 > >>> (lldb) r > >>> > >>>> BTW, the check wouldn't have affected code from .defs files because I > >>>> only do it if the original breakpoint specification and the "function > >>>> start" are from the same source file. > >>> > >>> Sure enough - no doubt someone out there has some file that #includes > >>> itself in some entertaining way, but likely quite rare. > >> > >> Given that the penalty you have to pay for such a deed is having to be a > >> little more careful how you set breakpoints, I don't feel the need for > >> super-human efforts in this area. > >> > >> Jim > >> > >> > >>> > >>>> And we know about inlined blocks so inlining isn't going to fool us > >>>> either. > >>>> > >>>> Jim > >>>> > >>>> > >>>> > >>>> > >>>>> On Jan 15, 2021, at 5:09 PM, David Blaikie <dblai...@gmail.com> wrote: > >>>>> > >>>>> "But because their source lines are outside the function source range" > >>>>> > >>>>> Not sure I understand that - the DWARF doesn't describe a function > >>>>> source range, right? Only the line a function starts on. And a > >>>>> function can have code from source lines in many files/offsets that > >>>>> are unrelated to the function start line (LLVM in several places > >>>>> #includes .def files into functions to stamp out tables, switches, > >>>>> arrays, etc, for instance) > >>>>> > >>>>> On Fri, Jan 15, 2021 at 4:47 PM Jim Ingham via Phabricator via > >>>>> lldb-commits <lldb-commits@lists.llvm.org> wrote: > >>>>>> > >>>>>> jingham created this revision. > >>>>>> jingham requested review of this revision. > >>>>>> Herald added a project: LLDB. > >>>>>> Herald added a subscriber: lldb-commits. > >>>>>> > >>>>>> The inline initializers contribute code to the constructor(s). You > >>>>>> will step onto them in the source view as you step through the > >>>>>> constructor, for instance. But because their source lines are outside > >>>>>> the function source range, lldb thought a breakpoint on the > >>>>>> initializer line was crossing from one function to another, which file > >>>>>> & line breakpoints don't allow. That meant if you tried to set a > >>>>>> breakpoint on one of these lines it doesn't create any locations. > >>>>>> > >>>>>> This patch fixes that by asserting that if the LineEntry in one of the > >>>>>> SymbolContexts that the search produced exactly matches the file & > >>>>>> line specifications in the breakpoint, it has to be a valid place to > >>>>>> set the breakpoint, and we should just set it. > >>>>>> > >>>>>> > >>>>>> Repository: > >>>>>> rG LLVM Github Monorepo > >>>>>> > >>>>>> https://reviews.llvm.org/D94846 > >>>>>> > >>>>>> Files: > >>>>>> lldb/source/Breakpoint/BreakpointResolverFileLine.cpp > >>>>>> lldb/test/API/lang/cpp/break-on-initializers/Makefile > >>>>>> lldb/test/API/lang/cpp/break-on-initializers/TestBreakOnCPP11Initializers.py > >>>>>> lldb/test/API/lang/cpp/break-on-initializers/main.cpp > >>>>>> > >>>>>> _______________________________________________ > >>>>>> lldb-commits mailing list > >>>>>> lldb-commits@lists.llvm.org > >>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits