[Lldb-commits] [PATCH] D60254: Add dropped ManualDWARFIndex assert()

jankratochvil created this revision. jankratochvil added a reviewer: labath. jankratochvil added a project: LLDB. Herald added subscribers: arphaman, aprantl. D47253 dropped this assertion. So either add it back by this patch or otherwise I will remove currently

[Lldb-commits] [PATCH] D59826: [expression] Explicitly build the lookup table of any TagDecl's context

labath added a comment. In D59826#1452771 , @martong wrote: > In D59826#1451536 , @clayborg wrote: > > > Any way to dump the AST in a test to verify we don't create multiple? > > > I think I might be able to use the

[Lldb-commits] [PATCH] D60254: Add dropped ManualDWARFIndex assert()

labath added a comment. I don't exactly remember my thought processes there, but I guess I have dropped that assert during the refactor because it was obvious to me. Now that you mention it, I don't have any particular opinion on what we should do. If it makes your job any easier, then go ahead

[Lldb-commits] [PATCH] D60254: Add dropped ManualDWARFIndex assert()

jankratochvil added a comment. Maybe I will have to update `GetBaseCompileUnit`, maybe not. But as I find the assert useful (for `-fdebug-types-section -gsplit-dwarf`) going to add back the assertion. Thanks for the reply. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.l

[Lldb-commits] [lldb] r357678 - Add dropped ManualDWARFIndex assert()

Author: jankratochvil Date: Thu Apr 4 02:24:02 2019 New Revision: 357678 URL: http://llvm.org/viewvc/llvm-project?rev=357678&view=rev Log: Add dropped ManualDWARFIndex assert() D47253 dropped this assertion. Differential Revision: https://reviews.llvm.org/D60254 Modified: lldb/trunk/source

[Lldb-commits] [PATCH] D60254: Add dropped ManualDWARFIndex assert()

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 rLLDB357678: Add dropped ManualDWARFIndex assert() (authored by jankratochvil, committed by ). Changed prior to commit: h

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

