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

2017-09-12 Thread 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 
> mailto: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)

2017-09-12 Thread 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)

2017-09-12 Thread 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] [lldb-dev] PTYPE replacement in LLDB

2017-09-12 Thread Greg Clayton via lldb-commits

> On Sep 11, 2017, at 10:10 PM, vignesh balu via lldb-dev 
>  wrote:
> 
> Hi All,
> 
> I see in the command Mapping Documnet, we can use "image lookup -type" in 
> place of "ptype" from gdb. Currently, this can be used only for Type(link 
> int, char) , or gloal, static variable or function symbol.

Another option is:

(lldb) type lookup
> 
> This is not as efficient as GDB, where it  will whole structure if we point 
> to variable.
> Here we have to find the variable type first and then we have to do "image 
> lookup".
> 
> do we have a single command which will do that ?

Not at the moment. 

> If not, shall we implement one or integrating with one of existing command is 
> good ?

The question is where does it belong? We could add it in one of two places as 
far as I see:

(lldb) frame variable --type my_var [my_var2]

The "--type" option (no arguments for the option) would indicate we want to 
dump the types of any variable names listed as arguments to the command.

(lldb) type lookup --variable my_var --variable my_var2

where "--variable " means to look at the type of a variable.

I think both would be useful as they would be easy to add.

Please let me know if anyone has any other ideas.

BTW: it would be easy to add this as a python command for now until we get this 
into lldb. If you want to know more about the lldb commands, let me know.

Greg Clayton.
> 
> thanks,
> vigneshbalu
> 
> 
> ___
> lldb-dev mailing list
> lldb-...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

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

2017-09-12 Thread Greg Clayton via lldb-commits

> 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. Knowing that we have an unsupported 
relocation doesn't help us without knowing what the expression is. So while we 
end up seeing the crash, we often don't have enough info to do anything about 
it. So I don't see that as better.


> 
> 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 
>> mailto:lldb-commits@lists.llvm.org>> wrote:
>> 
> 
>> 
>> 
>> On Mon, Sep 11, 2017 at 4:22 PM Jason Molenda via lldb-commits 
>> mailto: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)

2017-09-12 Thread 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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] [lldb-dev] PTYPE replacement in LLDB

2017-09-12 Thread Greg Clayton via lldb-commits

> On Sep 11, 2017, at 10:10 PM, vignesh balu via lldb-dev 
>  wrote:
> 
> Hi All,
> 
> I see in the command Mapping Documnet, we can use "image lookup -type" in 
> place of "ptype" from gdb. Currently, this can be used only for Type(link 
> int, char) , or gloal, static variable or function symbol.

Another option is:

(lldb) type lookup
> 
> This is not as efficient as GDB, where it  will whole structure if we point 
> to variable.
> Here we have to find the variable type first and then we have to do "image 
> lookup".
> 
> do we have a single command which will do that ?

Not at the moment. 

> If not, shall we implement one or integrating with one of existing command is 
> good ?

The question is where does it belong? We could add it in one of two places as 
far as I see:

(lldb) frame variable --type my_var [my_var2]

The "--type" option (no arguments for the option) would indicate we want to 
dump the types of any variable names listed as arguments to the command.

(lldb) type lookup --variable my_var --variable my_var2

where "--variable " means to look at the type of a variable.

I think both would be useful as they would be easy to add.

Please let me know if anyone has any other ideas.

BTW: it would be easy to add this as a python command for now until we get this 
into lldb.

> 
> thanks,
> vigneshbalu
> 
> 
> ___
> lldb-dev mailing list
> lldb-...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

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


Re: [Lldb-commits] [lldb-dev] PTYPE replacement in LLDB

2017-09-12 Thread Jim Ingham via lldb-commits
The original plan was to add a "type eval" to go alongside "type lookup".  
"type eval" would take an expression, evaluate it, and then print the resulting 
type.  gdb's type takes either a type name or an expression, but there's no way 
to disambiguate if you have a type that has the same name as a local variable, 
so I'm more inclined to make them separate commands or use options.

Note, gdb also has "whatis" which doesn't do any evaluation, just looks up the 
argument asa name.  That is pretty much the --variable option that Greg 
suggests.  That could be done either by having "type eval" first try local vars 
and then evaluate the expression or by having an explicit "--vars" flag.

Jim



