Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

I agree with Jason here, but I guess that's no surprise.  

Jim

> On Sep 12, 2017, at 2:57 PM, Jason Molenda via lldb-commits 
>  wrote:
> 
> Or another way, I don't object to asserts because we hit them so often today. 
>  I object to asserts in a shipping debugger fundamentally - it's not an error 
> mode we should encourage or depend on for an interactive tool.
> 
>> On Sep 12, 2017, at 2:56 PM, Jason Molenda  wrote:
>> 
>> I think debug-build asserts & more testing is something everyone can agree 
>> on.  I don't want to speak for Greg, but I don't think anyone is going to 
>> sign on to the idea that the lldb source base becomes super well tested & 
>> bug free and then we start using asserts liberally.  There are always going 
>> to be scenarios that our customers hit, with compilers we don't have access 
>> to, with programs we don't have access to, with targets we don't have access 
>> to, that we don't cover in our testsuite.  We can work to expand our 
>> testsuite, and expand the scope of what / how we test the debugger, but 
>> we'll never match the behavior someone running lldb on an AutoCAD built with 
>> a three year old gcc or whatever.
>> 
>>> On Sep 12, 2017, at 2:53 PM, Zachary Turner  wrote:
>>> 
>>> So let's say that in a hypothetical future we had very good test coverage.  
>>> Would there still be resistance to asserts?  Because if your test coverage 
>>> is good, then your bots should be catching most stuff.
>>> 
>>> On Tue, Sep 12, 2017 at 2:42 PM Greg Clayton  wrote:
>>> Agreed. We need to do better on the test coverage part, don't think you'll 
>>> get any pushback on that front.
>>> 
 On Sep 12, 2017, at 2:36 PM, Zachary Turner  wrote:
 
 Right, and the only reason it's a bigger problem is because of   test 
 coverage.
 
 On Tue, Sep 12, 2017 at 2:34 PM Jason Molenda  wrote:
 
 
> On Sep 12, 2017, at 11:19 AM, Zachary Turner  wrote:
> 
> 
> 
> On Tue, Sep 12, 2017 at 11:07 AM Greg Clayton  wrote:
> 
>> On Sep 12, 2017, at 10:10 AM, Zachary Turner  wrote:
>> 
>> 
>> 
>> On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton  wrote:
>>> On Sep 12, 2017, at 9:53 AM, Zachary Turner  wrote:
>>> 
>>> If you had just logged it, the bug would still not be fixed because 
>>> nobody would know about it.  I also can't believe we have to keep 
>>> saying this :-/
>> 
>> By log, I mean Host::SystemLog(...) which would come out in the command 
>> line. Not "log enable ...". So users would see the issue and report the 
>> bug. Crashing doesn't mean people always report the bug.
>> I mentioned earlier in the thread that I assumed Xcode had an automatic 
>> crash that would handle the crash and automatically upload it to Apple.  
>> Is this really not the case?  If core dumps are too big, why not just a 
>> stack trace?  Surely the Xcode team must have some kind of internal 
>> metrics system to track stability.
> 
> They do just upload text crash logs. It doesn't tell us what expression 
> triggered the issue though. It shows a crash in an expression, but 
> doesn't show the expression text as this violates privacy.
> 
> So, you do get a bug report when a crash occurs then.  In contrast to the 
> case where you simply log something, and don't get a crash report.
> 
> In some cases, you can look at the code and figure out why it crashed.  
> In other cases the bug occurs extremely infrequently (you can build 
> heuristic matching of call-stacks into your infrastructure that processes 
> the crash logs).  If it's a high incidence crasher then you do some 
> investigation.  And the good news is, once you fix it, you've *actually* 
> fixed it.  Now instead of hundreds of thousands of people using something 
> that doesn't work quite right for presumably the rest of the software's 
> life (since nobody knows about the bug), they have something that 
> actually works.
> 
> There's probably some initial pain associated with this approach since 
> the test coverage is so low right now (I came up with about ~25% code 
> coverage in a test I ran a while back).  When you get this higher, your 
> tests start catching all of the high incidence stuff, and then you're 
> left with only occasional crashes.
> 
> And since you have the out of process stuff, it doesn't even bring down 
> Xcode anymore.  Just a debugging session.  That's an amazing price to pay 
> for having instant visibility into a huge class of bugs that LLDB is 
> currently willfully ignoring.
 
 
 I guess I'm speaking at someone who is supporting a debugger used by tens 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jason Molenda via lldb-commits]

Or another way, I don't object to asserts because we hit them so often today.  
I object to asserts in a shipping debugger fundamentally - it's not an error 
mode we should encourage or depend on for an interactive tool.

> On Sep 12, 2017, at 2:56 PM, Jason Molenda  wrote:
> 
> I think debug-build asserts & more testing is something everyone can agree 
> on.  I don't want to speak for Greg, but I don't think anyone is going to 
> sign on to the idea that the lldb source base becomes super well tested & bug 
> free and then we start using asserts liberally.  There are always going to be 
> scenarios that our customers hit, with compilers we don't have access to, 
> with programs we don't have access to, with targets we don't have access to, 
> that we don't cover in our testsuite.  We can work to expand our testsuite, 
> and expand the scope of what / how we test the debugger, but we'll never 
> match the behavior someone running lldb on an AutoCAD built with a three year 
> old gcc or whatever.
> 
>> On Sep 12, 2017, at 2:53 PM, Zachary Turner  wrote:
>> 
>> So let's say that in a hypothetical future we had very good test coverage.  
>> Would there still be resistance to asserts?  Because if your test coverage 
>> is good, then your bots should be catching most stuff.
>> 
>> On Tue, Sep 12, 2017 at 2:42 PM Greg Clayton  wrote:
>> Agreed. We need to do better on the test coverage part, don't think you'll 
>> get any pushback on that front.
>> 
>>> On Sep 12, 2017, at 2:36 PM, Zachary Turner  wrote:
>>> 
>>> Right, and the only reason it's a bigger problem is because of   test 
>>> coverage.
>>> 
>>> On Tue, Sep 12, 2017 at 2:34 PM Jason Molenda  wrote:
>>> 
>>> 
 On Sep 12, 2017, at 11:19 AM, Zachary Turner  wrote:
 
 
 
 On Tue, Sep 12, 2017 at 11:07 AM Greg Clayton  wrote:
 
> On Sep 12, 2017, at 10:10 AM, Zachary Turner  wrote:
> 
> 
> 
> On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton  wrote:
>> On Sep 12, 2017, at 9:53 AM, Zachary Turner  wrote:
>> 
>> If you had just logged it, the bug would still not be fixed because 
>> nobody would know about it.  I also can't believe we have to keep saying 
>> this :-/
> 
> By log, I mean Host::SystemLog(...) which would come out in the command 
> line. Not "log enable ...". So users would see the issue and report the 
> bug. Crashing doesn't mean people always report the bug.
> I mentioned earlier in the thread that I assumed Xcode had an automatic 
> crash that would handle the crash and automatically upload it to Apple.  
> Is this really not the case?  If core dumps are too big, why not just a 
> stack trace?  Surely the Xcode team must have some kind of internal 
> metrics system to track stability.
 
 They do just upload text crash logs. It doesn't tell us what expression 
 triggered the issue though. It shows a crash in an expression, but doesn't 
 show the expression text as this violates privacy.
 
 So, you do get a bug report when a crash occurs then.  In contrast to the 
 case where you simply log something, and don't get a crash report.
 
 In some cases, you can look at the code and figure out why it crashed.  In 
 other cases the bug occurs extremely infrequently (you can build heuristic 
 matching of call-stacks into your infrastructure that processes the crash 
 logs).  If it's a high incidence crasher then you do some investigation.  
 And the good news is, once you fix it, you've *actually* fixed it.  Now 
 instead of hundreds of thousands of people using something that doesn't 
 work quite right for presumably the rest of the software's life (since 
 nobody knows about the bug), they have something that actually works.
 
 There's probably some initial pain associated with this approach since the 
 test coverage is so low right now (I came up with about ~25% code coverage 
 in a test I ran a while back).  When you get this higher, your tests start 
 catching all of the high incidence stuff, and then you're left with only 
 occasional crashes.
 
 And since you have the out of process stuff, it doesn't even bring down 
 Xcode anymore.  Just a debugging session.  That's an amazing price to pay 
 for having instant visibility into a huge class of bugs that LLDB is 
 currently willfully ignoring.
>>> 
>>> 
>>> I guess I'm speaking at someone who is supporting a debugger used by tens 
>>> of thousands of people.  The debugger crashing is a vastly bigger problem 
>>> to us than bugs that didn't announce themselves by taking down the 
>>> debugger.  lldb_asserts that activate when we're running debug builds are 
>>> fine, that's exactly when I want to 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jason Molenda via lldb-commits]

I think debug-build asserts & more testing is something everyone can agree on.  
I don't want to speak for Greg, but I don't think anyone is going to sign on to 
the idea that the lldb source base becomes super well tested & bug free and 
then we start using asserts liberally.  There are always going to be scenarios 
that our customers hit, with compilers we don't have access to, with programs 
we don't have access to, with targets we don't have access to, that we don't 
cover in our testsuite.  We can work to expand our testsuite, and expand the 
scope of what / how we test the debugger, but we'll never match the behavior 
someone running lldb on an AutoCAD built with a three year old gcc or whatever.

> On Sep 12, 2017, at 2:53 PM, Zachary Turner  wrote:
> 
> So let's say that in a hypothetical future we had very good test coverage.  
> Would there still be resistance to asserts?  Because if your test coverage is 
> good, then your bots should be catching most stuff.
> 
> On Tue, Sep 12, 2017 at 2:42 PM Greg Clayton  wrote:
> Agreed. We need to do better on the test coverage part, don't think you'll 
> get any pushback on that front.
> 
>> On Sep 12, 2017, at 2:36 PM, Zachary Turner  wrote:
>> 
>> Right, and the only reason it's a bigger problem is because of   test 
>> coverage.
>> 
>> On Tue, Sep 12, 2017 at 2:34 PM Jason Molenda  wrote:
>> 
>> 
>> > On Sep 12, 2017, at 11:19 AM, Zachary Turner  wrote:
>> >
>> >
>> >
>> > On Tue, Sep 12, 2017 at 11:07 AM Greg Clayton  wrote:
>> >
>> >> On Sep 12, 2017, at 10:10 AM, Zachary Turner  wrote:
>> >>
>> >>
>> >>
>> >> On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton  wrote:
>> >>> On Sep 12, 2017, at 9:53 AM, Zachary Turner  wrote:
>> >>>
>> >>> If you had just logged it, the bug would still not be fixed because 
>> >>> nobody would know about it.  I also can't believe we have to keep saying 
>> >>> this :-/
>> >>
>> >> By log, I mean Host::SystemLog(...) which would come out in the command 
>> >> line. Not "log enable ...". So users would see the issue and report the 
>> >> bug. Crashing doesn't mean people always report the bug.
>> >> I mentioned earlier in the thread that I assumed Xcode had an automatic 
>> >> crash that would handle the crash and automatically upload it to Apple.  
>> >> Is this really not the case?  If core dumps are too big, why not just a 
>> >> stack trace?  Surely the Xcode team must have some kind of internal 
>> >> metrics system to track stability.
>> >
>> > They do just upload text crash logs. It doesn't tell us what expression 
>> > triggered the issue though. It shows a crash in an expression, but doesn't 
>> > show the expression text as this violates privacy.
>> >
>> > So, you do get a bug report when a crash occurs then.  In contrast to the 
>> > case where you simply log something, and don't get a crash report.
>> >
>> > In some cases, you can look at the code and figure out why it crashed.  In 
>> > other cases the bug occurs extremely infrequently (you can build heuristic 
>> > matching of call-stacks into your infrastructure that processes the crash 
>> > logs).  If it's a high incidence crasher then you do some investigation.  
>> > And the good news is, once you fix it, you've *actually* fixed it.  Now 
>> > instead of hundreds of thousands of people using something that doesn't 
>> > work quite right for presumably the rest of the software's life (since 
>> > nobody knows about the bug), they have something that actually works.
>> >
>> > There's probably some initial pain associated with this approach since the 
>> > test coverage is so low right now (I came up with about ~25% code coverage 
>> > in a test I ran a while back).  When you get this higher, your tests start 
>> > catching all of the high incidence stuff, and then you're left with only 
>> > occasional crashes.
>> >
>> > And since you have the out of process stuff, it doesn't even bring down 
>> > Xcode anymore.  Just a debugging session.  That's an amazing price to pay 
>> > for having instant visibility into a huge class of bugs that LLDB is 
>> > currently willfully ignoring.
>> 
>> 
>> I guess I'm speaking at someone who is supporting a debugger used by tens of 
>> thousands of people.  The debugger crashing is a vastly bigger problem to us 
>> than bugs that didn't announce themselves by taking down the debugger.  
>> lldb_asserts that activate when we're running debug builds are fine, that's 
>> exactly when I want to see the debugger crash on my desktop or when it's 
>> running the testsuite.  Asserting when it's on the customer's desktop is a 
>> no-go.
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

So let's say that in a hypothetical future we had very good test coverage.
Would there still be resistance to asserts?  Because if your test coverage
is good, then your bots should be catching most stuff.

On Tue, Sep 12, 2017 at 2:42 PM Greg Clayton  wrote:

> Agreed. We need to do better on the test coverage part, don't think you'll
> get any pushback on that front.
>
> On Sep 12, 2017, at 2:36 PM, Zachary Turner  wrote:
>
> Right, and the only reason it's a bigger problem is because of   test
> coverage.
>
> On Tue, Sep 12, 2017 at 2:34 PM Jason Molenda  wrote:
>
>>
>>
>> > On Sep 12, 2017, at 11:19 AM, Zachary Turner 
>> wrote:
>> >
>> >
>> >
>> > On Tue, Sep 12, 2017 at 11:07 AM Greg Clayton 
>> wrote:
>> >
>> >> On Sep 12, 2017, at 10:10 AM, Zachary Turner 
>> wrote:
>> >>
>> >>
>> >>
>> >> On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton 
>> wrote:
>> >>> On Sep 12, 2017, at 9:53 AM, Zachary Turner 
>> wrote:
>> >>>
>> >>> If you had just logged it, the bug would still not be fixed because
>> nobody would know about it.  I also can't believe we have to keep saying
>> this :-/
>> >>
>> >> By log, I mean Host::SystemLog(...) which would come out in the
>> command line. Not "log enable ...". So users would see the issue and report
>> the bug. Crashing doesn't mean people always report the bug.
>> >> I mentioned earlier in the thread that I assumed Xcode had an
>> automatic crash that would handle the crash and automatically upload it to
>> Apple.  Is this really not the case?  If core dumps are too big, why not
>> just a stack trace?  Surely the Xcode team must have some kind of internal
>> metrics system to track stability.
>> >
>> > They do just upload text crash logs. It doesn't tell us what expression
>> triggered the issue though. It shows a crash in an expression, but doesn't
>> show the expression text as this violates privacy.
>> >
>> > So, you do get a bug report when a crash occurs then.  In contrast to
>> the case where you simply log something, and don't get a crash report.
>> >
>> > In some cases, you can look at the code and figure out why it crashed.
>> In other cases the bug occurs extremely infrequently (you can build
>> heuristic matching of call-stacks into your infrastructure that processes
>> the crash logs).  If it's a high incidence crasher then you do some
>> investigation.  And the good news is, once you fix it, you've *actually*
>> fixed it.  Now instead of hundreds of thousands of people using something
>> that doesn't work quite right for presumably the rest of the software's
>> life (since nobody knows about the bug), they have something that actually
>> works.
>> >
>> > There's probably some initial pain associated with this approach since
>> the test coverage is so low right now (I came up with about ~25% code
>> coverage in a test I ran a while back).  When you get this higher, your
>> tests start catching all of the high incidence stuff, and then you're left
>> with only occasional crashes.
>> >
>> > And since you have the out of process stuff, it doesn't even bring down
>> Xcode anymore.  Just a debugging session.  That's an amazing price to pay
>> for having instant visibility into a huge class of bugs that LLDB is
>> currently willfully ignoring.
>>
>>
>> I guess I'm speaking at someone who is supporting a debugger used by tens
>> of thousands of people.  The debugger crashing is a vastly bigger problem
>> to us than bugs that didn't announce themselves by taking down the
>> debugger.  lldb_asserts that activate when we're running debug builds are
>> fine, that's exactly when I want to see the debugger crash on my desktop or
>> when it's running the testsuite.  Asserting when it's on the customer's
>> desktop is a no-go.
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Greg Clayton via lldb-commits]

Agreed. We need to do better on the test coverage part, don't think you'll get 
any pushback on that front.

