[lldb-dev] Where "thread until <line-number>" should set breakpoints?
Venkata Ramanaiah Nalamothu via lldb-dev
lldb-dev at lists.llvm.org
Thu Aug 2 20:56:58 PDT 2018
Thanks Jim for the elaborate reply.
I knew what is happening in below piece of code and also has a patch ready
but now needs a bit of fine tuning based on your below comments. I just
wanted to hear from you folks that my understanding is correct and I am not
missing something.
Also, the code around it needs modification of error messages to refer to
'thread->GetID()' instead of 'm_options.m_thread_idx'. I will create a
review on phabricator with all these changes.
- Ramana
On Thu, Aug 2, 2018 at 10:56 PM, Jim Ingham <jingham at apple.com> wrote:
> That's a bug. The code in CommandObjectThreadUntil needs to be a little
> more careful about sliding the line number to the "nearest source line that
> has code." It currently does:
>
> for (uint32_t line_number : line_numbers) {
> uint32_t start_idx_ptr = index_ptr;
> while (start_idx_ptr <= end_ptr) {
> LineEntry line_entry;
> const bool exact = false;
> start_idx_ptr = sc.comp_unit->FindLineEntry(
> start_idx_ptr, line_number, sc.comp_unit, exact,
> &line_entry);
> if (start_idx_ptr == UINT32_MAX)
> break;
>
> addr_t address =
> line_entry.range.GetBaseAddress().GetLoadAddress(target);
> if (address != LLDB_INVALID_ADDRESS) {
> if (fun_addr_range.ContainsLoadAddress(address, target))
> address_list.push_back(address);
> else
> all_in_function = false;
> }
> start_idx_ptr++;
> }
> }
>
> The problem is that in the while loop we set:
>
> const bool exact = false;
>
> Having exact == false through the whole loop means that all the other line
> table entries after the last exact match will match because from the index
> ptr on there are no more entries, so we slide it to the next one.
>
> In general it's not always easy to tell which lines will actually
> contribute code so lldb is lax about line number matching, and slides the
> breakpoint to the nearest subsequent line that has code when there's no
> exact match. That seems a good principle to follow here as well. So I
> don't want to just set exact to "true".
>
> I think the better behavior is to try FindLineEntry the first time with
> exact = false. If that picks up a different line from the one given, reset
> the line we are looking for to the found line. In either case, then go on
> to search for all the other instances of that line with exact = false. It
> might actually be handy to have another function on CompUnit like
> FindAllEntriesForLine that bundles up this behavior as it seems a generally
> useful one.
>
> If you want to give this a try, please do. Otherwise, file a bug and I'll
> get to it when I have a chance.
>
> Jim
>
>
>
> > On Aug 1, 2018, at 10:22 PM, Ramana <ramana.venkat83 at gmail.com> wrote:
> >
> >
> > On Thu, Aug 2, 2018 at 3:32 AM, Jim Ingham <jingham at apple.com> wrote:
> >
> >
> > > On Jul 24, 2018, at 9:05 PM, Ramana via lldb-dev <
> lldb-dev at lists.llvm.org> wrote:
> > >
> > > On the subject line, the ToT lldb (see code around
> CommandObjectThread.cpp:1230) sets the breakpoint on the first exact
> matching line of 'line-number' or the closest line number > 'line-number'
> i.e. the best match.
> > >
> > > And along with that, starting from the above exact/best matching line
> number index in the line table, the breakpoints are also being set on every
> other line number available in the line table in the current function
> scope. This latter part, I believe, is incorrect.
> >
> > Why do you think this is incorrect?
> >
> > The requirements for "thread until <line number>" are:
> >
> > a) If any code contributed by <line number> is executed before leaving
> the function, stop
> > b) If you end up leaving the function w/o triggering (a), then stop
> >
> > Understood and no concerns on this.
> >
> > Correct or incorrect should be determined by how well the implementation
> fits those requirements.
> >
> > There isn't currently a reliable indication from the debug information
> or line tables that "line N will always be entered starting with the block
> at 0x123". So you can't tell without doing control flow analysis, which if
> any of the separate entries in the line table for the same line will get
> hit in the course of executing the function. So the safest thing to do is
> to set breakpoints on them all.
> >
> > From the above, I understand that we have to do this when the debug line
> table has more than one entry for a particular source line. And this is
> what I referred to as "machine code for one single source line is scattered
> across" in my previous mail. Thanks for sharing why we had to do that.
> >
> > Besides setting a few more breakpoints - which should be pretty cheap -
> I don't see much downside to the way it is currently implemented.
> >
> > Anyway, why did this bother you?
> >
> > Jim
> >
> > However, I am concerned about the below 'thread until' behaviour. For
> the attached test case (kernels.cpp - OpenCL code), following is the debug
> line table generated by the compiler.
> >
> > File name Line number Starting address
> > ./kernels.cpp:[++]
> > kernels.cpp 9 0xacc74d00
> > kernels.cpp 12 0xacc74d00
>
> > kernels.cpp 14 0xacc74d40
> > kernels.cpp 13 0xacc74dc0
> > kernels.cpp 14 0xacc74e00
> > kernels.cpp 25 0xacc74e80
> > kernels.cpp 25 0xacc74ec0
> > kernels.cpp 26 0xacc74f00
> > kernels.cpp 26 0xacc74f40
> > kernels.cpp 26 0xacc74f80
> > kernels.cpp 17 0xacc74fc0
> > kernels.cpp 18 0xacc75000
> > kernels.cpp 18 0xacc75040
> > kernels.cpp 19 0xacc75080
> > kernels.cpp 27 0xacc750c0
> > kernels.cpp 27 0xacc75140
> > kernels.cpp 28 0xacc75180
> > kernels.cpp 28 0xacc751c0
> > kernels.cpp 29 0xacc75200
> > kernels.cpp 29 0xacc75240
> > kernels.cpp 30 0xacc75280
> >
> > With the ToT lldb, when I am at line 12 (0xacc74d00), if I say 'thread
> until 18', the lldb log gives me the following w.r.t breakpoints.
> >
> > GDBRemoteCommunicationClient::SendGDBStoppointTypePacket() add at addr
> = 0xacc75280
> > Thread::PushPlan(0x0xa48b38f0): "Stepping from address 0xacc74d00 until
> we reach one of:
> > 0xacc75000 (bp: -4)
> > 0xacc75040 (bp: -5)
> > 0xacc75080 (bp: -6)
> > 0xacc750c0 (bp: -7)
> > 0xacc75140 (bp: -8)
> > 0xacc75180 (bp: -9)
> > 0xacc751c0 (bp: -10)
> > 0xacc75200 (bp: -11)
> > 0xacc75240 (bp: -12)
> > 0xacc75280 (bp: -13)
> >
> > Setting two breakpoints for line number 18 i.e. at 0xacc75000 and
> 0xacc75040 is understandable from your above reasoning and since we are
> anyway setting a breakpoint at the end of the function (line 30 -
> 0xacc75280), is it necessary to set the breakpoints on line numbers 19, 27,
> 28, 29 as well i.e. at 0xacc75080 (line 19), 0xacc750c0 (line 27),
> 0xacc75140 (line 27), 0xacc75180 (line 28), 0xacc751c0 (line 28),
> 0xacc75200 (line 29), 0xacc75240 (line 29)?
> >
> > The latter part i.e. setting breakpoints on 19, 27, 28, 29 as well is
> what I think is incorrect. Am I missing something here?
> >
> >
> > >
> > > What, I think, should happen is we just set only one breakpoint on the
> first exact/best match for the given 'line-number' and another on the
> return address from the current frame. And this leaves us with one special
> case where the machine code for one single source line is scattered across
> (aka scheduled across) and I do not know what is the expected behaviour in
> this case.
> > >
> > > If I my above understanding is correct and after discussing here on
> how to handle the scattered code scenario, I will submit a patch.
> > >
> > > Regards,
> > > Venkata Ramanaiah
> > > _______________________________________________
> > > lldb-dev mailing list
> > > lldb-dev at lists.llvm.org
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> >
> >
> > <kernels.cpp>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20180803/ab30f527/attachment-0001.html>
More information about the lldb-dev
mailing list