> On Sep 12, 2017, at 11:21 AM, Greg Clayton via lldb-commits 
>  wrote:
> 
> 
>> On Sep 11, 2017, at 10:10 PM, vignesh balu via lldb-dev 
>>  wrote:
>> 
>> Hi All,
>> 
>> I see in the command Mapping Documnet, we can use "image lookup -type" in 
>> place of "ptype" from gdb. Currently, this can be used only for Type(link 
>> int, char) , or gloal, static variable or function symbol.
> 
> Another option is:
> 
> (lldb) type lookup
>> 
>> This is not as efficient as GDB, where it  will whole structure if we point 
>> to variable.
>> Here we have to find the variable type first and then we have to do "image 
>> lookup".
>> 
>> do we have a single command which will do that ?
> 
> Not at the moment. 
> 
>> If not, shall we implement one or integrating with one of existing command 
>> is good ?
> 
> The question is where does it belong? We could add it in one of two places as 
> far as I see:
> 
> (lldb) frame variable --type my_var [my_var2]
> 
> The "--type" option (no arguments for the option) would indicate we want to 
> dump the types of any variable names listed as arguments to the command.
> 
> (lldb) type lookup --variable my_var --variable my_var2
> 
> where "--variable " means to look at the type of a variable.
> 
> I think both would be useful as they would be easy to add.
> 
> Please let me know if anyone has any other ideas.
> 
> BTW: it would be easy to add this as a python command for now until we get 
> this into lldb.
> 
>> 
>> thanks,
>> vigneshbalu
>> 
>> 
>> ___
>> lldb-dev mailing list
>> lldb-...@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> 
> ___
> 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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


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

2017-09-12 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo updated this revision to Diff 114890.
lemo added a reviewer: markmentovai.
lemo added a comment.

Revised patch based on the review feedback: I looked into updating the running 
state after everything else succeeds, but it proved a bit awkward since 1) 
TrySetRunning() can fail and 2) if we check the running state then use 
SetRunning() instead of TrySetRunning() it opens the possibility for a race.

For reference, here are the considered options:

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

The updated patch is using #2 - rollback the running state to "stopped" if the 
resume operation fails. The downside is the possibility of a "partial rollback" 
but from a cursory review of the code paths the risk seems no higher than 
option #1. Thoughts?

I'm looking into adding a test case as well but I'm uploading the WIP patch to 
get everyone a chance to take an early look.

Thanks!
Lemo.


https://reviews.llvm.org/D37651

Files:
  source/Commands/CommandObjectThread.cpp
  source/Plugins/Process/elf-core/ProcessElfCore.h
  source/Target/Process.cpp


Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1621,7 +1621,12 @@
   log->Printf("Process::Resume: -- TrySetRunning failed, not resuming.");
 return error;
   }
-  return PrivateResume();
+  Status error = PrivateResume();
+  if (!error.Success()) {
+// Undo running state change
+m_public_run_lock.SetStopped();
+  }
+  return error;
 }
 
 Status Process::ResumeSynchronous(Stream *stream) {
@@ -1650,6 +1655,9 @@
   error.SetErrorStringWithFormat(
   "process not in stopped state after synchronous resume: %s",
   StateAsCString(state));
+  } else {
+// Undo running state change
+m_public_run_lock.SetStopped();
   }
 
   // Undo the hijacking of process events...
Index: source/Plugins/Process/elf-core/ProcessElfCore.h
===
--- source/Plugins/Process/elf-core/ProcessElfCore.h
+++ source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -89,6 +89,8 @@
   //--
   bool IsAlive() override;
 
+  bool WarnBeforeDetach() const override { return false; }
+
   //--
   // Process Memory
   //--
Index: source/Commands/CommandObjectThread.cpp
===
--- source/Commands/CommandObjectThread.cpp
+++ source/Commands/CommandObjectThread.cpp
@@ -94,7 +94,7 @@
 bool all_threads = false;
 if (command.GetArgumentCount() == 0) {
   Thread *thread = m_exe_ctx.GetThreadPtr();
-  if (!HandleOneThread(thread->GetID(), result))
+  if (!thread || !HandleOneThread(thread->GetID(), result))
 return false;
   return result.Succeeded();
 } else if (command.GetArgumentCount() == 1) {
@@ -775,6 +775,12 @@
   else
 error = process->Resume();
 
+  if (!error.Success()) {
+result.AppendMessage(error.AsCString());
+result.SetStatus(eReturnStatusFailed);
+return false;
+  }
+
   // There is a race condition where this thread will return up the call
   // stack to the main command handler
   // and show an (lldb) prompt before HandlePrivateEvent (from


Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1621,7 +1621,12 @@
   log->Printf("Process::Resume: -- TrySetRunning failed, not resuming.");
 return error;
   }
-  return PrivateResume();
+  Status error = PrivateResume();
+  if (!error.Success()) {
+// Undo running state change
+m_public_run_lock.SetStopped();
+  }
+  return error;
 }
 
 Status Process::ResumeSynchronous(Stream *stream) {
@@ -1650,6 +1655,9 @@
   error.SetErrorStringWithFormat(
   "process not in stopped state after synchronous resume: %s",
   StateAsCString(state));
+  } else {
+// Undo running state change
+m_public_run_lock.SetStopped();
   }
 
   // Undo the hijacking of process events...