> On Sep 12, 2017, at 2:36 PM, Zachary Turner  wrote:
> 
> Right, and the only reason it's a bigger problem is because of   test 
> coverage.
> 
> On Tue, Sep 12, 2017 at 2:34 PM Jason Molenda  > wrote:
> 
> 
> > On Sep 12, 2017, at 11:19 AM, Zachary Turner  > > wrote:
> >
> >
> >
> > On Tue, Sep 12, 2017 at 11:07 AM Greg Clayton  > > wrote:
> >
> >> On Sep 12, 2017, at 10:10 AM, Zachary Turner  >> > wrote:
> >>
> >>
> >>
> >> On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton  >> > wrote:
> >>> On Sep 12, 2017, at 9:53 AM, Zachary Turner  >>> > wrote:
> >>>
> >>> If you had just logged it, the bug would still not be fixed because 
> >>> nobody would know about it.  I also can't believe we have to keep saying 
> >>> this :-/
> >>
> >> By log, I mean Host::SystemLog(...) which would come out in the command 
> >> line. Not "log enable ...". So users would see the issue and report the 
> >> bug. Crashing doesn't mean people always report the bug.
> >> I mentioned earlier in the thread that I assumed Xcode had an automatic 
> >> crash that would handle the crash and automatically upload it to Apple.  
> >> Is this really not the case?  If core dumps are too big, why not just a 
> >> stack trace?  Surely the Xcode team must have some kind of internal 
> >> metrics system to track stability.
> >
> > They do just upload text crash logs. It doesn't tell us what expression 
> > triggered the issue though. It shows a crash in an expression, but doesn't 
> > show the expression text as this violates privacy.
> >
> > So, you do get a bug report when a crash occurs then.  In contrast to the 
> > case where you simply log something, and don't get a crash report.
> >
> > In some cases, you can look at the code and figure out why it crashed.  In 
> > other cases the bug occurs extremely infrequently (you can build heuristic 
> > matching of call-stacks into your infrastructure that processes the crash 
> > logs).  If it's a high incidence crasher then you do some investigation.  
> > And the good news is, once you fix it, you've *actually* fixed it.  Now 
> > instead of hundreds of thousands of people using something that doesn't 
> > work quite right for presumably the rest of the software's life (since 
> > nobody knows about the bug), they have something that actually works.
> >
> > There's probably some initial pain associated with this approach since the 
> > test coverage is so low right now (I came up with about ~25% code coverage 
> > in a test I ran a while back).  When you get this higher, your tests start 
> > catching all of the high incidence stuff, and then you're left with only 
> > occasional crashes.
> >
> > And since you have the out of process stuff, it doesn't even bring down 
> > Xcode anymore.  Just a debugging session.  That's an amazing price to pay 
> > for having instant visibility into a huge class of bugs that LLDB is 
> > currently willfully ignoring.
> 
> 
> I guess I'm speaking at someone who is supporting a debugger used by tens of 
> thousands of people.  The debugger crashing is a vastly bigger problem to us 
> than bugs that didn't announce themselves by taking down the debugger.  
> lldb_asserts that activate when we're running debug builds are fine, that's 
> exactly when I want to see the debugger crash on my desktop or when it's 
> running the testsuite.  Asserting when it's on the customer's desktop is a 
> no-go.

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

Right, and the only reason it's a bigger problem is because of   test
coverage.

On Tue, Sep 12, 2017 at 2:34 PM Jason Molenda  wrote:

>
>
> > On Sep 12, 2017, at 11:19 AM, Zachary Turner  wrote:
> >
> >
> >
> > On Tue, Sep 12, 2017 at 11:07 AM Greg Clayton 
> wrote:
> >
> >> On Sep 12, 2017, at 10:10 AM, Zachary Turner 
> wrote:
> >>
> >>
> >>
> >> On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton 
> wrote:
> >>> On Sep 12, 2017, at 9:53 AM, Zachary Turner 
> wrote:
> >>>
> >>> If you had just logged it, the bug would still not be fixed because
> nobody would know about it.  I also can't believe we have to keep saying
> this :-/
> >>
> >> By log, I mean Host::SystemLog(...) which would come out in the command
> line. Not "log enable ...". So users would see the issue and report the
> bug. Crashing doesn't mean people always report the bug.
> >> I mentioned earlier in the thread that I assumed Xcode had an automatic
> crash that would handle the crash and automatically upload it to Apple.  Is
> this really not the case?  If core dumps are too big, why not just a stack
> trace?  Surely the Xcode team must have some kind of internal metrics
> system to track stability.
> >
> > They do just upload text crash logs. It doesn't tell us what expression
> triggered the issue though. It shows a crash in an expression, but doesn't
> show the expression text as this violates privacy.
> >
> > So, you do get a bug report when a crash occurs then.  In contrast to
> the case where you simply log something, and don't get a crash report.
> >
> > In some cases, you can look at the code and figure out why it crashed.
> In other cases the bug occurs extremely infrequently (you can build
> heuristic matching of call-stacks into your infrastructure that processes
> the crash logs).  If it's a high incidence crasher then you do some
> investigation.  And the good news is, once you fix it, you've *actually*
> fixed it.  Now instead of hundreds of thousands of people using something
> that doesn't work quite right for presumably the rest of the software's
> life (since nobody knows about the bug), they have something that actually
> works.
> >
> > There's probably some initial pain associated with this approach since
> the test coverage is so low right now (I came up with about ~25% code
> coverage in a test I ran a while back).  When you get this higher, your
> tests start catching all of the high incidence stuff, and then you're left
> with only occasional crashes.
> >
> > And since you have the out of process stuff, it doesn't even bring down
> Xcode anymore.  Just a debugging session.  That's an amazing price to pay
> for having instant visibility into a huge class of bugs that LLDB is
> currently willfully ignoring.
>
>
> I guess I'm speaking at someone who is supporting a debugger used by tens
> of thousands of people.  The debugger crashing is a vastly bigger problem
> to us than bugs that didn't announce themselves by taking down the
> debugger.  lldb_asserts that activate when we're running debug builds are
> fine, that's exactly when I want to see the debugger crash on my desktop or
> when it's running the testsuite.  Asserting when it's on the customer's
> desktop is a no-go.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jason Molenda via lldb-commits]

> On Sep 12, 2017, at 11:19 AM, Zachary Turner  wrote:
> 
> 
> 
> On Tue, Sep 12, 2017 at 11:07 AM Greg Clayton  wrote:
> 
>> On Sep 12, 2017, at 10:10 AM, Zachary Turner  wrote:
>> 
>> 
>> 
>> On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton  wrote:
>>> On Sep 12, 2017, at 9:53 AM, Zachary Turner  wrote:
>>> 
>>> If you had just logged it, the bug would still not be fixed because nobody 
>>> would know about it.  I also can't believe we have to keep saying this :-/
>> 
>> By log, I mean Host::SystemLog(...) which would come out in the command 
>> line. Not "log enable ...". So users would see the issue and report the bug. 
>> Crashing doesn't mean people always report the bug.
>> I mentioned earlier in the thread that I assumed Xcode had an automatic 
>> crash that would handle the crash and automatically upload it to Apple.  Is 
>> this really not the case?  If core dumps are too big, why not just a stack 
>> trace?  Surely the Xcode team must have some kind of internal metrics system 
>> to track stability.
> 
> They do just upload text crash logs. It doesn't tell us what expression 
> triggered the issue though. It shows a crash in an expression, but doesn't 
> show the expression text as this violates privacy. 
> 
> So, you do get a bug report when a crash occurs then.  In contrast to the 
> case where you simply log something, and don't get a crash report.
> 
> In some cases, you can look at the code and figure out why it crashed.  In 
> other cases the bug occurs extremely infrequently (you can build heuristic 
> matching of call-stacks into your infrastructure that processes the crash 
> logs).  If it's a high incidence crasher then you do some investigation.  And 
> the good news is, once you fix it, you've *actually* fixed it.  Now instead 
> of hundreds of thousands of people using something that doesn't work quite 
> right for presumably the rest of the software's life (since nobody knows 
> about the bug), they have something that actually works.
> 
> There's probably some initial pain associated with this approach since the 
> test coverage is so low right now (I came up with about ~25% code coverage in 
> a test I ran a while back).  When you get this higher, your tests start 
> catching all of the high incidence stuff, and then you're left with only 
> occasional crashes.
> 
> And since you have the out of process stuff, it doesn't even bring down Xcode 
> anymore.  Just a debugging session.  That's an amazing price to pay for 
> having instant visibility into a huge class of bugs that LLDB is currently 
> willfully ignoring.


I guess I'm speaking at someone who is supporting a debugger used by tens of 
thousands of people.  The debugger crashing is a vastly bigger problem to us 
than bugs that didn't announce themselves by taking down the debugger.  
lldb_asserts that activate when we're running debug builds are fine, that's 
exactly when I want to see the debugger crash on my desktop or when it's 
running the testsuite.  Asserting when it's on the customer's desktop is a 
no-go.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

Yeah, seems like there's too much heat right now of this to be a useful avenue.

Jim

> On Sep 12, 2017, at 1:35 PM, Zachary Turner  wrote:
> 
> 4ad5334bfcff803f3765e444785b8f9d3a73c683: Don't pass a null StringRef.  
> simple.
> f7b079263a751fdf3adea8e549803aaf92d465f8: Maybe fix it instead, as the 
> comment suggests? 
> f3647763b02ddef65c6244f91939d997f7733ecd: Probably fine since you don't have 
> control over the code of the plugin.
> f67e0b92fbd9e0bc0267ae5210478c85a44b8afc: Probably fine since you're at the 
> mercy of the system
> 0ac65423e2ba99a42b246d191520ff13bdca5cb0: There's no indication here why the 
> TypeSP might be empty.  Is it bad debug info?  If so it should be easy to 
> construct a test.  If the answer is "I don't know", then this doesn't really 
> fix anything.
> 7043340bc58b0d751fcf66001f62cbf0d9527623: Probably fine, debug info is bad.
> d7019a8d5ccd5dfdb79d74312ef449b734627ec3: Seems easy to write a test for.  
> That said, I would argue that the function should return a reference and the 
> person calling the function ensure that we *do* have a TypeSystem for the 
> specified language.  There are plenty of places where the function is already 
> called with the knowledge that a TypeSystem exists.  In those cases, the 
> function absolutely should assert.  By moving the responsibility up and 
> clearly documenting it, you allow
> 
> On Tue, Sep 12, 2017 at 1:23 PM Jim Ingham  wrote:
> Huh, yeah I don't see any of these bugs as making your point.  These are all 
> fixes so I don't see how they argue that we're willfully ignoring anything.  
> To the extent that they deal with asserts they fixes for lldb not remembering 
> that it has to tread carefully to avoid triggering other asserts elsewhere.
> 
> Jim
> 
> > On Sep 12, 2017, at 1:17 PM, Zachary Turner  wrote:
> >
> > (Some of those look like correct fixes btw, since they deal with user input)
> >
> > On Tue, Sep 12, 2017 at 1:16 PM Zachary Turner  wrote:
> > On Tue, Sep 12, 2017 at 1:04 PM Jim Ingham  wrote:
> >
> > I don't see any evidence for lldb suffering from "a huge class of bugs that 
> > we are willfully ignoring..." particularly ones that would be easily caught 
> > if we just had more asserts.  Can you give some examples?
> >
> > Probably all of these, for starters:
> >
> > D:\src\llvm-mono>git log --grep "Don't crash" lldb
> > commit 4ad5334bfcff803f3765e444785b8f9d3a73c683
> > Author: Greg Clayton 
> > Date:   Mon Jul 24 16:47:04 2017 +
> >
> > Don't crash when hostname is empty. StringRef will assert and kill your 
> > program.
> >
> > commit f7b079263a751fdf3adea8e549803aaf92d465f8
> > Author: Sean Callanan 
> > Date:   Fri Aug 26 18:12:39 2016 +
> >
> > Don't crash when trying to capture persistent variables in a block.
> >
> > Reports an error instead.  We can fix this later to make persistent 
> > variables
> > work, but right now we hit an LLVM assertion if we get this wrong.
> >
> > 
> >
> > commit f3647763b02ddef65c6244f91939d997f7733ecd
> > Author: Greg Clayton 
> > Date:   Mon May 16 20:07:38 2016 +
> >
> > Don't crash when OS plug-in returns None from any of the functions we 
> > might call.
> >
> > 
> >
> > commit f67e0b92fbd9e0bc0267ae5210478c85a44b8afc
> > Author: Greg Clayton 
> > Date:   Thu May 12 22:36:47 2016 +
> >
> > Don't crash when a process' task port goes bad.
> >
> > 
> >
> > commit 0ac65423e2ba99a42b246d191520ff13bdca5cb0
> > Author: Greg Clayton 
> > Date:   Tue Mar 15 21:58:28 2016 +
> >
> > Don't crash if the TypeSP is empty.
> >
> > commit 7043340bc58b0d751fcf66001f62cbf0d9527623
> > Author: Greg Clayton 
> > Date:   Fri Feb 12 00:07:16 2016 +
> >
> > Don't crash if we have a DIE that has a DW_AT_ranges attribute and yet 
> > the SymbolFileDWARF doesn't have a DebugRanges. If this happens print a 
> > nice error message to prompt the user to file a bug and attach the 
> > offending DWARF file so we can get the correct compiler fixed.
> >
> > 
> >
> > commit 0d1591d3b74d518b9edd7482f65976092c14e951
> > Author: Greg Clayton 
> > Date:   Wed Oct 28 20:49:34 2015 +
> >
> > Don't crash when opening a fuzzed mach-o file that has bad dyld trie 
> > data.
> >
> > 
> >
> > commit d7019a8d5ccd5dfdb79d74312ef449b734627ec3
> > Author: Greg Clayton 
> > Date:   Fri Aug 14 23:15:48 2015 +
> >
> > Don't crash if we don't have a type system for a language.
> >
> > commit b5838b5a4870ba8f620e7a5733038f02f45b1a78
> > Author: Greg Clayton 
> > Date:   Thu Aug 13 23:16:15 2015 +
> >
> > Don't crash when we have a .a file that contains an object with a 16 
> > character name. Any calls to std::string::erase must be bounds checked.
> 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

Got that part, didn't get the last PP.

Jim

> On Sep 12, 2017, at 1:36 PM, Zachary Turner  wrote:
> 
> 
> 
> On Tue, Sep 12, 2017 at 1:30 PM Jim Ingham  wrote:
> Can you elaborate on this comment.  I must be being dense but it didn't parse 
> for me.
> 
> Jim
> 
> git log --stat shows you the files that were changed as part of the CL.  If I 
> re-run that command with --stat, out of the first 30 CLs, only 1 had a test. 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

On Tue, Sep 12, 2017 at 1:30 PM Jim Ingham  wrote:

> Can you elaborate on this comment.  I must be being dense but it didn't
> parse for me.
>
> Jim
>

git log --stat shows you the files that were changed as part of the CL.  If
I re-run that command with --stat, out of the first 30 CLs, only 1 had a
test.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Greg Clayton via lldb-commits]

The first one is a nice example of what we don't want to crash on. Before the 
switch to StringRef, hostname could be NULL. After it, StringRef handily 
crashed for us instead of dutifully just setting itself to the empty string. I 
say this is exactly the kind of crash we don't want. But thanks to StringRefs 
assertive behavior, this became a crash when it never should have. 

> On Sep 12, 2017, at 1:16 PM, Zachary Turner  wrote:
> 
> 
> 
> On Tue, Sep 12, 2017 at 1:04 PM Jim Ingham  > wrote:
> 
> I don't see any evidence for lldb suffering from "a huge class of bugs that 
> we are willfully ignoring..." particularly ones that would be easily caught 
> if we just had more asserts.  Can you give some examples?
> 
> Probably all of these, for starters:
> 
> D:\src\llvm-mono>git log --grep "Don't crash" lldb
> commit 4ad5334bfcff803f3765e444785b8f9d3a73c683
> Author: Greg Clayton >
> Date:   Mon Jul 24 16:47:04 2017 +
> 
> Don't crash when hostname is empty. StringRef will assert and kill your 
> program.
> 
> commit f7b079263a751fdf3adea8e549803aaf92d465f8
> Author: Sean Callanan >
> Date:   Fri Aug 26 18:12:39 2016 +
> 
> Don't crash when trying to capture persistent variables in a block.
> 
> Reports an error instead.  We can fix this later to make persistent 
> variables
> work, but right now we hit an LLVM assertion if we get this wrong.
> 
> 
> 
> commit f3647763b02ddef65c6244f91939d997f7733ecd
> Author: Greg Clayton >
> Date:   Mon May 16 20:07:38 2016 +
> 
> Don't crash when OS plug-in returns None from any of the functions we 
> might call.
> 
> 
> 
> commit f67e0b92fbd9e0bc0267ae5210478c85a44b8afc
> Author: Greg Clayton >
> Date:   Thu May 12 22:36:47 2016 +
> 
> Don't crash when a process' task port goes bad.
> 
> 
> 
> commit 0ac65423e2ba99a42b246d191520ff13bdca5cb0
> Author: Greg Clayton >
> Date:   Tue Mar 15 21:58:28 2016 +
> 
> Don't crash if the TypeSP is empty.
> 
> commit 7043340bc58b0d751fcf66001f62cbf0d9527623
> Author: Greg Clayton >
> Date:   Fri Feb 12 00:07:16 2016 +
> 
> Don't crash if we have a DIE that has a DW_AT_ranges attribute and yet 
> the SymbolFileDWARF doesn't have a DebugRanges. If this happens print a nice 
> error message to prompt the user to file a bug and attach the offending DWARF 
> file so we can get the correct compiler fixed.
> 
> 
> 
> commit 0d1591d3b74d518b9edd7482f65976092c14e951
> Author: Greg Clayton >
> Date:   Wed Oct 28 20:49:34 2015 +
> 
> Don't crash when opening a fuzzed mach-o file that has bad dyld trie data.
> 
> 
> 
> commit d7019a8d5ccd5dfdb79d74312ef449b734627ec3
> Author: Greg Clayton >
> Date:   Fri Aug 14 23:15:48 2015 +
> 
> Don't crash if we don't have a type system for a language.
> 
> commit b5838b5a4870ba8f620e7a5733038f02f45b1a78
> Author: Greg Clayton >
> Date:   Thu Aug 13 23:16:15 2015 +
> 
> Don't crash when we have a .a file that contains an object with a 16 
> character name. Any calls to std::string::erase must be bounds checked.
> 
> 
> 
> commit 4234fd5de6adb471728df83670ebbe0ae3c7ee68
> Author: Greg Clayton >
> Date:   Tue Aug 11 21:01:32 2015 +
> 
> Don't crash if the file we want to touch doesn't exist.
> 
> commit b385164c734997af6367271d33dc1c4618bfb754
> Author: Greg Clayton >
> Date:   Mon Jul 13 22:08:16 2015 +
> 
> Don't crash if we are unable to get the member type.
> 
> 
> 
> commit a5deec8dc9a9e7fd977049f6fb5977796906e8ca
> Author: Sean Callanan >
> Date:   Thu May 28 20:06:40 2015 +
> 
> Don't crash if we don't have a process and need
> to check for alternate manglings.
> 
> commit 4427526ffa55675b623702452ff6e13c33c79763
> Author: Greg Clayton >
> Date:   Fri May 15 22:31:18 2015 +
> 
> Don't crash if we have bad debug info that has a DW_TAG_inheritance with 
> a bad DW_AT_type reference. Emit an error with instructions to file a bug.
> 
> 
> 
> commit 4506ae8792a5b726133a58d1bc887313bceccd93
> Author: Greg Clayton >
> Date:   Fri May 15 22:20:29 2015 +
> 
> Don't crash if a function has no name by calling 'strcmp(name, "main")'.
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

