[Lldb-commits] [PATCH] D62756: Be consistent when adding names and mangled names to the index

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Being consistent definitely sounds like a good idea. Since this does change behavior somewhat, I'm wondering whether it would make sense to add a test here. The thing that's not clear to me is whether this change in behavior is only theoretical, or if it can also happen

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I agree with your reasoning about language runtime dependencies, and I think this is the right way to accomplish this. However, I just want to add that from an lldb-server POV, even the fact that we pull in the `Process` class into it's dependency graph is a bug. So, wha

[Lldb-commits] [PATCH] D62788: [lldb-server unittest] Add missing teardown logic

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. > Not sure why this isn't failing the build bots.. (unless they're running > without asserts?). The build bots (and lit, in general) runs each of the unit tests in isolation (so it first gets a list of all tests, then runs the executable wi

[Lldb-commits] [PATCH] D62788: [lldb-server unittest] Add missing teardown logic

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I should add (since I'm guessing you found this problem while trying to add unit tests for your lldb-server changes) a bit of history about these tests. There are two kinds of tests for lldb-server. The python tests (test/testcases/tools/lldb-server) are older ones. The

[Lldb-commits] [PATCH] D62796: [Target] Generalize some behavior in Target::SymbolsDidLoad

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: source/Target/Target.cpp:1670-1672 + if (LanguageRuntime *objc_runtime = + m_process_sp->GetLanguageRuntime(lldb::eLanguageTypeObjC)) objc_runtime->SymbolsDidLoad(module_list); Same question abo

[Lldb-commits] [PATCH] D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. If this is supposed to be truly language-agnostic, shouldn't we be iterating over all language plugins and asking all of them for the "runtime symbol" (not that I know what a runtime symbol is, really)? Otherwise, this is still language-specific behavior, even though it'

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::SetCompilerTypeFromPersistentDecl

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. The idea seems right, but I'm not too familiar with this part of the code. Someone else ought to look at this too. Comment at: include/lldb/Expression/ExpressionVariable.h:235-236 + virtual bool SetCompilerTypeFromPersistentDecl(ConstString type_name

[Lldb-commits] [PATCH] D62499: Create a generic handler for Xfer packets

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thanks for writing the test, I have a bunch of more comments on the test itself, but hopefully this will be the last iteration. :) Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2766 // Grab the auxv data.

[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:629-631 + llvm::Optional vdso_base = + m_auxv->GetAuxValue(AuxVector::AUXV_AT_SYSINFO_EHDR); + if (vdso_base)

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Regarding the test, I am wondering whether requiring the `/proc/%d/maps` and the linker rendezvous structure to match isn't too strict of a requirement. Dynamic linkers (and particularly android dynamic linkers) do a lot of crazy stuff, and trying to assert this invarian

[Lldb-commits] [PATCH] D62714: [FormatEntity] Ignore ASCII escape sequences when colors are disabled.

2019-06-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: lldb/trunk/include/lldb/Core/FormatEntity.h:44 ParentString, - InsertString, + EscapeCode, Root, InsertString is used for more than just escape codes so it is now not named correctly. For a vari

[Lldb-commits] [PATCH] D62732: [WIP][RISCV] Initial port of LLDB for RISC-V

2019-06-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. You are following all of the patterns for all of the architectures. It would be nice for us to cleanup DynamicRegisterInfo.cpp, Platform.cpp, and Thread.cpp eventually so we don't need to modify these files when a few arch is added by abstracting things into lldb_priva

[Lldb-commits] [PATCH] D62812: [llvm] [CodeView] Move Triple::ArchType → CPUType mapping from LLDB

2019-06-03 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision. mgorny added reviewers: labath, mtrent, compnerd. Herald added a project: LLVM. Since neither the CodeView::CPUType nor Triple::ArchType constants are specific to LLDB, it makes no sense to maintain their mapping there. Instead, introduce a small inline function to pr

[Lldb-commits] [PATCH] D62743: Add color to the default thread and frame format.

2019-06-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Attaching my versions in case you like anything that I did: settings set frame-format frame #${frame.index}: ${ansi.fg.yellow}${frame.pc}${ansi.normal}{ ${ansi.bold}${ansi.underline}${ansi.fg.cyan}${module.file.basename}${ansi.normal}${ansi.fg.cyan} ${function.name-

[Lldb-commits] [PATCH] D62788: [lldb-server unittest] Add missing teardown logic

2019-06-03 Thread António Afonso via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL362406: [lldb-server unittest] Add missing teardown logic (authored by aadsm, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://revi

[Lldb-commits] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a subscriber: dblaikie. labath added a comment. Actually, there was a subtle change of behavior here. Before this change, if we tried to parse a DIE using an abbreviation table from a different file, we would most likely fail immediately because of the fail-safe in GetAbbreviationD

[Lldb-commits] [PATCH] D62796: [Target] Generalize some behavior in Target::SymbolsDidLoad

2019-06-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Seems like we need a Process::SymbolsDidLoad(...) function that we can call. It should iterate over all loaded language runtimes (only loaded ones) and forward the list. =

[Lldb-commits] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

2019-06-03 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment. In D62634#1527533 , @labath wrote: > Actually, there was a subtle change of behavior here. Before this change, if > we tried to parse a DIE using an abbreviation table from a different file, we > would most likely fail immediate

[Lldb-commits] [PATCH] D62743: Add color to the default thread and frame format.

2019-06-03 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Sorry, I got more comments :-) Comment at: lldb/source/Core/Debugger.cpp:124 +#define FILE_COLOR "${ansi.fg.yellow}" +#define STOP_COLOR "${ansi.fg.red}$" + Thanks, but I was thinking more about something that could be replaced dynamic

[Lldb-commits] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

2019-06-03 Thread Paul Robinson via Phabricator via lldb-commits
probinson added a comment. In D62634#1527575 , @dblaikie wrote: > This is intentional behavior in clang (controllable under the > -f[no-]split-dwarf-inlining flag, and modified by the > -f[no-]debug-info-for-profiling flag). > > This extra debug info is

Re: [Lldb-commits] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

2019-06-03 Thread Greg Clayton via lldb-commits
> On Jun 3, 2019, at 8:33 AM, Pavel Labath via Phabricator > wrote: > > labath added a subscriber: dblaikie. > labath added a comment. > > Actually, there was a subtle change of behavior here. Before this change, if > we tried to parse a DIE using an abbreviation table from a different file,

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
xiaobai marked an inline comment as done. xiaobai added a comment. In D62755#1527004 , @labath wrote: > However, I just want to add that from an lldb-server POV, even the fact that > we pull in the `Process` class into it's dependency graph is a bug. Ag

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

2019-06-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:37-39 + lldb::addr_t link_map; + lldb::addr_t base_addr; + lldb::addr_t ld_addr; These seem very linux specific. Why do we need 3 addresses here? Seems like we s

[Lldb-commits] [PATCH] D62812: [llvm] [CodeView] Move Triple::ArchType → CPUType mapping from LLDB

2019-06-03 Thread Saleem Abdulrasool via Phabricator via lldb-commits
compnerd added inline comments. Comment at: llvm/include/llvm/DebugInfo/CodeView/CodeView.h:145 + switch (ArchType) { + case Triple::ArchType::aarch64: +return CPUType::ARM64; I that `aarch64_be` and `aarch64_32` should be included in this. ==

[Lldb-commits] [PATCH] D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment. In D62795#1527035 , @labath wrote: > If this is supposed to be truly language-agnostic, shouldn't we be iterating > over all language plugins and asking all of them for the "runtime symbol" > (not that I know what a runtime symbo

[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

2019-06-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:91-92 m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID); - - m_auxv.reset(new AuxVector(m_process)); + DataExtractor auxv_data(m_process

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::SetCompilerTypeFromPersistentDecl

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
xiaobai marked an inline comment as done. xiaobai added inline comments. Comment at: include/lldb/Expression/ExpressionVariable.h:235-236 + virtual bool SetCompilerTypeFromPersistentDecl(ConstString type_name, + CompilerType &com

[Lldb-commits] [PATCH] D62796: [Target] Generalize some behavior in Target::SymbolsDidLoad

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
xiaobai marked 2 inline comments as done. xiaobai added inline comments. Comment at: source/Target/Target.cpp:1670 if (m_process_sp) { - LanguageRuntime *runtime = - m_process_sp->GetLanguageRuntime(lldb::eLanguageTypeObjC); - if (runtime) { -ObjCL

[Lldb-commits] [PATCH] D62796: [Target] Generalize some behavior in Target::SymbolsDidLoad

2019-06-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: source/Target/Target.cpp:1670 if (m_process_sp) { - LanguageRuntime *runtime = - m_process_sp->GetLanguageRuntime(lldb::eLanguageTypeObjC); - if (runtime) { -ObjCLanguageRuntime *objc_runtime = (ObjCLangu

[Lldb-commits] [PATCH] D62501: Implement GetSharedLibraryInfoAddress

2019-06-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:430 + lldb::addr_t GetELFImageInfoAddress(); + This doesn't seem like

[Lldb-commits] [PATCH] D62771: [LLDBRegisterNum] Update function call llvm::codeview::getRegisterNames(CPUType) in lldb

2019-06-03 Thread Wanyi Ye via Phabricator via lldb-commits
kusmour abandoned this revision. kusmour marked an inline comment as done. kusmour added a comment. D62772 toke care of this Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62771/new/ https://reviews.llvm.org/D62771 __

[Lldb-commits] [PATCH] D62501: Implement GetSharedLibraryInfoAddress

2019-06-03 Thread António Afonso via Phabricator via lldb-commits
aadsm marked an inline comment as done. aadsm added inline comments. Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:430 + lldb::addr_t GetELFImageInfoAddress(); + clayborg wrote: > This doesn't seem like it belongs in the base class. Maybe j

[Lldb-commits] [PATCH] D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
xiaobai updated this revision to Diff 202763. xiaobai added a comment. Make the behavior in IRExecutionUnit actually language agnostic. Also, reverse the order of the for loops. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62795/new/ https://reviews.llvm.org/D62795 Files: include/l

[Lldb-commits] [PATCH] D62796: [Target] Generalize some behavior in Target::SymbolsDidLoad

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
xiaobai updated this revision to Diff 202782. xiaobai added a comment. Address comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62796/new/ https://reviews.llvm.org/D62796 Files: include/lldb/Target/LanguageRuntime.h include/lldb/Target/ObjCLanguageRuntime.h source/Target/Ta

[Lldb-commits] [PATCH] D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

2019-06-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: include/lldb/Target/ObjCLanguageRuntime.h:270-272 - virtual lldb::addr_t LookupRuntimeSymbol(ConstString name) { -return LLDB_INVALID_ADDRESS; - } Which language has this filled in? Only Swift? CHANGES SINCE LAS

[Lldb-commits] [PATCH] D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
xiaobai marked an inline comment as done. xiaobai added inline comments. Comment at: include/lldb/Target/ObjCLanguageRuntime.h:270-272 - virtual lldb::addr_t LookupRuntimeSymbol(ConstString name) { -return LLDB_INVALID_ADDRESS; - } clayborg wrote: > Which l

[Lldb-commits] [PATCH] D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

2019-06-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision. clayborg added inline comments. This revision is now accepted and ready to land. Comment at: include/lldb/Target/ObjCLanguageRuntime.h:270-272 - virtual lldb::addr_t LookupRuntimeSymbol(ConstString name) { -return LLDB_INVALID_ADDRESS; - } -

[Lldb-commits] [PATCH] D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
xiaobai marked an inline comment as done. xiaobai added inline comments. Comment at: include/lldb/Target/ObjCLanguageRuntime.h:270-272 - virtual lldb::addr_t LookupRuntimeSymbol(ConstString name) { -return LLDB_INVALID_ADDRESS; - } clayborg wrote: > xiaobai

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::SetCompilerTypeFromPersistentDecl

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
xiaobai updated this revision to Diff 202805. xiaobai added a comment. Pavel's suggestion Renamed method from "SetCompilerTypeFromPersistentDecl" to "GetCompilerTypeFromPersistentDecl" CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62797/new/ https://reviews.llvm.org/D62797 Files: in

[Lldb-commits] [lldb] r362456 - Add support for mid-function epilogues on x86 that end in a non-local jump.

2019-06-03 Thread Jason Molenda via lldb-commits
Author: jmolenda Date: Mon Jun 3 15:34:12 2019 New Revision: 362456 URL: http://llvm.org/viewvc/llvm-project?rev=362456&view=rev Log: Add support for mid-function epilogues on x86 that end in a non-local jump. The x86 assembly inspection engine has code to support detecting a mid-function epilog

[Lldb-commits] [PATCH] D62764: Detect x86 mid-function epilogues that end in a jump

2019-06-03 Thread Phabricator 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 rL362456: Add support for mid-function epilogues on x86 that end in a non-local jump. (authored by jmolenda, committed by ).

[Lldb-commits] [lldb] r362458 - [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

2019-06-03 Thread Alex Langford via lldb-commits
Author: xiaobai Date: Mon Jun 3 15:41:48 2019 New Revision: 362458 URL: http://llvm.org/viewvc/llvm-project?rev=362458&view=rev Log: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime Summary: LookupRuntimeSymbol seems like a general LanguageRuntime method. Although no o

[Lldb-commits] [PATCH] D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL362458: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime (authored by xiaobai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository

[Lldb-commits] [lldb] r362461 - [Target] Generalize some behavior in Target::SymbolsDidLoad

2019-06-03 Thread Alex Langford via lldb-commits
Author: xiaobai Date: Mon Jun 3 16:12:11 2019 New Revision: 362461 URL: http://llvm.org/viewvc/llvm-project?rev=362461&view=rev Log: [Target] Generalize some behavior in Target::SymbolsDidLoad Summary: SymbolsDidLoad is currently only implemented for ObjCLanguageRuntime, but that doesn't mean th

[Lldb-commits] [PATCH] D62796: [Target] Generalize some behavior in Target::SymbolsDidLoad

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL362461: [Target] Generalize some behavior in Target::SymbolsDidLoad (authored by xiaobai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHAN

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

2019-06-03 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments. Comment at: include/lldb/Target/CPPLanguageRuntime.h:47 + static CPPLanguageRuntime *GetCPPLanguageRuntime(Process &process) { +return static_cast( +process.GetLanguageRuntime(lldb::eLanguageTypeC_plus_plus)); xiao

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

2019-06-03 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This deals with my objections so I'm marking as accepted because that's the only way I can see to unblock. Seems like you were still discussing return types, and I don't want to preempt tha

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

2019-06-03 Thread Wanyi Ye via Phabricator via lldb-commits
kusmour updated this revision to Diff 202824. kusmour added a comment. 1. Limit the `TestReturnValue` for nested struct and class (cpp support) to only x86_64 2. This patch somehow fix the bug: pr36870 for Systerm V ABI (windows is waiting for this

[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

2019-06-03 Thread António Afonso via Phabricator via lldb-commits
aadsm marked 2 inline comments as done. aadsm added inline comments. Comment at: lldb/source/Plugins/Process/Utility/AuxVector.cpp:11 -using namespace lldb; -using namespace lldb_private; +AuxVector::AuxVector(lldb_private::DataExtractor &data) { ParseAuxv(data); } --

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

2019-06-03 Thread Alex Langford via Phabricator via lldb-commits
xiaobai marked an inline comment as done. xiaobai added inline comments. Comment at: include/lldb/Target/CPPLanguageRuntime.h:47 + static CPPLanguageRuntime *GetCPPLanguageRuntime(Process &process) { +return static_cast( +process.GetLanguageRuntime(lldb::eLanguageTyp

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

2019-06-03 Thread António Afonso via Phabricator via lldb-commits
aadsm marked 4 inline comments as done. aadsm added a comment. @labath > Regarding the test, I am wondering whether requiring the /proc/%d/maps and > the linker rendezvous structure to match isn't too strict of a requirement. > Dynamic linkers (and particularly android dynamic linkers) do a lot

[Lldb-commits] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

2019-06-03 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment. In D62634#1527634 , @probinson wrote: > In D62634#1527575 , @dblaikie wrote: > > > This is intentional behavior in clang (controllable under the > > -f[no-]split-dwarf-inlining flag, and m

[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

2019-06-03 Thread António Afonso via Phabricator via lldb-commits
aadsm marked an inline comment as done. aadsm added inline comments. Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:183 + m_process->GetAddressByteSize()); + m_auxv = llvm::make_unique(auxv_data);

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::GetCompilerTypeFromPersistentDecl

2019-06-03 Thread Saleem Abdulrasool via Phabricator via lldb-commits
compnerd accepted this revision. compnerd added a subscriber: clayborg. compnerd added a comment. This revision is now accepted and ready to land. Would be nice to get someone like @clayborg to chime in, but, I think that @labath also seems to think that this is fine. Comment

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

2019-06-03 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Fine by me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62755/new/ https://reviews.llvm.org/D62755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-com

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::GetCompilerTypeFromPersistentDecl

2019-06-03 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I have no problem with the change in general. However, you've introduced the possibility of name collision between convenience type definition in various languages. So it would be good to run through the persistent DECL's for ALL the supported languages and report col

[Lldb-commits] [PATCH] D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

2019-06-03 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. This looks fine to me. For context, ObjC has symbols that point into the runtime that tell you things like the current offsets of the members of an ObjC class. In debug builds the symbols are present, but the runtime doesn't depend on the symbols per se, since it read

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

2019-06-03 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments. Comment at: include/lldb/Target/CPPLanguageRuntime.h:47 + static CPPLanguageRuntime *GetCPPLanguageRuntime(Process &process) { +return static_cast( +process.GetLanguageRuntime(lldb::eLanguageTypeC_plus_plus)); xiao

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

2019-06-03 Thread António Afonso via Phabricator via lldb-commits
aadsm marked an inline comment as done. aadsm added inline comments. Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:37-39 + lldb::addr_t link_map; + lldb::addr_t base_addr; + lldb::addr_t ld_addr; aadsm wrote: > clayborg wrote: > > These see

[Lldb-commits] [PATCH] D62499: Create a generic handler for Xfer packets

2019-06-03 Thread António Afonso via Phabricator via lldb-commits
aadsm updated this revision to Diff 202852. aadsm marked 2 inline comments as done. aadsm added a comment. Move test to a better place, address other comments related to tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62499/new/ https://review

[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

2019-06-03 Thread António Afonso via Phabricator via lldb-commits
aadsm updated this revision to Diff 202853. aadsm marked an inline comment as done. aadsm added a comment. Herald added a subscriber: emaste. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62500/new/ https://reviews.llvm.org/D62500

[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

2019-06-03 Thread António Afonso via Phabricator via lldb-commits
aadsm added inline comments. Comment at: lldb/source/Plugins/Process/Utility/AuxVector.cpp:24-28 + while (true) { +uint64_t type = 0; +uint64_t value = 0; +if (!ReadUInt(data, &offset, &type) || !ReadUInt(data, &offset, &value)) break; claybor

[Lldb-commits] [PATCH] D62501: Implement GetSharedLibraryInfoAddress

2019-06-03 Thread António Afonso via Phabricator via lldb-commits
aadsm updated this revision to Diff 202854. aadsm added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62501/new/ https://reviews.llvm.org/D62501 Files: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp lldb/s

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

2019-06-03 Thread António Afonso via Phabricator via lldb-commits
aadsm updated this revision to Diff 202855. aadsm added a comment. Address comments except the one about a different test plan. Also removed the main property, after searching on the internet I'm not confident with the way I was using to detect the main executable. Since the "lm-main" is an opti

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

2019-06-03 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. Comment at: include/lldb/Target/CPPLanguageRuntime.h:47 + static CPPLanguageRuntime *GetCPPLanguageRuntime(Process &process) { +return static_cast( +process.GetLanguageRuntime(lldb::eLanguageTypeC_plus_plus

[Lldb-commits] [PATCH] D62499: Create a generic handler for Xfer packets

2019-06-03 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 to me now. Thanks for your patience. Comment at: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerTest.cpp:1-2 +//===-- GDBRemoteGDBRemoteCommunicati