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

Yes, this is the culprit indeed, thanks Ed for reporting it! The fix is trivial and I can take care of it on Monday, but if anyone is blocked by it we should revert the change. On Sat, Sep 16, 2017 at 5:21 AM, Ed Maste via lldb-commits < lldb-commits@lists.llvm.org> wrote: > On 13 September

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

It's a good question - here's a two part answer: 1. The actual printing of the output is the longest blocking in many cases (as mentioned in the description: the actual data gathering for "target module dump symtab" can take 1-2sec, but printing it can take 20min. For quick experiment, try dis -p

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Thanks for the context. On Tue, Sep 19, 2017 at 11:58 AM, Jim Ingham wrote: > > > On Sep 19, 2017, at 11:25 AM, Leonard Mosescu > wrote: > > > > These are all great suggestions, thanks everyone! > > > > > We should have a test. The test would need to use

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Any pointers on the steps required to make changes to the SB APIs? Is it documented anywhere? Thanks! On Tue, Sep 19, 2017 at 11:58 AM, Jim Ingham wrote: > > > On Sep 19, 2017, at 11:25 AM, Leonard Mosescu > wrote: > > > > These are all great suggestions,

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Yes, using line endings as split points is somewhat arbitrary, anything that's a reasonable compromise between interruption responsiveness and low interrupt polling overhead would do. I feel that the lines are somewhat nicer since arbitrary splitting may lead to confusion and/or formatting

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

This looks beautiful indeed. The problem is that it doesn't quite work with the current MemoryBuffer and the line_iterator : for one thing there's no way to construct a MemoryBuffer from a StringRef, or to use the line_iterator directly with a StringRef. On Tue, Sep 19, 2017 at 10:39 AM, Zachary

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

These are all great suggestions, thanks everyone! > We should have a test. The test would need to use pexpect or something similar. If anyone has any pointer on how to make a test for this, please chime in. I was thinking just a pexpect based test. I love tests but what exactly do we want to test

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Looking at line_iterator, it seems to be designed to work with MemoryBuffer, which in turn seems specialized for dealing with file content (so while it may be possible to force init a MemoryBuffer from a StringRef it seems a bit awkward to me). Also, line_iterator has extra stuff which we don't

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Hi Greg, are you referring to the manual line splitting vs. using StringRef::split()? This is how it's documented: /// Split into two substrings around the first occurrence of a separator /// character. /// /// If \p Separator is in the string, then the result is a pair (LHS, RHS) /// such that

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

I agree Jim. I'd like like to build thing incrementally - checking in the current change as is does not preclude adding the SB APIs while it does provide the foundation. I think that going after the scenarios you mention is a significant increase in scope. Are people even hooking up

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

So, how about I look into exposing WasInterrupted() through SB APIs in a follow up change? On Tue, Sep 19, 2017 at 11:36 AM, Jim Ingham wrote: > Xcode does, I don't know about other UI's. > > Jim > > > On Sep 19, 2017, at 11:35 AM, Leonard Mosescu >

[Lldb-commits] [lldb] r314856 - updating svn:ignore

Author: lemo Date: Tue Oct 3 15:30:02 2017 New Revision: 314856 URL: http://llvm.org/viewvc/llvm-project?rev=314856=rev Log: updating svn:ignore Modified: lldb/trunk/ (props changed) Propchange: lldb/trunk/ --

[Lldb-commits] [lldb] r315037 - Implement interactive command interruption

Author: lemo Date: Thu Oct 5 16:41:28 2017 New Revision: 315037 URL: http://llvm.org/viewvc/llvm-project?rev=315037=rev Log: Implement interactive command interruption The core of this change is the new CommandInterpreter::m_command_state, which models the state transitions for interactive

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

+Mark Mentovai On Mon, Sep 11, 2017 at 12:24 PM, Leonard Mosescu wrote: > Thanks Jim, I'll give option #1 a try and upload a new patch. > > think your divisions into options is too simplistic. lldb is built of >> many different subsystems, and a lot of "inconsistent state"

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

Thanks Jim, I'll give option #1 a try and upload a new patch. think your divisions into options is too simplistic. lldb is built of > many different subsystems, and a lot of "inconsistent state" can be > confined to one subsystem or other. So for instance, if one CU's debug is > unrecoverably

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

I think everyone is in agreement that input should be sanitized, and ill-formed input should not trigger an abrupt termination if there's a way to recover (which is not always feasible). The conversation started around fail-fast for inconsistent and unrecoverable state, where I think the math is

Re: [Lldb-commits] [PATCH] D38552: LLDB cmake fix: define LLDB_CONFIGURATION_xxx based on the build type

Identical, no - note the checks are inverted. Definitely uglier, but it's defaulting to LLDB_CONFIGURATION_DEBUG if CMAKE_BUILD_TYPE is not defined, right? With the current defaulting rule I pointed out both versions should be equivalent. If anything I prefer the 1st version since I like to avoid

[Lldb-commits] [lldb] r314929 - LLDB cmake fix: define LLDB_CONFIGURATION_xxx based on the build type

Author: lemo Date: Wed Oct 4 13:23:56 2017 New Revision: 314929 URL: http://llvm.org/viewvc/llvm-project?rev=314929=rev Log: LLDB cmake fix: define LLDB_CONFIGURATION_xxx based on the build type Neither LLDB_CONFIGURATION_DEBUG nor LLDB_CONFIGURATION_RELEASE were ever set in the CMake LLDB

Re: [Lldb-commits] [lldb] r334439 - Add a new SBTarget::LoadCore() overload which surfaces errors if the load fails

anted the actual error for external diagnostic purposes (for example it's valuable to know what was the actual problem: corrupted, truncated or unsupported minidump, etc.) On Mon, Jun 11, 2018 at 5:47 PM, Jim Ingham wrote: > > > > On Jun 11, 2018, at 2:19 PM, Leonard Mosescu via lldb-comm

Re: [Lldb-commits] [lldb] r334439 - Add a new SBTarget::LoadCore() overload which surfaces errors if the load fails

anted the actual error for external diagnostic purposes (for example it's valuable to know what was the actual problem: corrupted, truncated or unsupported minidump, etc.) On Mon, Jun 11, 2018 at 5:47 PM, Jim Ingham wrote: > > > > On Jun 11, 2018, at 2:19 PM, Leonard Mosescu via lldb-comm

Re: [Lldb-commits] [PATCH] D47991: Improve SBThread's stepping API using SBError parameter.

> > Ah I see. That's because the last argument is a C++ default argument. It > looks like the convention in this file is that the error argument should be > the last non-defaulted argument. I think it should be: void StepOver(lldb::RunMode stop_other_threads, lldb::SBError ); // no default

Re: [Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

Doesn't the LIT based test drop the split-function case (originally produced with PGO)? Sorry for being late to the party, but it seems beneficial to have both LIT *and* checked in binaries since in general they are complementary: checking against freshly built binaries only covers a matching set

Re: [Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

I agree, checked in binaries are not always pretty. But some coverage depends checked in binaries (or at very least is dramatically harder to get the same thing from source) Are we saying that sacrificing coverage to keep tests smaller should be the default trade off? On Fri, Jun 8, 2018 at

Re: [Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

> > For this particular case, I don't think having a split function is > particularly interesting. I agree that conceptually all disjoint ranges are the same. Until they are not: I haven't looked closely at the code changes but it's easy to introduce assumptions about the layout hierarchy

Re: [Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

So what's your take on this particular case? As far as I can see (please correct me if I'm wrong), the LIT-only tests: 1. Don't cover the case where a function is split into disjoint regions, right? 2. Also don't cover the cross-targeting case (ex. the native PDB reader hosted on Linux) 3. A bug

Re: [Lldb-commits] [PATCH] D48049: Add a new SBTarget::LoadCore() overload which surfaces errors if the load fails

The intention in the scope of this change is just to check that the new overload is exposed correctly through the Python API. In general, guaranteeing specific error codes (messages?) is likely impractical: 1. It's not always easy to do the proper checks (ex. for 'file-not-found' you'd actually

[Lldb-commits] [lldb] r334439 - Add a new SBTarget::LoadCore() overload which surfaces errors if the load fails

Author: lemo Date: Mon Jun 11 14:19:26 2018 New Revision: 334439 URL: http://llvm.org/viewvc/llvm-project?rev=334439=rev Log: Add a new SBTarget::LoadCore() overload which surfaces errors if the load fails There was no way to find out what's wrong if SBProcess SBTarget::LoadCore(const char

Re: [Lldb-commits] [PATCH] D48049: Add a new SBTarget::LoadCore() overload which surfaces errors if the load fails

> > remove space before ( > I'd be happy to make the change, but looking at the rest of the file it seems that almost everything uses the opposite convention: Foo (...). So, to make sure I'm making the right change, is this how it should look? lldb::SBProcess LoadCore(const char

Re: [Lldb-commits] Support of MSVC function-level linking

If anyone's working on this I'd suggest adding a test case for the "split code" case as well (where even a single function is split into multiple ranges). MSVC with PGO should help produce hot/cold cold split repros. On Thu, May 31, 2018 at 10:24 AM, Greg Clayton via lldb-commits <

Re: [Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

> > However, during parsing you need to know the meaning of a "...0" UUID. > In a MachO file (at least based on the comments in the code) this value is > used to denote the fact that the object file has no UUID. For elf, a > "000..0" build-id is a perfectly valid identifier (and the lack of a

Re: [Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

> > I'm not sure if there is a suitable place for that function. This is > needed in "ObjectFileMachO" and two dynamic loader plugins. Then your factory idea may be the next best thing. While we're at it, maybe we can remove the UUID::SetBytes() from the public interface, and make the UUID an

[Lldb-commits] [lldb] r331394 - Use the UUID from the minidump's CodeView Record for placeholder modules

Author: lemo Date: Wed May 2 13:06:17 2018 New Revision: 331394 URL: http://llvm.org/viewvc/llvm-project?rev=331394=rev Log: Use the UUID from the minidump's CodeView Record for placeholder modules This change adds support for two types of Minidump CodeView records: PDB70 (reference:

Re: [Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

FYI, Breakpad & Crashpad will start generating the Microsoft flavor of ARM minidumps soon. On Wed, Jul 25, 2018 at 9:44 AM, Leonard Mosescu via Phabricator < revi...@reviews.llvm.org> wrote: > lemo added inline comments. > > > > Comment at:

Re: [Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

The new test cases did catch a real bug: [ RUN ] VMRange.CollectionContains llvm/src/tools/lldb/unittests/Utility/VMRangeTest.cpp:146: Failure Value of: VMRange::ContainsRange(collection, VMRange(0x100, 0x104)) Actual: false Expected: true class RangeInRangeUnaryPredicate { public:

Re: [Lldb-commits] [PATCH] D50290: Fix a bug in VMRange

Thanks Zach. I can't find llvm::contains() though, any pointers to it? On Fri, Aug 3, 2018 at 5:58 PM, Zachary Turner via Phabricator via lldb-commits wrote: > zturner added a subscriber: lemo. > zturner added a comment. > > I think we have llvm::contains() which would allow you to make this >

Re: [Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

> > I never *ran LLDB tests*, not sure where they are and what they are. I hope you're planning to look into this before submitting the change :) On Fri, Jul 27, 2018 at 11:28 AM, Eugene Birukov via Phabricator < revi...@reviews.llvm.org> wrote: > EugeneBi added a comment. > > Hmm... I never

Re: [Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

> > The LLDB MI plugin didn't work very well as was quite flaky when I tested > it a while back. > Just curious, what was the flaky part, the debug adapter or the LLDB MI interface? On Wed, Aug 8, 2018 at 8:40 AM, Greg Clayton via Phabricator < revi...@reviews.llvm.org> wrote: > clayborg added

[Lldb-commits] [lldb] r338949 - Fix a bug in VMRange

Author: lemo Date: Fri Aug 3 19:15:26 2018 New Revision: 338949 URL: http://llvm.org/viewvc/llvm-project?rev=338949=rev Log: Fix a bug in VMRange I noticed a suspicious failure: [ RUN ] VMRange.CollectionContains llvm/src/tools/lldb/unittests/Utility/VMRangeTest.cpp:146: Failure Value of:

[Lldb-commits] [lldb] r339161 - Misc module/dwarf logging improvements

Author: lemo Date: Tue Aug 7 11:00:30 2018 New Revision: 339161 URL: http://llvm.org/viewvc/llvm-project?rev=339161=rev Log: Misc module/dwarf logging improvements This change improves the logging for the lldb.module category to note a few interesting cases: 1. Local object file found, but

Re: [Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

Really cool! Are you planning to add some documentation for it? (set up instructions, etc...) On Tue, Aug 7, 2018 at 9:24 AM, Greg Clayton via Phabricator via lldb-commits wrote: > clayborg updated this revision to Diff 159526. > clayborg added a comment. > > Removed dead code from python

Re: [Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump

It's an interesting idea, thanks! I don't object moving code around if there's a strong case for it, but I'd like to keep the fix small and simple for now, but it's worth considering if the current minidump loading path will need more flexibility. On Thu, Aug 23, 2018 at 1:27 PM, Greg Clayton

[Lldb-commits] [lldb] r340578 - Restrict the set of plugins used for ProcessMinidump

Author: lemo Date: Thu Aug 23 14:34:33 2018 New Revision: 340578 URL: http://llvm.org/viewvc/llvm-project?rev=340578=rev Log: Restrict the set of plugins used for ProcessMinidump 1. The dynamic loaders should not be needed for loading minidumps and they may create problems (ex. the macOS loader

Re: [Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

Great timing! ARM support would be most welcome. Are you planning to support the Breakpad flavor or ARM minidumps or the Microsoft one? (Mark Mentovai just reminded me that the ARM support was added independently and some of the structures are different) Regarding the invalid minidumps, I used my

Re: [Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

The problem is not returning an error from Minidump::Create() - if that was the case this could easily be improved indeed. The two-phase initialization is a consequence of the LLDB plugin lookup: 1. Target::CreateProcess() calls Process::FindPlugin() 2. ProcessMinidump::CreateInstance() then has

Re: [Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

That sounds reasonable to me. I'll make a note to revisit this (I don't the the cycles to do it right away but I'm planning a few more changes in the area soon). On Mon, Jul 16, 2018 at 10:36 AM, Pavel Labath wrote: > Ok, I see what you mean now. > > Looking at the other core file plugins (elf,

Re: [Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

> > The problem is that shared libraries differ on these machines and > LLDB either fails to load some libraries *or loads wrong ones*. > Not finding the modules is not surprising but the latter (loading the wrong modules) is a bit concerning. Do you know why the module build-id is not used when

Re: [Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

I'm curious too: where did the PDB70 age create matching problems? On a related note, I just noticed that ObjectFilePECOFF::GetUUID() doesn't have a real implementation (just returns false). How do we extract module UUID for PE/COFF files? On Wed, Aug 29, 2018 at 10:28 AM, Zachary Turner via

Re: [Lldb-commits] [lldb] r341080 - Remove redundant initialization

Just curious, what prompted this change? (compiler diagnostic? forcing value initialization in the member initializer list is harmless in this case) Also, if we want to do this kind of cleanup, m_compiler initialization is also redundant. On Thu, Aug 30, 2018 at 8:39 AM, Adrian Prantl via

[Lldb-commits] [lldb] r336918 - Restructure the minidump loading path and add early & explicit consistency checks

Author: lemo Date: Thu Jul 12 10:27:18 2018 New Revision: 336918 URL: http://llvm.org/viewvc/llvm-project?rev=336918=rev Log: Restructure the minidump loading path and add early & explicit consistency checks Corrupted minidumps was leading to unpredictable behavior. This change adds explicit

Re: [Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

Greg/Pavel, does the latest revision look good to you? Thanks! On Wed, Apr 18, 2018 at 10:31 AM, Leonard Mosescu via Phabricator < revi...@reviews.llvm.org> wrote: > lemo marked an inline comment as done. > lemo added a comment. > > In https://reviews.llvm.org/D45700#1070491, @amccarth wrote: >

Re: [Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

> > It looks like nobody except me is worried about the > module-without-an-object-file situation, so I guess we can try this out and > see how it goes. > Sorry Pavel, I meant to respond to this: most of the code seems to explicitly handle this case (module-without-object-file), I just had to fix

Re: [Lldb-commits] [lldb] r330314 - Attempt to fix TestMiniDump on windows

The mix of backward and forward slashes doesn't impact my current project but it would be nice to have a consistent path syntax (both within a single path and also cross platforms). > Leonard, is it reasonable to assume that all paths in the minidumps will be > absolute (and thus resolving is

Re: [Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)

Hi Erik, the review is still marked as requiring changes. Once that is sorted out I'd be happy to submit this on your behalf (what is the base SVN revision for the latest patch?) Davide Italiano, is all the CR feedback addressed in the latest revision? On Tue, Apr 24, 2018 at 1:38 PM, Erik

[Lldb-commits] [lldb] r330302 - Improve LLDB's handling of non-local minidumps

Author: lemo Date: Wed Apr 18 16:10:46 2018 New Revision: 330302 URL: http://llvm.org/viewvc/llvm-project?rev=330302=rev Log: Improve LLDB's handling of non-local minidumps Normally, LLDB is creating a high-fidelity representation of a live process, including a list of modules and sections, with

Re: [Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

Hi Greg, looking at request_evaluate() I noticed that it will evaluate the string as a lldb command if prefixed by ` . This is a great feature (it allows building REPL consoles on top of DAP), but I'm curious how you picked up this convention? For example I believe that the gdb DAP uses -exec

Re: [Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

Great. Do you think that having an abstracted stream I/O tunneled through DAP would lose anything compared to the direct file handle? I can see a few problems using a native platform file handle: 1. It's specific to the platform (with subtle differences between platforms) 2. It assumes a local

Re: [Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

Thanks Greg, this is what I had in mind. I have some ideas which involve this kind of debugger console. We'll likely start with a basic version built on top of evaluate `cmd and once we get around to building a full console I'll ping you. On Sat, Sep 22, 2018 at 8:50 AM, Greg Clayton wrote: >

Re: [Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

> > The solution I would love to see is to have the initialize packet return > something via the DAP that says "I have a command line interpreter, please > send a packet with a file handle (slave path to slave side of pseudo > terminal maybe)". VS Code and Nuclide both emulated tty already, so we

Re: [Lldb-commits] [PATCH] D57037: BreakpadRecords: Address post-commit feedback

> > After some internal discussion, it seems that the situation with the > all-zero UUIDs is as follows: > - breakpad symbol files do not attach a special meaning to a zero UUID - > if a file does not have a build-id, the dump_syms tool will use a hash of > the first page of the text section (or

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

I ended up implementing the support for "target symbols add" since it's something we needed anyway. This allowed the removal of the contentious implicit search in the current directory. I tried to verify this behavior, but it seems like it should already work > out of the box? So we're on the

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

> How large is the PDB file here? 100Kb On Mon, Dec 10, 2018 at 11:48 AM Zachary Turner via Phabricator < revi...@reviews.llvm.org> wrote: > zturner added a comment. > > How large is the PDB file here? > > > CHANGES SINCE LAST ACTION > https://reviews.llvm.org/D55142/new/ > >

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

> BTW: check my changes in: https://reviews.llvm.org/D55522 > It will be interesting to you since it parses the linux maps info if it is available in breakpad generated minidump files. This will give us enough info to create correct sections for object files when we have no ELF file or no symbol

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

Thanks Zach. Don't get me wrong, I have no problem tweaking it as long as necessary assuming we all agree on the plan: we could implement a ObjectFilePDB and a PDB SymbolVendor first, then the contentious PDB lookup detail goes away. My intention was to enable other developers to start consuming

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

I can see how this works for the PDB, no-module-binary case. What about the PDB & module-binary case? Maybe I missed the rationale, but I think that modeling the general case: module and symbols are separate files, is a better foundation for the particular case where the symbols are embedded in

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

> > The Windowsy thing to do is what Zach said: Check the directory that > contains the .dmp for the .pdb. It's the first place the VS debugger looks > when opening a minidump. It's less sensitive to the user's environment. > (Making the user change the current working directory for this could

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

> > I guess I don't see why we need a temporary solution at all. If we can > have logic that can be rolled into the SymbolVendor when we get it, and > makes sense there, and is also simple, why not go with it? Failing that, > doesn't the `target symbols add` solution also work fine? > I just

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

> > But here, we're talking about a situation where there is no EXE, only a > minidump. If there is a minidump and no EXE then neither WinDbg nor VS > will search the minidump folder for the PDB. Indeed, this is key part. > In my mind, the algorithm could be something like: ... > I'm not a

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

Thanks Pavel and Greg. It sounds to me like it would be better to have a separate command > (let's call it "target modules replace" for now) for adding an "object > file" to a "placeholder" module, instead of repurposing "target symbols > add" to do that. Yes, that would be my preference as

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

> > But if the minidump and PDBs are in the same directory, then wouldn't my > proposed solution also work (while also being a permanent solution)? > If we're looking in the same directory as the binary file (which is how I read your suggestion) then it would not be found in this case, since the

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

> > We talked about this offline, but bringing the discussion back here. Can > you describe the use case that this is addressing? As you mention, this is > a temporary hack until we have proper symbol searching logic, but proper > symbol searching logic will do more than just look up symbols in

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

> > I think we can fix that by changing the line to: > > if (!object_file || object_file->GetFileSpec() == symbol_fspec) { > } > The problem is that SymbolFileNativePDB returns the module object file as it's own object file. Are you suggesting we should track the module object file separate from

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

What's the consensus? Personally I think that, even considering the potential issue that Paval pointed out, the "target symbols add ..." is the most conservative approach in terms of introducing new behavior. I'm fine with the current directory lookup as well (the original change) since it's

Re: [Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file

Hi Aleksandr, yes, no objections to this patch. I was responding to Pavel's comments, which I also assume are forward-looking as well, not strictly related to this patch. On Thu, Nov 29, 2018 at 12:08 PM Aleksandr Urakov < aleksandr.ura...@jetbrains.com> wrote: > Yes, I agree that the current

Re: [Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file

Great observations Pavel! I think it's really important to have orthogonal/composable abstractions here: the symbols should be decoupled from the container format IMO. You know more about the ObjectFile than me so I can't say if ObjectFileBreakpad is the best interface, but here are my initial

[Lldb-commits] [lldb] r350546 - Use the minidump exception record if present

Author: lemo Date: Mon Jan 7 09:55:42 2019 New Revision: 350546 URL: http://llvm.org/viewvc/llvm-project?rev=350546=rev Log: Use the minidump exception record if present If the minidump contains a saved exception record use it automatically. Differential Revision:

Re: [Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

> > ouldn’t we have lldb generate the mini dump itself as the first step of > the test? > How would this work cross-platform? On Fri, Jan 4, 2019 at 8:48 AM Zachary Turner wrote: > For those kinds of cases, we could use obj2yaml and check in yaml right? > Fwiw I tried to round-trip an exe

Re: [Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

> > I have a minidump generator if you need me to make any specific minidump > files for you. > Maybe not in this case, but it seems an interesting idea. What are the capabilities of this generator tool? On Thu, Jan 3, 2019 at 3:49 PM Greg Clayton via Phabricator < revi...@reviews.llvm.org>

Re: [Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

Great! I can see how we can put this to good use. In the meantime, I'd like to submit this change as is - the included input files are intended to be reused for future test cases as well (they are extracted from my larger change to add support for the native PDB reader + minidumps). On Fri, Jan

Re: [Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

Sounds very useful. Are you planning to add it to the LLDB repository? On Fri, Jan 4, 2019 at 10:56 AM Greg Clayton wrote: > > > On Jan 4, 2019, at 9:45 AM, Leonard Mosescu wrote: > > I have a minidump generator if you need me to make any specific minidump >> files for you. >> > > Maybe not