4ad5334bfcff803f3765e444785b8f9d3a73c683: Don't pass a null StringRef.
 simple.
f7b079263a751fdf3adea8e549803aaf92d465f8: Maybe fix it instead, as the
comment suggests?
f3647763b02ddef65c6244f91939d997f7733ecd: Probably fine since you don't
have control over the code of the plugin.
f67e0b92fbd9e0bc0267ae5210478c85a44b8afc: Probably fine since you're at the
mercy of the system
0ac65423e2ba99a42b246d191520ff13bdca5cb0: There's no indication here why
the TypeSP might be empty.  Is it bad debug info?  If so it should be easy
to construct a test.  If the answer is "I don't know", then this doesn't
really fix anything.
7043340bc58b0d751fcf66001f62cbf0d9527623: Probably fine, debug info is bad.
d7019a8d5ccd5dfdb79d74312ef449b734627ec3: Seems easy to write a test for.
That said, I would argue that the function should return a reference and
the person calling the function ensure that we *do* have a TypeSystem for
the specified language.  There are plenty of places where the function is
already called with the knowledge that a TypeSystem exists.  In those
cases, the function absolutely should assert.  By moving the responsibility
up and clearly documenting it, you allow

On Tue, Sep 12, 2017 at 1:23 PM Jim Ingham  wrote:

> Huh, yeah I don't see any of these bugs as making your point.  These are
> all fixes so I don't see how they argue that we're willfully ignoring
> anything.  To the extent that they deal with asserts they fixes for lldb
> not remembering that it has to tread carefully to avoid triggering other
> asserts elsewhere.
>
> Jim
>
> > On Sep 12, 2017, at 1:17 PM, Zachary Turner  wrote:
> >
> > (Some of those look like correct fixes btw, since they deal with user
> input)
> >
> > On Tue, Sep 12, 2017 at 1:16 PM Zachary Turner 
> wrote:
> > On Tue, Sep 12, 2017 at 1:04 PM Jim Ingham  wrote:
> >
> > I don't see any evidence for lldb suffering from "a huge class of bugs
> that we are willfully ignoring..." particularly ones that would be easily
> caught if we just had more asserts.  Can you give some examples?
> >
> > Probably all of these, for starters:
> >
> > D:\src\llvm-mono>git log --grep "Don't crash" lldb
> > commit 4ad5334bfcff803f3765e444785b8f9d3a73c683
> > Author: Greg Clayton 
> > Date:   Mon Jul 24 16:47:04 2017 +
> >
> > Don't crash when hostname is empty. StringRef will assert and kill
> your program.
> >
> > commit f7b079263a751fdf3adea8e549803aaf92d465f8
> > Author: Sean Callanan 
> > Date:   Fri Aug 26 18:12:39 2016 +
> >
> > Don't crash when trying to capture persistent variables in a block.
> >
> > Reports an error instead.  We can fix this later to make persistent
> variables
> > work, but right now we hit an LLVM assertion if we get this wrong.
> >
> > 
> >
> > commit f3647763b02ddef65c6244f91939d997f7733ecd
> > Author: Greg Clayton 
> > Date:   Mon May 16 20:07:38 2016 +
> >
> > Don't crash when OS plug-in returns None from any of the functions
> we might call.
> >
> > 
> >
> > commit f67e0b92fbd9e0bc0267ae5210478c85a44b8afc
> > Author: Greg Clayton 
> > Date:   Thu May 12 22:36:47 2016 +
> >
> > Don't crash when a process' task port goes bad.
> >
> > 
> >
> > commit 0ac65423e2ba99a42b246d191520ff13bdca5cb0
> > Author: Greg Clayton 
> > Date:   Tue Mar 15 21:58:28 2016 +
> >
> > Don't crash if the TypeSP is empty.
> >
> > commit 7043340bc58b0d751fcf66001f62cbf0d9527623
> > Author: Greg Clayton 
> > Date:   Fri Feb 12 00:07:16 2016 +
> >
> > Don't crash if we have a DIE that has a DW_AT_ranges attribute and
> yet the SymbolFileDWARF doesn't have a DebugRanges. If this happens print a
> nice error message to prompt the user to file a bug and attach the
> offending DWARF file so we can get the correct compiler fixed.
> >
> > 
> >
> > commit 0d1591d3b74d518b9edd7482f65976092c14e951
> > Author: Greg Clayton 
> > Date:   Wed Oct 28 20:49:34 2015 +
> >
> > Don't crash when opening a fuzzed mach-o file that has bad dyld trie
> data.
> >
> > 
> >
> > commit d7019a8d5ccd5dfdb79d74312ef449b734627ec3
> > Author: Greg Clayton 
> > Date:   Fri Aug 14 23:15:48 2015 +
> >
> > Don't crash if we don't have a type system for a language.
> >
> > commit b5838b5a4870ba8f620e7a5733038f02f45b1a78
> > Author: Greg Clayton 
> > Date:   Thu Aug 13 23:16:15 2015 +
> >
> > Don't crash when we have a .a file that contains an object with a 16
> character name. Any calls to std::string::erase must be bounds checked.
> >
> > 
> >
> > commit 4234fd5de6adb471728df83670ebbe0ae3c7ee68
> > Author: Greg Clayton 
> > Date:   Tue Aug 11 21:01:32 2015 +
> >
> > Don't crash if the file we want to touch doesn't exist.
> >
> > commit 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

Can you elaborate on this comment.  I must be being dense but it didn't parse 
for me.

Jim

> On Sep 12, 2017, at 1:21 PM, Zachary Turner  wrote:
> 
> Incidentally, if you add --stat to the command line there, you'll see that 
> only 1 of those CLs has any test coverage at all.  I pressed Enter a few more 
> times and I couldn't find any more tests.
> 
> If you want to look into reducing crashes, that's where you should be 
> looking.  Not at reducing the prevalence of the ONE signal that tells you 
> exactly where test coverage is lacking.
> 
> On Tue, Sep 12, 2017 at 1:17 PM Zachary Turner  wrote:
> (Some of those look like correct fixes btw, since they deal with user input)
> 
> On Tue, Sep 12, 2017 at 1:16 PM Zachary Turner  wrote:
> On Tue, Sep 12, 2017 at 1:04 PM Jim Ingham  wrote:
> 
> I don't see any evidence for lldb suffering from "a huge class of bugs that 
> we are willfully ignoring..." particularly ones that would be easily caught 
> if we just had more asserts.  Can you give some examples?
> 
> Probably all of these, for starters:
> 
> D:\src\llvm-mono>git log --grep "Don't crash" lldb
> commit 4ad5334bfcff803f3765e444785b8f9d3a73c683
> Author: Greg Clayton 
> Date:   Mon Jul 24 16:47:04 2017 +
> 
> Don't crash when hostname is empty. StringRef will assert and kill your 
> program.
> 
> commit f7b079263a751fdf3adea8e549803aaf92d465f8
> Author: Sean Callanan 
> Date:   Fri Aug 26 18:12:39 2016 +
> 
> Don't crash when trying to capture persistent variables in a block.
> 
> Reports an error instead.  We can fix this later to make persistent 
> variables
> work, but right now we hit an LLVM assertion if we get this wrong.
> 
> 
> 
> commit f3647763b02ddef65c6244f91939d997f7733ecd
> Author: Greg Clayton 
> Date:   Mon May 16 20:07:38 2016 +
> 
> Don't crash when OS plug-in returns None from any of the functions we 
> might call.
> 
> 
> 
> commit f67e0b92fbd9e0bc0267ae5210478c85a44b8afc
> Author: Greg Clayton 
> Date:   Thu May 12 22:36:47 2016 +
> 
> Don't crash when a process' task port goes bad.
> 
> 
> 
> commit 0ac65423e2ba99a42b246d191520ff13bdca5cb0
> Author: Greg Clayton 
> Date:   Tue Mar 15 21:58:28 2016 +
> 
> Don't crash if the TypeSP is empty.
> 
> commit 7043340bc58b0d751fcf66001f62cbf0d9527623
> Author: Greg Clayton 
> Date:   Fri Feb 12 00:07:16 2016 +
> 
> Don't crash if we have a DIE that has a DW_AT_ranges attribute and yet 
> the SymbolFileDWARF doesn't have a DebugRanges. If this happens print a nice 
> error message to prompt the user to file a bug and attach the offending DWARF 
> file so we can get the correct compiler fixed.
> 
> 
> 
> commit 0d1591d3b74d518b9edd7482f65976092c14e951
> Author: Greg Clayton 
> Date:   Wed Oct 28 20:49:34 2015 +
> 
> Don't crash when opening a fuzzed mach-o file that has bad dyld trie data.
> 
> 
> 
> commit d7019a8d5ccd5dfdb79d74312ef449b734627ec3
> Author: Greg Clayton 
> Date:   Fri Aug 14 23:15:48 2015 +
> 
> Don't crash if we don't have a type system for a language.
> 
> commit b5838b5a4870ba8f620e7a5733038f02f45b1a78
> Author: Greg Clayton 
> Date:   Thu Aug 13 23:16:15 2015 +
> 
> Don't crash when we have a .a file that contains an object with a 16 
> character name. Any calls to std::string::erase must be bounds checked.
> 
> 
> 
> commit 4234fd5de6adb471728df83670ebbe0ae3c7ee68
> Author: Greg Clayton 
> Date:   Tue Aug 11 21:01:32 2015 +
> 
> Don't crash if the file we want to touch doesn't exist.
> 
> commit b385164c734997af6367271d33dc1c4618bfb754
> Author: Greg Clayton 
> Date:   Mon Jul 13 22:08:16 2015 +
> 
> Don't crash if we are unable to get the member type.
> 
> 
> 
> commit a5deec8dc9a9e7fd977049f6fb5977796906e8ca
> Author: Sean Callanan 
> Date:   Thu May 28 20:06:40 2015 +
> 
> Don't crash if we don't have a process and need
> to check for alternate manglings.
> 
> commit 4427526ffa55675b623702452ff6e13c33c79763
> Author: Greg Clayton 
> Date:   Fri May 15 22:31:18 2015 +
> 
> Don't crash if we have bad debug info that has a DW_TAG_inheritance with 
> a bad DW_AT_type reference. Emit an error with instructions to file a bug.
> 
> 
> 
> commit 4506ae8792a5b726133a58d1bc887313bceccd93
> Author: Greg Clayton 
> Date:   Fri May 15 22:20:29 2015 +
> 
> Don't crash if a function has no name by calling 'strcmp(name, "main")'.
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

Huh, yeah I don't see any of these bugs as making your point.  These are all 
fixes so I don't see how they argue that we're willfully ignoring anything.  To 
the extent that they deal with asserts they fixes for lldb not remembering that 
it has to tread carefully to avoid triggering other asserts elsewhere.

Jim

> On Sep 12, 2017, at 1:17 PM, Zachary Turner  wrote:
> 
> (Some of those look like correct fixes btw, since they deal with user input)
> 
> On Tue, Sep 12, 2017 at 1:16 PM Zachary Turner  wrote:
> On Tue, Sep 12, 2017 at 1:04 PM Jim Ingham  wrote:
> 
> I don't see any evidence for lldb suffering from "a huge class of bugs that 
> we are willfully ignoring..." particularly ones that would be easily caught 
> if we just had more asserts.  Can you give some examples?
> 
> Probably all of these, for starters:
> 
> D:\src\llvm-mono>git log --grep "Don't crash" lldb
> commit 4ad5334bfcff803f3765e444785b8f9d3a73c683
> Author: Greg Clayton 
> Date:   Mon Jul 24 16:47:04 2017 +
> 
> Don't crash when hostname is empty. StringRef will assert and kill your 
> program.
> 
> commit f7b079263a751fdf3adea8e549803aaf92d465f8
> Author: Sean Callanan 
> Date:   Fri Aug 26 18:12:39 2016 +
> 
> Don't crash when trying to capture persistent variables in a block.
> 
> Reports an error instead.  We can fix this later to make persistent 
> variables
> work, but right now we hit an LLVM assertion if we get this wrong.
> 
> 
> 
> commit f3647763b02ddef65c6244f91939d997f7733ecd
> Author: Greg Clayton 
> Date:   Mon May 16 20:07:38 2016 +
> 
> Don't crash when OS plug-in returns None from any of the functions we 
> might call.
> 
> 
> 
> commit f67e0b92fbd9e0bc0267ae5210478c85a44b8afc
> Author: Greg Clayton 
> Date:   Thu May 12 22:36:47 2016 +
> 
> Don't crash when a process' task port goes bad.
> 
> 
> 
> commit 0ac65423e2ba99a42b246d191520ff13bdca5cb0
> Author: Greg Clayton 
> Date:   Tue Mar 15 21:58:28 2016 +
> 
> Don't crash if the TypeSP is empty.
> 
> commit 7043340bc58b0d751fcf66001f62cbf0d9527623
> Author: Greg Clayton 
> Date:   Fri Feb 12 00:07:16 2016 +
> 
> Don't crash if we have a DIE that has a DW_AT_ranges attribute and yet 
> the SymbolFileDWARF doesn't have a DebugRanges. If this happens print a nice 
> error message to prompt the user to file a bug and attach the offending DWARF 
> file so we can get the correct compiler fixed.
> 
> 
> 
> commit 0d1591d3b74d518b9edd7482f65976092c14e951
> Author: Greg Clayton 
> Date:   Wed Oct 28 20:49:34 2015 +
> 
> Don't crash when opening a fuzzed mach-o file that has bad dyld trie data.
> 
> 
> 
> commit d7019a8d5ccd5dfdb79d74312ef449b734627ec3
> Author: Greg Clayton 
> Date:   Fri Aug 14 23:15:48 2015 +
> 
> Don't crash if we don't have a type system for a language.
> 
> commit b5838b5a4870ba8f620e7a5733038f02f45b1a78
> Author: Greg Clayton 
> Date:   Thu Aug 13 23:16:15 2015 +
> 
> Don't crash when we have a .a file that contains an object with a 16 
> character name. Any calls to std::string::erase must be bounds checked.
> 
> 
> 
> commit 4234fd5de6adb471728df83670ebbe0ae3c7ee68
> Author: Greg Clayton 
> Date:   Tue Aug 11 21:01:32 2015 +
> 
> Don't crash if the file we want to touch doesn't exist.
> 
> commit b385164c734997af6367271d33dc1c4618bfb754
> Author: Greg Clayton 
> Date:   Mon Jul 13 22:08:16 2015 +
> 
> Don't crash if we are unable to get the member type.
> 
> 
> 
> commit a5deec8dc9a9e7fd977049f6fb5977796906e8ca
> Author: Sean Callanan 
> Date:   Thu May 28 20:06:40 2015 +
> 
> Don't crash if we don't have a process and need
> to check for alternate manglings.
> 
> commit 4427526ffa55675b623702452ff6e13c33c79763
> Author: Greg Clayton 
> Date:   Fri May 15 22:31:18 2015 +
> 
> Don't crash if we have bad debug info that has a DW_TAG_inheritance with 
> a bad DW_AT_type reference. Emit an error with instructions to file a bug.
> 
> 
> 
> commit 4506ae8792a5b726133a58d1bc887313bceccd93
> Author: Greg Clayton 
> Date:   Fri May 15 22:20:29 2015 +
> 
> Don't crash if a function has no name by calling 'strcmp(name, "main")'.
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

Incidentally, if you add --stat to the command line there, you'll see that
only 1 of those CLs has any test coverage at all.  I pressed Enter a few
more times and I couldn't find any more tests.

If you want to look into reducing crashes, that's where you should be
looking.  Not at reducing the prevalence of the ONE signal that tells you
exactly where test coverage is lacking.

On Tue, Sep 12, 2017 at 1:17 PM Zachary Turner  wrote:

