Re: [Lldb-commits] [PATCH] D94846: Allow breakpoints to be set on C++11 inline initializers

2021-01-19 Thread Jim Ingham via lldb-commits


> On Jan 19, 2021, at 3:34 PM, David Blaikie  wrote:
> 
> On Tue, Jan 19, 2021 at 2:55 PM Jim Ingham  wrote:
>> 
>> 
>> 
>>> On Jan 19, 2021, at 11:40 AM, David Blaikie  wrote:
>>> 
>>> On Tue, Jan 19, 2021 at 10:17 AM Jim Ingham  wrote:
 
 
 
> On Jan 17, 2021, at 10:47 AM, David Blaikie  wrote:
> 
> On Fri, Jan 15, 2021 at 6:22 PM Jim Ingham  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 

Re: [Lldb-commits] [PATCH] D94846: Allow breakpoints to be set on C++11 inline initializers

2021-01-19 Thread David Blaikie via lldb-commits
On Tue, Jan 19, 2021 at 2:55 PM Jim Ingham  wrote:
>
>
>
> > On Jan 19, 2021, at 11:40 AM, David Blaikie  wrote:
> >
> > On Tue, Jan 19, 2021 at 10:17 AM Jim Ingham  wrote:
> >>
> >>
> >>
> >>> On Jan 17, 2021, at 10:47 AM, David Blaikie  wrote:
> >>>
> >>> On Fri, Jan 15, 2021 at 6:22 PM Jim Ingham  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 

Re: [Lldb-commits] [PATCH] D94846: Allow breakpoints to be set on C++11 inline initializers

2021-01-19 Thread Jim Ingham via lldb-commits


> On Jan 19, 2021, at 11:40 AM, David Blaikie  wrote:
> 
> On Tue, Jan 19, 2021 at 10:17 AM Jim Ingham  wrote:
>> 
>> 
>> 
>>> On Jan 17, 2021, at 10:47 AM, David Blaikie  wrote:
>>> 
>>> On Fri, Jan 15, 2021 at 6:22 PM Jim Ingham  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 

Re: [Lldb-commits] [PATCH] D94846: Allow breakpoints to be set on C++11 inline initializers

2021-01-19 Thread David Blaikie via lldb-commits
On Tue, Jan 19, 2021 at 10:17 AM Jim Ingham  wrote:
>
>
>
> > On Jan 17, 2021, at 10:47 AM, David Blaikie  wrote:
> >
> > On Fri, Jan 15, 2021 at 6:22 PM Jim Ingham  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.

> >> 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:
> >
> > AddressLine   Column File   ISA Discriminator Flags
> > -- -- -- -- --- - -
> > 0x00401140  1  0  1   0 0  is_stmt
> > 0x00401144  1 37  1   0 0  is_stmt 
> > prologue_end
> > 0x00401150 13  0  1   0 0  is_stmt
> > 

Re: [Lldb-commits] [PATCH] D94846: Allow breakpoints to be set on C++11 inline initializers

2021-01-19 Thread Jim Ingham via lldb-commits


> On Jan 17, 2021, at 10:47 AM, David Blaikie  wrote:
> 
> On Fri, Jan 15, 2021 at 6:22 PM Jim Ingham  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.

> 
>> 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:
> 
> AddressLine   Column File   ISA Discriminator Flags
> -- -- -- -- --- - -
> 0x00401140  1  0  1   0 0  is_stmt
> 0x00401144  1 37  1   0 0  is_stmt 
> prologue_end
> 0x00401150 13  0  1   0 0  is_stmt
> 0x00401154 14  1  1   0 0  is_stmt 
> prologue_end
> 0x00401158 14  1  1   0 0  is_stmt 
> end_sequence
> 0x00401020  0  0  1   0 0  is_stmt
> 0x00401024 12  4  1   0 0  is_stmt 
> prologue_end
> 0x00401040  0  0  1   0 0  is_stmt
> 0x0040104b  0  0  1   0 0  is_stmt 
> end_sequence
> 

Re: [Lldb-commits] [PATCH] D94846: Allow breakpoints to be set on C++11 inline initializers

2021-01-17 Thread David Blaikie via lldb-commits
On Fri, Jan 15, 2021 at 6:22 PM Jim Ingham  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).

> 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.

> 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?)

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:

AddressLine   Column File   ISA Discriminator Flags
-- -- -- -- --- - -
0x00401140  1  0  1   0 0  is_stmt
0x00401144  1 37  1   0 0  is_stmt prologue_end
0x00401150 13  0  1   0 0  is_stmt
0x00401154 14  1  1   0 0  is_stmt prologue_end
0x00401158 14  1  1   0 0  is_stmt end_sequence
0x00401020  0  0  1   0 0  is_stmt
0x00401024 12  4  1   0 0  is_stmt prologue_end
0x00401040  0  0  1   0 0  is_stmt
0x0040104b  0  0  1   0 0  is_stmt end_sequence
0x00401160  8  0  1   0 0  is_stmt
0x00401174  6  7  1   0 0  is_stmt prologue_end
0x0040117f  4  7  1   0 0  is_stmt
0x00401181  9  3  1   0 0  is_stmt
0x00401187  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) 

Re: [Lldb-commits] [PATCH] D94846: Allow breakpoints to be set on C++11 inline initializers

2021-01-15 Thread Jim Ingham via lldb-commits
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.

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.  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...

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.

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.

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.  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  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  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


Re: [Lldb-commits] [PATCH] D94846: Allow breakpoints to be set on C++11 inline initializers

2021-01-15 Thread David Blaikie via lldb-commits
"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  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