[Lldb-commits] [PATCH] D55614: Fix MinidumpParser::GetFilteredModuleList() and test it

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55614/new/ https://reviews.llvm.org/D55614 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

labath added a comment. I am very sorry about this, but I am having some second thoughts about the `RunReplay` thingy. I don't think I've correctly expressed what is bothering me about it (and that's probably because I wasn't clear with myself about what is bothering me). I'll try to formulate

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

labath added inline comments. Comment at: source/Commands/CommandObjectTarget.cpp:4246 if (symbol_file) { -ObjectFile *object_file = symbol_file->GetObjectFile(); lemo wrote: > note I had to bypass this check: we don't (yet) have a Object

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

On irc earlier i was talking about this with Greg and he said it should be fine in his opinion. I’ll point him to this review in the morning so he can comment On Thu, Dec 13, 2018 at 3:30 AM Pavel Labath via Phabricator < revi...@reviews.llvm.org> wrote: > labath added inline comments. > > > =

[Lldb-commits] [lldb] r349027 - Classify tests in lit/Modules

Author: labath Date: Thu Dec 13 04:13:29 2018 New Revision: 349027 URL: http://llvm.org/viewvc/llvm-project?rev=349027&view=rev Log: Classify tests in lit/Modules We've recently developed a convention where the tests are placed into subfolders according to the object file type. This applies that

[Lldb-commits] [PATCH] D55653: Check pointer results on nullptr before using them

tatyana-krasnukha created this revision. tatyana-krasnukha added reviewers: ki.stfu, abidh. Herald added a subscriber: lldb-commits. Prevent crashing like llvm.org/pr37054 Repository: rLLDB LLDB https://reviews.llvm.org/D55653 Files: MICmdCmdData.cpp MICmdCmdGdbInfo.cpp MICmdCmdMiscell

[Lldb-commits] [lldb] r349036 - Add missing Initialize/Terminate for Architecture plugins

Author: tkrasnukha Date: Thu Dec 13 06:28:25 2018 New Revision: 349036 URL: http://llvm.org/viewvc/llvm-project?rev=349036&view=rev Log: Add missing Initialize/Terminate for Architecture plugins Modified: lldb/trunk/source/API/SystemInitializerFull.cpp Modified: lldb/trunk/source/API/SystemI

[Lldb-commits] [PATCH] D55575: [NativePDB] Support local variables

labath added a comment. The test looks really good now. This looks fine to me, though I don't know much about PBDs. I still think the seemingly random change in `BinaryStreamArray` would be better served as a standalone patch. Comment at: lldb/lit/SymbolFile/NativePDB/Inputs

[Lldb-commits] [PATCH] D55626: [Reproducers] Add tests for functionality

JDevlieghere updated this revision to Diff 178075. JDevlieghere marked an inline comment as done. JDevlieghere added a comment. Address Adrian's feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55626/new/ https://reviews.llvm.org/D55626 Files: lit/Reproducer/Functionalities/Inp

[Lldb-commits] [PATCH] D55626: [Reproducers] Add tests for functionality

JDevlieghere marked 4 inline comments as done. JDevlieghere added inline comments. Comment at: lit/Reproducer/Functionalities/Inputs/stepping.c:8 +// +//===--===// +#include aprantl wrote: > Why

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere added a comment. In D55582#1329479 , @labath wrote: > I am very sorry about this, but I am having some second thoughts about the > `RunReplay` thingy. I don't think I've correctly expressed what is bothering > me about it (and that's probabl

[Lldb-commits] [lldb] r349062 - Fix MinidumpParser::GetFilteredModuleList() and test it

Author: gclayton Date: Thu Dec 13 09:24:30 2018 New Revision: 349062 URL: http://llvm.org/viewvc/llvm-project?rev=349062&view=rev Log: Fix MinidumpParser::GetFilteredModuleList() and test it The MinidumpParser::GetFilteredModuleList() code was attempting to iterate through the entire module list

[Lldb-commits] [PATCH] D55614: Fix MinidumpParser::GetFilteredModuleList() and test it

This revision was automatically updated to reflect the committed changes. Closed by commit rL349062: Fix MinidumpParser::GetFilteredModuleList() and test it (authored by gclayton, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55614?v

[Lldb-commits] [PATCH] D55631: Delay replacing the process till after we've finalized it

clayborg added inline comments. Comment at: source/Target/Target.cpp:2867-2880 // Get a weak pointer to the previous process if we have one ProcessWP process_wp; if (m_process_sp) process_wp = m_process_sp; -m_process_sp = -GetPlatform()->DebugPr

[Lldb-commits] [lldb] r349067 - [NativePDB] Add support for local variables.

Author: zturner Date: Thu Dec 13 10:17:51 2018 New Revision: 349067 URL: http://llvm.org/viewvc/llvm-project?rev=349067&view=rev Log: [NativePDB] Add support for local variables. This patch adds support for parsing and evaluating local variables. using the native pdb plugin. Differential Revisio

[Lldb-commits] [PATCH] D55575: [NativePDB] Support local variables

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB349067: [NativePDB] Add support for local variables. (authored by zturner, committed by ). Herald added subscribers: te

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 consi

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

Well, Visual Studio also supports remote debugging, and searching next to the .dmp is just one of several places it looks. And LLDB also supports local debugging, and so looking next to the .dmp file, being consistent with Visual Studio, will be the expected behavior for people using LLDB in local

[Lldb-commits] [PATCH] D55116: [CMake] llvm_codesign workaround for Xcode double-signing errors

This revision was automatically updated to reflect the committed changes. Closed by commit rL349070: [CMake] llvm_codesign workaround for Xcode double-signing errors (authored by stefan.graenitz, committed by ). Herald added a subscriber: michaelplatings. Repository: rL LLVM CHANGES SINCE LAST

[Lldb-commits] [PATCH] D55328: [CMake] Revised LLDB.framework builds

aprantl added inline comments. Herald added a subscriber: michaelplatings. Comment at: cmake/modules/LLDBConfig.cmake:275 -if (CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR) - message(FATAL_ERROR "In-source builds are not allowed. CMake would overwrite " -"the makefiles distribute

[Lldb-commits] [PATCH] D55631: Delay replacing the process till after we've finalized it

jingham added a comment. Yeah, I'll simplify the logic after this change. I've had mysterious crashes that I've never been able to reproduce for quite some time now. Things like we go to tear down a process, the thread is deleting its thread plans, a thread plan goes to remove one of it's brea

[Lldb-commits] [PATCH] D55328: [CMake] Revised LLDB.framework builds

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Let's give this a try. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55328/new/ https://reviews.llvm.org/D55328 ___ lldb-commits maili

[Lldb-commits] [PATCH] D55328: [CMake] Revised LLDB.framework builds

aprantl added a comment. Please be sure to watch all LLDB bots (green dragon and lab.llvm.org:8011) closely when landing this change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55328/new/ https://reviews.llvm.org/D55328 ___ lldb-commits

[Lldb-commits] [PATCH] D55328: [CMake] Revised LLDB.framework builds

JDevlieghere accepted this revision. JDevlieghere added a comment. Sorry for the late review, I had a look originally but didn't have any comments. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55328/new/ https://reviews.llvm.org/D55328

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

On 13/12/2018 19:32, Leonard Mosescu wrote: 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 look

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) { } ``` On Thu, Dec 13, 2018 at 12:04 PM Pavel Labath wrote: > On 13/12/2018 19:32, Leonard Mosescu wrote: > > What's the consensus? > > > > Personally I think that, even conside

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 S

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

The problems I have with current directory lookup are: * It makes the behavior dependent on the environment, much like using an environment variable. This is a potential source of flakiness in tests, or different behavior on different peoples' machines. * It doesn't match WinDbg or MSVC * It's te

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just need a way to verify symbols are good. See my inline comment. Comment at: source/Commands/CommandObjectTarget.cpp:4246 if (symbol_file) { -

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

At this point it seems like perpetuating the hack, or at least even if that's the direction we decide to go longterm, not implementing that solution fully and missing some of the corner cases. So I think I'd rather just go with the original hack of checking the current directory at this point. St

[Lldb-commits] [PATCH] D55571: [ast] CreateParameterDeclaration should use an appropriate DeclContext.

friss added a comment. Zachary, how did you figure out this can be an issue? Does it fix something we should be testing? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55571/new/ https://reviews.llvm.org/D55571 ___

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 P

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

The permanent solution would be figuring out what to do about the ObjectFile situation (e.g. do we want to make an ObjectFilePDB or do we want to live in a world where SymbolFiles need not be backed by ObjectFiles?), and then regardless of what we decide there, implementing the SymbolVendor that ca

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

That said, as you mentioned this enables other developers to start working on things, and if that means we can get the SymbolVendor in more quickly, then we can get rid of this more quickly. I just really want to converge towards the permanent solution, rather than away from it, so if committing t

[Lldb-commits] [PATCH] D55571: [ast] CreateParameterDeclaration should use an appropriate DeclContext.

zturner added a comment. In D55571#1330354 , @friss wrote: > Zachary, how did you figure out this can be an issue? Does it fix something > we should be testing? Recently I added a `target modules dump ast` command and I've been using it to write tests

[Lldb-commits] [PATCH] D55116: [CMake] llvm_codesign workaround for Xcode double-signing errors

beanz added a comment. Sorry for being behind on this, but I don't think this is the right solution to the problem. I don't think we should ever pass `--force`. To avoid duplicate code signing when using the Xcode generator we should set the `XCODE_ATTRIBUTE_CODE_SIGN_IDENTITY` target property

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere added a comment. I played around with this today: - I'm totally convinced that we should replay by swapping out stdin with a file from the reproducer. This makes this better overall. - I'm not convinced that capturing just stdin is an improvement. What I like about the current appr

[Lldb-commits] [lldb] r349122 - Fix test failures that depended on module order

Author: gclayton Date: Thu Dec 13 19:07:15 2018 New Revision: 349122 URL: http://llvm.org/viewvc/llvm-project?rev=349122&view=rev Log: Fix test failures that depended on module order Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.p

[Lldb-commits] [PATCH] D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available.

clayborg updated this revision to Diff 178168. clayborg added a comment. Herald added a subscriber: mgorny. Fixed errors, fully test, and make static function that are local to MinidumpParser.cpp that parse the region info from linux maps, memory info list, memory list and memory 64 list. CHAN

[Lldb-commits] [PATCH] D55116: [CMake] llvm_codesign workaround for Xcode double-signing errors

sgraenitz added a comment. Fair enough. Will create a ticket for it. At least this works for now. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55116/new/ https://reviews.llvm.org/D55116 ___ lldb-commits mailing list