> (Some of those look like correct fixes btw, since they deal with user
> input)
>
> On Tue, Sep 12, 2017 at 1:16 PM Zachary Turner  wrote:
>
>> On Tue, Sep 12, 2017 at 1:04 PM Jim Ingham  wrote:
>>
>>>
>>> I don't see any evidence for lldb suffering from "a huge class of bugs
>>> that we are willfully ignoring..." particularly ones that would be easily
>>> caught if we just had more asserts.  Can you give some examples?
>>>
>>
>> Probably all of these, for starters:
>>
>> D:\src\llvm-mono>git log --grep "Don't crash" lldb
>> commit 4ad5334bfcff803f3765e444785b8f9d3a73c683
>> Author: Greg Clayton 
>> Date:   Mon Jul 24 16:47:04 2017 +
>>
>> Don't crash when hostname is empty. StringRef will assert and kill
>> your program.
>>
>> commit f7b079263a751fdf3adea8e549803aaf92d465f8
>> Author: Sean Callanan 
>> Date:   Fri Aug 26 18:12:39 2016 +
>>
>> Don't crash when trying to capture persistent variables in a block.
>>
>> Reports an error instead.  We can fix this later to make persistent
>> variables
>> work, but right now we hit an LLVM assertion if we get this wrong.
>>
>> 
>>
>> commit f3647763b02ddef65c6244f91939d997f7733ecd
>> Author: Greg Clayton 
>> Date:   Mon May 16 20:07:38 2016 +
>>
>> Don't crash when OS plug-in returns None from any of the functions we
>> might call.
>>
>> 
>>
>> commit f67e0b92fbd9e0bc0267ae5210478c85a44b8afc
>> Author: Greg Clayton 
>> Date:   Thu May 12 22:36:47 2016 +
>>
>> Don't crash when a process' task port goes bad.
>>
>> 
>>
>> commit 0ac65423e2ba99a42b246d191520ff13bdca5cb0
>> Author: Greg Clayton 
>> Date:   Tue Mar 15 21:58:28 2016 +
>>
>> Don't crash if the TypeSP is empty.
>>
>> commit 7043340bc58b0d751fcf66001f62cbf0d9527623
>> Author: Greg Clayton 
>> Date:   Fri Feb 12 00:07:16 2016 +
>>
>> Don't crash if we have a DIE that has a DW_AT_ranges attribute and
>> yet the SymbolFileDWARF doesn't have a DebugRanges. If this happens print a
>> nice error message to prompt the user to file a bug and attach the
>> offending DWARF file so we can get the correct compiler fixed.
>>
>> 
>>
>> commit 0d1591d3b74d518b9edd7482f65976092c14e951
>> Author: Greg Clayton 
>> Date:   Wed Oct 28 20:49:34 2015 +
>>
>> Don't crash when opening a fuzzed mach-o file that has bad dyld trie
>> data.
>>
>> 
>>
>> commit d7019a8d5ccd5dfdb79d74312ef449b734627ec3
>> Author: Greg Clayton 
>> Date:   Fri Aug 14 23:15:48 2015 +
>>
>> Don't crash if we don't have a type system for a language.
>>
>> commit b5838b5a4870ba8f620e7a5733038f02f45b1a78
>> Author: Greg Clayton 
>> Date:   Thu Aug 13 23:16:15 2015 +
>>
>> Don't crash when we have a .a file that contains an object with a 16
>> character name. Any calls to std::string::erase must be bounds checked.
>>
>> 
>>
>> commit 4234fd5de6adb471728df83670ebbe0ae3c7ee68
>> Author: Greg Clayton 
>> Date:   Tue Aug 11 21:01:32 2015 +
>>
>> Don't crash if the file we want to touch doesn't exist.
>>
>> commit b385164c734997af6367271d33dc1c4618bfb754
>> Author: Greg Clayton 
>> Date:   Mon Jul 13 22:08:16 2015 +
>>
>> Don't crash if we are unable to get the member type.
>>
>> 
>>
>> commit a5deec8dc9a9e7fd977049f6fb5977796906e8ca
>> Author: Sean Callanan 
>> Date:   Thu May 28 20:06:40 2015 +
>>
>> Don't crash if we don't have a process and need
>> to check for alternate manglings.
>>
>> commit 4427526ffa55675b623702452ff6e13c33c79763
>> Author: Greg Clayton 
>> Date:   Fri May 15 22:31:18 2015 +
>>
>> Don't crash if we have bad debug info that has a DW_TAG_inheritance
>> with a bad DW_AT_type reference. Emit an error with instructions to file a
>> bug.
>>
>> 
>>
>> commit 4506ae8792a5b726133a58d1bc887313bceccd93
>> Author: Greg Clayton 
>> Date:   Fri May 15 22:20:29 2015 +
>>
>> Don't crash if a function has no name by calling 'strcmp(name,
>> "main")'.
>>
>> 
>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

(Some of those look like correct fixes btw, since they deal with user input)

On Tue, Sep 12, 2017 at 1:16 PM Zachary Turner  wrote:

> On Tue, Sep 12, 2017 at 1:04 PM Jim Ingham  wrote:
>
>>
>> I don't see any evidence for lldb suffering from "a huge class of bugs
>> that we are willfully ignoring..." particularly ones that would be easily
>> caught if we just had more asserts.  Can you give some examples?
>>
>
> Probably all of these, for starters:
>
> D:\src\llvm-mono>git log --grep "Don't crash" lldb
> commit 4ad5334bfcff803f3765e444785b8f9d3a73c683
> Author: Greg Clayton 
> Date:   Mon Jul 24 16:47:04 2017 +
>
> Don't crash when hostname is empty. StringRef will assert and kill
> your program.
>
> commit f7b079263a751fdf3adea8e549803aaf92d465f8
> Author: Sean Callanan 
> Date:   Fri Aug 26 18:12:39 2016 +
>
> Don't crash when trying to capture persistent variables in a block.
>
> Reports an error instead.  We can fix this later to make persistent
> variables
> work, but right now we hit an LLVM assertion if we get this wrong.
>
> 
>
> commit f3647763b02ddef65c6244f91939d997f7733ecd
> Author: Greg Clayton 
> Date:   Mon May 16 20:07:38 2016 +
>
> Don't crash when OS plug-in returns None from any of the functions we
> might call.
>
> 
>
> commit f67e0b92fbd9e0bc0267ae5210478c85a44b8afc
> Author: Greg Clayton 
> Date:   Thu May 12 22:36:47 2016 +
>
> Don't crash when a process' task port goes bad.
>
> 
>
> commit 0ac65423e2ba99a42b246d191520ff13bdca5cb0
> Author: Greg Clayton 
> Date:   Tue Mar 15 21:58:28 2016 +
>
> Don't crash if the TypeSP is empty.
>
> commit 7043340bc58b0d751fcf66001f62cbf0d9527623
> Author: Greg Clayton 
> Date:   Fri Feb 12 00:07:16 2016 +
>
> Don't crash if we have a DIE that has a DW_AT_ranges attribute and yet
> the SymbolFileDWARF doesn't have a DebugRanges. If this happens print a
> nice error message to prompt the user to file a bug and attach the
> offending DWARF file so we can get the correct compiler fixed.
>
> 
>
> commit 0d1591d3b74d518b9edd7482f65976092c14e951
> Author: Greg Clayton 
> Date:   Wed Oct 28 20:49:34 2015 +
>
> Don't crash when opening a fuzzed mach-o file that has bad dyld trie
> data.
>
> 
>
> commit d7019a8d5ccd5dfdb79d74312ef449b734627ec3
> Author: Greg Clayton 
> Date:   Fri Aug 14 23:15:48 2015 +
>
> Don't crash if we don't have a type system for a language.
>
> commit b5838b5a4870ba8f620e7a5733038f02f45b1a78
> Author: Greg Clayton 
> Date:   Thu Aug 13 23:16:15 2015 +
>
> Don't crash when we have a .a file that contains an object with a 16
> character name. Any calls to std::string::erase must be bounds checked.
>
> 
>
> commit 4234fd5de6adb471728df83670ebbe0ae3c7ee68
> Author: Greg Clayton 
> Date:   Tue Aug 11 21:01:32 2015 +
>
> Don't crash if the file we want to touch doesn't exist.
>
> commit b385164c734997af6367271d33dc1c4618bfb754
> Author: Greg Clayton 
> Date:   Mon Jul 13 22:08:16 2015 +
>
> Don't crash if we are unable to get the member type.
>
> 
>
> commit a5deec8dc9a9e7fd977049f6fb5977796906e8ca
> Author: Sean Callanan 
> Date:   Thu May 28 20:06:40 2015 +
>
> Don't crash if we don't have a process and need
> to check for alternate manglings.
>
> commit 4427526ffa55675b623702452ff6e13c33c79763
> Author: Greg Clayton 
> Date:   Fri May 15 22:31:18 2015 +
>
> Don't crash if we have bad debug info that has a DW_TAG_inheritance
> with a bad DW_AT_type reference. Emit an error with instructions to file a
> bug.
>
> 
>
> commit 4506ae8792a5b726133a58d1bc887313bceccd93
> Author: Greg Clayton 
> Date:   Fri May 15 22:20:29 2015 +
>
> Don't crash if a function has no name by calling 'strcmp(name,
> "main")'.
>
> 
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

On Tue, Sep 12, 2017 at 1:04 PM Jim Ingham  wrote:

>
> I don't see any evidence for lldb suffering from "a huge class of bugs
> that we are willfully ignoring..." particularly ones that would be easily
> caught if we just had more asserts.  Can you give some examples?
>

Probably all of these, for starters:

D:\src\llvm-mono>git log --grep "Don't crash" lldb
commit 4ad5334bfcff803f3765e444785b8f9d3a73c683
Author: Greg Clayton 
Date:   Mon Jul 24 16:47:04 2017 +

Don't crash when hostname is empty. StringRef will assert and kill your
program.

commit f7b079263a751fdf3adea8e549803aaf92d465f8
Author: Sean Callanan 
Date:   Fri Aug 26 18:12:39 2016 +

Don't crash when trying to capture persistent variables in a block.

Reports an error instead.  We can fix this later to make persistent
variables
work, but right now we hit an LLVM assertion if we get this wrong.



commit f3647763b02ddef65c6244f91939d997f7733ecd
Author: Greg Clayton 
Date:   Mon May 16 20:07:38 2016 +

Don't crash when OS plug-in returns None from any of the functions we
might call.



commit f67e0b92fbd9e0bc0267ae5210478c85a44b8afc
Author: Greg Clayton 
Date:   Thu May 12 22:36:47 2016 +

Don't crash when a process' task port goes bad.



commit 0ac65423e2ba99a42b246d191520ff13bdca5cb0
Author: Greg Clayton 
Date:   Tue Mar 15 21:58:28 2016 +

Don't crash if the TypeSP is empty.

commit 7043340bc58b0d751fcf66001f62cbf0d9527623
Author: Greg Clayton 
Date:   Fri Feb 12 00:07:16 2016 +

Don't crash if we have a DIE that has a DW_AT_ranges attribute and yet
the SymbolFileDWARF doesn't have a DebugRanges. If this happens print a
nice error message to prompt the user to file a bug and attach the
offending DWARF file so we can get the correct compiler fixed.



commit 0d1591d3b74d518b9edd7482f65976092c14e951
Author: Greg Clayton 
Date:   Wed Oct 28 20:49:34 2015 +

Don't crash when opening a fuzzed mach-o file that has bad dyld trie
data.



commit d7019a8d5ccd5dfdb79d74312ef449b734627ec3
Author: Greg Clayton 
Date:   Fri Aug 14 23:15:48 2015 +

Don't crash if we don't have a type system for a language.

commit b5838b5a4870ba8f620e7a5733038f02f45b1a78
Author: Greg Clayton 
Date:   Thu Aug 13 23:16:15 2015 +

Don't crash when we have a .a file that contains an object with a 16
character name. Any calls to std::string::erase must be bounds checked.



commit 4234fd5de6adb471728df83670ebbe0ae3c7ee68
Author: Greg Clayton 
Date:   Tue Aug 11 21:01:32 2015 +

Don't crash if the file we want to touch doesn't exist.

commit b385164c734997af6367271d33dc1c4618bfb754
Author: Greg Clayton 
Date:   Mon Jul 13 22:08:16 2015 +

Don't crash if we are unable to get the member type.



commit a5deec8dc9a9e7fd977049f6fb5977796906e8ca
Author: Sean Callanan 
Date:   Thu May 28 20:06:40 2015 +

Don't crash if we don't have a process and need
to check for alternate manglings.

commit 4427526ffa55675b623702452ff6e13c33c79763
Author: Greg Clayton 
Date:   Fri May 15 22:31:18 2015 +

Don't crash if we have bad debug info that has a DW_TAG_inheritance
with a bad DW_AT_type reference. Emit an error with instructions to file a
bug.



commit 4506ae8792a5b726133a58d1bc887313bceccd93
Author: Greg Clayton 
Date:   Fri May 15 22:20:29 2015 +

Don't crash if a function has no name by calling 'strcmp(name, "main")'.


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

> On Sep 12, 2017, at 11:19 AM, Zachary Turner via lldb-commits 
>  wrote:
> 
> 
> 
> On Tue, Sep 12, 2017 at 11:07 AM Greg Clayton  wrote:
> 
>> On Sep 12, 2017, at 10:10 AM, Zachary Turner  wrote:
>> 
>> 
>> 
>> On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton  wrote:
>>> On Sep 12, 2017, at 9:53 AM, Zachary Turner  wrote:
>>> 
>>> If you had just logged it, the bug would still not be fixed because nobody 
>>> would know about it.  I also can't believe we have to keep saying this :-/
>> 
>> By log, I mean Host::SystemLog(...) which would come out in the command 
>> line. Not "log enable ...". So users would see the issue and report the bug. 
>> Crashing doesn't mean people always report the bug.
>> I mentioned earlier in the thread that I assumed Xcode had an automatic 
>> crash that would handle the crash and automatically upload it to Apple.  Is 
>> this really not the case?  If core dumps are too big, why not just a stack 
>> trace?  Surely the Xcode team must have some kind of internal metrics system 
>> to track stability.
> 
> They do just upload text crash logs. It doesn't tell us what expression 
> triggered the issue though. It shows a crash in an expression, but doesn't 
> show the expression text as this violates privacy. 
> 
> So, you do get a bug report when a crash occurs then.  In contrast to the 
> case where you simply log something, and don't get a crash report.
> 
> In some cases, you can look at the code and figure out why it crashed.  In 
> other cases the bug occurs extremely infrequently (you can build heuristic 
> matching of call-stacks into your infrastructure that processes the crash 
> logs).  If it's a high incidence crasher then you do some investigation.  And 
> the good news is, once you fix it, you've *actually* fixed it.  Now instead 
> of hundreds of thousands of people using something that doesn't work quite 
> right for presumably the rest of the software's life (since nobody knows 
> about the bug), they have something that actually works.
> 
> There's probably some initial pain associated with this approach since the 
> test coverage is so low right now (I came up with about ~25% code coverage in 
> a test I ran a while back).  When you get this higher, your tests start 
> catching all of the high incidence stuff, and then you're left with only 
> occasional crashes.
> 
> And since you have the out of process stuff, it doesn't even bring down Xcode 
> anymore.  Just a debugging session.  That's an amazing price to pay for 
> having instant visibility into a huge class of bugs that LLDB is currently 
> willfully ignoring.

I don't see any evidence for lldb suffering from "a huge class of bugs that we 
are willfully ignoring..." particularly ones that would be easily caught if we 
just had more asserts.  Can you give some examples?

From what I see (and I see all the bugs for lldb that come through the Apple 
reporting system which is where most of the people who use lldb on 
macOS/iOS/etc file them so I see lots of lldb bugs) we have a few big systemic 
issues.  The process event handling is racy somewhere - I've found a few places 
where this is true but not all yet.And the synthetic child provider has 
memory management issue (though this seems to mostly effect Swift so not such 
an issue here.)  I'm not sure either of these would have been easier to find if 
we had a pervasive assert-based architecture.  Most of the other bugs are 
behavior problems - e.g. the name lookup in the expression parser doesn't 
mirror the order of lookup that would have happened in source code, so we find 
type names when we should find local variable names first, etc.  There are also 
not the sort of thing you can express in asserts.  They are the sort of thing 
that can be checked by testing, and when we fix bugs we always add test cases, 
so that coverage will increase over time.

Jim


> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

On Tue, Sep 12, 2017 at 11:07 AM Greg Clayton  wrote:

>
> On Sep 12, 2017, at 10:10 AM, Zachary Turner  wrote:
>
>
>
> On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton  wrote:
>
>> On Sep 12, 2017, at 9:53 AM, Zachary Turner  wrote:
>>
>> If you had just logged it, the bug would still not be fixed because
>> nobody would know about it.  I also can't believe we have to keep saying
>> this :-/
>>
>>
>> By log, I mean Host::SystemLog(...) which would come out in the command
>> line. Not "log enable ...". So users would see the issue and report the
>> bug. Crashing doesn't mean people always report the bug.
>>
> I mentioned earlier in the thread that I assumed Xcode had an automatic
> crash that would handle the crash and automatically upload it to Apple.  Is
> this really not the case?  If core dumps are too big, why not just a stack
> trace?  Surely the Xcode team must have some kind of internal metrics
> system to track stability.
>
>
> They do just upload text crash logs. It doesn't tell us what expression
> triggered the issue though. It shows a crash in an expression, but doesn't
> show the expression text as this violates privacy.
>

So, you do get a bug report when a crash occurs then.  In contrast to the
case where you simply log something, and don't get a crash report.

In some cases, you can look at the code and figure out why it crashed.  In
other cases the bug occurs extremely infrequently (you can build heuristic
matching of call-stacks into your infrastructure that processes the crash
logs).  If it's a high incidence crasher then you do some investigation.
And the good news is, once you fix it, you've *actually* fixed it.  Now
instead of hundreds of thousands of people using something that doesn't
work quite right for presumably the rest of the software's life (since
nobody knows about the bug), they have something that actually works.

There's probably some initial pain associated with this approach since the
test coverage is so low right now (I came up with about ~25% code coverage
in a test I ran a while back).  When you get this higher, your tests start
catching all of the high incidence stuff, and then you're left with only
occasional crashes.

And since you have the out of process stuff, it doesn't even bring down
Xcode anymore.  Just a debugging session.  That's an amazing price to pay
for having instant visibility into a huge class of bugs that LLDB is
currently willfully ignoring.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Greg Clayton via lldb-commits]

> On Sep 12, 2017, at 10:10 AM, Zachary Turner  wrote:
> 
> 
> 
> On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton  > wrote:
>> On Sep 12, 2017, at 9:53 AM, Zachary Turner > > wrote:
>> 
>> If you had just logged it, the bug would still not be fixed because nobody 
>> would know about it.  I also can't believe we have to keep saying this :-/
> 
> By log, I mean Host::SystemLog(...) which would come out in the command line. 
> Not "log enable ...". So users would see the issue and report the bug. 
> Crashing doesn't mean people always report the bug.
> I mentioned earlier in the thread that I assumed Xcode had an automatic crash 
> that would handle the crash and automatically upload it to Apple.  Is this 
> really not the case?  If core dumps are too big, why not just a stack trace?  
> Surely the Xcode team must have some kind of internal metrics system to track 
> stability.

They do just upload text crash logs. It doesn't tell us what expression 
triggered the issue though. It shows a crash in an expression, but doesn't show 
the expression text as this violates privacy. ___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Greg Clayton via lldb-commits]

