[lldb-dev] Where "thread until <line-number>" should set breakpoints?
Venkata Ramanaiah Nalamothu via lldb-dev
lldb-dev at lists.llvm.org
Sun Aug 5 04:02:04 PDT 2018
I have created https://reviews.llvm.org/D50304.
BTW, I almost forgot the fact that the error message changes were already
covered in my other patch https://reviews.llvm.org/D48865.
On Fri, Aug 3, 2018 at 11:41 PM, Jim Ingham <jingham at apple.com> wrote:
> Thanks! I look forward to the patch.
>
> Jim
>
>
> > On Aug 2, 2018, at 8:56 PM, Venkata Ramanaiah Nalamothu <
> ramana.venkat83 at gmail.com> wrote:
> >
> > 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/20180805/1de52e88/attachment.html>
More information about the lldb-dev
mailing list