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

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

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

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 accept

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

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

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

> 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

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

> 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 "l

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

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::SystemL

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

> 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, t

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

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

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

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 be

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

> 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

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

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

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

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

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

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

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

(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

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

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 pre

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

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

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

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

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

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 ch

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

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

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

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

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

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

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

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 file

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

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 th

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

> 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

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

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,

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

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 M

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

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 t

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

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 goi

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

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

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

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

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

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:

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

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 err

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

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