[Lldb-commits] [PATCH] D43506: Fix a couple of more tests to not create files in the source tree

labath added a comment. In https://reviews.llvm.org/D43506#1013276, @aprantl wrote: > Thanks! > > By the way, just in case you are motivated :-), one outstanding task is also > to replace all uses of the `clean` target in the makefiles with simply > removing the test build directory. I think

[Lldb-commits] [PATCH] D43506: Fix a couple of more tests to not create files in the source tree

This revision was automatically updated to reflect the committed changes. Closed by commit rL325690: Fix a couple of more tests to not create files in the source tree (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/

[Lldb-commits] [PATCH] D43419: Fix TestBreakpointInGlobalConstructor for Windows

labath added a comment. Yes, regardless of the target platform, if you specify num_locations = -2, then we just don't check the number of locations. https://reviews.llvm.org/D43419 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://li

[Lldb-commits] [PATCH] D43512: DWZ 11/11: Fix for symlinked .build-id/**.debug files

labath added inline comments. Comment at: source/Host/common/Symbols.cpp:288-290 if (llvm::sys::fs::equivalent(file_spec.GetPath(), module_file_spec.GetPath())) continue; Do we need to check the equivalen

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

labath added a comment. Greg, with my last comment in mind, how do you feel about this patch? https://reviews.llvm.org/D42955 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43546: Fix TestMultithreaded when specifying an alternative debugserver.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. The LLDB_DEBUGSERVER_PATH is the "official" way to force a debugserver path in lldb. So unless you want to change the implementation in GDBRemoteCommunication, I don't think there is anything

[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash

labath created this revision. labath added reviewers: aprantl, davide. Herald added a subscriber: JDevlieghere. The llvm function is equivalent to this one. Where possible I tried to replace const char* with llvm::StringRef to avoid extra strlen computations. In most places, I was able to track th

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

labath added inline comments. Comment at: source/Core/Module.cpp:1283-1284 if (!m_sections_ap) { -ObjectFile *obj_file = GetObjectFile(); -if (obj_file != nullptr) +if (ObjectFile *obj_file = GetObjectFile()) obj_file->CreateSections(*GetUnifiedSectionList()

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

labath added inline comments. Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-175 + class Guard final { +std::recursive_mutex *m_mutex; - typedef void (*BreakpointSiteSPMapFunc)(lldb::BreakpointSiteSP &bp, - void *bato

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

labath added inline comments. Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-161 + class Guard : private std::unique_lock { +using std::unique_lock::unique_lock; + }; zturner wrote: > Err, I meant to just deleted the `Guard` class entirely and

[Lldb-commits] [PATCH] D43647: Generate most of the target properties from a central specification.

labath added a comment. I'm not against this in any way, but my long-term goal for this would be so that I can declare a property with something like: Property InputPath(ParentProperty, "input-path", "description-string") where the Property class would know have an appropriate conversion ope

[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash

labath updated this revision to Diff 135568. labath added a comment. This adds a test which looks up a type with unicode characters. I have verified this test does not pass when run against a pre-D42740 compiler. https://reviews.llvm.org/D43596 Files: include/lldb/Core/MappedHash.h packages

[Lldb-commits] [PATCH] D43647: Generate most of the target properties from a central specification.

labath added a comment. Yes, the property would probably be a member variable of some object (maybe even of the parent property, although we would also want to have a setup where the properties can be declared outside of the parent object (for plugins, etc.)). Then something would need to insta

[Lldb-commits] [PATCH] D43662: [Utility] Simplify and generalize the CleanUp helper, NFC

labath added a comment. What do you think about a syntax like: lldb_utility::CleanUp cleanup_our(::close, our_socket); // The CleanUp constructor creates the std::function internally This would be a generalization of your syntax, so you could still use a lambda when needed, but you could avo

[Lldb-commits] [PATCH] D43662: [Utility] Simplify and generalize the CleanUp helper, NFC

labath added a comment. Btw, I think we should also move the CleanUp class out of the lldb_utility and into lldb_private namespace. we have been slowly getting rid of these... https://reviews.llvm.org/D43662 ___ lldb-commits mailing list lldb-commi

[Lldb-commits] [PATCH] D43662: [Utility] Simplify and generalize the CleanUp helper, NFC

labath added a comment. In https://reviews.llvm.org/D43662#1016939, @vsk wrote: > > What do you think about a syntax like: > > > > lldb_utility::CleanUp cleanup_our(::close, our_socket); // The CleanUp > > constructor creates the std::function internally > > @labath I find the current version o

[Lldb-commits] [PATCH] D43662: [Utility] Simplify and generalize the CleanUp helper, NFC

labath added a comment. While we wait for the verdict on the template stuff, it occurred to me that you don't need the PerformCleanup bool member as std::function has an "empty" state. So the destructor can just do `if(Cleanup) Cleanup()` and the disable function can reset the cleanup object. (

[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash

This revision was automatically updated to reflect the committed changes. Closed by commit rL325927: Replace HashStringUsingDJB with llvm::djbHash (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D43596?vs=135568&id=

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

labath created this revision. labath added reviewers: davide, zturner, jingham. The command takes two input arguments: a module to use as a debug target and a file containing a list of commands. The command will execute each of the breakpoint commands in the file and dump the breakpoint state afte

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

labath added a comment. In https://reviews.llvm.org/D43686#1017577, @zturner wrote: > Curious if we need lldb-test for this. Could we get by with just using lldb > in batch mode? Obviously I'm not opposed to adding whatever we need to > lldb-test, just want to make sure it's something that ca

[Lldb-commits] [PATCH] D43688: Partial fix for TestConflictingSymbol.py on Windows

labath added a comment. This test deliberately compiles without `-g`, so the variable should not be in the debug info. On non-windows systems we're expected to pick up the name from the symbol table. Is there such a thing on windows as well? Maybe you need to lookup the symbol as `_conflicting_

[Lldb-commits] [PATCH] D43688: Partial fix for TestConflictingSymbol.py on Windows

labath added a comment. In https://reviews.llvm.org/D43688#1017652, @amccarth wrote: > In https://reviews.llvm.org/D43688#1017638, @labath wrote: > > > On non-windows systems we're expected to pick up the name from the symbol > > table. Is there such a thing on windows as well? > > > Given that

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

labath added a comment. I believe lldb-server does not work around breakpoint opcodes during writing (it does for reading), but that can be fixed, of course. https://reviews.llvm.org/D39967 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D43662: [Utility] Simplify and generalize the CleanUp helper, NFC

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This looks great, thanks. I've been wanting to do something about the CleanUp class for a long time. I have just a couple of nits, but no need to go through review again. C

[Lldb-commits] [PATCH] D43694: Add a sanity check for inline tests

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good. One extra possible check would be to make sure that at the exit of the loop, the inferior is in an expected state (eStateExited with result 0?) https://reviews.llvm.org/D43694

[Lldb-commits] [PATCH] D43688: Partial fix for TestConflictingSymbol.py on Windows

labath added a subscriber: spyffe. labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D43688#1017831, @amccarth wrote: > OK, as far as I can tell, Windows doesn't have anything equivalent to the > .symtab: a DLL ha

[Lldb-commits] [PATCH] D43698: Plug errno in TCPSocket::Connect()

labath added a comment. This seems weird. I have never seen anyone resetting errno *after* a call. Wouldn't it be better to log strerror(errno) somewhere, or make sure that the Status object reflects the actual error? Repository: rL LLVM https://reviews.llvm.org/D43698 __

[Lldb-commits] [PATCH] D43698: Plug errno in TCPSocket::Connect()

labath added a comment. Yes, but that's why we have the Error objects, so we don't have to do the errno save-restore dance. If this is just a debugging aid, then maybe you should keep it as such (locally). There are plenty of places that will leave a stray errno around and I don't think we wan

[Lldb-commits] [PATCH] D43705: Fix tabs/spaces in TestUnicodeSymbols.py

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. lgtm, of course. This happened because I created this file on my mac, which is not my regular development environment. https://reviews.llvm.org/D43705 _

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

labath added a comment. So, I tried this out on my machine, and got just one failure: == FAIL: test_with_run_command_dwarf_type_units (TestInlinedBreakpoints.InlinedBreakpointsTestCase) Test 'b basic_types.cpp:176' does

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

This revision was automatically updated to reflect the committed changes. Closed by commit rL326112: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use… (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://review

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

labath added a comment. In https://reviews.llvm.org/D32167#1019504, @clayborg wrote: > In https://reviews.llvm.org/D32167#1019467, @labath wrote: > > > However, I am not so sure about the proliferation of debug info variants > > that we seem to be having. Right now we have two outstanding patche

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

labath added a comment. In https://reviews.llvm.org/D32167#1019635, @jingham wrote: > Note, I have (though very very occasionally) seen a dsymutil bug that broke > line tables. So it would be good to keep a few "set a breakpoint and run to > it" tests just to sanity check this. But most tests

[Lldb-commits] [PATCH] D43784: Un-XFAIL TestCallStdStringFunction.test for Windows

labath added a comment. Btw, today I found out that this test is passing just accidentally because the CHECK lines are matching the output printed by the inferior and not the results of the evaluated expressions (the situation may be different on windows, if you don't do stdin/out redirection t

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

labath updated this revision to Diff 135971. labath added a comment. Herald added subscribers: JDevlieghere, mgorny. This version moves the logic for the building of unified section list into the symbol vendor plugin. The idea was to keep the construction of section lists of individual object fil

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

labath updated this revision to Diff 135975. labath added a comment. Remove the changes to source/Plugins/SymbolVendor/CMakeLists.txt that snuck in (I was experimenting with enabling the plugin on non-mac systems, but realized that's not possible right now). https://reviews.llvm.org/D42955 File

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

labath accepted this revision. labath added a comment. Sorry, I forgot about that. This looks fine as far as I am concerned. Thank you for flying lldb, and in particular, for creating the gdb-client testing framework. https://reviews.llvm.org/D42145 _

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

labath added a comment. In https://reviews.llvm.org/D32167#1019702, @clayborg wrote: > I am afraid of the opposite: we test what we think we need to test and our > simple tests cases don't adequately test the feature we are adding. I can > certainly limit the testing to very simple test cases w

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

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 rL326261: [lldb] Use vFlash commands when writing to target's flash memory regions (authored by labath, committed by ). Hera

[Lldb-commits] [PATCH] D43837: Move option parsing out of the Args class

labath created this revision. labath added reviewers: davide, jingham, zturner. The args class is used in plenty of places (a lot of them in the lower lldb layers) for representing a list of arguments, and most of these places don't care about option parsing. Moving the option parsing out of the c

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

labath added a comment. It seems this messed up the computation of load addresses for symbols in the androids vdso: Symtab, num_symbols = 5: Debug symbol |Synthetic symbol ||Externally Visible ||| Index UserID DSX Type

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

labath added a comment. Ok, so this is what seems to be happening: The vdso "file" has the following program headers: Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align LOAD 0x00 0xe000 0x 0x00434 0x00434 R E 0x1000 DYN

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

labath added inline comments. Comment at: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:814 value = value - header->p_vaddr; found_offset = true; Ok so the issue is that here we use the virtual address to compute the load bia

[Lldb-commits] [PATCH] D43857: Speed up TestWatchpointMultipleThreads

labath created this revision. labath added reviewers: davide, jingham. Herald added a subscriber: aprantl. The inferior was sleeping before doing any interesting work. I remove that to make the test faster. While looking at the purpose of the test (to check that watchpoints are propagated to all

[Lldb-commits] [PATCH] D43837: Move option parsing out of the Args class

labath added a comment. In https://reviews.llvm.org/D43837#1022303, @jingham wrote: > I don't have any objections to the contents of this patch per se. But I > wonder if having to do all this work to separate the uses of Args from the > options parser so we don't drag it in in some low level u

[Lldb-commits] [PATCH] D43857: Speed up TestWatchpointMultipleThreads

labath added a comment. That's a good point. I think that at least TestConcurrentEvents do it that way, but they are testing a different thing. I think it makes sense to test both things here actually. I'll create one test which sets the watchpoint before thread creation and one after. BTW, do

[Lldb-commits] [PATCH] D43837: Move option parsing out of the Args class

labath added a comment. In https://reviews.llvm.org/D43837#1022430, @jingham wrote: > I still worry a bit because there's another unstated responsibility for Args > which is that even though it is going to get used at a very high level in > lldb it has to NOT depend on anything you don't want l

[Lldb-commits] [PATCH] D43837: Move option parsing out of the Args class

labath added inline comments. Comment at: include/lldb/Interpreter/Options.h:123-126 + llvm::Expected Parse(const Args &args, + ExecutionContext *execution_context, + lldb::PlatformSP platform_sp, +

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

labath added inline comments. Comment at: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:814 value = value - header->p_vaddr; found_offset = true; owenpshaw wrote: > labath wrote: > > Ok so the issue is that here we use the vir

[Lldb-commits] [PATCH] D43837: Move option parsing out of the Args class

labath added a comment. In https://reviews.llvm.org/D43837#1022554, @jingham wrote: > Okay, that sounds good then. Will you enforce the rule about the Utilities > directory socially or by some mechanism? You will get a link error because the Utility unittest executable will have missing symb

[Lldb-commits] [PATCH] D43837: Move option parsing out of the Args class

labath updated this revision to Diff 136355. labath added a comment. Add a slightly longer comment to the Parse function operation. https://reviews.llvm.org/D43837 Files: include/lldb/Interpreter/Args.h include/lldb/Interpreter/Options.h packages/Python/lldbsuite/test/functionalities/comp

[Lldb-commits] [PATCH] D43857: Speed up TestWatchpointMultipleThreads

labath updated this revision to Diff 136361. labath added a comment. This makes the inferior more simple and deterministic by using a single thread. Now we have one test which sets the watchpoint before the thread is created and one that sets it after. I've also simplified the test a bit with new

[Lldb-commits] [PATCH] D43857: Speed up TestWatchpointMultipleThreads

labath updated this revision to Diff 136368. labath added a comment. Replace all tabs in the python file and clang-format the cpp file. https://reviews.llvm.org/D43857 Files: packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_threads/TestWatchpointMultipleThreads.py packag

[Lldb-commits] [PATCH] D42917: Adapt some tests to work with PPC64le architecture

This revision was automatically updated to reflect the committed changes. Closed by commit rL326369: Adapt some tests to work with PPC64le architecture (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42917?vs=13285

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

labath added a comment. In https://reviews.llvm.org/D42145#1022717, @owenpshaw wrote: > > And I'm not even completely clear about your case. I understand you write > > the module to the physical address, but what happens when you actually go > > around to debugging it? Is it still there or does

[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidType, a force-checked recoverable error

labath added a comment. I agree that we should start using this in a lot more places. I think you should be able to shorten the logging code a bit by using the LLDB_LOG_ERROR macro, which I've created for this purpose. Comment at: source/Plugins/ExpressionParser/Clang/ClangE

[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidType, a force-checked recoverable error

labath added inline comments. Comment at: include/lldb/Utility/Log.h:249-254 + ::lldb_private::Log *log_private = (log); \ + if (log_private) \ +log_private->FormatError(::s

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

labath added a comment. Sorry, I've been waiting to give others (Davide?) a chance to express their opinion. I personally don't think having a new debug info flavour is a good idea. Tests written specifically to test this functionality will be easier to maintain and debug when they break. And k

[Lldb-commits] [PATCH] D43857: Speed up TestWatchpointMultipleThreads

This revision was automatically updated to reflect the committed changes. Closed by commit rL326514: Speed up TestWatchpointMultipleThreads (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D43857 Files: lldb/trunk/pa

[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidTypeError, a force-checked recoverable error

labath added inline comments. Comment at: include/lldb/Utility/Log.h:259-260 +// +// For convenience, log may either be a Log instance or an unsigned value +// specifying log categories. +// The thing to remember here is that we have multiple log channels. So the

[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidTypeError, a force-checked recoverable error

labath added a comment. Thanks. The logging stuff looks good to me. I know very little about the TypeFromUser class and friends, so someone else should probably ack that. https://reviews.llvm.org/D43912 ___ lldb-commits mailing list lldb-commits@li

[Lldb-commits] [PATCH] D44041: Only replace object file sections when non-empty

labath requested changes to this revision. labath added a comment. This revision now requires changes to proceed. This looks like a perfect case for lldb-test. You will run into a couple of problems which will prevent this from working out of the box. I tried to fix those in https://reviews.llvm

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

labath requested review of this revision. labath added a comment. Explicitly requesting re-review to make sure this is on your radar. We have a new patch that would benefit from this, but if this is not ready to go in yet (it's a turned into a fairly big refact

[Lldb-commits] [PATCH] D44042: Ensure that trailing characters aren't included in PECOFF section names

labath added a comment. Is it possible to test this? Is it possible to make yaml2obj generate a file that would trigger this? https://reviews.llvm.org/D44042 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

labath added a comment. I can try to split the patch up a bit if you think it will make things easier. I'm not sure how much I will be able to do that, as the patch is not that big, it just needs to touch a bunch of files for the changed interfaces. In https://reviews.llvm.org/D42955#1026061,

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

labath added a comment. In https://reviews.llvm.org/D42955#1026175, @clayborg wrote: > The dSYM file is a mach-o file that contains symbols only, It is because the > dSYM file (stand alone debug info file) has all of the section definitions > from the main executable, but no section content for

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

labath added a comment. In https://reviews.llvm.org/D42955#1026216, @clayborg wrote: > AS for the ELF example where only debug info is around, it might not be > running into issues if it doesn't contain a symbol table. If it does contain > a symbol table, hopefully it is using the unified secti

[Lldb-commits] [PATCH] D44055: [lldb] Fix "requires global constructor" warning in g_range_specifiers

labath added inline comments. Comment at: source/Breakpoint/BreakpointID.cpp:49 llvm::ArrayRef BreakpointID::GetRangeSpecifiers() { + static llvm::StringRef g_range_specifiers[] = {"-", "to", "To", "TO"}; return llvm::makeArrayRef(g_range_specifiers); You ca

[Lldb-commits] [PATCH] D44056: [lldb] Fix "code unreachable" warning in HostThreadPosix::Cancel

labath added inline comments. Comment at: source/Host/posix/HostThreadPosix.cpp:44 if (IsJoinable()) { #ifndef __ANDROID__ #ifndef __FreeBSD__ xiaobai wrote: > aprantl wrote: > > What about: > > ``` > > #ifdef __ANDROID__ > > error.SetErrorString("HostTh

[Lldb-commits] [PATCH] D44055: [lldb] Fix "requires global constructor" warning in g_range_specifiers

labath added a comment. BTW, to appease older versions of gcc you will need to write the initializer as `= {{"-"}, {"to"}, ...}` (i.e. add extra {} around the strings). https://reviews.llvm.org/D44055 ___ lldb-commits mailing list lldb-commits@list

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

labath abandoned this revision. labath added a comment. I don't think this approach is good while ObjectFileMachO is messing with sections of other object files. At this point I think, I've actually learned enough about MachO to understand what the code is doing there, so I'm going to see if I

[Lldb-commits] [PATCH] D44074: ObjectFileMachO: split CreateSections mega-function into more manageable chunks

labath created this revision. labath added reviewers: clayborg, davide. In an effort to understand the function's operation, I've split it into logical pieces. Parsing of a single segment is moved to a separate function (and the parsing state that is carried from one segment to another is explicit

[Lldb-commits] [PATCH] D44015: Fix std unique pointer not printing.

labath added inline comments. Comment at: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp:65-76 ValueObjectSP tuple_sp = valobj_sp->GetChildMemberWithName(ConstString("_M_t"), true); + + ValueObjectSP tuple_sp_child = + tuple_sp->GetChildMemberWithNa

[Lldb-commits] [PATCH] D44056: [lldb] Fix "code unreachable" warning in HostThreadPosix::Cancel

labath added inline comments. Comment at: source/Host/posix/HostThreadPosix.cpp:44 if (IsJoinable()) { #ifndef __ANDROID__ #ifndef __FreeBSD__ xiaobai wrote: > labath wrote: > > xiaobai wrote: > > > aprantl wrote: > > > > What about: > > > > ``` > > > > #ifd

[Lldb-commits] [PATCH] D44056: [lldb] Fix "code unreachable" warning in HostThreadPosix::Cancel

labath added inline comments. Comment at: source/Host/posix/HostThreadPosix.cpp:44 if (IsJoinable()) { #ifndef __ANDROID__ #ifndef __FreeBSD__ xiaobai wrote: > labath wrote: > > xiaobai wrote: > > > labath wrote: > > > > xiaobai wrote: > > > > > aprantl wrot

[Lldb-commits] [PATCH] D44015: Fix std unique pointer not printing.

labath added a comment. Pretty close, just a couple of style issues. Comment at: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp:62 if (!valobj_backend_sp) -return false; +return NULL; nullptr. Comment at: source/Plu

[Lldb-commits] [PATCH] D44139: WIP: Update selected thread after loading mach core

labath added a comment. I don't know much about OS plugins, but I don't see a reason why this would be mach-specific (so yes, it should probably be done for elf cores as well). Oh, and we also have windows minidumps, but I'm not sure if it should be up to you to update all of these (I'm saying

[Lldb-commits] [PATCH] D44056: [lldb] Fix "code unreachable" warning in HostThreadPosix::Cancel

labath added a comment. Since the android part wasn't obviously NFC, I figured I should be the one to do it. And since the pthread_cancel is not available on android, I've had to add the #else clause as well, which should fix the unreachable warning as well. So I guess you could say this got co

[Lldb-commits] [PATCH] D44074: ObjectFileMachO: split CreateSections mega-function into more manageable chunks

labath added a comment. I agree we should move things to the cpp file when possible, and this is possible. I'm going to update the patch before committing. Thanks for the review. https://reviews.llvm.org/D44074 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

labath added a comment. In https://reviews.llvm.org/D42955#1027142, @clayborg wrote: > Again, we need any objects coming out of the ObjectFile to have the correct > sections. Not sure how we would accomplish that with a solution where the > dSYM file uses it own useless sections. I think we h

[Lldb-commits] [PATCH] D44074: ObjectFileMachO: split CreateSections mega-function into more manageable chunks

This revision was automatically updated to reflect the committed changes. Closed by commit rL326791: ObjectFileMachO: split CreateSections mega-function into more manageable chunks (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[Lldb-commits] [PATCH] D42914: Rewrite TestTargetSymbolsBuildidCase to be more focused

This revision was automatically updated to reflect the committed changes. Closed by commit rL326805: Rewrite TestTargetSymbolsBuildidCase to be more focused (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42914?vs=

[Lldb-commits] [PATCH] D44041: Only replace object file sections when non-empty

labath added a comment. Ok, as of r326805 it should be very easy to write a test for this. You can look at the test in that commit for inspiration. Btw, do you happen to know why we have two copies of the section merging code? While working on that patch I noticed an issue that is caused (in pa

[Lldb-commits] [PATCH] D44157: [elf] Remove one copy of the section merging code

labath created this revision. labath added reviewers: fjricci, jankratochvil. Herald added subscribers: arichardson, emaste. Besides being superfluous, this double merging was actually wrong and causing some sections to be added twice. The reason for that was that the code assumes sectoin IDs are

[Lldb-commits] [PATCH] D44159: Next batch of test-tree-cleaning changes

labath created this revision. labath added a reviewer: aprantl. Herald added subscribers: mehdi_amini, ki.stfu. The changes here fall into several categories. - some tests were redirecting inferior stdout/err to a file. For these I make sure we use an absolute path for the file. I also create a

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

labath added inline comments. Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:3500-3518 + // Create a list of write entries from loadable segments, + // using physical addresses if they aren't all null + size_t header_count = ParseProgramHeaders(); + bool should_u

[Lldb-commits] [PATCH] D43837: Move option parsing out of the Args class

labath added a comment. Is there anything else you wanted me to do here? https://reviews.llvm.org/D43837 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44015: Fix std unique pointer not printing.

This revision was automatically updated to reflect the committed changes. Closed by commit rL327017: Fix std unique pointer pretty-printer for new stl versions (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44015 Fil

[Lldb-commits] [PATCH] D43837: Move option parsing out of the Args class

This revision was automatically updated to reflect the committed changes. Closed by commit rL327110: Move option parsing out of the Args class (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D43837?vs=136355&id=1377

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

labath added a comment. I thought the DoLoadInMemory function (which should probably be called differently then) could just return a vector and the actual writing would still happen in the LoadInMemory function. https://reviews.llvm.org/D42145 ___

[Lldb-commits] [PATCH] D44157: [elf] Remove one copy of the section merging code

This revision was automatically updated to reflect the committed changes. Closed by commit rL327123: [elf] Remove one copy of the section merging code (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44157 Files: lld

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath created this revision. labath added reviewers: jingham, zturner. StringToAddress could end up evaluating an expression, which is a somewhat unexpected behavior of a seemingly simple method. I rename it to EvaluateAddressExpression and move it to the Target class, next to the standard Evalua

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

labath added a comment. Ah, sorry. I didn't get where you were going with your previous comment. The ObjectFile->Process dependency is something that I think we shouldn't have, but whether it's in an header or implementation file makes little difference to me. So I think moving the #include is

[Lldb-commits] [PATCH] D44139: Update selected thread after loading mach core

labath added a comment. I'll leave the cpp change for others to approve (though it certainly looks simpler than the previous one). I just have a couple of drive-by comments on the test. Comment at: packages/Python/lldbsuite/test/functionalities/postmortem/mach-core/TestMachC

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

labath accepted this revision. labath added a comment. I like this a lot. That's the kind of change I wanted to do as a follow-up one day. Thank you. https://reviews.llvm.org/D42145 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://l

[Lldb-commits] [PATCH] D44342: Introduce a setting to disable Spotlight while running the test suite

labath added a comment. Does that mean we can remove the `.noindex` thingy from the build folder name? (In any case, the change seems fine to me). https://reviews.llvm.org/D44342 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://list

[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)

labath created this revision. labath added reviewers: krytarowski, zturner. Herald added a subscriber: mgorny. This patch consists of two relatively independent changes: - first, I teach the standalone build to detect the case when llvm was built in the shared mode and set the appropriate cmake

[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)

labath added a comment. Ah, yes, another build mode :/. Are you doing a standalone build or not? I am pretty sure this will be fine for an integrated build, but I have no idea what will it do in the standalone mode... https://reviews.llvm.org/D44379 __

[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)

labath planned changes to this revision. labath added a comment. Right, so this will not work for the BUILD_SHARED_LIBS case, and there doesn't seem to be an easy way to make it work from this end. I'm going to try fixing this from the llvm side and come back to this if we still need the pthread

[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)

labath added inline comments. Comment at: cmake/modules/LLDBConfig.cmake:354 + + check_library_exists(dl dlopen "" HAVE_LIBDL) + if (HAVE_LIBDL) krytarowski wrote: > krytarowski wrote: > > labath wrote: > > > krytarowski wrote: > > > > mgorny wrote: > > > > > k

<    4   5   6   7   8   9   10   11   12   13   >