labath added a comment. To me, the greatest advantage in using Expected (ErrorOr is the old way of doing things; I wouldn't use that in new code) is greater clarity in what are the possible results of the function. If the function returns the object and the error in two pieces, then this leaves

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

sgraenitz added a comment. > From the looks of it, clang needs to set it manually because they rely on > llvm-config instead of using find_package. Please note that clang *is* using `find_package`, just that instead of removing the old llvm-config invocation, it was deprecated first. The code I

[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

labath added a comment. In D60001#1453457 , @clayborg wrote: > ok, so I think I figured out what was going on: I had the .so files still in > my build > packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new > directory. They didn't show

[Lldb-commits] [lldb] r357680 - modify-python-lldb.py: (Re)move __len__ and __iter__ support

Author: labath Date: Thu Apr 4 03:13:59 2019 New Revision: 357680 URL: http://llvm.org/viewvc/llvm-project?rev=357680&view=rev Log: modify-python-lldb.py: (Re)move __len__ and __iter__ support Summary: This patch moves the modify-python-lldb code for adding new functions to the SBModule class in

[Lldb-commits] [PATCH] D60195: modify-python-lldb.py: (Re)move __len__ and __iter__ support

This revision was automatically updated to reflect the committed changes. Closed by commit rL357680: modify-python-lldb.py: (Re)move __len__ and __iter__ support (authored by labath, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHAN

[Lldb-commits] [lldb] r357691 - Breakpad: Refine record classification code

Author: labath Date: Thu Apr 4 06:23:25 2019 New Revision: 357691 URL: http://llvm.org/viewvc/llvm-project?rev=357691&view=rev Log: Breakpad: Refine record classification code Previously we would classify all STACK records into a single bucket. This is not really helpful, because there are three

[Lldb-commits] [PATCH] D59775: Minidump: Add support for reading/writing strings

jhenderson added inline comments. Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:70 minidump::SystemInfo Info; + std::string CSDVersion; Don't know about lifetime here, but could this be a `StringRef`? Comment at: unittests/Object/Min

[Lldb-commits] [PATCH] D60121: Object/Minidump: Add support for reading the ModuleList stream

jhenderson added a comment. Code basically looks fine, but somebody else should confirm that the format is correct. Comment at: lib/Object/Minidump.cpp:55 + if (!OptionalStream) +return createError("No such stream", object_error::invalid_section_index); + auto ExpectedSi

[Lldb-commits] [PATCH] D59775: Minidump: Add support for reading/writing strings

labath marked 3 inline comments as done. labath added inline comments. Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:70 minidump::SystemInfo Info; + std::string CSDVersion; jhenderson wrote: > Don't know about lifetime here, but could this be a `String

[Lldb-commits] [PATCH] D59775: Minidump: Add support for reading/writing strings

labath updated this revision to Diff 193698. labath marked an inline comment as done. labath added a comment. add the size-does-not-fit test case Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59775/new/ https://reviews.llvm.org/D59775 Files: include/llvm/Object

[Lldb-commits] [PATCH] D60121: Object/Minidump: Add support for reading the ModuleList stream

labath marked 2 inline comments as done. labath added a comment. In D60121#1454817 , @jhenderson wrote: > Code basically looks fine, but somebody else should confirm that the format > is correct. BTW, the structure definitions are just copied from lldb

[Lldb-commits] [PATCH] D59775: Minidump: Add support for reading/writing strings

jhenderson accepted this revision. jhenderson added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59775/new/ https://reviews.llvm.org/D59775 ___ lldb-c

[Lldb-commits] [PATCH] D60121: Object/Minidump: Add support for reading the ModuleList stream

labath updated this revision to Diff 193713. labath marked an inline comment as done. labath added a comment. - avoid using mismatched error codes - use decimal numbers more consistently in the tests Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60121/new/ https:/

[Lldb-commits] [PATCH] D60121: Object/Minidump: Add support for reading the ModuleList stream

labath marked an inline comment as done. labath added inline comments. Comment at: lib/Object/Minidump.cpp:55 + if (!OptionalStream) +return createError("No such stream", object_error::invalid_section_index); + auto ExpectedSize = labath wrote: > jhenderson

[Lldb-commits] [PATCH] D60121: Object/Minidump: Add support for reading the ModuleList stream

jhenderson accepted this revision. jhenderson added a comment. This revision is now accepted and ready to land. Okay, no more comments from me. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60121/new/ https://reviews.llvm.org/D60121

[Lldb-commits] [PATCH] D60268: Breakpad: Parse Stack CFI records

labath created this revision. labath added reviewers: clayborg, amccarth, markmentovai. Herald added a subscriber: aprantl. This patch adds support for parsing STACK CFI records from breakpad files. For the expressions specifying the values of registers, only a basic parse is performed -- the reco

[Lldb-commits] [PATCH] D60271: PDBFPO: Use references instead of pointers, where possible

labath created this revision. labath added reviewers: aleksandr.urakov, amccarth. The code was passing pointers around, expecting they would be not null. In c++ it is possible to convey this notion explicitly by using a reference instead. Not all uses of pointers could be converted to references

[Lldb-commits] [PATCH] D60268: Breakpad: Parse Stack CFI records

amccarth added a comment. This looks good. I have a few questions inline, but none of those are major concerns. Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:173 + llvm::DenseMap UnwindRules; +}; + I'm not a fan of deep class hierarchies,

[Lldb-commits] [PATCH] D60271: PDBFPO: Use references instead of pointers, where possible

amccarth accepted this revision. amccarth added a comment. This revision is now accepted and ready to land. LGTM. I noticed this also deleted two overloads of Visit from FPOProgramASTVisitorDWARFCodegen, but that appears to be harmless (the base class overloads were also no-ops). CHANGES SINC

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

ted added a comment. llvm/tools/lldb/tools/driver/Driver.cpp:495:12: error: comparison of integers of different signs: 'ssize_t' (aka 'long') and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare] if (nrwr != commands_size) { ^ ~ 1 error generated. Repository:

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

xiaobai added a comment. In D60180#1454634 , @sgraenitz wrote: > > LLVM_LIBRARY_DIR is being set correctly. It appears to be getting it from > > the LLVMConfig we get from find_package(LLVM). > > Yes LLVMConfig sets the variable, but it's not going to be

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

jasonmolenda added a comment. Thanks Pavel, I'll convert this to use Expected<> while I'm working on it. I'm writing a test case right now, and looking at how DynamicPluginDarwin works more closely for adding & removing binaries, e.g. there's also no way to batch remove libraries and have a gr

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

JDevlieghere created this revision. JDevlieghere added reviewers: aprantl, davide, labath. Herald added a project: LLDB. JDevlieghere updated this revision to Diff 193822. JDevlieghere added a comment. Fix CF test The testcase for objective-c data formatters is very big as it checks a bunch of

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

JDevlieghere updated this revision to Diff 193822. JDevlieghere added a comment. Fix CF test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60300/new/ https://reviews.llvm.org/D60300 Files: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/ObjCDat

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

davide added a comment. I think this is good regardless for readability, but I would really appreciate if we can collect some numbers to see how effective this change actually is. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60300/new/ https://reviews.llvm.org/D60300 __