> On Sep 12, 2017, at 10:10 AM, Zachary Turner  > wrote:
> 
> 
> 
> On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton  > wrote:
>> On Sep 12, 2017, at 9:53 AM, Zachary Turner > > wrote:
>> 
>> If you had just logged it, the bug would still not be fixed because nobody 
>> would know about it.  I also can't believe we have to keep saying this :-/
> 
> By log, I mean Host::SystemLog(...) which would come out in the command line. 
> Not "log enable ...". So users would see the issue and report the bug. 
> Crashing doesn't mean people always report the bug.
> I mentioned earlier in the thread that I assumed Xcode had an automatic crash 
> that would handle the crash and automatically upload it to Apple.  Is this 
> really not the case?  If core dumps are too big, why not just a stack trace?  
> Surely the Xcode team must have some kind of internal metrics system to track 
> stability.

They do just upload text crash logs. It doesn't tell us what expression 
triggered the issue though. It shows a crash in an expression, but doesn't show 
the expression text as this violates privacy. ___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

On Tue, Sep 12, 2017 at 10:03 AM Greg Clayton  wrote:

> On Sep 12, 2017, at 9:53 AM, Zachary Turner  wrote:
>
> If you had just logged it, the bug would still not be fixed because nobody
> would know about it.  I also can't believe we have to keep saying this :-/
>
>
> By log, I mean Host::SystemLog(...) which would come out in the command
> line. Not "log enable ...". So users would see the issue and report the
> bug. Crashing doesn't mean people always report the bug.
>
I mentioned earlier in the thread that I assumed Xcode had an automatic
crash that would handle the crash and automatically upload it to Apple.  Is
this really not the case?  If core dumps are too big, why not just a stack
trace?  Surely the Xcode team must have some kind of internal metrics
system to track stability.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

And for the record, I *do* get where your concern comes from.  It comes
from the fact that even if you crash, as I'm suggesting, you still won't
know about it until it's too late (i.e. in the wild).

That of course goes back to my constant hounding on having more tests.  If
you have more tests, you would catch it much earlier.

"Hide bugs and try to pretend they didn't happen" is *very obviously* worse
than "catch bugs immediately and fix them".

What that means is that someone might have to spend months, or even years,
doing nothing but re-designing things to be more testable, and re-designing
the test infrastructure to make the overhead of writing a test much lower.

Alternatively, do nothing, and continue having thousands of latent bugs
that annoy users and lead to death by 1000 cuts.  Which is, as I said
earlier, an existential threat to the project.

On Tue, Sep 12, 2017 at 9:53 AM Zachary Turner  wrote:

> If you had just logged it, the bug would still not be fixed because nobody
> would know about it.  I also can't believe we have to keep saying this :-/
>
> On Tue, Sep 12, 2017 at 9:50 AM Greg Clayton  wrote:
>
>> Success? No. Log something. Return an error. Anything but crashing.
>> Crashing is not acceptable. I can't believe we have to keep saying this.
>>
>> On Sep 11, 2017, at 4:39 PM, Zachary Turner via lldb-commits <
>> lldb-commits@lists.llvm.org> wrote:
>>
>>
>>
>> On Mon, Sep 11, 2017 at 4:22 PM Jason Molenda via lldb-commits <
>> lldb-commits@lists.llvm.org> wrote:
>>
>>> fwiw the reason the JIT came up is because we had an instance where the
>>> older MCJIT wasn't handling a relocation in thumb code about six weeks ago
>>> and we only caught the crash a couple days before we released a beta of
>>> it.  It definitely can happen with MCJIT.  I think with ORC JIT this is a
>>> not going to be a problem -- but it's a good example of a class of problem
>>> where the subsystem (jit) considers the failure catastrophic, whereas the
>>> user will find another way to do their work.  When it takes the developer
>>> an hour to get to the point of failure, they try to print a variable, lldb
>>> ingests a ton of debug info and then we crash because some little detail
>>> was not valid, or they try to run an expression and the debugger crashes
>>> with an unsupported relocation, I can't overstate what an enormous failure
>>> of the debugger that is.
>>>
>>
>> I disagree.  It sounds like a success.  As a result of it crashing six
>> weeks ago, you learned the bug exists, and now Lang has fixed it.
>>
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

If you had just logged it, the bug would still not be fixed because nobody
would know about it.  I also can't believe we have to keep saying this :-/

On Tue, Sep 12, 2017 at 9:50 AM Greg Clayton  wrote:

> Success? No. Log something. Return an error. Anything but crashing.
> Crashing is not acceptable. I can't believe we have to keep saying this.
>
> On Sep 11, 2017, at 4:39 PM, Zachary Turner via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>
>
> On Mon, Sep 11, 2017 at 4:22 PM Jason Molenda via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>> fwiw the reason the JIT came up is because we had an instance where the
>> older MCJIT wasn't handling a relocation in thumb code about six weeks ago
>> and we only caught the crash a couple days before we released a beta of
>> it.  It definitely can happen with MCJIT.  I think with ORC JIT this is a
>> not going to be a problem -- but it's a good example of a class of problem
>> where the subsystem (jit) considers the failure catastrophic, whereas the
>> user will find another way to do their work.  When it takes the developer
>> an hour to get to the point of failure, they try to print a variable, lldb
>> ingests a ton of debug info and then we crash because some little detail
>> was not valid, or they try to run an expression and the debugger crashes
>> with an unsupported relocation, I can't overstate what an enormous failure
>> of the debugger that is.
>>
>
> I disagree.  It sounds like a success.  As a result of it crashing six
> weeks ago, you learned the bug exists, and now Lang has fixed it.
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Greg Clayton via lldb-commits]

Success? No. Log something. Return an error. Anything but crashing. Crashing is 
not acceptable. I can't believe we have to keep saying this. 

> On Sep 11, 2017, at 4:39 PM, Zachary Turner via lldb-commits 
>  wrote:
> 
> 
> 
> On Mon, Sep 11, 2017 at 4:22 PM Jason Molenda via lldb-commits 
> > wrote:
> fwiw the reason the JIT came up is because we had an instance where the older 
> MCJIT wasn't handling a relocation in thumb code about six weeks ago and we 
> only caught the crash a couple days before we released a beta of it.  It 
> definitely can happen with MCJIT.  I think with ORC JIT this is a not going 
> to be a problem -- but it's a good example of a class of problem where the 
> subsystem (jit) considers the failure catastrophic, whereas the user will 
> find another way to do their work.  When it takes the developer an hour to 
> get to the point of failure, they try to print a variable, lldb ingests a ton 
> of debug info and then we crash because some little detail was not valid, or 
> they try to run an expression and the debugger crashes with an unsupported 
> relocation, I can't overstate what an enormous failure of the debugger that 
> is.
>  
> I disagree.  It sounds like a success.  As a result of it crashing six weeks 
> ago, you learned the bug exists, and now Lang has fixed it.
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

On Mon, Sep 11, 2017 at 4:22 PM Jason Molenda via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> fwiw the reason the JIT came up is because we had an instance where the
> older MCJIT wasn't handling a relocation in thumb code about six weeks ago
> and we only caught the crash a couple days before we released a beta of
> it.  It definitely can happen with MCJIT.  I think with ORC JIT this is a
> not going to be a problem -- but it's a good example of a class of problem
> where the subsystem (jit) considers the failure catastrophic, whereas the
> user will find another way to do their work.  When it takes the developer
> an hour to get to the point of failure, they try to print a variable, lldb
> ingests a ton of debug info and then we crash because some little detail
> was not valid, or they try to run an expression and the debugger crashes
> with an unsupported relocation, I can't overstate what an enormous failure
> of the debugger that is.
>

I disagree.  It sounds like a success.  As a result of it crashing six
weeks ago, you learned the bug exists, and now Lang has fixed it.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

> On Sep 11, 2017, at 4:05 PM, Leonard Mosescu via lldb-commits 
>  wrote:
> 
> 
> Process already has "Error Process::WillResume()" for this very reason. Just 
> have the Windows mini dump core file plug-in override it. The code in 
> Process::Resume already checks this:
> Status Process::PrivateResume() {
>   Status error(WillResume());
>   // Tell the process it is about to resume before the thread list
>   if (error.Success()) {
>   ...
> }
>   }
>   return error;
> }
> Problem solved no???
> 
> It would be great, but that also happens too late (by the time 
> PrivateResume() is called the state has been already changed)
> 
> lldb command interruption requires two parts, one is the rollback, and the 
> other is to have someone listening for interruption.  To give an instance 
> that works in lldb today, if you run the "expr" command, while we are in the 
> process of compiling there's no way to interrupt "expr" because clang doesn't 
> listen for interrupts.  But once the expression is running, we're back in the 
> lldb event loop and so we can handle interruption.  And of course if you 
> interrupt an expression while running it knows how to clean up after itself.  
> That wasn't done for interruption primarily, rather "expr" already had to 
> know that to be able to clean up from expressions that crashed or raised 
> signals while running.
> 
> I don't think we need a general "interrupt me anywhere you want" mechanism to 
> support command interruption however.  Most of the time-consuming work in 
> lldb comes in self-contained chunks (reading in symbols from shared 
> libraries, reading in the debug info index tables, etc) or happens while lldb 
> is actually waiting for something to happen (that part already is 
> interruptible at a low level).  We try to do work as lazily as possible, so 
> if the chunk boundaries and the lazy work boundaries line up you won't really 
> need to roll-back at all.   The model where you check for interrupts after 
> each chunk of work is done will I think be responsive enough for user's 
> needs, and also greatly simplify the work to handle interruption.  This is a 
> little more manual, since you have to insert appropriate "yields", but I 
> think will be good enough for our purposes and much easier to make stable.
>  
> @Jim: I think we're thinking along the same lines, something intended for the 
> poor soul who's does something like "target modules dump symtab", rather than 
> the ability to interrupt at any arbitrary point (which as you point out it's 
> not really feasible).

Exactly.  The ^C in the driver is currently wired up to 
SBDebugger::DispatchInputInterrupt().  All that does at present is see if any 
targets under the debugger have running processes, and interrupt them.  That 
could be extended to do more general interrupting.  But I haven't taken any 
time to think of a good strategy for implementing that.
 
Jim


>  
> 
> On Mon, Sep 11, 2017 at 3:43 PM, Greg Clayton via lldb-commits 
>  wrote:
> 
>> On Sep 11, 2017, at 3:37 PM, Zachary Turner  wrote:
>> 
>> On Mon, Sep 11, 2017 at 3:31 PM Greg Clayton via lldb-commits 
>>  wrote:
>> I know there are two points of view here so I will give my two cents:
>> 
>> If it comes down to something as easy as "always check for NULL before doing 
>> something", or something that is similar and very easy, I would say just 
>> code around it. Don't "assert(...)" or "llvm_unreachable(...)". Things that 
>> fall into this category: switch statements with defaults that might catch 
>> things, checking var/ivar for NULL, seeing if a collection is empty, bounds 
>> checking things.
>> 
>> One example here is LLDB has its own object file readers for mach-o and ELF. 
>> Mach-o has load commands in the file header and each load command has a 
>> "cmd" uint32_t whose value is an enum. It is followed by a uint32_t "length" 
>> of this data. The LLVM version of the mach-o file parser, a while back 
>> before changes were made, wanted to assert or llvm_unreachable for any load 
>> command that it didn't recognize. This kind of thing can't happen in a 
>> debugger.
>> 
>> Right, but that's user input.  On the other hand, suppose LLDB manually 
>> constructs some IR that it passes to the interpreter.  Since LLDB is in full 
>> control of the IR it generates, it can safely assert that the interpreter 
>> was successful at interpreting it.
> 
> Like when you run an expression, it generated some relocation that the JIT 
> doesn't handle and it crashes??? We ran into this a few times. So I don't 
> agree that anything in the expression stack is not subject to these rules. It 
> is something we should fix and we should not crash. So while unexpected 
> things are harder to handle, I still say we should recover if at all possible 
> and we ran into the relocations stuff not being handles many times over the 
> years. 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jason Molenda via lldb-commits]

fwiw the reason the JIT came up is because we had an instance where the older 
MCJIT wasn't handling a relocation in thumb code about six weeks ago and we 
only caught the crash a couple days before we released a beta of it.  It 
definitely can happen with MCJIT.  I think with ORC JIT this is a not going to 
be a problem -- but it's a good example of a class of problem where the 
subsystem (jit) considers the failure catastrophic, whereas the user will find 
another way to do their work.  When it takes the developer an hour to get to 
the point of failure, they try to print a variable, lldb ingests a ton of debug 
info and then we crash because some little detail was not valid, or they try to 
run an expression and the debugger crashes with an unsupported relocation, I 
can't overstate what an enormous failure of the debugger that is.

> On Sep 11, 2017, at 4:10 PM, Greg Clayton via lldb-commits 
>  wrote:
> 
> 
>> On Sep 11, 2017, at 4:05 PM, Leonard Mosescu  wrote:
>> 
>> 
>> Process already has "Error Process::WillResume()" for this very reason. Just 
>> have the Windows mini dump core file plug-in override it. The code in 
>> Process::Resume already checks this:
>> Status Process::PrivateResume() {
>>   Status error(WillResume());
>>   // Tell the process it is about to resume before the thread list
>>   if (error.Success()) {
>>   ...
>> }
>>   }
>>   return error;
>> }
>> Problem solved no???
>> 
>> It would be great, but that also happens too late (by the time 
>> PrivateResume() is called the state has been already changed)
> 
> We could call Process::WillResume() before Process::PrivateResume() before it 
> changes the state... Probably still need to call it in PrivateResume() since 
> a complex thread plan might start and stop the process many times and the 
> process might crash and there would only be one main Process::Continue, or 
> thread step that kicks off the initial run.
> 
> Greg
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Greg Clayton via lldb-commits]

> On Sep 11, 2017, at 4:05 PM, Leonard Mosescu  wrote:
> 
> 
> Process already has "Error Process::WillResume()" for this very reason. Just 
> have the Windows mini dump core file plug-in override it. The code in 
> Process::Resume already checks this:
> Status Process::PrivateResume() {
>   Status error(WillResume());
>   // Tell the process it is about to resume before the thread list
>   if (error.Success()) {
>   ...
> }
>   }
>   return error;
> }
> Problem solved no???
> 
> It would be great, but that also happens too late (by the time 
> PrivateResume() is called the state has been already changed)

We could call Process::WillResume() before Process::PrivateResume() before it 
changes the state... Probably still need to call it in PrivateResume() since a 
complex thread plan might start and stop the process many times and the process 
might crash and there would only be one main Process::Continue, or thread step 
that kicks off the initial run.

Greg


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

Maybe I'm missing something, but Why can't you just not generate a
relocation that the JIT doesn't handle?

On Mon, Sep 11, 2017 at 3:43 PM Greg Clayton  wrote:

>
> On Sep 11, 2017, at 3:37 PM, Zachary Turner  wrote:
>
> On Mon, Sep 11, 2017 at 3:31 PM Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>> I know there are two points of view here so I will give my two cents:
>>
>> If it comes down to something as easy as "always check for NULL before
>> doing something", or something that is similar and very easy, I would say
>> just code around it. Don't "assert(...)" or "llvm_unreachable(...)". Things
>> that fall into this category: switch statements with defaults that might
>> catch things, checking var/ivar for NULL, seeing if a collection is empty,
>> bounds checking things.
>>
>> One example here is LLDB has its own object file readers for mach-o and
>> ELF. Mach-o has load commands in the file header and each load command has
>> a "cmd" uint32_t whose value is an enum. It is followed by a uint32_t
>> "length" of this data. The LLVM version of the mach-o file parser, a while
>> back before changes were made, wanted to assert or llvm_unreachable for any
>> load command that it didn't recognize. This kind of thing can't happen in a
>> debugger.
>
>
> Right, but that's user input.  On the other hand, suppose LLDB manually
> constructs some IR that it passes to the interpreter.  Since LLDB is in
> full control of the IR it generates, it can safely assert that the
> interpreter was successful at interpreting it.
>
>
> Like when you run an expression, it generated some relocation that the JIT
> doesn't handle and it crashes??? We ran into this a few times. So I don't
> agree that anything in the expression stack is not subject to these rules.
> It is something we should fix and we should not crash. So while unexpected
> things are harder to handle, I still say we should recover if at all
> possible and we ran into the relocations stuff not being handles many times
> over the years. We don't control user input that is in the expression.
>
> If you are a shared library, you shouldn't crash if you can help it. If
> you are an executable, do what you want and feel free to crash yourself. We
> took LLDB out of process in Xcode so we could load unsigned code from Swift
> nightly builds, and another benefit was it didn't crash Xcode when things
> went wrong in LLDB.
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Greg Clayton via lldb-commits]

> On Sep 11, 2017, at 3:37 PM, Zachary Turner  wrote:
> 
> On Mon, Sep 11, 2017 at 3:31 PM Greg Clayton via lldb-commits 
> > wrote:
> I know there are two points of view here so I will give my two cents:
> 
> If it comes down to something as easy as "always check for NULL before doing 
> something", or something that is similar and very easy, I would say just code 
> around it. Don't "assert(...)" or "llvm_unreachable(...)". Things that fall 
> into this category: switch statements with defaults that might catch things, 
> checking var/ivar for NULL, seeing if a collection is empty, bounds checking 
> things.
> 
> One example here is LLDB has its own object file readers for mach-o and ELF. 
> Mach-o has load commands in the file header and each load command has a "cmd" 
> uint32_t whose value is an enum. It is followed by a uint32_t "length" of 
> this data. The LLVM version of the mach-o file parser, a while back before 
> changes were made, wanted to assert or llvm_unreachable for any load command 
> that it didn't recognize. This kind of thing can't happen in a debugger.
> 
> Right, but that's user input.  On the other hand, suppose LLDB manually 
> constructs some IR that it passes to the interpreter.  Since LLDB is in full 
> control of the IR it generates, it can safely assert that the interpreter was 
> successful at interpreting it.