Index: source/Plugins/Process/elf-core/ProcessElfCore.h
===
--- source/Plugins/Process/elf-core/ProcessElfCore.h
+++ source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -89,6 +89,8 @@
   //--
   bool IsAlive() override;
 
+  bool WarnBeforeDetach() const override { return false; }
+
   //--
   // Process Memory
   //---

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

2017-09-12 Thread 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 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

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

2017-09-12 Thread 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 mailto:gclay...@apple.com>>
> 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 mailto:scalla...@apple.com>>
> 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 mailto:gclay...@apple.com>>
> 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 mailto:gclay...@apple.com>>
> Date:   Thu May 12 22:36:47 2016 +
> 
> Don't crash when a process' task port goes bad.
> 
> 
> 
> commit 0ac65423e2ba99a42b246d191520ff13bdca5cb0
> Author: Greg Clayton mailto:gclay...@apple.com>>
> Date:   Tue Mar 15 21:58:28 2016 +
> 
> Don't crash if the TypeSP is empty.
> 
> commit 7043340bc58b0d751fcf66001f62cbf0d9527623
> Author: Greg Clayton mailto:gclay...@apple.com>>
> 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 mailto:gclay...@apple.com>>
> 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 mailto:gclay...@apple.com>>
> 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 mailto:gclay...@apple.com>>
> 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 mailto:gclay...@apple.com>>
> 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 mailto:gclay...@apple.com>>
> 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 mailto:scalla...@apple.com>>
> 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 mailto:gclay...@apple.com>>
> 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 mailto:gclay...@apple.com>>
> 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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.
> >
> > 
> >
> > 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 Cl

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

2017-09-12 Thread 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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)

2017-09-12 Thread 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 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.
>> 
> 

___
ll

[Lldb-commits] [lldb] r313083 - [lldb] Adjust UpdateExternalModuleListIfNeeded method for the case of *.dwo

2017-09-12 Thread Alexander Shaposhnikov via lldb-commits
Author: alexshap
Date: Tue Sep 12 15:14:36 2017
New Revision: 313083

URL: http://llvm.org/viewvc/llvm-project?rev=313083&view=rev
Log:
[lldb] Adjust UpdateExternalModuleListIfNeeded method for the case of *.dwo

When LLDB loads "external" modules it looks at the
presence of DW_AT_GNU_dwo_name.
However, when the already created module
(corresponding to .dwo itself) is being processed,
it will see the presence of DW_AT_GNU_dwo_name
(which contains the name of dwo file) and
will try to call ModuleList::GetSharedModule again.
In some cases (i.e. for empty files) Clang 4.0
generates a *.dwo file which has DW_AT_GNU_dwo_name,
but no DW_AT_comp_dir. In this case the method
ModuleList::GetSharedModule will fail and
the warning will be printed. To workaround this issue, 
one can notice that in this case we don't actually need 
to try to load the already loaded module (corresponding to .dwo).

Test plan: make check-all

Differential revision: https://reviews.llvm.org/D37295

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=313083&r1=313082&r2=313083&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Tue Sep 12 
15:14:36 2017
@@ -1640,7 +1640,29 @@ void SymbolFileDWARF::UpdateExternalModu
 }
 dwo_module_spec.GetArchitecture() =
 m_obj_file->GetModule()->GetArchitecture();
-// printf ("Loading dwo = '%s'\n", dwo_path);
+
+// When LLDB loads "external" modules it looks at the
+// presence of DW_AT_GNU_dwo_name.
+// However, when the already created module
+// (corresponding to .dwo itself) is being processed,
+// it will see the presence of DW_AT_GNU_dwo_name
+// (which contains the name of dwo file) and
+// will try to call ModuleList::GetSharedModule again.
+// In some cases (i.e. for empty files) Clang 4.0
+// generates a *.dwo file which has DW_AT_GNU_dwo_name,
+// but no DW_AT_comp_dir. In this case the method
+// ModuleList::GetSharedModule will fail and
+// the warning will be printed. However, as one can notice
+// in this case we don't actually need to try to load the already
+// loaded module (corresponding to .dwo) so we simply skip it.
+if (m_obj_file->GetFileSpec()
+.GetFileNameExtension()
+.GetStringRef() == "dwo" &&
+llvm::StringRef(m_obj_file->GetFileSpec().GetPath())
+.endswith(dwo_module_spec.GetFileSpec().GetPath())) {
+  continue;
+}
+
 Status error = ModuleList::GetSharedModule(
 dwo_module_spec, module_sp, NULL, NULL, NULL);
 if (!module_sp) {


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


[Lldb-commits] [PATCH] D37295: [lldb] Adjust UpdateExternalModuleListIfNeeded method for the case of *.dwo

2017-09-12 Thread Alexander Shaposhnikov via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313083: [lldb] Adjust UpdateExternalModuleListIfNeeded 
method for the case of *.dwo (authored by alexshap).

Changed prior to commit:
  https://reviews.llvm.org/D37295?vs=113249&id=114923#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37295

Files:
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1640,7 +1640,29 @@
 }
 dwo_module_spec.GetArchitecture() =
 m_obj_file->GetModule()->GetArchitecture();
