[Lldb-commits] [PATCH] D47250: Move ObjectFile initialization out of SystemInitializerCommon

2018-05-24 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL333182: Move ObjectFile initialization out of SystemInitializerCommon (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47250

[Lldb-commits] [PATCH] D47275: 1/3: DWARFDIE split out to DWARFBasicDIE

2018-05-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47275#215, @clayborg wrote: > I like DWARFFirstDIE. Pavel should ok this too. I said what I think of DWARFFirstDIE. I'd like to hear from you what you think about the is-a issue I mentioned. https://reviews.llvm.org/D47275 _

[Lldb-commits] [PATCH] D47275: 1/3: DWARFDIE split out to DWARFBasicDIE

2018-05-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47275#254, @clayborg wrote: > In https://reviews.llvm.org/D47275#1110772, @labath wrote: > > > I don't think a name like `DWARFUnitDIE` is a good one bacause it would > > make a weird `is-a` relationship (a DWARFDIE represetning a DW_TAG_v

[Lldb-commits] [PATCH] D47275: 1/3: DWARFDIE split out to DWARFBasicDIE

2018-05-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Yes, Base is fine. Thank you. https://reviews.llvm.org/D47275 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47342: Move SystemInitializerFull header to source/API

2018-05-24 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Fine by me. https://reviews.llvm.org/D47342 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

[Lldb-commits] [PATCH] D46810: 3/3: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-05-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. +1 for the cleanup. The `m_die_array_size` function is just weird Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:312 void DWARFUnit::ClearDIEs(bool keep_compile_unit_die) { + m_die_array.clear(); You are ignoring the keep_

[Lldb-commits] [PATCH] D47302: [lldb, lldb-mi] Add method AddCurrentTargetSharedObjectPath to the SBDebugger.

2018-05-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47302#641, @clayborg wrote: > Be sure to not pass through any experimental settings. About that, do we want to have some non-api breaking way of accessing the settings (like, `SBSomething GetSetting(std::string setting_name)`). This coul

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

2018-05-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Moving the assertion into `ClangModulesDeclVendor` seems like a good idea to me. If that's the only place that actually requires the value to be non-empty, then it should be the thing asserting it (we can put a helpful message in the assert statement telling the user wha

[Lldb-commits] [PATCH] D47368: ManualDWARFIndex: Treat DW_TAG_subprogram and DW_TAG_inlined_subroutine the same way

2018-05-25 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: clayborg, JDevlieghere. Herald added a subscriber: eraman. We were treating subprograms and inlined subroutines differently when building the index. The difference was in which indexes were individual tags inserted (subprograms went to all inde

[Lldb-commits] [PATCH] D47384: Remove dependency from Host to clang

2018-05-26 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: aprantl. labath added a comment. +Adrian. I remember looking at the path computation code a while ago (I think it was in the context of python though) and concluding that something like this would be needed. Overall the patch seems fine to me, but I want to make sure it

[Lldb-commits] [PATCH] D47368: ManualDWARFIndex: Treat DW_TAG_subprogram and DW_TAG_inlined_subroutine the same way

2018-05-29 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL98: ManualDWARFIndex: Treat DW_TAG_subprogram and DW_TAG_inlined_subroutine the… (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.ll

[Lldb-commits] [PATCH] D47420: Remove Linux-specific includes for posix/FileSystem.cpp

2018-05-29 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I bet this stopped being necessary when most of the filesystem code was moved into llvm. https://reviews.llvm.org/D47420 ___ lldb-commits mailin

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

2018-05-29 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: clayborg, JDevlieghere. Herald added a subscriber: aprantl. When searching for methods only, we need to do extra work to make sure the functions we get from the apple tables are indeed methods. Previously we were resolving the DIE into a Symbol

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

2018-05-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:222 +return true; + return GetReferencedDIE(DW_AT_specification) + .GetParent() clayborg wrote: > I can never remember when a DW_AT_abstract_origin might be used. Mig

[Lldb-commits] [PATCH] D47495: Support relative paths with less than two components in DBGSourcePathRemapping

2018-05-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I was also thinking whether this behavior needs to be conditional. If nothing depends on this, then I'm all for changing the condition. However, my question is whether "." is the only path we should treat this way. I'm thinking it would be more consistent to give the roo

[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

2018-05-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. The idea that came to me while looking at this is testing this gdb-client style. This would allow you to mock the server responses to allocation and e.g. test handling of allocation failures. However, the problem is these tests sit on top of SBAPI and there seems to be

[Lldb-commits] [PATCH] D47495: [FileSpec] Re-implmenet removeLastPathComponent

2018-05-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: unittests/Utility/FileSpecTest.cpp:342 + EXPECT_FALSE(fs_posix_relative.RemoveLastPathComponent()); + EXPECT_STREQ("foo", fs_posix_relative.GetCString()); + Is this the behavior you want here? I was thinking we could fo

[Lldb-commits] [PATCH] D47495: [FileSpec] Re-implmenet removeLastPathComponent

2018-05-30 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. Comment at: unittests/Utility/FileSpecTest.cpp:342 + EXPECT_FALSE(fs_posix_relative.RemoveLastPathComponent()); + EXPECT_STREQ("foo", fs_posix_relative.GetCString()); + JDevlieghere wrote: > labath wr

[Lldb-commits] [PATCH] D47535: [FileSpec] Add support for lambdas to EnumerateDirectory. NFC

2018-05-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Could we just get rid of the baton version? Repository: rL LLVM https://reviews.llvm.org/D47535 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47535: [FileSpec] Add support for lambdas to EnumerateDirectory. NFC

2018-05-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Actually, I wonder if we shouldn't just deprecate this function altogether. What was your motivation for this patch? It seems we already have `llvm::fs::(recursive_)directory_iterator` for this purpose. It would be great if we could use that for all new code. Have you lo

[Lldb-commits] [PATCH] D47535: [FileSpec] Add support for lambdas to EnumerateDirectory. NFC

2018-05-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: zturner. labath added a comment. In https://reviews.llvm.org/D47535#1116392, @JDevlieghere wrote: > In https://reviews.llvm.org/D47535#1116364, @labath wrote: > > > Actually, I wonder if we shouldn't just deprecate this function altogether. > > What was your motivation f

[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

2018-05-30 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Thank you for making the changes. This looks fine to me. The more testing, the better. Comment at: tools/lldb-test/lldb-test.cpp:532 + // Print the result of the allocation

[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc

2018-05-31 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I don't know much about IRMemoryMap myself, but this does seem uncontroversial. Nonetheless, I did manage to find something to rip into. :D Comment at: source/Expression/IRMemoryMap.cpp:312-316 +// Round up the requested size to an aligned value, if

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

2018-05-31 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. shrink_to_fit should be fine. I wouldn't expect to see any difference as it is implemented (in libstdc++ at least) using the swap trick too. https://reviews.llvm.org/D47492 ___

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

2018-05-31 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Well, if a type is trivially copyable (which DWARFDebugInfo **should** be, though I haven't checked), it should be able to do the copy via a single `memcpy` instead of a bunch of constructor calls. That should be more-or-less equivalent to realloc. If this does not end

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

2018-05-31 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 149253. labath added a comment. Implement recursive search in IsMethod() https://reviews.llvm.org/D47470 Files: lit/SymbolFile/DWARF/find-method-local-struct.cpp source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp source/Plugins/SymbolFile/DWARF/DWARFD

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

2018-05-31 Thread Pavel Labath via Phabricator via lldb-commits
labath marked 4 inline comments as done. labath added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:153 +return true; + bool looking_for_methods = name_type_mask & eFunctionNameTypeMethod; + return looking_for_methods == die.IsMethod(); --

[Lldb-commits] [PATCH] D46885: Remove append parameter to FindGlobalVariables

2018-05-31 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thanks for the patch. I've clang-formatted it and committed in r333639. https://reviews.llvm.org/D46885 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46885: Remove append parameter to FindGlobalVariables

2018-05-31 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL333639: Remove append parameter to FindGlobalVariables (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D46885?vs=146848&id

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

2018-05-31 Thread Pavel Labath via Phabricator via lldb-commits
labath marked 4 inline comments as done. labath added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:222 +return true; + return GetReferencedDIE(DW_AT_specification) + .GetParent() JDevlieghere wrote: > labath wrote: > > clayb

[Lldb-commits] [PATCH] D47579: dotest: make inline tests compatible with -f

2018-05-31 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: JDevlieghere, aprantl, tberghammer. Herald added a subscriber: eraman. This is split off from https://reviews.llvm.org/D47265 where I needed to be able to invoke every test with -f. That patch is kinda dead now, but this part seems like a good

[Lldb-commits] [PATCH] D47579: dotest: make inline tests compatible with -f

2018-05-31 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47579#1117457, @JDevlieghere wrote: > If I correctly understand this change, this might make it possible to apply > the `add_test_categories` decorator to an inline test. We had issues with > this when introducing the swiftpr category because

[Lldb-commits] [PATCH] D47579: dotest: make inline tests compatible with -f

2018-05-31 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 149269. labath added a comment. - obliterate using_dsym property - reduce confusion in the MakeInlineTest function (variable "test" used for multiple things) https://reviews.llvm.org/D47579 Files: packages/Python/lldbsuite/test/lldbinline.py Index: pack

[Lldb-commits] [PATCH] D47612: Add dependency on clang-headers when building LLDB.framework using CMake

2018-06-01 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I don't understand the interactions between lldb framework and clang headers, but if there isn't a better person to review this, I'm happy to rubber-stamp it. :) https://reviews.llvm.org/D47

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

2018-06-01 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 149423. labath marked an inline comment as done. labath added a comment. I have a feeling we are starting to over-engineer this. Visiting referenced dies sounds like a useful utility in general, though I think most of the users will be content with just visitin

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

2018-06-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. ps: the specificationsAndAbstractOrigins sounded like too big of a mouthful. I've invented the name "elaborating dies" for that. https://reviews.llvm.org/D47470 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http:

[Lldb-commits] [PATCH] D47625: [cmake] Detect presence of wide-char libedit at build time

2018-06-01 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: krytarowski, uweigand, jankratochvil, timshen, beanz. Herald added a subscriber: mgorny. Instead of hardcoding a list of platforms where libedit is known to have wide char support we detect this in cmake. The main motivation for this is attempt

[Lldb-commits] [PATCH] D47625: [cmake] Detect presence of wide-char libedit at build time

2018-06-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: include/lldb/Host/Editline.h:72-76 #ifdef EL_CLIENTDATA /* editline with wide support + wide char read function */ using EditLineGetCharType = wchar_t; #else using EditLineGetCharType = char; #endif It's not fully

[Lldb-commits] [PATCH] D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

2018-06-01 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: JDevlieghere, clayborg, jingham. Herald added subscribers: aprantl, mehdi_amini, mgorny. This patch is pretty much a big noop. It adds the ability to create a DebugNamesDWARFIndex class, but the class itself is not implemented in any useful way

[Lldb-commits] [PATCH] D47625: [cmake] Detect presence of wide-char libedit at build time

2018-06-04 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: include/lldb/Host/Editline.h:72-76 #ifdef EL_CLIENTDATA /* editline with wide support + wide char read function */ using EditLineGetCharType = wchar_t; #else using EditLineGetCharType = char; #endif christos wrote

[Lldb-commits] [PATCH] D47646: [IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands

2018-06-04 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks great, thanks for doing this. As far as windows goes, I believe that there we simply don't have the memory allocation part implemented, so it's not surprising that these tests fail. It

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

2018-06-04 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL333878: AppleDWARFIndex: Get function method-ness directly from debug info (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47

[Lldb-commits] [PATCH] D47147: DWARFIndex: Reduce duplication in the GetFunctions methods

2018-06-04 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 149702. labath added a comment. This is a rewrite of the original patch with the same idea but different approach. Now that Apple index determines method-ness straight from the debug info, we don't need to resolve the functions into SymbolContexts. This removes

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

2018-06-05 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47492#1122039, @jankratochvil wrote: > In https://reviews.llvm.org/D47492#1121543, @dblaikie wrote: > > > Happy to help explain it - which part(s) are you having a bit of trouble > > with? > > > What's wrong on this implementation >

[Lldb-commits] [PATCH] D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

2018-06-05 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:122 +PropertyDefinition g_experimental_properties[] = { +{"use-debug-names", OptionValue::eTypeBoolean, true, 0, nullptr, nullptr, + "Use .debug_names index section."}, -

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

2018-06-05 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Well, with these kinds of safeguards **I think** this could work. If I was the maintainer of std::vector in libstdc++ I am not sure if I would accept such a patch, but I am not, so feel free to try. :) Repository: rL LLVM https://reviews.llvm.org/D47492 ___

[Lldb-commits] [PATCH] D47147: DWARFIndex: Reduce duplication in the GetFunctions methods

2018-06-05 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL334004: DWARFIndex: simplify GetFunctions methods (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47147?vs=149702&id=1499

[Lldb-commits] [PATCH] D47579: dotest: make inline tests compatible with -f

2018-06-05 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL334009: dotest: make inline tests compatible with -f (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47579 Files: lldb/tru

[Lldb-commits] [PATCH] D47265: WIP: lit: Run each test separately

2018-06-05 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 149936. labath added a comment. No real change in functionality, just updating the diff to remove the unrelated changes. https://reviews.llvm.org/D47265 Files: lit/Suite/lldbtest.py Index: lit/Suite/lldbtest.py ===

[Lldb-commits] [PATCH] D47062: Suggest lldb-dotest to reproduce a failure.

2018-06-05 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47062#1109399, @JDevlieghere wrote: > I'm going to hold off on this until we have decided what to do for > https://reviews.llvm.org/D46005. If we run the test cases as separate lit > invocations, then the test format is in a better position t

[Lldb-commits] [PATCH] D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

2018-06-05 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 149964. labath added a comment. This makes the setting non-experimental and renames it to "ignore-file-indexes". I've also wired up the index dump methods to Module::Dump so that I could write a test that apple indexes are indeed being used after this. https:

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

2018-06-05 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: clayborg, JDevlieghere. Herald added a subscriber: aprantl. This adds the ability to lookup variables to the DWARF v5 index class. find-basic-variable.cpp test is extended to cover this scenario as well, and I have added a new test which verifi

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

2018-06-05 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:70 + continue; + +Append(entry, offsets); clayborg wrote: > Need to check for the context matches here if context is not empty. Hmm... thanks for pointing

[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47801#1123051, @sas wrote: > Don't we risk creating circular dependencies with this? What happens when a > tool depends on liblldb? you'll have theTool depend on liblldb and liblldb > depend on theTool. The same thought has occurred to me,

[Lldb-commits] [PATCH] D47797: [lldb-mi] Re-implement MI -exec-next command.

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: tools/lldb-mi/MICmdCmdExec.cpp:384 if (nThreadId != UINT64_MAX) -strCmd += CMIUtilString::Format(" %llu", nThreadId); - rDebugger.GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult, -

[Lldb-commits] [PATCH] D47812: [lldb] [lit] Do not run Python tests w/ LLDB_DISABLE_PYTHON

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Makes sense to me. https://reviews.llvm.org/D47812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

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

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I have reverted this because of the broken tests. However, I have to also ask: isn't there better way to test this? (one that does not depend on opaque checked-in binaries). On linux, I could check-in a .s file which has the line table exactly as I want it and then have

[Lldb-commits] [PATCH] D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL334088: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 150149. labath added a comment. Ok, so it turns out we **were** passing qualified names into the GetGlobalVariables function. In the apple-tables case we were then searching based on the basename and completely ignoring the context. This resulted in incorrect m

[Lldb-commits] [PATCH] D47832: DebugNamesDWARFIndex: Add support for partial indexes

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: JDevlieghere, clayborg. Herald added a subscriber: aprantl. It possible that a single module has indexed and non-indexed compile units. In this case, we can use the fast indexed lookup for the first ones and fall back to the manual index for th

[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47801#1123895, @xiaobai wrote: > > - rename INCLUDE_IN_FRAMEWORK to something more neutral (USED_BY_LIBLLDB or > > whatever) > > - make the liblldb -> tool dependency not conditioned by > > LLDB_BUILD_FRAMEWORK > > - remove the lldb->tool dep

[Lldb-commits] [PATCH] D47797: [lldb-mi] Re-implement MI -exec-next command.

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lit/tools/lldb-mi/exec/exec-next.test:19 + +-exec-next --thread 0 +# Check that exec-next can process the case of invalid thread ID. aprantl wrote: > 0 feels like it might actually be a valid thread id on some systems.. p

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

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47708#1124429, @zturner wrote: > Do you just need a pdb, or does it really need to be a vs pdb? lld can > generate high quality pdbs now. So it might be possible to use lld to link > and produce a pdb when you run the test. > > Pavel’s sugges

[Lldb-commits] [PATCH] D47838: [lldb-mi] Re-implement MI -exec-step command.

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I don't know if there is any spec about what the gdb-mi protocol should use for thread identification, but I think the most important part is to be consistent. If you use indexes in one place and tid's in another you'll confuse the hell out of clients trying to talk to l

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL334181: DebugNamesDWARFIndex: Add ability to lookup variables (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47781 Files:

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

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47708#1124733, @aleksandr.urakov wrote: > > I think these kinds of checks would be a nice fit for a lldb-test symbols > > --verify option. > > Can you explain this in detail, please? We have an lldb-test executable, which we use for writing

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

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I don't think that's necessary. The actual fix is really simple and this whole discussion is just about figuring out what to do with tests. https://reviews.llvm.org/D47708 ___ lldb-commits mailing list lldb-commits@lists.llv

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

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Btw, would it be possible to use the `/order` directive to achive what you want? (/order:function_from_first_file,function_from_second_file,another_function_from_first_file) https://reviews.llvm.org/D47708 ___ lldb-commits

[Lldb-commits] [PATCH] D47832: DebugNamesDWARFIndex: Add support for partial indexes

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL334185: DebugNamesDWARFIndex: Add support for partial indexes (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47832?vs=15

[Lldb-commits] [PATCH] D47881: DebugNamesDWARFIndex: Implement GetFunctions method

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: clayborg, JDevlieghere. Herald added a subscriber: aprantl. This patch implements the non-regex variant of GetFunctions. To share more code with the Apple implementation, I've extracted the common filtering code from that class into a utility f

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

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thank you for implementing the lldb-test extension. Now that we have that, and the `/order`, we should be able to get rid of the binaries for the function-level-linking test. You should be able to rewrite it into something like this: lit/SymbolFile/PDB/function-level-li

[Lldb-commits] [PATCH] D47797: [lldb-mi] Re-implement MI -exec-next command.

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. looks good to me https://reviews.llvm.org/D47797 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47708#1125030, @aleksandr.urakov wrote: > In https://reviews.llvm.org/D47708#1124994, @zturner wrote: > > > As a general rule, lld-link is command line compatible with MSVC and > > clang-cl is command line compatible with cl. So, the /order o

[Lldb-commits] [PATCH] D47889: Use llvm::VersionTuple instead of manual version marshalling

2018-06-07 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: zturner, friss, clayborg, jingham. Herald added a subscriber: emaste. This has multiple advantages: - we need only one function argument/instance variable instead of three - no need to default initialize variables - no custom parsing code - Ve

[Lldb-commits] [PATCH] D47415: [lldb, lldb-mi] Re-implement MI -exec-continue command.

2018-06-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: tools/lldb-mi/MICmdCmdExec.cpp:248 } - } else { -// ToDo: Re-evaluate if this is required when application near finished as -// this is parsing LLDB error message -// which seems a hack and is code brittle -const cha

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

2018-06-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thank you for your patience. I am very happy with the final result here. I personally like to keep number of files in a test minimal (hence I inlined the test into the cpp file), but overall that's not really important. https://reviews.llvm.org/D47708 ___

[Lldb-commits] [PATCH] D47881: DebugNamesDWARFIndex: Implement GetFunctions method

2018-06-08 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL334273: DebugNamesDWARFIndex: Implement GetFunctions method (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47881 Files: l

[Lldb-commits] [PATCH] D47415: [lldb, lldb-mi] Re-implement MI -exec-continue command.

2018-06-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: tools/lldb-mi/MICmdCmdExec.cpp:248 } - } else { -// ToDo: Re-evaluate if this is required when application near finished as -// this is parsing LLDB error message -// which seems a hack and is code brittle -const cha

[Lldb-commits] [PATCH] D47625: [cmake] Detect presence of wide-char libedit at build time

2018-06-11 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: include/lldb/Host/Editline.h:72-76 #ifdef EL_CLIENTDATA /* editline with wide support + wide char read function */ using EditLineGetCharType = wchar_t; #else using EditLineGetCharType = char; #endif christos wrote

[Lldb-commits] [PATCH] D47625: [cmake] Detect presence of wide-char libedit at build time

2018-06-11 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL334393: [cmake] Detect presence of wide-char libedit at build time (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47625?

[Lldb-commits] [PATCH] D47991: Add method SBThread::StepOver with SBError parameter.

2018-06-11 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D47991#1128433, @aprantl wrote: > Hmm.. it looks like most C++ API calls don't have any documentation. > > http://lldb.llvm.org/cpp_reference/html/classlldb_1_1SBThread.html#a42755a170e127881a5dd65162217f68b > > It does look like the python API

[Lldb-commits] [PATCH] D48060: Introduce lldb-framework CMake target and centralize its logic

2018-06-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I like the idea of centralizing the framework code as much as possible. However, I am not sure how the dependency management fits into this. More details in inline comments. Comment at: CMakeLists.txt:46 + if (NOT APPLE) +message(FATAL_ERROR "LLDB

[Lldb-commits] [PATCH] D48060: Introduce lldb-framework CMake target and centralize its logic

2018-06-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: cmake/modules/LLDBFramework.cmake:45 + + add_dependencies(lldb-framework liblldb lldb-argdumper lldb-server lldb-framework-headers) + add_dependencies(finish_swig lldb-framework) xiaobai wrote: > labath wrote: > > Mayb

[Lldb-commits] [PATCH] D48084: [FileSpec] Delegate common operations to llvm::sys::path

2018-06-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: source/Utility/FileSpec.cpp:244-246 + // Only update style if explicitly requested. + if (style) +m_style = (*style == Style::native) ? GetNativeStyle() : *style; I don't really have a clear opinion on whether the n

[Lldb-commits] [PATCH] D48084: [FileSpec] Delegate common operations to llvm::sys::path

2018-06-12 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. Comment at: source/Utility/FileSpec.cpp:244-246 + // Only update style if explicitly requested. + if (style) +m_style = (*style == Style::native) ? GetNativeStyle() : *style; JDevlieghere wrote: >

[Lldb-commits] [PATCH] D48060: Introduce lldb-framework CMake target and centralize its logic

2018-06-13 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: CMakeLists.txt:140 +# lldb-suite is a dummy target that encompasses all the necessary tools and +# libraries for building a fully-functioning lldb. +add_custom_target(lldb-suite) fully-functioning lldb **library**. (this

[Lldb-commits] [PATCH] D48084: [FileSpec] Delegate common operations to llvm::sys::path

2018-06-13 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: source/Utility/FileSpec.cpp:244-246 + // Only update style if explicitly requested. + if (style) +m_style = (*style == Style::native) ? GetNativeStyle() : *style; JDevlieghere wrote: > labath wrote: > > JDevlieghere

[Lldb-commits] [PATCH] D48177: Suppress SIGSEGV on Android when the program will recover

2018-06-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thank you for implementing this. We've had code like this in android studio for a long time, but it's definitely better doing it in lldb instead. I'll give some more background so that Jim and anyone else looking at this can understand what's going on. These files (I for

[Lldb-commits] [PATCH] D47889: Use llvm::VersionTuple instead of manual version marshalling

2018-06-15 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 151484. labath added a comment. Fix a typo in netbsd code. https://reviews.llvm.org/D47889 Files: include/lldb/Core/Module.h include/lldb/Host/freebsd/HostInfoFreeBSD.h include/lldb/Host/linux/HostInfoLinux.h include/lldb/Host/macosx/HostInfoMacOSX.h

[Lldb-commits] [PATCH] D48215: Remove dependency from Host to python

2018-06-15 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: zturner, jingham, davide. Herald added a subscriber: mgorny. The only reason python was used in the Host module was to compute the python path. I resolve this the same way as https://reviews.llvm.org/D47384 did for clang, moving the path compu

[Lldb-commits] [PATCH] D48215: Remove dependency from Host to python

2018-06-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Actually, my plan was to follow this up with a patch which removes usages of that enumeration from the internal api altogether (basically do a `s/GetLLDBPath(ePathTypeXXX/GetXXXPath`). That should make the internal api nicer, and provide a clean separation between the in

[Lldb-commits] [PATCH] D48215: Remove dependency from Host to python

2018-06-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. (pressed return too soon) Would that address your reservations about this idea? https://reviews.llvm.org/D48215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-co

[Lldb-commits] [PATCH] D48215: Remove dependency from Host to python

2018-06-15 Thread Pavel Labath via Phabricator via lldb-commits
labath planned changes to this revision. labath added a comment. Ok, I'll start with the other patch first. https://reviews.llvm.org/D48215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb

[Lldb-commits] [PATCH] D48060: Introduce lldb-framework CMake target and centralize its logic

2018-06-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. This is looking very good, I just have one more quick question. I am wondering whether we couldn't simplify this dependency management a tad bit more. Would the thing I'm proposing below work for you? Comment at: CMakeLists.txt:147-153 + if (LLDB_CAN

[Lldb-commits] [PATCH] D48060: Introduce lldb-framework CMake target and centralize its logic

2018-06-18 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good. I'm really happy with how this turned out. Thank you for your patience. https://reviews.llvm.org/D48060 ___ lldb-commits mailing lis

[Lldb-commits] [PATCH] D48272: Replace HostInfo::GetLLDBPath with specific functions

2018-06-18 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: clayborg, zturner. Instead of a function taking an enum value determining which path to return, we now have a suite of functions, each returning a single path kind. This makes it easy to move the python-path function into a specific plugin in a

[Lldb-commits] [PATCH] D47889: Use llvm::VersionTuple instead of manual version marshalling

2018-06-18 Thread Pavel Labath via Phabricator via lldb-commits
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 rL334950: Use llvm::VersionTuple instead of manual version marshalling (authored by labath, committed by ). Herald added a s

[Lldb-commits] [PATCH] D48272: Replace HostInfo::GetLLDBPath with specific functions

2018-06-19 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D48272#1135373, @clayborg wrote: > Looks good. My only question is do we not require people to only fill in the > directory portion of the FileSpec anymore for these functions? I am fine with > way since hopefully FileSpec::AppendPathComponent

[Lldb-commits] [PATCH] D48272: Replace HostInfo::GetLLDBPath with specific functions

2018-06-19 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL335052: Replace HostInfo::GetLLDBPath with specific functions (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48272 Files:

[Lldb-commits] [PATCH] D48215: Remove dependency from Host to python

2018-06-19 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 151922. labath added a comment. Rebase this patch on trunk. Now this patch is simply about moving GetPythonDir from Host module to ScriptInterpreterPython. https://reviews.llvm.org/D48215 Files: include/lldb/Host/macosx/HostInfoMacOSX.h include/lldb/Host

<    1   2   3   4   5   6   7   8   9   10   >