Like when you run an expression, it generated some relocation that the JIT 
doesn't handle and it crashes??? We ran into this a few times. So I don't agree 
that anything in the expression stack is not subject to these rules. It is 
something we should fix and we should not crash. So while unexpected things are 
harder to handle, I still say we should recover if at all possible and we ran 
into the relocations stuff not being handles many times over the years. We 
don't control user input that is in the expression.

If you are a shared library, you shouldn't crash if you can help it. If you are 
an executable, do what you want and feel free to crash yourself. We took LLDB 
out of process in Xcode so we could load unsigned code from Swift nightly 
builds, and another benefit was it didn't crash Xcode when things went wrong in 
LLDB. ___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

On Mon, Sep 11, 2017 at 3:31 PM Greg Clayton via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> I know there are two points of view here so I will give my two cents:
>
> If it comes down to something as easy as "always check for NULL before
> doing something", or something that is similar and very easy, I would say
> just code around it. Don't "assert(...)" or "llvm_unreachable(...)". Things
> that fall into this category: switch statements with defaults that might
> catch things, checking var/ivar for NULL, seeing if a collection is empty,
> bounds checking things.
>
> One example here is LLDB has its own object file readers for mach-o and
> ELF. Mach-o has load commands in the file header and each load command has
> a "cmd" uint32_t whose value is an enum. It is followed by a uint32_t
> "length" of this data. The LLVM version of the mach-o file parser, a while
> back before changes were made, wanted to assert or llvm_unreachable for any
> load command that it didn't recognize. This kind of thing can't happen in a
> debugger.


Right, but that's user input.  On the other hand, suppose LLDB manually
constructs some IR that it passes to the interpreter.  Since LLDB is in
full control of the IR it generates, it can safely assert that the
interpreter was successful at interpreting it.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Greg Clayton via lldb-commits]

I know there are two points of view here so I will give my two cents:

If it comes down to something as easy as "always check for NULL before doing 
something", or something that is similar and very easy, I would say just code 
around it. Don't "assert(...)" or "llvm_unreachable(...)". Things that fall 
into this category: switch statements with defaults that might catch things, 
checking var/ivar for NULL, seeing if a collection is empty, bounds checking 
things. 

One example here is LLDB has its own object file readers for mach-o and ELF. 
Mach-o has load commands in the file header and each load command has a "cmd" 
uint32_t whose value is an enum. It is followed by a uint32_t "length" of this 
data. The LLVM version of the mach-o file parser, a while back before changes 
were made, wanted to assert or llvm_unreachable for any load command that it 
didn't recognize. This kind of thing can't happen in a debugger. We will load 
files in the future that weren't made with the exact version of tools used for 
LLDB. We can't assert and die saying "I don't know what this blob of stuff is". 
This is a very easy call to make, since if we don't understand what the command 
is, we can skip it because there is a length. 

Same goes for DWARF: we can't assert if we don't recognize a DWARF tag or 
attribute. DW_FORM_ values that the debugger doesn't recognized might 
currently cause us to crash as this stops us from being able to parse the file. 
I would say this shouldn't crash us. We should say "I don't know how to parse 
this debug information". There might be many other debug files we can parse.

Just because Xcode runs LLDB out of process, doesn't mean that CodeLion, 
Nuclide, AppCode and many other IDE's that link against a native LLDB.framework 
or liblldb.so don't matter. When you run in this environment, you don't control 
your heap as the LLDB shared library exists in the heap along with all of the 
other code in the current applicaiton. The heap can get messed up sometimes. 
Once your heap gets messed up, it is nice to try and survive if you can and if 
it is easy. So just saying "I did an assert in my implementation because I 
didn't want to check if something was NULL" seems too easy to fix. 

Also, when you put an assert in your implementation, you are assuming that 
people will read through all of the implementation code before using it? I 
usually just looking at the header file. Often things are not documented in 
header files saying "don't call this function with NULL". We just happen to run 
into it later when NULL happens to be passed to a function and we crash.

So I say make your code bullet proof when it is easy. Stop the crashes that 
aren't necessary if at all possible.


Following up on the approach to fix resuming core files from earlier:

>>> Going back to the original fix, I think most observations are very 
>>> pertinent - yes, the result is not ideal. This is because the current 
>>> logic, changing the state in Process::Resume() before the whole operation 
>>> completes successfully is ... unfortunate. So here are the options I see:
>>> 
>>> 1. Fix Process::Resume() so we only change the state late, after everything 
>>> succeds
>>> 2. Rollback state change if anything fails while resuming
>>> 3. Patch the specific case of "resume is not supported"
>>> 4. Do nothing

Process already has "Error Process::WillResume()" for this very reason. Just 
have the Windows mini dump core file plug-in override it. The code in 
Process::Resume already checks this:

Status Process::PrivateResume() {
  Status error(WillResume());
  // Tell the process it is about to resume before the thread list
  if (error.Success()) {
  ...
  SetPrivateState(eStateRunning);
  SetPrivateState(eStateStopped);
}
  }
  return error;
}

Problem solved no???


> On Sep 11, 2017, at 2:57 PM, Jim Ingham via lldb-commits 
>  wrote:
> 
> 
>> On Sep 11, 2017, at 12:24 PM, Leonard Mosescu  wrote:
>> 
>> Thanks Jim, I'll give option #1 a try and upload a new patch.
>> 
>> think your divisions into options is too simplistic.  lldb is built of many 
>> different subsystems, and a lot of "inconsistent state" can be confined to 
>> one subsystem or other.  So for instance, if one CU's debug is unrecoverably 
>> damaged, we can just pretend that that .o file was built w/o debug info and 
>> the user can do lots of useful debugging still.  It is much better if we can 
>> say "Found irrecoverable errors in this CU, dumping debug info" and continue 
>> rather than crashing because one CU that the user may not even care about is 
>> all horked up.  Similarly a single JIT execution can get itself all balled 
>> up and yet if we can discard the work it did we can continue doing lots of 
>> other things.
>> 
>> The key here is the recoverable part - if you can isolate the state and 
>> recover (throw-away, rollback), then I agree, that can be a preferable 
>> approach. My 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

> On Sep 11, 2017, at 12:24 PM, Leonard Mosescu  wrote:
> 
> Thanks Jim, I'll give option #1 a try and upload a new patch.
> 
>  think your divisions into options is too simplistic.  lldb is built of many 
> different subsystems, and a lot of "inconsistent state" can be confined to 
> one subsystem or other.  So for instance, if one CU's debug is unrecoverably 
> damaged, we can just pretend that that .o file was built w/o debug info and 
> the user can do lots of useful debugging still.  It is much better if we can 
> say "Found irrecoverable errors in this CU, dumping debug info" and continue 
> rather than crashing because one CU that the user may not even care about is 
> all horked up.  Similarly a single JIT execution can get itself all balled up 
> and yet if we can discard the work it did we can continue doing lots of other 
> things.
> 
> The key here is the recoverable part - if you can isolate the state and 
> recover (throw-away, rollback), then I agree, that can be a preferable 
> approach. My point is about truly irrecoverable states - WTF states where you 
> don't expect to get into, can't do anything meaningful in response and will 
> likely cause cascading issues.

Sure, but whether a state is recoverable is determined in part by how hard you 
are willing to work to recover.  In some tools, you won't get enough benefit 
from making recovery possible to justify the effort.  I think the compiler is 
probably one such tool.  But I assert that lldb is a tool where there are real 
human beings that will thank us each time we manage to side-step some little 
present left in our pathway by the debug info, the system API's, the way flakey 
programs that are about to crash behave, etc...

And how much you rule out is also not a unitary thing.  For instance, when 
swift is reading in it's module files to recover swift type information, it can 
(and still does way too often) find itself in an inconsistent state such that 
if you try to look at the module any further, swift is likely to crash.  So we 
changed all accesses to the SwiftASTContext to check that the context was still 
copacetic, and if it was not, we would return an error.  Then Enrico actually 
put in a build phase parser that went through the lldb sources and ensured that 
all functions that used AST contexts checked that they were okay before 
touching them, so you couldn't make the mistake of not checking & handling the 
error.  That was, even if one swift module was bad, the debugging session could 
go on safely.

It took a fair bit of work to get this right, but it was definitely worth the 
effort.  

BTW, that's what I meant when I said that if you assert in all the 
subcomponents you're putting the decision of how important an error at the 
wrong place in the stack.  For the swift compiler, not being able to read in 
its swift module is an irrecoverable state.  For lldb it is not.

> 
> As a side note, I would *love* a generic roll-back mechanism in LLDB. Not 
> only that it can give us a viable alternative to failing fast, but it can 
> also allow things like LLDB-command interruption (which is one thing I'd like 
> to take a closer look when I get a chance)

lldb command interruption requires two parts, one is the rollback, and the 
other is to have someone listening for interruption.  To give an instance that 
works in lldb today, if you run the "expr" command, while we are in the process 
of compiling there's no way to interrupt "expr" because clang doesn't listen 
for interrupts.  But once the expression is running, we're back in the lldb 
event loop and so we can handle interruption.  And of course if you interrupt 
an expression while running it knows how to clean up after itself.  That wasn't 
done for interruption primarily, rather "expr" already had to know that to be 
able to clean up from expressions that crashed or raised signals while running.

I don't think we need a general "interrupt me anywhere you want" mechanism to 
support command interruption however.  Most of the time-consuming work in lldb 
comes in self-contained chunks (reading in symbols from shared libraries, 
reading in the debug info index tables, etc) or happens while lldb is actually 
waiting for something to happen (that part already is interruptible at a low 
level).  We try to do work as lazily as possible, so if the chunk boundaries 
and the lazy work boundaries line up you won't really need to roll-back at all. 
  The model where you check for interrupts after each chunk of work is done 
will I think be responsive enough for user's needs, and also greatly simplify 
the work to handle interruption.  This is a little more manual, since you have 
to insert appropriate "yields", but I think will be good enough for our 
purposes and much easier to make stable.

This is something we've wanted to add to lldb since the start, so if you're 
interested in looking into this further that would be great.

Jim

> 
> On Mon, Sep 11, 2017 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Leonard Mosescu via lldb-commits]

+Mark Mentovai

On Mon, Sep 11, 2017 at 12:24 PM, Leonard Mosescu 
wrote:

> Thanks Jim, I'll give option #1 a try and upload a new patch.
>
>  think your divisions into options is too simplistic.  lldb is built of
>> many different subsystems, and a lot of "inconsistent state" can be
>> confined to one subsystem or other.  So for instance, if one CU's debug is
>> unrecoverably damaged, we can just pretend that that .o file was built w/o
>> debug info and the user can do lots of useful debugging still.  It is much
>> better if we can say "Found irrecoverable errors in this CU, dumping debug
>> info" and continue rather than crashing because one CU that the user may
>> not even care about is all horked up.  Similarly a single JIT execution can
>> get itself all balled up and yet if we can discard the work it did we can
>> continue doing lots of other things.
>
>
> The key here is the recoverable part - if you can isolate the state and
> recover (throw-away, rollback), then I agree, that can be a preferable
> approach. My point is about truly irrecoverable states - WTF states where
> you don't expect to get into, can't do anything meaningful in response and
> will likely cause cascading issues.
>
> As a side note, I would *love* a generic roll-back mechanism in LLDB. Not
> only that it can give us a viable alternative to failing fast, but it can
> also allow things like LLDB-command interruption (which is one thing I'd
> like to take a closer look when I get a chance)
>
> On Mon, Sep 11, 2017 at 10:55 AM, Jim Ingham  wrote:
>
>>
>> > On Sep 11, 2017, at 10:24 AM, Leonard Mosescu 
>> wrote:
>> >
>> > I think everyone is in agreement that input should be sanitized, and
>> ill-formed input should not trigger an abrupt termination if there's a way
>> to recover (which is not always feasible).
>> >
>> > The conversation started around fail-fast for inconsistent and
>> unrecoverable state, where I think the math is very clear in this case.
>> Using one the the comments about the long-lasting debugging sessions as
>> example, let's say that we have a bug and the debugger state gets corrupted
>> after 1h of debugging:
>> > 1. debugger crashes immediately: lost 1h of debugging -> angry bug
>> report (or ideally automated bug report)
>> > 2. debugger limps along for a few more hours behaving erratically (not
>> necessarily in an obvious fashion)
>> > a. developer gives up after many hours or debugging without
>> understanding what's going on
>> > b. developer figures out that the debugger is unreliable
>> >
>> > While it is choosing between two evils, I think #1 is clearly the right
>> choice: #2 results in more lost time and frustration, less chance of
>> getting a bug report and can result in a general impression that the
>> debugger is not to be trusted (which for a developer tool is infinitely
>> worse than an unstable tool)
>>
>> I think your divisions into options is too simplistic.  lldb is built of
>> many different subsystems, and a lot of "inconsistent state" can be
>> confined to one subsystem or other.  So for instance, if one CU's debug is
>> unrecoverably damaged, we can just pretend that that .o file was built w/o
>> debug info and the user can do lots of useful debugging still.  It is much
>> better if we can say "Found irrecoverable errors in this CU, dumping debug
>> info" and continue rather than crashing because one CU that the user may
>> not even care about is all horked up.  Similarly a single JIT execution can
>> get itself all balled up and yet if we can discard the work it did we can
>> continue doing lots of other things.
>>
>> So I think there are lots of places where you can get into a bad state
>> locally, but that doesn't mean #2 is the outcome.  Sometimes you even get
>> into a state where a specific target has done something totally
>> incomprehensible.  In that case, you may have to jettison that target, but
>> that might not be the only target/debugger in this lldb session.  So you
>> still wouldn't want to quit, those targets are probably fine.
>>
>> As you note, this is a little off from the original discussion which was
>> more about setting up and enforcing logic flows using the lldb_private
>> API's.  If you really have to call B only after calling A, and you can
>> clearly signal to yourself that A was called, then it is okay put an assert
>> there.  But in your case, CanResume seems like an optimization - and you
>> are clearly using it as such to avoid having to fix something deeper down
>> in lldb.  But in that case DoResume can error for other reasons, and so the
>> system should deal with it.  So adding another hard requirement was
>> inelegant and exiting if it wasn't met just made that more obvious...
>>
>> >
>> > --
>> >
>> > Going back to the original fix, I think most observations are very
>> pertinent - yes, the result is not ideal. This is because the current
>> logic, 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Leonard Mosescu via lldb-commits]

Thanks Jim, I'll give option #1 a try and upload a new patch.

 think your divisions into options is too simplistic.  lldb is built of
> many different subsystems, and a lot of "inconsistent state" can be
> confined to one subsystem or other.  So for instance, if one CU's debug is
> unrecoverably damaged, we can just pretend that that .o file was built w/o
> debug info and the user can do lots of useful debugging still.  It is much
> better if we can say "Found irrecoverable errors in this CU, dumping debug
> info" and continue rather than crashing because one CU that the user may
> not even care about is all horked up.  Similarly a single JIT execution can
> get itself all balled up and yet if we can discard the work it did we can
> continue doing lots of other things.


The key here is the recoverable part - if you can isolate the state and
recover (throw-away, rollback), then I agree, that can be a preferable
approach. My point is about truly irrecoverable states - WTF states where
you don't expect to get into, can't do anything meaningful in response and
will likely cause cascading issues.

As a side note, I would *love* a generic roll-back mechanism in LLDB. Not
only that it can give us a viable alternative to failing fast, but it can
also allow things like LLDB-command interruption (which is one thing I'd
like to take a closer look when I get a chance)

On Mon, Sep 11, 2017 at 10:55 AM, Jim Ingham  wrote:

>
> > On Sep 11, 2017, at 10:24 AM, Leonard Mosescu 
> wrote:
> >
> > I think everyone is in agreement that input should be sanitized, and
> ill-formed input should not trigger an abrupt termination if there's a way
> to recover (which is not always feasible).
> >
> > The conversation started around fail-fast for inconsistent and
> unrecoverable state, where I think the math is very clear in this case.
> Using one the the comments about the long-lasting debugging sessions as
> example, let's say that we have a bug and the debugger state gets corrupted
> after 1h of debugging:
> > 1. debugger crashes immediately: lost 1h of debugging -> angry bug
> report (or ideally automated bug report)
> > 2. debugger limps along for a few more hours behaving erratically (not
> necessarily in an obvious fashion)
> > a. developer gives up after many hours or debugging without
> understanding what's going on
> > b. developer figures out that the debugger is unreliable
> >
> > While it is choosing between two evils, I think #1 is clearly the right
> choice: #2 results in more lost time and frustration, less chance of
> getting a bug report and can result in a general impression that the
> debugger is not to be trusted (which for a developer tool is infinitely
> worse than an unstable tool)
>
> I think your divisions into options is too simplistic.  lldb is built of
> many different subsystems, and a lot of "inconsistent state" can be
> confined to one subsystem or other.  So for instance, if one CU's debug is
> unrecoverably damaged, we can just pretend that that .o file was built w/o
> debug info and the user can do lots of useful debugging still.  It is much
> better if we can say "Found irrecoverable errors in this CU, dumping debug
> info" and continue rather than crashing because one CU that the user may
> not even care about is all horked up.  Similarly a single JIT execution can
> get itself all balled up and yet if we can discard the work it did we can
> continue doing lots of other things.
>
> So I think there are lots of places where you can get into a bad state
> locally, but that doesn't mean #2 is the outcome.  Sometimes you even get
> into a state where a specific target has done something totally
> incomprehensible.  In that case, you may have to jettison that target, but
> that might not be the only target/debugger in this lldb session.  So you
> still wouldn't want to quit, those targets are probably fine.
>
> As you note, this is a little off from the original discussion which was
> more about setting up and enforcing logic flows using the lldb_private
> API's.  If you really have to call B only after calling A, and you can
> clearly signal to yourself that A was called, then it is okay put an assert
> there.  But in your case, CanResume seems like an optimization - and you
> are clearly using it as such to avoid having to fix something deeper down
> in lldb.  But in that case DoResume can error for other reasons, and so the
> system should deal with it.  So adding another hard requirement was
> inelegant and exiting if it wasn't met just made that more obvious...
>
> >
> > --
> >
> > Going back to the original fix, I think most observations are very
> pertinent - yes, the result is not ideal. This is because the current
> logic, changing the state in Process::Resume() before the whole operation
> completes successfully is ... unfortunate. So here are the options I see:
> >
> > 1. Fix Process::Resume() so we only change the 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