-// printf ("Loading dwo = '%s'\n", dwo_path);
+
+// When LLDB loads "external" modules it looks at the
+// presence of DW_AT_GNU_dwo_name.
+// However, when the already created module
+// (corresponding to .dwo itself) is being processed,
+// it will see the presence of DW_AT_GNU_dwo_name
+// (which contains the name of dwo file) and
+// will try to call ModuleList::GetSharedModule again.
+// In some cases (i.e. for empty files) Clang 4.0
+// generates a *.dwo file which has DW_AT_GNU_dwo_name,
+// but no DW_AT_comp_dir. In this case the method
+// ModuleList::GetSharedModule will fail and
+// the warning will be printed. However, as one can notice
+// in this case we don't actually need to try to load the already
+// loaded module (corresponding to .dwo) so we simply skip it.
+if (m_obj_file->GetFileSpec()
+.GetFileNameExtension()
+.GetStringRef() == "dwo" &&
+llvm::StringRef(m_obj_file->GetFileSpec().GetPath())
+.endswith(dwo_module_spec.GetFileSpec().GetPath())) {
+  continue;
+}
+
 Status error = ModuleList::GetSharedModule(
 dwo_module_spec, module_sp, NULL, NULL, NULL);
 if (!module_sp) {


Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1640,7 +1640,29 @@
 }
 dwo_module_spec.GetArchitecture() =
 m_obj_file->GetModule()->GetArchitecture();
-// printf ("Loading dwo = '%s'\n", dwo_path);
+
+// When LLDB loads "external" modules it looks at the
+// presence of DW_AT_GNU_dwo_name.
+// However, when the already created module
+// (corresponding to .dwo itself) is being processed,
+// it will see the presence of DW_AT_GNU_dwo_name
+// (which contains the name of dwo file) and
+// will try to call ModuleList::GetSharedModule again.
+// In some cases (i.e. for empty files) Clang 4.0
+// generates a *.dwo file which has DW_AT_GNU_dwo_name,
+// but no DW_AT_comp_dir. In this case the method
+// ModuleList::GetSharedModule will fail and
+// the warning will be printed. However, as one can notice
+// in this case we don't actually need to try to load the already
+// loaded module (corresponding to .dwo) so we simply skip it.
+if (m_obj_file->GetFileSpec()
+.GetFileNameExtension()
+.GetStringRef() == "dwo" &&
+llvm::StringRef(m_obj_file->GetFileSpec().GetPath())
+.endswith(dwo_module_spec.GetFileSpec().GetPath())) {
+  continue;
+}
+
 Status error = ModuleList::GetSharedModule(
 dwo_module_spec, module_sp, NULL, NULL, NULL);
 if (!module_sp) {
___
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)

2017-09-12 Thread 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 
 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 

[Lldb-commits] [lldb] r313113 - Fix test_attach_to_process_from_different_dir_by_id test on Windows

2017-09-12 Thread Eugene Zemtsov via lldb-commits
Author: eugene
Date: Tue Sep 12 19:44:24 2017
New Revision: 313113

URL: http://llvm.org/viewvc/llvm-project?rev=313113&view=rev
Log:
Fix test_attach_to_process_from_different_dir_by_id test on Windows

On Windows a process can't delete its own current direcotry, that's why the test
needs to return to the original direcotry before removing newdir.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py?rev=313113&r1=313112&r2=313113&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py
 Tue Sep 12 19:44:24 2017
@@ -46,15 +46,18 @@ class ProcessAttachTestCase(TestBase):
 except OSError, e:
 if e.errno != os.errno.EEXIST:
 raise
-
self.buildProgram('main.cpp',os.path.join(os.getcwd(),'newdir','proc_attach'))
-exe = os.path.join('.','newdir','proc_attach')
-self.addTearDownHook(lambda: shutil.rmtree(os.path.join(os.getcwd(
+testdir = os.getcwd()
+newdir = os.path.join(testdir,'newdir')
+exe = os.path.join(newdir, 'proc_attach')
+self.buildProgram('main.cpp', exe)
+self.addTearDownHook(lambda: shutil.rmtree(newdir))
 
 # Spawn a new process
 popen = self.spawnSubprocess(exe)
 self.addTearDownHook(self.cleanupSubprocesses)
 
 os.chdir('newdir')
+self.addTearDownHook(lambda: os.chdir(testdir))
 self.runCmd("process attach -p " + str(popen.pid))
 
 target = self.dbg.GetSelectedTarget()


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