> On Sep 11, 2017, at 10:24 AM, Leonard Mosescu  wrote:
> 
> I think everyone is in agreement that input should be sanitized, and 
> ill-formed input should not trigger an abrupt termination if there's a way to 
> recover (which is not always feasible). 
> 
> The conversation started around fail-fast for inconsistent and unrecoverable 
> state, where I think the math is very clear in this case. Using one the the 
> comments about the long-lasting debugging sessions as example, let's say that 
> we have a bug and the debugger state gets corrupted after 1h of debugging:
> 1. debugger crashes immediately: lost 1h of debugging -> angry bug report (or 
> ideally automated bug report)
> 2. debugger limps along for a few more hours behaving erratically (not 
> necessarily in an obvious fashion)
> a. developer gives up after many hours or debugging without understanding 
> what's going on
> b. developer figures out that the debugger is unreliable
> 
> While it is choosing between two evils, I think #1 is clearly the right 
> choice: #2 results in more lost time and frustration, less chance of getting 
> a bug report and can result in a general impression that the debugger is not 
> to be trusted (which for a developer tool is infinitely worse than an 
> unstable tool)

I think your divisions into options is too simplistic.  lldb is built of many 
different subsystems, and a lot of "inconsistent state" can be confined to one 
subsystem or other.  So for instance, if one CU's debug is unrecoverably 
damaged, we can just pretend that that .o file was built w/o debug info and the 
user can do lots of useful debugging still.  It is much better if we can say 
"Found irrecoverable errors in this CU, dumping debug info" and continue rather 
than crashing because one CU that the user may not even care about is all 
horked up.  Similarly a single JIT execution can get itself all balled up and 
yet if we can discard the work it did we can continue doing lots of other 
things.

So I think there are lots of places where you can get into a bad state locally, 
but that doesn't mean #2 is the outcome.  Sometimes you even get into a state 
where a specific target has done something totally incomprehensible.  In that 
case, you may have to jettison that target, but that might not be the only 
target/debugger in this lldb session.  So you still wouldn't want to quit, 
those targets are probably fine.

As you note, this is a little off from the original discussion which was more 
about setting up and enforcing logic flows using the lldb_private API's.  If 
you really have to call B only after calling A, and you can clearly signal to 
yourself that A was called, then it is okay put an assert there.  But in your 
case, CanResume seems like an optimization - and you are clearly using it as 
such to avoid having to fix something deeper down in lldb.  But in that case 
DoResume can error for other reasons, and so the system should deal with it.  
So adding another hard requirement was inelegant and exiting if it wasn't met 
just made that more obvious...

> 
> --
> 
> Going back to the original fix, I think most observations are very pertinent 
> - yes, the result is not ideal. This is because the current logic, changing 
> the state in Process::Resume() before the whole operation completes 
> successfully is ... unfortunate. So here are the options I see:
> 
> 1. Fix Process::Resume() so we only change the state late, after everything 
> succeds
> 2. Rollback state change if anything fails while resuming
> 3. Patch the specific case of "resume is not supported"
> 4. Do nothing
> 
> The proposed fix is #3, since it's the less intrusive (ie. doesn't change the 
> core resume logic) patch for a very specific and narrow case. IMO the "right" 
> fix is #1, but it seems a more radical change. #2 seems tempting in terms of 
> code changes, but I find it less compelling overall since rollbacks are 
> generally fragile if the code is not designed with that in mind.
> 
> So, LLDB experts, what do you think? I'd be happy to go with #1 if someone 
> with experience in this area can review the change. Anything else that I 
> missed?
> 

Seems like #1 is the right choice if you're willing to give it a try.  You are 
right that this code is tricky, but it would be worth the effort to fix it 
right, if for no other reason than to get some other eyes on it.  I'm happy to 
answer any questions as you go along.  This code has ping-ponged back and forth 
between Greg and me, so we should both be able to help you out.

Jim

> Thanks!
> 
> 
> 
> 
> 
> 
> On Sun, Sep 10, 2017 at 1:52 PM, Jim Ingham via lldb-commits 
>  wrote:
> 
>> On Sep 9, 2017, at 7:38 PM, Zachary Turner  wrote:
>> 
>> Sure, but reading a core file involves handling user input. Obviously that 
>> has to be sanitized and you can't crash on user input. I don't think there's 
>> ever been 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Leonard Mosescu via lldb-commits]

I think everyone is in agreement that input should be sanitized, and
ill-formed input should not trigger an abrupt termination if there's a way
to recover (which is not always feasible).

The conversation started around fail-fast for inconsistent and
unrecoverable state, where I think the math is very clear in this case.
Using one the the comments about the long-lasting debugging sessions as
example, let's say that we have a bug and the debugger state gets corrupted
after 1h of debugging:
1. debugger crashes immediately: lost 1h of debugging -> angry bug report
(or ideally automated bug report)
2. debugger limps along for a few more hours behaving erratically (not
necessarily in an obvious fashion)

a. developer gives up after many hours or debugging without understanding
what's going on

b. developer figures out that the debugger is unreliable

While it is choosing between two evils, I think #1 is clearly the right
choice: #2 results in more lost time and frustration, less chance of
getting a bug report and can result in a general impression that the
debugger is not to be trusted (which for a developer tool is infinitely
worse than an unstable tool)

--

Going back to the original fix, I think most observations are very
pertinent - yes, the result is not ideal. This is because the current
logic, changing the state in Process::Resume() before the whole operation
completes successfully is ... unfortunate. So here are the options I see:

1. Fix Process::Resume() so we only change the state late, after everything
succeds
2. Rollback state change if anything fails while resuming
3. Patch the specific case of "resume is not supported"
4. Do nothing

The proposed fix is #3, since it's the less intrusive (ie. doesn't change
the core resume logic) patch for a very specific and narrow case. IMO the
"right" fix is #1, but it seems a more radical change. #2 seems tempting in
terms of code changes, but I find it less compelling overall since
rollbacks are generally fragile if the code is not designed with that in
mind.

So, LLDB experts, what do you think? I'd be happy to go with #1 if someone
with experience in this area can review the change. Anything else that I
missed?

Thanks!






On Sun, Sep 10, 2017 at 1:52 PM, Jim Ingham via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

>
> On Sep 9, 2017, at 7:38 PM, Zachary Turner  wrote:
>
> Sure, but reading a core file involves handling user input. Obviously that
> has to be sanitized and you can't crash on user input. I don't think
> there's ever been any disagreement about that. On the other hand, if you
> just type an expression in the debugger and expect an answer back, after
> clang says the ast is valid, the whole stack should be asserts all the way
> down, because everything is validated
>
>
> Yes, but even that ends up not matching with the facts on the ground,
> because the debug information for types as used by lldb has to be produced
> from debug info, and the debug info can be malformed and not infrequently
> is.  So at some point clang is going to ask for something that it assumes
> must always be available because if it had read the types in from header
> files it would have asserted at an early stage but when it is fed types
> from debug info incrementally - which is the only efficient way the
> debugger can do it - is going to cause something to look wrong at a point
> where that “shouldn’t be possible.”  clang's rigid structure means it can’t
> cope with this and will crash on something the user didn’t themselves get
> wrong.
>
> According to your classification, we should really consider debug
> information “user input” as well.  But that means we’re wiring “user input”
> into a pretty deep place in clang and you’d probably have to weaken your
> strict assert model to handle that. And it makes me wonder if there’s
> anything the debugger consumes that wouldn’t fall into this category?
>
> For instance, we also let plugins provide threads and their stacks, and we
> try pretty hard not to crash and burn if these plugins get it a little
> wrong.  So process state really needs to be treated as “user data” as
> well.  And since we use SPI’s for most of this info that we are pretty much
> the only clients of, they can be expected to be flakey as well, again a
> fact that we should not inflict on users of the debugger if we can help it.
>
> The answer of trying to do everything dangerous out of process I think
> will make a  slow and fragile architecture.  And while I agree with the
> llvm.org decision not to use C++ exceptions because they make reasoning
> about the program more difficult, replacing that with SIGSEGV as the
> exception mechanism of choice seems really weak.  You are not the first
> person to point out that we could use the crash-protected threads, but I’m
> remain very unenthusiastic about this approach...
>
> Moreover, there are a bunch of problematic areas in lldb, but I don’t

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

> On Sep 9, 2017, at 7:38 PM, Zachary Turner  wrote:
> 
> Sure, but reading a core file involves handling user input. Obviously that 
> has to be sanitized and you can't crash on user input. I don't think there's 
> ever been any disagreement about that. On the other hand, if you just type an 
> expression in the debugger and expect an answer back, after clang says the 
> ast is valid, the whole stack should be asserts all the way down, because 
> everything is validated 

Yes, but even that ends up not matching with the facts on the ground, because 
the debug information for types as used by lldb has to be produced from debug 
info, and the debug info can be malformed and not infrequently is.  So at some 
point clang is going to ask for something that it assumes must always be 
available because if it had read the types in from header files it would have 
asserted at an early stage but when it is fed types from debug info 
incrementally - which is the only efficient way the debugger can do it - is 
going to cause something to look wrong at a point where that “shouldn’t be 
possible.”  clang's rigid structure means it can’t cope with this and will 
crash on something the user didn’t themselves get wrong.

According to your classification, we should really consider debug information 
“user input” as well.  But that means we’re wiring “user input” into a pretty 
deep place in clang and you’d probably have to weaken your strict assert model 
to handle that. And it makes me wonder if there’s anything the debugger 
consumes that wouldn’t fall into this category?

For instance, we also let plugins provide threads and their stacks, and we try 
pretty hard not to crash and burn if these plugins get it a little wrong.  So 
process state really needs to be treated as “user data” as well.  And since we 
use SPI’s for most of this info that we are pretty much the only clients of, 
they can be expected to be flakey as well, again a fact that we should not 
inflict on users of the debugger if we can help it.

The answer of trying to do everything dangerous out of process I think will 
make a  slow and fragile architecture.  And while I agree with the llvm.org 
decision not to use C++ exceptions because they make reasoning about the 
program more difficult, replacing that with SIGSEGV as the exception mechanism 
of choice seems really weak.  You are not the first person to point out that we 
could use the crash-protected threads, but I’m remain very unenthusiastic about 
this approach...

Moreover, there are a bunch of problematic areas in lldb, but I don’t think any 
of them are problematic for the reasons that you are describing here.  There 
are areas of weak memory management in the data formatters & some places where 
we don’t manage locking right in handling process stops & the reconstruction of 
program state after that, and those cause the vast majority of the infrequent 
but annoying crashes we see.  I don’t myself believe that putting in this 
assert everywhere architecture would pay off in reduced crashes to compensate 
for the brittleness it would introduce.

Finally, and most crucially, no failure of any expression evaluation and no 
malformed type is worth taking down a debug session, and it’s our 
responsibility working on lldb to make sure it doesn't.  If the makes our job a 
little harder, so be it.  This was Jason’s point earlier, but it is worth 
reiterating because it makes lldb a very different kind of program from all the 
other programs in the suite collected under the llvm project.  If the compiler 
gets into an inconsistent state compiling a source file, it’s fine for it to 
just exit.  It wasn’t going to do any more useful work anyway.  That is 
definitely NOT true of lldb, and makes reasonable the notion that general 
solutions that seem appropriate for the other tools are not appropriate for 
lldb.


Jim


> On Sat, Sep 9, 2017 at 6:53 PM Jim Ingham  > wrote:
> I think we are talking at cross purposes.  Seems to me you are saying “If we 
> can assert that the answers to questions I ask must always copacetic, then we 
> can express that in software, and that will make things so much easier".  I’m 
> saying "my experience of the data debuggers have to deal with is such that 
> assuming such assertions will lead you to over-react to such errors.  Instead 
> you should write your code so that if somebody gives you bad data you don’t 
> fall over allowing the people who called you to decide how important the 
> error was.”  Every core file that was written out by OS X for years had a 
> section that was ill-formed.  Asserting when you get an ill-formed object 
> file might seem a good way to ensure that you don’t have to make guesses that 
> might lead you astray.  But the people who need to load core files on OS X 
> are rightly unmoved by such arguments if lldb disappears out from under them 
> when reading in core files.
>  
> 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

Sure, but reading a core file involves handling user input. Obviously that
has to be sanitized and you can't crash on user input. I don't think
there's ever been any disagreement about that. On the other hand, if you
just type an expression in the debugger and expect an answer back, after
clang says the ast is valid, the whole stack should be asserts all the way
down, because everything is validated
On Sat, Sep 9, 2017 at 6:53 PM Jim Ingham  wrote:

> I think we are talking at cross purposes.  Seems to me you are saying “If
> we can assert that the answers to questions I ask must always copacetic,
> then we can express that in software, and that will make things so much
> easier".  I’m saying "my experience of the data debuggers have to deal with
> is such that assuming such assertions will lead you to over-react to such
> errors.  Instead you should write your code so that if somebody gives you
> bad data you don’t fall over allowing the people who called you to decide
> how important the error was.”  Every core file that was written out by OS X
> for years had a section that was ill-formed.  Asserting when you get an
> ill-formed object file might seem a good way to ensure that you don’t have
> to make guesses that might lead you astray.  But the people who need to
> load core files on OS X are rightly unmoved by such arguments if lldb
> disappears out from under them when reading in core files.
>
> Jim
>
>
>
> On Sep 9, 2017, at 1:31 PM, Zachary Turner  wrote:
>
>
>
> On Sat, Sep 9, 2017 at 12:04 PM Jim Ingham  wrote:
>
>>
>> I disagree here.  If you can reasonably unwind from an error you should
>> do so even if you can’t figure out how you could have gotten an answer you
>> didn’t expect.  If an API is returning a pointer to something you should
>> assume it might return nullptr unless the API explicitly states otherwise.
>>
> But that's exactly what an assert is.  It's an explicit statement by the
> API about what should happen.  Which is why by adding them liberally, these
> assumptions can then be propagated all the way up through many layers of
> the code, vastly simplifying the codebase.
>
> if you have
>
> void *foo(int x) {
>   // do some stuff
>
>   assert(x < 0 || foo != nullptr);
> }
>
> Then you're documenting that if x is greater than 0, the caller doesn't
> need to check the return value for nullptr.  Now instead of this:
>
> void *bar(unsigned x) {
>   void *ptr = foo(x);
>   if (!ptr) {
> // log an error
> return nullptr;
>   }
>   return ptr;
> }
>
> You just have
>
> void *bar(unsigned x) {
>   void *ptr = foo(x);
>   assert(x);
>   return x;
> }
>
> And now the caller of bar doesn't have to check either.  The code has
> greatly reduced complexity due to the butterfly efflect of propagating
> these assumptions up.
>
> This is a simple example but the point is that building assumptions into
> your API is a good thing, because you can enforce them and it vastly
> simplifies the code.
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

I think we are talking at cross purposes.  Seems to me you are saying “If we 
can assert that the answers to questions I ask must always copacetic, then we 
can express that in software, and that will make things so much easier".  I’m 
saying "my experience of the data debuggers have to deal with is such that 
assuming such assertions will lead you to over-react to such errors.  Instead 
you should write your code so that if somebody gives you bad data you don’t 
fall over allowing the people who called you to decide how important the error 
was.”  Every core file that was written out by OS X for years had a section 
that was ill-formed.  Asserting when you get an ill-formed object file might 
seem a good way to ensure that you don’t have to make guesses that might lead 
you astray.  But the people who need to load core files on OS X are rightly 
unmoved by such arguments if lldb disappears out from under them when reading 
in core files.
 
Jim


> On Sep 9, 2017, at 1:31 PM, Zachary Turner  wrote:
> 
> 
> 
> On Sat, Sep 9, 2017 at 12:04 PM Jim Ingham  > wrote:
> 
> I disagree here.  If you can reasonably unwind from an error you should do so 
> even if you can’t figure out how you could have gotten an answer you didn’t 
> expect.  If an API is returning a pointer to something you should assume it 
> might return nullptr unless the API explicitly states otherwise.  
> But that's exactly what an assert is.  It's an explicit statement by the API 
> about what should happen.  Which is why by adding them liberally, these 
> assumptions can then be propagated all the way up through many layers of the 
> code, vastly simplifying the codebase.
> 
> if you have
> 
> void *foo(int x) {
>   // do some stuff
> 
>   assert(x < 0 || foo != nullptr);
> }
> 
> Then you're documenting that if x is greater than 0, the caller doesn't need 
> to check the return value for nullptr.  Now instead of this:
> 
> void *bar(unsigned x) {
>   void *ptr = foo(x);
>   if (!ptr) {
> // log an error
> return nullptr;
>   }
>   return ptr;
> }
> 
> You just have
> 
> void *bar(unsigned x) {
>   void *ptr = foo(x);
>   assert(x);
>   return x;
> }
> 
> And now the caller of bar doesn't have to check either.  The code has greatly 
> reduced complexity due to the butterfly efflect of propagating these 
> assumptions up.
> 
> This is a simple example but the point is that building assumptions into your 
> API is a good thing, because you can enforce them and it vastly simplifies 
> the code.

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

On Sat, Sep 9, 2017 at 12:04 PM Jim Ingham  wrote:

>
> I disagree here.  If you can reasonably unwind from an error you should do
> so even if you can’t figure out how you could have gotten an answer you
> didn’t expect.  If an API is returning a pointer to something you should
> assume it might return nullptr unless the API explicitly states otherwise.
>
But that's exactly what an assert is.  It's an explicit statement by the
API about what should happen.  Which is why by adding them liberally, these
assumptions can then be propagated all the way up through many layers of
the code, vastly simplifying the codebase.

if you have

void *foo(int x) {
  // do some stuff

  assert(x < 0 || foo != nullptr);
}

Then you're documenting that if x is greater than 0, the caller doesn't
need to check the return value for nullptr.  Now instead of this:

void *bar(unsigned x) {
  void *ptr = foo(x);
  if (!ptr) {
// log an error
return nullptr;
  }
  return ptr;
}

You just have

void *bar(unsigned x) {
  void *ptr = foo(x);
  assert(x);
  return x;
}

And now the caller of bar doesn't have to check either.  The code has
greatly reduced complexity due to the butterfly efflect of propagating
these assumptions up.

This is a simple example but the point is that building assumptions into
your API is a good thing, because you can enforce them and it vastly
simplifies the code.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

On Sat, Sep 9, 2017 at 11:18 AM Jim Ingham  wrote:

> On Sep 8, 2017, at 11:45 PM, Zachary Turner  wrote:
>
> On Fri, Sep 8, 2017 at 8:19 PM Jason Molenda  wrote:
>
>> Also keep in mind that debug sessions have a tendency to be long lived.
>> I may be working through a problem for half an hour -- or this may be the
>> one rare instance where a bug reproduces -- and crashing because some bogus
>> piece of debug info that I don't care about is invalid is not acceptable.
>> I've gotten those bug reports, where someone has spent all day getting to
>> just the point where they have the problem in front of them, only to have
>> the debugger crash, and they are rightfully enraged at the debugger for
>> this.
>>
>> An interactive tool like the debugger cannot use a model where asserting
>> is acceptable.  Even if lldb is out-of-process, you're losing all of the
>> work you'd done until then, and complex bugs can involve extraordinarily
>> long running debug sessions.  The fact that llvm/clang can assert out from
>> under us is to the detriment of lldb's quality.
>>
>> J
>
> Fwiw i think we all agree that it shouldn't crash.  What we disagree on is
> the best way of making it crash less.  It's easy to see why someone would
> argue that adding an assert increases the incidence of crashes.  I mean on
> the surface it seems ridiculous to argue otherwise.
>
> But otherwise is precisely what we all continue to argue.
>
> Your position is that by going out of your way to avoid crashes, it will
> crash less and be more stable.  Pretty intuitive position if I'm being
> honest.  But I don't agree.
>
> I think you are trading short term metrics for long term technical debt
> You fix one crash by adding a null check, without figuring out why null
> ever got in there in the first place.
>
> You get fewer bug reports because instead of crashing, which would
> hopefully trigger some automatic crash uploader in Xcode that automatically
> files a rdar, you just never find out about the bug in the first place.
>
> You introduce even more bugs, because when you go to edit a function, its
> complexity and branching is through the roof, and you overlook something
> some corner case.
>
> You have less test coverage because you introduce another untested branch
> in the code, reducing the already abysmal code coverage even further.
>
> The way to reduce crashing is to *increase* the code coverage of your test
> suite.  That is the solution.  And you don't do that by adding null checks
> everywhere, you do it by removing them and asserting.
>
> Yes, it means Xcode might crash.  But you know the bug exists!  How many
> bugs are out there right now that nobody even knows about because they are
> hidden by a null pointer check and instead of the user trying to figure out
> what's going and filing a bug report, they just give up and use gdb
> instead.  That's one more person you're never going to get back.
>
> Crashing Xcode is annoying, but it's fixable.  But when your technical
> debt is going up and to the right, and your test coverage is going down and
> to the right, that's an existential threat to the project .
>
> Btw, i still don't understand why asserts cause anything to crash, given
> that they're supposed to be turned off in a release build
>
>
>  I was objecting to the use of llvm_report_fatal_error.  That crashes no
> matter what.  And I was suggesting replacing that with an lldb_assert.  So
> I’m not sure we are so much in disagreement about that.
>
You're right that crashes no matter what, but when an lldb_assert fires,
you only find out about it if someone is kind enough to submit a bug
report.  I'm assuming that if it were to crash you would find out about it
with an automatically filed rdar with an attached core file.  (I might be
wrong about this)


>
> I was also objecting to the approach where instead of figuring out why the
> errors from one call - DoResmume - were not getting handled properly - we
> add ANOTHER earlier check - CanResume - and then we have to deal with what
> happens when there’s a code path that doesn’t get caught by the added
> check.  That is just adding weakness, and a chance that some obscure
> programmer error gets us confused.
>
I agree with you here.


>
> I also am skeptical about the "just assert if things aren’t quite right"
> approach, because it removes the responsibility for thinking about how you
> would get to that state and how you would get out of it safely.  But more
> crucially, it exaggerates the importance of subsections of lldb’s
> functionality.  For instance, to the JIT, being asked to handle a
> relocation that it doesn’t support is a fatal error, so it seems okay by
> the fail soon lights to just fatal error.  But to lldb no expression is
> important enough to quit if it doesn’t work, so we would have really very
> much preferred if the JIT had been forced to back out from the error.
> Ditto with llvm & clang and 

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

On Fri, Sep 8, 2017 at 8:19 PM Jason Molenda  wrote:

> Also keep in mind that debug sessions have a tendency to be long lived.  I
> may be working through a problem for half an hour -- or this may be the one
> rare instance where a bug reproduces -- and crashing because some bogus
> piece of debug info that I don't care about is invalid is not acceptable.
> I've gotten those bug reports, where someone has spent all day getting to
> just the point where they have the problem in front of them, only to have
> the debugger crash, and they are rightfully enraged at the debugger for
> this.
>
> An interactive tool like the debugger cannot use a model where asserting
> is acceptable.  Even if lldb is out-of-process, you're losing all of the
> work you'd done until then, and complex bugs can involve extraordinarily
> long running debug sessions.  The fact that llvm/clang can assert out from
> under us is to the detriment of lldb's quality.
>
> J

Fwiw i think we all agree that it shouldn't crash.  What we disagree on is
the best way of making it crash less.  It's easy to see why someone would
argue that adding an assert increases the incidence of crashes.  I mean on
the surface it seems ridiculous to argue otherwise.

But otherwise is precisely what we all continue to argue.

Your position is that by going out of your way to avoid crashes, it will
crash less and be more stable.  Pretty intuitive position if I'm being
honest.  But I don't agree.

I think you are trading short term metrics for long term technical debt.

You fix one crash by adding a null check, without figuring out why null
ever got in there in the first place.

You get fewer bug reports because instead of crashing, which would
hopefully trigger some automatic crash uploader in Xcode that automatically
files a rdar, you just never find out about the bug in the first place.

You introduce even more bugs, because when you go to edit a function, its
complexity and branching is through the roof, and you overlook something
some corner case.

You have less test coverage because you introduce another untested branch
in the code, reducing the already abysmal code coverage even further.

The way to reduce crashing is to *increase* the code coverage of your test
suite.  That is the solution.  And you don't do that by adding null checks
everywhere, you do it by removing them and asserting.

Yes, it means Xcode might crash.  But you know the bug exists!  How many
bugs are out there right now that nobody even knows about because they are
hidden by a null pointer check and instead of the user trying to figure out
what's going and filing a bug report, they just give up and use gdb
instead.  That's one more person you're never going to get back.

Crashing Xcode is annoying, but it's fixable.  But when your technical debt
is going up and to the right, and your test coverage is going down and to
the right, that's an existential threat to the project .

Btw, i still don't understand why asserts cause anything to crash, given
that they're supposed to be turned off in a release build
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jason Molenda via lldb-commits]

Just to comment on one aside from earlier, about the fact that Apple moved lldb 
out of process from Xcode.  This was necessitated by some swift support issues, 
but it's true that the fact that lldb crashing no longer took down Xcode was a 
big plus to moving lldb out of process.  This shouldn't be seen as a model to 
be emulated, but a shortcoming and failure of lldb as a mature piece of 
software.  We can't provide this great debugger API that we encourage people to 
adopt and then say 'Well yeah sure but if you don't want your app crashing out 
from under you, better not use that'.  Apple would have had to move lldb out of 
process regardless of this issue, but the fact that there had been discussions 
about moving lldb out of process BECAUSE of the crashing for a long time is a 
truth that pains me.


> On Sep 8, 2017, at 8:19 PM, Jason Molenda via lldb-commits 
>  wrote:
> 
> Also keep in mind that debug sessions have a tendency to be long lived.  I 
> may be working through a problem for half an hour -- or this may be the one 
> rare instance where a bug reproduces -- and crashing because some bogus piece 
> of debug info that I don't care about is invalid is not acceptable.  I've 
> gotten those bug reports, where someone has spent all day getting to just the 
> point where they have the problem in front of them, only to have the debugger 
> crash, and they are rightfully enraged at the debugger for this.
> 
> An interactive tool like the debugger cannot use a model where asserting is 
> acceptable.  Even if lldb is out-of-process, you're losing all of the work 
> you'd done until then, and complex bugs can involve extraordinarily long 
> running debug sessions.  The fact that llvm/clang can assert out from under 
> us is to the detriment of lldb's quality. 
> 
> J
> 
>> On Sep 8, 2017, at 7:41 PM, Jim Ingham via lldb-commits 
>>  wrote:
>> 
>> Yes, the fact that llvm & clang and the MCJIT abort willy nilly has caused 
>> much pain to lldb.  We don’t control that behavior and so are resigned to 
>> some continued suffering along those lines - though it seems like Lang is 
>> going to do a much cleaner job of handling problems in the new ORC JIT which 
>> will remove some significant source of that pain.  And we have no intention 
>> of exacerbating it in areas we have more control over.  That has been both 
>> Greg and my strong opinion since we started the work on lldb.  Greg can 
>> weigh in on whether he has changed his mind about this, I have not.
>> 
>> BTW, the fact that lldb can run out of process doesn’t make it that much 
>> better.  Because lldb caches the parse of object files from debugger to 
>> debugger it is much more efficient to keep one out of process lldb session 
>> running per targeted OS.  So Xcode runs one for all the Native processes and 
>> one for each iOS device.  But that means that you are still going to take 
>> down all the debug sessions for that target when the debugger goes down.  So 
>> still quite undesirable.
>> 
>> Jim
>> 
>> 
>>> On Sep 8, 2017, at 7:18 PM, Zachary Turner  wrote:
>>> 
>>> Note that if something originates from user input, then i agree there's no 
>>> excuse for a fail fast. But after all the inputs have been sanitized, I 
>>> think it's fine to fast fail
>>> On Fri, Sep 8, 2017 at 7:12 PM Zachary Turner  wrote:
>>> On Fri, Sep 8, 2017 at 6:11 PM Jim Ingham via Phabricator 
>>>  wrote:
>>> 
>>> I know there is debate about this one side and another but for lldb this is 
>>> a settled issue.  Unless you really are in a state where the world is about 
>>> to come crashing down around you, you can't raise a fatal error in lldb.  
>>> And in this case, the world is only very minorly strange, so it is 
>>> certainly not appropriate.
>>> 
>>> I don't agree that this is a settled issue in LLDB.  
>>> 
>>> For starters, clang already does this, and LLDB links against libclang.
>>> 
>>> Second, a year or so ago, i recall Greg saying that LLDB now runs out of 
>>> process in Xcode, which should make this a non issue.
>>> 
>>> Finally, by allowing inconsistent state to propagate through the code you 
>>> are only hurting yourself, as you're ultimately not preventing any crashes. 
>>>  It'll just crash later when you de reference a null pointer or read some 
>>> corrupt memory.  Furthermore, it actually increases the likelihood of 
>>> introducing bugs, due to the added complexity and technical debt introduced 
>>> by untested code paths
>> 
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jason Molenda via lldb-commits]

Also keep in mind that debug sessions have a tendency to be long lived.  I may 
be working through a problem for half an hour -- or this may be the one rare 
instance where a bug reproduces -- and crashing because some bogus piece of 
debug info that I don't care about is invalid is not acceptable.  I've gotten 
those bug reports, where someone has spent all day getting to just the point 
where they have the problem in front of them, only to have the debugger crash, 
and they are rightfully enraged at the debugger for this.

An interactive tool like the debugger cannot use a model where asserting is 
acceptable.  Even if lldb is out-of-process, you're losing all of the work 
you'd done until then, and complex bugs can involve extraordinarily long 
running debug sessions.  The fact that llvm/clang can assert out from under us 
is to the detriment of lldb's quality. 

J

> On Sep 8, 2017, at 7:41 PM, Jim Ingham via lldb-commits 
>  wrote:
> 
> Yes, the fact that llvm & clang and the MCJIT abort willy nilly has caused 
> much pain to lldb.  We don’t control that behavior and so are resigned to 
> some continued suffering along those lines - though it seems like Lang is 
> going to do a much cleaner job of handling problems in the new ORC JIT which 
> will remove some significant source of that pain.  And we have no intention 
> of exacerbating it in areas we have more control over.  That has been both 
> Greg and my strong opinion since we started the work on lldb.  Greg can weigh 
> in on whether he has changed his mind about this, I have not.
> 
> BTW, the fact that lldb can run out of process doesn’t make it that much 
> better.  Because lldb caches the parse of object files from debugger to 
> debugger it is much more efficient to keep one out of process lldb session 
> running per targeted OS.  So Xcode runs one for all the Native processes and 
> one for each iOS device.  But that means that you are still going to take 
> down all the debug sessions for that target when the debugger goes down.  So 
> still quite undesirable.
> 
> Jim
> 
> 
>> On Sep 8, 2017, at 7:18 PM, Zachary Turner  wrote:
>> 
>> Note that if something originates from user input, then i agree there's no 
>> excuse for a fail fast. But after all the inputs have been sanitized, I 
>> think it's fine to fast fail
>> On Fri, Sep 8, 2017 at 7:12 PM Zachary Turner  wrote:
>> On Fri, Sep 8, 2017 at 6:11 PM Jim Ingham via Phabricator 
>>  wrote:
>> 
>> I know there is debate about this one side and another but for lldb this is 
>> a settled issue.  Unless you really are in a state where the world is about 
>> to come crashing down around you, you can't raise a fatal error in lldb.  
>> And in this case, the world is only very minorly strange, so it is certainly 
>> not appropriate.
>> 
>> I don't agree that this is a settled issue in LLDB.  
>> 
>> For starters, clang already does this, and LLDB links against libclang.
>> 
>> Second, a year or so ago, i recall Greg saying that LLDB now runs out of 
>> process in Xcode, which should make this a non issue.
>> 
>> Finally, by allowing inconsistent state to propagate through the code you 
>> are only hurting yourself, as you're ultimately not preventing any crashes.  
>> It'll just crash later when you de reference a null pointer or read some 
>> corrupt memory.  Furthermore, it actually increases the likelihood of 
>> introducing bugs, due to the added complexity and technical debt introduced 
>> by untested code paths
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Jim Ingham via lldb-commits]

Yes, the fact that llvm & clang and the MCJIT abort willy nilly has caused much 
pain to lldb.  We don’t control that behavior and so are resigned to some 
continued suffering along those lines - though it seems like Lang is going to 
do a much cleaner job of handling problems in the new ORC JIT which will remove 
some significant source of that pain.  And we have no intention of exacerbating 
it in areas we have more control over.  That has been both Greg and my strong 
opinion since we started the work on lldb.  Greg can weigh in on whether he has 
changed his mind about this, I have not.

BTW, the fact that lldb can run out of process doesn’t make it that much 
better.  Because lldb caches the parse of object files from debugger to 
debugger it is much more efficient to keep one out of process lldb session 
running per targeted OS.  So Xcode runs one for all the Native processes and 
one for each iOS device.  But that means that you are still going to take down 
all the debug sessions for that target when the debugger goes down.  So still 
quite undesirable.

Jim


> On Sep 8, 2017, at 7:18 PM, Zachary Turner  wrote:
> 
> Note that if something originates from user input, then i agree there's no 
> excuse for a fail fast. But after all the inputs have been sanitized, I think 
> it's fine to fast fail
> On Fri, Sep 8, 2017 at 7:12 PM Zachary Turner  > wrote:
> On Fri, Sep 8, 2017 at 6:11 PM Jim Ingham via Phabricator 
> > wrote:
> 
> I know there is debate about this one side and another but for lldb this is a 
> settled issue.  Unless you really are in a state where the world is about to 
> come crashing down around you, you can't raise a fatal error in lldb.  And in 
> this case, the world is only very minorly strange, so it is certainly not 
> appropriate.
> 
> I don't agree that this is a settled issue in LLDB.  
> 
> For starters, clang already does this, and LLDB links against libclang.
> 
> Second, a year or so ago, i recall Greg saying that LLDB now runs out of 
> process in Xcode, which should make this a non issue.
> 
> Finally, by allowing inconsistent state to propagate through the code you are 
> only hurting yourself, as you're ultimately not preventing any crashes.  
> It'll just crash later when you de reference a null pointer or read some 
> corrupt memory.  Furthermore, it actually increases the likelihood of 
> introducing bugs, due to the added complexity and technical debt introduced 
> by untested code paths

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Zachary Turner via lldb-commits]

Note that if something originates from user input, then i agree there's no
excuse for a fail fast. But after all the inputs have been sanitized, I
think it's fine to fast fail
On Fri, Sep 8, 2017 at 7:12 PM Zachary Turner  wrote:

> On Fri, Sep 8, 2017 at 6:11 PM Jim Ingham via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>>
>> I know there is debate about this one side and another but for lldb this
>> is a settled issue.  Unless you really are in a state where the world is
>> about to come crashing down around you, you can't raise a fatal error in
>> lldb.  And in this case, the world is only very minorly strange, so it is
>> certainly not appropriate.
>
>
> I don't agree that this is a settled issue in LLDB.
>
> For starters, clang already does this, and LLDB links against libclang.
>
> Second, a year or so ago, i recall Greg saying that LLDB now runs out of
> process in Xcode, which should make this a non issue.
>
> Finally, by allowing inconsistent state to propagate through the code you
> are only hurting yourself, as you're ultimately not preventing any
> crashes.  It'll just crash later when you de reference a null pointer or
> read some corrupt memory.  Furthermore, it actually increases the
> likelihood of introducing bugs, due to the added complexity and technical
> debt introduced by untested code paths
>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits