[Lldb-commits] [PATCH] D62221: [lldb-server][LLGS] Support 'g' packets

[Pavel Labath via Phabricator via lldb-commits]

labath added a subscriber: omjavaid.
labath added a comment.

I have a couple of comments inline, but overall I think this looks pretty good 
now.

It would be interesting to also test reading %ymm registers, as those are 
stored in a funny way in the cpu context (and so we are most likely to get them 
wrong), however, I don't think this has to be done now.




Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py:140-147
+@llgs_test
+def test_g_returns_correct_data_with_suffix_llgs(self):
+self.init_llgs_test()
+self.build()
+self.set_inferior_startup_launch()
+self.g_returns_correct_data(True)
+

Right now, these tests will only work on x86_64, so you'll need to add 
something like `@skipIf(archs=no_match(["x86_64"]))`.

 + @omjavaid in case he's interested in contributing an arm version of these.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1936-1937
+// Write the register data to the buffer.
+const uint8_t *const data =
+reinterpret_cast(reg_value.GetBytes());
+

It doesn't look like this `reinterpret_cast` is needed. You should be able to 
feed `reg_value.GetBytes()` directly to `memcpy`...



Comment at: lldb/source/Utility/StringExtractorGDBRemote.cpp:379-382
   case 'g':
-if (packet_size == 1)
-  return eServerPacketType_g;
-break;

guiandrade wrote:
> clayborg wrote:
> > What if another packet starts with 'g'?
> I removed the if because I thought the packet could have the thread id 
> appended to it in [[ 
> https://github.com/llvm/llvm-project/blob/1f46d524a1c6ac66bbd803caf5f1206603759a5f/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp#L535
>| 
> ldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp#L535 
> ]]. If that's really the case, should we still try to make it more robust to 
> avoid prefix collisions?
I think this is fine because *we* don't support any other packet starting with 
`g`. So, we can just classify that packet as `g`, and later fail due to a 
syntax error. If we end up supporting another packet like that, we can refine 
the classification logic then.

This is also the same pattern used by other single-letter packets. (eg. below).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62221/new/

https://reviews.llvm.org/D62221



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thank you for working on this. This can speed up the handling of shared library 
loading by a lot. Also, thank you for splitting the work up into logical 
pieces. Please find my comments inline.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2697-2717
+  std::string xfer_object;
+  if (packet.GetStringBeforeChar(xfer_object, ':') == 0)
+return SendIllFormedResponse(packet, "qXfer packet missing object name");
+  // Consume ':'
+  packet.GetChar();
+  // Parse action (read/write).
+  std::string xfer_action;

I think it would be simpler to just forget about the StringExtractor here, and 
do something like
```
SmallVector fields;
StringRef(packet.GetStringRef()).split(fields, 3);
if (fields.size() != 4) return "Malformed packet";
// After this, "object" is in fields[0], "action" is in fields[1], annex is in 
fields[2], and the rest is in fields[3]
std::string buffer_key = fields[0] + fields[2];
...
```



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2738
+  if (!memory_buffer_sp) {
+if (xfer_object == "auxv") {
+// *BSD impls should be able to do this too.

Given that this function is going to grow, it would be good to split it in 
smaller chunks. Maybe move this code into something like 
`ErrorOr ReadObject(StringRef object)`?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2740
+// *BSD impls should be able to do this too.
+#if defined(__linux__) || defined(__NetBSD__)
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));

I think we should just remove this ifdef. NetBSD and linux are the only 
platforms supported by lldb-server right now. And anyway, any implementation 
which does not support auxv reading can just return an error from 
`process->GetAuxvData`...

If we want to differentiate between SendErrorResponse and 
SendUnimplementedResponse we can just develop a convention that a specific 
error code (`llvm::errc::not_supported`) means "unimplemented".



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:85
   lldb::StateType m_inferior_prev_state = lldb::StateType::eStateInvalid;
-  std::unique_ptr m_active_auxv_buffer_up;
+  std::unordered_map>
+  m_xfer_buffer_map;

This could be an `llvm::StringMap`. Also, it should be enough to use 
`unique_ptr` here. You just need to make sure that you don't copy the value out 
of the map. I recommend a pattern like
```
iter = map.find("foo");
if (iter == end()) {
  auto buffer_or_error = getFoo();
  if (buffer_or_error)
iter = map.insert("foo", std::move(*buffer_or_error))
  else
report(buffer_or_error.getError());
}
StringRef buffer = iter->second->getBuffer();

do_stuff(buffer);

if (done)
  map.erase(iter);
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62499/new/

https://reviews.llvm.org/D62499



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Pavel Labath via Phabricator via lldb-commits]

labath requested changes to this revision.
labath added a reviewer: JDevlieghere.
labath added a subscriber: JDevlieghere.
labath added a comment.
This revision now requires changes to proceed.

Definitely go for the option of refactoring the DYLD AuxVector class to make it 
usable from lldb-server.

It doesn't look like it should be that complicated even. Instead of passing it 
a `Process` instance, we can just pass it the data we have obtained from the 
right process (liblldb process, or lldb-server process), along with any other 
data it needs to interpret it (looks like it's just the pointer size (?)).

The question of plugins depending on each other is just a matter of finding the 
right place to put this new class. In my mind, Plugins/Process/Utility` is not 
a real plugin, and I don't see much of a problem in the Posix dynamic loader 
depending on that (it makes sense that it would need *some* data about the 
process in order to do its job. Or, given that the new class will depend on 
just the data extractor and nothing else, we could put it all the way up to 
`Utility`.

A third option would be to create a completely new library for this. In the 
past we've talked about a new library for "classes describing various 
properties of a process", where we'd have MemoryRegionInfo, ProcessInfo, etc, 
but so far it hasn't materialized. It seems like this could fit nicely into 
this description, so we could start with the new library with this class. 
+@JDevlieghere for any thoughts on this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62500/new/

https://reviews.llvm.org/D62500



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks relatively straight-forward, but I'll wait with the full review of this 
and subsequent patches until we sort out the first two patches in this series.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62501/new/

https://reviews.llvm.org/D62501



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62505: Fix multiple module loaded with the same UUID

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: jingham.
labath added a subscriber: jingham.
labath added a comment.

I agree we need to support a module being loaded multiple times, but I'm not 
sure if this is the right way to achieve that. I mean, if two files are exactly 
the same, then it should be fine (and even preferable) to download them just 
once, store them under a single entry (based on the UUID) in the module cache, 
and parse their symbols just once. It seems like somebody even intended for us 
to support a module being loaded multiple times as there is this TODO 
https://github.com/llvm-mirror/lldb/blob/1b2170d0116d52a219574780e7eb01043c3712e1/source/Target/SectionLoadList.cpp#L50
 in the SectionLoadList class.

However, this is not an area I'm super familiar with. Maybe @jingham knows more 
about this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62505/new/

https://reviews.llvm.org/D62505



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It sounds like these improved reads would be beneficial for all kinds of reads, 
and not just for reading c strings. Could this chunking logic be implemented 
directly in ReadMemory?

That would also allow you to test this by sending `m` which are deliberately 
chosen to cross readable/unreadable page boundaries in interesting ways...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62503/new/

https://reviews.llvm.org/D62503



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r361799 - [CMake] Folder structure for generated Xcode project to cover more targets

[Stefan Granitz via lldb-commits]

Author: stefan.graenitz
Date: Tue May 28 02:29:05 2019
New Revision: 361799

URL: http://llvm.org/viewvc/llvm-project?rev=361799&view=rev
Log:
[CMake] Folder structure for generated Xcode project to cover more targets

Modified:
lldb/trunk/CMakeLists.txt
lldb/trunk/cmake/modules/AddLLDB.cmake
lldb/trunk/cmake/modules/LLDBConfig.cmake
lldb/trunk/cmake/modules/LLDBStandalone.cmake
lldb/trunk/lit/CMakeLists.txt
lldb/trunk/source/API/CMakeLists.txt
lldb/trunk/test/CMakeLists.txt
lldb/trunk/tools/debugserver/source/CMakeLists.txt
lldb/trunk/tools/debugserver/source/MacOSX/CMakeLists.txt
lldb/trunk/tools/debugserver/source/MacOSX/DarwinLog/CMakeLists.txt
lldb/trunk/tools/driver/CMakeLists.txt
lldb/trunk/unittests/CMakeLists.txt
lldb/trunk/unittests/tools/lldb-mi/utils/CMakeLists.txt
lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
lldb/trunk/utils/lit-cpuid/CMakeLists.txt
lldb/trunk/utils/lldb-dotest/CMakeLists.txt

Modified: lldb/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=361799&r1=361798&r2=361799&view=diff
==
--- lldb/trunk/CMakeLists.txt (original)
+++ lldb/trunk/CMakeLists.txt Tue May 28 02:29:05 2019
@@ -153,6 +153,7 @@ if(LLDB_INCLUDE_TESTS)
 
   add_custom_target(lldb-test-deps)
   add_dependencies(lldb-test-deps ${LLDB_TEST_DEPS})
+  set_target_properties(lldb-test-deps PROPERTIES FOLDER "lldb misc")
 
   add_subdirectory(test)
   add_subdirectory(unittests)
@@ -193,6 +194,7 @@ if (NOT LLDB_DISABLE_PYTHON)
   set(readline_dep readline)
 endif()
 add_dependencies(finish_swig swig_wrapper liblldb lldb-argdumper 
${readline_dep})
+set_target_properties(finish_swig swig_wrapper PROPERTIES FOLDER "lldb 
misc")
 
 # Ensure we do the python post-build step when building lldb.
 add_dependencies(lldb finish_swig)

Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=361799&r1=361798&r2=361799&view=diff
==
--- lldb/trunk/cmake/modules/AddLLDB.cmake (original)
+++ lldb/trunk/cmake/modules/AddLLDB.cmake Tue May 28 02:29:05 2019
@@ -100,7 +100,11 @@ function(add_lldb_library name)
   # Add in any extra C++ compilation flags for this library.
   target_compile_options(${name} PRIVATE ${PARAM_EXTRA_CXXFLAGS})
 
-  set_target_properties(${name} PROPERTIES FOLDER "lldb libraries")
+  if(PARAM_PLUGIN)
+set_target_properties(${name} PROPERTIES FOLDER "lldb plugins")
+  else()
+set_target_properties(${name} PROPERTIES FOLDER "lldb libraries")
+  endif()
 endfunction(add_lldb_library)
 
 function(add_lldb_executable name)

Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=361799&r1=361798&r2=361799&view=diff
==
--- lldb/trunk/cmake/modules/LLDBConfig.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake Tue May 28 02:29:05 2019
@@ -364,7 +364,7 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
 )
 
   add_custom_target(lldb-headers)
-  set_target_properties(lldb-headers PROPERTIES FOLDER "Misc")
+  set_target_properties(lldb-headers PROPERTIES FOLDER "lldb misc")
 
   if (NOT CMAKE_CONFIGURATION_TYPES)
 add_llvm_install_targets(install-lldb-headers

Modified: lldb/trunk/cmake/modules/LLDBStandalone.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBStandalone.cmake?rev=361799&r1=361798&r2=361799&view=diff
==
--- lldb/trunk/cmake/modules/LLDBStandalone.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBStandalone.cmake Tue May 28 02:29:05 2019
@@ -88,6 +88,14 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
   set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
   set(LLVM_INCLUDE_TESTS ON CACHE INTERNAL "")
 
+  option(LLVM_USE_FOLDERS "Enable solution folders in Visual Studio. Disable 
for Express versions." ON)
+  if(LLVM_USE_FOLDERS)
+set_property(GLOBAL PROPERTY USE_FOLDERS ON)
+  endif()
+
+  set_target_properties(clang-tablegen-targets PROPERTIES FOLDER "lldb misc")
+  set_target_properties(intrinsics_gen PROPERTIES FOLDER "lldb misc")
+
   set(CMAKE_INCLUDE_CURRENT_DIR ON)
   include_directories(
 "${CMAKE_BINARY_DIR}/include"

Modified: lldb/trunk/lit/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/CMakeLists.txt?rev=361799&r1=361798&r2=361799&view=diff
==
--- lldb/trunk/lit/CMakeLists.txt (original)
+++ lldb/trunk/lit/CMakeLists.txt Tue May 28 02:29:05 2019
@@ -68,7 +68,7 @@ add_lit_testsuite(check-lldb-lit "Runnin
   DEPENDS ${LLDB_TEST_DEPS}
   )
 
-set_target_properties(

[Lldb-commits] [PATCH] D62517: Revert "D11003: Tolerate DWARF compile unit without filename."

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, tberghammer, dsrbecky.
Herald added a subscriber: aprantl.

This code is modifying a support file list after it has been created.
This makes it hard to share the file list between type units and
compile units in DWARF. It's not a total showstopper, but supporting
this while also sharing the lists would make things more complicated.

Given that this was added to support a project which never fully
materialised, and that even back then there were some concerns about the
correctness of this approach (according to D11003#200772 
 the compile
unit name is not guaranteed to be the first one in the support file
list), I think we should just delete this workaround.


https://reviews.llvm.org/D62517

Files:
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -680,19 +680,6 @@
 module_sp, dwarf_cu, cu_file_spec, dwarf_cu->GetID(),
 cu_language, is_optimized ? eLazyBoolYes : eLazyBoolNo);
 
-// If we just created a compile unit with an invalid file spec,
-// try and get the first entry in the supports files from the
-// line table as that should be the compile unit.
-if (!cu_file_spec) {
-  cu_file_spec = cu_sp->GetSupportFiles().GetFileSpecAtIndex(1);
-  if (cu_file_spec) {
-(FileSpec &)(*cu_sp) = cu_file_spec;
-// Also fix the invalid file spec which was copied from the
-// compile unit.
-cu_sp->GetSupportFiles().Replace(0, cu_file_spec);
-  }
-}
-
 dwarf_cu->SetUserData(cu_sp.get());
 
 m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(


Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -680,19 +680,6 @@
 module_sp, dwarf_cu, cu_file_spec, dwarf_cu->GetID(),
 cu_language, is_optimized ? eLazyBoolYes : eLazyBoolNo);
 
-// If we just created a compile unit with an invalid file spec,
-// try and get the first entry in the supports files from the
-// line table as that should be the compile unit.
-if (!cu_file_spec) {
-  cu_file_spec = cu_sp->GetSupportFiles().GetFileSpecAtIndex(1);
-  if (cu_file_spec) {
-(FileSpec &)(*cu_sp) = cu_file_spec;
-// Also fix the invalid file spec which was copied from the
-// compile unit.
-cu_sp->GetSupportFiles().Replace(0, cu_file_spec);
-  }
-}
-
 dwarf_cu->SetUserData(cu_sp.get());
 
 m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62517: Revert "D11003: Tolerate DWARF compile unit without filename."

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

As long as we put an empty FileSpec at the start of the support list we are ok. 
All file indexes are 1 based.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62517/new/

https://reviews.llvm.org/D62517



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D62503#1518839 , @labath wrote:

> It sounds like these improved reads would be beneficial for all kinds of 
> reads, and not just for reading c strings. Could this chunking logic be 
> implemented directly in ReadMemory?


We should probably do this in ReadMemory directly to ensure we have the fastest 
memory reads possible for all situations. Then ignore my inline comment.

> That would also allow you to test this by sending `m` packets which are 
> deliberately chosen to cross readable/unreadable page boundaries in 
> interesting ways...

BTW: LLDB will usually always send down aligned reads due to it populating its 
memory cache.




Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1502-1504
+Status NativeProcessLinux::ReadCStringFromMemory(lldb::addr_t addr,
+ char *buffer, size_t max_size,
+ size_t &total_bytes_read) {

Seems like this function could be added to the NativeProcess base class? We 
would need to add "virtual size_t NativeProcess::GetCacheLineSize() = 0;" to 
NativeProcess too in that case.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:-2224
+  error = ReadCStringFromMemory(link_map.l_name,
+reinterpret_cast(&name_buffer),
+sizeof(name_buffer), bytes_read);

do we really need to read PATH_MAX each time?Seems like ReadCStringFromMemory 
could read 32 bytes at a time and check for NULL characters along the way and 
making sure we don't cross page boundaries?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62503/new/

https://reviews.llvm.org/D62503



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62505: Fix multiple module loaded with the same UUID

[António Afonso via Phabricator via lldb-commits]

aadsm added a comment.

Interesting, I did miss that comment when I checked that class. This is 
something @clayborg was concerned with when I discussed this solution with him. 
The main reason I was not keen in having multiple load addresses for a section 
is because that would change the GetLoadAddress to return an array instead of 
an addr_t or some kind of mechanism to be able to correctly resolve the load 
address of an Address (felt like too big of a change for what it looks like an 
edge case?). 
So I preferred to look at this situation (of 2 modules with the exact same 
data) as a coincidence and be ok with parsing the same data twice. It is 
actually not clear to me why Samsung ships these vndks if they're an exact copy 
of the system ones.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62505/new/

https://reviews.llvm.org/D62505



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62505: Fix multiple module loaded with the same UUID

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D62505#1518833 , @labath wrote:

> I agree we need to support a module being loaded multiple times, but I'm not 
> sure if this is the right way to achieve that. I mean, if two files are 
> exactly the same, then it should be fine (and even preferable) to download 
> them just once, store them under a single entry (based on the UUID) in the 
> module cache, and parse their symbols just once. It seems like somebody even 
> intended for us to support a module being loaded multiple times as there is 
> this TODO 
> https://github.com/llvm-mirror/lldb/blob/1b2170d0116d52a219574780e7eb01043c3712e1/source/Target/SectionLoadList.cpp#L50
>  in the SectionLoadList class.
>
> However, this is not an area I'm super familiar with. Maybe @jingham knows 
> more about this?


So I am familiar with this code and I was working with Antonio on this. I 
suggested the same thing, but then the amount of work we would need to do to 
support loading the same file in different areas was a lot more intrusive. What 
we really want is to have the same module loaded in two places, but each module 
would need to have its own platform path, one with /system/lib/libcutils.so and 
one with /system/lib/vndk-sp/libcutils.so. If we don't do this, then we show 
the user one of those file paths twice which doesn't match reality. That led us 
to think about a BaseModule class that wouldn't contain the platform file spec, 
and then we could have two Module instances that share the same BaseModule 
class. The BaseModule would always contain the local FileSpec that was uniqued. 
That seemed like a lot of hassle for not much gain. If we allow a 
lldb_private::Section to be loaded multiple times, there are a lot of places 
that would need to change in LLDB code where anytime someone tries to resolve a 
lldb_private::Address to a load address, it will need to be able to deal with 
multiple results. That will change many locations around the LLDB code base. 
That is the right fix IMHO, but it will make this code change quite large. So 
we decided to post this diff up for review so we could talk about the different 
solutions.

So if we end up going with one module, how do we propose to get around the fact 
that there are multiple platform paths ("/system/lib/libcutils.so" and 
"/system/lib/vndk-sp/libcutils.so") for this module? Just make platform path be 
an array instead of a single path?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62505/new/

https://reviews.llvm.org/D62505



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[António Afonso via Phabricator via lldb-commits]

aadsm added a comment.

Regarding changing the ReadMemory, yes, I was going to submit another patch 
with it. I was going to do it in a different way though, where I would read as 
much as possible with the process_vm_readv (fwiw, this function does not fail 
when it tries to read cross page, it will just partial read) and then read the 
rest with ptrace. But I guess it does makes more sense to adopt the same page 
boundary read strategy.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62503/new/

https://reviews.llvm.org/D62503



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D62503#1519179 , @aadsm wrote:

> Regarding changing the ReadMemory, yes, I was going to submit another patch 
> with it. I was going to do it in a different way though, where I would read 
> as much as possible with the process_vm_readv (fwiw, this function does not 
> fail when it tries to read cross page, it will just partial read) and then 
> read the rest with ptrace.


I think this is fine too, probably even better. I was going to suggest the same 
thing, but then I convinced myself that this wouldn't work in case the read 
*starts* in the unreadable memory region.

However, now that I think about it, that is nonsense, because there is no way 
for us to say to the user that "we failed to read some initial bytes, but this 
is the memory contents after that". So just using process_vm_readv, and 
finishing up with ptrace sounds fine to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62503/new/

https://reviews.llvm.org/D62503



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:2091-2093
+DataExtractor auxv_data(buffer_or_error.get()->getBufferStart(),
+buffer_or_error.get()->getBufferSize(),
+GetByteOrder(), GetAddressByteSize());

We need to get a copy of the data here right? I believe the "buffer_or_error" 
local variable will go out of scope and the data pointed to will be freed. You 
can use:
```
  lldb::offset_t DataExtractor::CopyData(lldb::offset_t offset, lldb::offset_t 
length, void *dst) const;
```
which will copy of the data and internally own it in a DataBufferSP. Or you can 
create a DataBufferSP yourself using DataBufferHeap and placing it into a 
DataBufferSP and then passing that to the DataExtractor constructor or SetData 
method.




Comment at: lldb/source/Plugins/Process/Utility/ELFAuxVector.cpp:18
+  lldb::offset_t saved_offset = *offset_ptr;
+  *value = data.GetMaxU64(offset_ptr, data.GetAddressByteSize());
+  return *offset_ptr != saved_offset;

```
data.GetAddress(offset_ptr);
```



Comment at: lldb/source/Plugins/Process/Utility/ELFAuxVector.h:67
+
+  std::unordered_map m_auxv_entries;
+};

Is there only ever one entry in the AUXV map for each EntryType?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62500/new/

https://reviews.llvm.org/D62500



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I like this being in a Utility class like it is coded, but we should fix the 
POSIX DYLD plug-in to use this common code as part of this patch. Comments?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62500/new/

https://reviews.llvm.org/D62500



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[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/ELFAuxVector.cpp:18
+  lldb::offset_t saved_offset = *offset_ptr;
+  *value = data.GetMaxU64(offset_ptr, data.GetAddressByteSize());
+  return *offset_ptr != saved_offset;

clayborg wrote:
> ```
> data.GetAddress(offset_ptr);
> ```
I know you pointed this out but I felt weird calling `GetAddress` since that's 
not the semantic value of the underlying data. That's why I still ended up with 
GetMaxU64, what do you think?



Comment at: lldb/source/Plugins/Process/Utility/ELFAuxVector.h:67
+
+  std::unordered_map m_auxv_entries;
+};

clayborg wrote:
> Is there only ever one entry in the AUXV map for each EntryType?
Yes, this was something I was concerned with (since the AuxVector returns an 
iterator). I checked the "official" API and that's what they assume: 
http://man7.org/linux/man-pages/man3/getauxval.3.html


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62500/new/

https://reviews.llvm.org/D62500



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62505: Fix multiple module loaded with the same UUID

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D62505#1519166 , @aadsm wrote:

> Interesting, I did miss that comment when I checked that class. This is 
> something @clayborg was concerned with when I discussed this solution with 
> him. The main reason I was not keen in having multiple load addresses for a 
> section is because that would change the GetLoadAddress to return an array 
> instead of an addr_t or some kind of mechanism to be able to correctly 
> resolve the load address of an Address (felt like too big of a change for 
> what it looks like an edge case?).


I wouldn't say this is specific to some class of samsung phones. It is easy to 
use dlmopen(3) to open the same library multiple times. Just try calling 
`dlmopen(LM_ID_NEWLM, "/lib64/libc.so.6", RTLD_NOW)` in a loop a couple of 
times and see what happens in /proc//maps. (I believe this is the same 
function that android uses to implement vndk separation.)

In D62505#1519178 , @clayborg wrote:

> So if we end up going with one module, how do we propose to get around the 
> fact that there are multiple platform paths ("/system/lib/libcutils.so" and 
> "/system/lib/vndk-sp/libcutils.so") for this module? Just make platform path 
> be an array instead of a single path?


TBH, I'm not really convinced that the "platform path" should be a property of 
the Module.

Imagine a situation where I have the same module loaded in multiple targets 
(maybe on different remote platforms, or maybe not). It makes sense that each 
target can have the module located at a different path. So maybe the "platform 
path" should be a property of the target,  saying "where did this target load 
the module from"? Of course, this won't help when you have the same module 
loaded multiple times in the same target, so maybe this does need to be a list 
of paths? I'm not sure, I'm just thinking aloud here...

Right now, I'm also lean towards making a Section be loadable multiple times, 
however this definitely needs more discussion. I'm interested to hear what 
@jingham thinks of this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62505/new/

https://reviews.llvm.org/D62505



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62425: [DWARFExpression] Remove ctor that takes just a compile unit.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/Expression/DWARFExpression.h:311-312
   lldb::addr_t m_loclist_slide; ///< A value used to slide the location list
-///offsets so that
+/// offsets so that
+/// m_c
   ///< they are relative to the object that owns the location list

correct the comment or remove changes?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:508-509
   uint32_t block_length = form_value.Unsigned();
-  frame_base->SetOpcodeData(module, debug_info_data, block_offset,
-block_length);
+  frame_base = new (frame_base) DWARFExpression(
+  module, debug_info_data, cu, block_offset, block_length);
 } else {

labath wrote:
> This is not the copy assignment constructor. That would be something like:
> `*frame_base = DWARFExpression(...)`.
> This code looks pretty dodgy, as you're trampling over an already constructed 
> object. I'm not sure whether that in itself is UB, but it definitely is not a 
> good idea.
Use move?:
```
*frame_base = std::move(DWARFExpression(...));
```



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:518-519
   if (loc_list_length > 0) {
-frame_base->SetOpcodeData(module, debug_loc_data,
-  debug_loc_offset, loc_list_length);
+frame_base = new (frame_base)
+DWARFExpression(module, debug_loc_data, cu,
+debug_loc_offset, loc_list_length);

use move like suggested above?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62425/new/

https://reviews.llvm.org/D62425



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62425: [DWARFExpression] Remove ctor that takes just a compile unit.

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:508-509
   uint32_t block_length = form_value.Unsigned();
-  frame_base->SetOpcodeData(module, debug_info_data, block_offset,
-block_length);
+  frame_base = new (frame_base) DWARFExpression(
+  module, debug_info_data, cu, block_offset, block_length);
 } else {

clayborg wrote:
> labath wrote:
> > This is not the copy assignment constructor. That would be something like:
> > `*frame_base = DWARFExpression(...)`.
> > This code looks pretty dodgy, as you're trampling over an already 
> > constructed object. I'm not sure whether that in itself is UB, but it 
> > definitely is not a good idea.
> Use move?:
> ```
> *frame_base = std::move(DWARFExpression(...));
> ```
move is not needed as `DWARFExpression(...)` is a temporary.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62425/new/

https://reviews.llvm.org/D62425



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57830: Add functionality to trace a function within lldb

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

For this to actually work we need to:

- Disable skipping function prologues using "settings set target.skip-prologue 
false", set a breakpoint at the function we want to trace and store breakpoint 
in "trace_breakpoint", restore skipping function prologues
- make a local variable "tracing_function = False"
- when we stop, detect if the "trace_breakpoint" was hit and set 
"tracing_function = True" if it was. Calculate the the return address using the 
thread's backtrace and set a breakpoint there, store breakpoint in local 
variable "trace_return_breakpoint"
- when we continue check "tracing_function" and step if it is true, and 
continue it not
- When we stop check if  "trace_return_breakpoint" was hit and if so, set 
"tracing_function = False"


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57830/new/

https://reviews.llvm.org/D57830



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62477: DWARFASTParserClang: Move attribute parsing into a single function

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Minor nit where we are storing name a ConstString but mangled name as "const 
char *". Fix if needed.

Would be good to verify performance doesn't regress from this somehow. Maybe 
having a large DWARF file with many types and accessing each type and making 
sure it is completed via the external AST.




Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:248
+  clang::StorageClass storage = clang::SC_None;
+  const char *mangled_name = nullptr;
+  ConstString name;

Why is this stored as a "const char *" and "name" below as a ConstString?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62477/new/

https://reviews.llvm.org/D62477



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62425: [DWARFExpression] Remove ctor that takes just a compile unit.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 201687.
JDevlieghere added a comment.

Address CR feedback


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62425/new/

https://reviews.llvm.org/D62425

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Symbol/Function.cpp

Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -184,7 +184,7 @@
const AddressRange &range)
 : UserID(func_uid), m_comp_unit(comp_unit), m_type_uid(type_uid),
   m_type(type), m_mangled(mangled), m_block(func_uid), m_range(range),
-  m_frame_base(nullptr), m_flags(), m_prologue_byte_size(0) {
+  m_frame_base(), m_flags(), m_prologue_byte_size(0) {
   m_block.SetParentScope(this);
   assert(comp_unit != nullptr);
 }
Index: lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
@@ -69,7 +69,7 @@
   is_constant = true;
 
   if (!module)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   const ArchSpec &architecture = module->GetArchitecture();
   llvm::Triple::ArchType arch_type = architecture.GetMachine();
@@ -77,7 +77,7 @@
   uint32_t address_size = architecture.GetAddressByteSize();
   uint32_t byte_size = architecture.GetDataByteSize();
   if (byte_order == eByteOrderInvalid || address_size == 0)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   RegisterKind register_kind = eRegisterKindDWARF;
   StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order);
@@ -88,13 +88,13 @@
 
 SectionList *section_list = module->GetSectionList();
 if (!section_list)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 uint32_t section_id = symbol.getAddressSection();
 
 auto section = section_list->FindSectionByID(section_id);
 if (!section)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 uint32_t offset = symbol.getAddressOffset();
 stream.PutMaxHex64(section->GetFileAddress() + offset, address_size,
@@ -129,7 +129,7 @@
   register_kind = eRegisterKindLLDB;
   reg_num = GetLLDBRegisterNumber(arch_type, reg_id);
   if (reg_num == LLDB_INVALID_REGNUM)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 }
 
 if (reg_num > 31) {
@@ -149,7 +149,7 @@
 register_kind = eRegisterKindLLDB;
 uint32_t reg_num = GetLLDBRegisterNumber(arch_type, symbol.getRegisterId());
 if (reg_num == LLDB_INVALID_REGNUM)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 if (reg_num > 31) {
   stream.PutHex8(DW_OP_regx);
@@ -168,7 +168,7 @@
 break;
   }
   default:
-return DWARFExpression(nullptr);
+return DWARFExpression();
   }
 
   DataBufferSP buffer =
Index: lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
@@ -111,13 +111,13 @@
   uint32_t address_size = architecture.GetAddressByteSize();
   uint32_t byte_size = architecture.GetDataByteSize();
   if (byte_order == eByteOrderInvalid || address_size == 0)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   RegisterKind register_kind = eRegisterKindDWARF;
   StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order);
 
   if (!writer(stream, register_kind))
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   DataBufferSP buffer =
   std::make_shared(stream.GetData(), stream.GetSize());
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3102,7 +3102,7 @@
   Declaration decl;
   uint32_t i;
   DWARFFormValue type_die_form;
-  DWARFExpression location(die.GetCU());
+  DWARFExpression location;
   bool is_external = false;
   bool is_artificial = false;
   bool location_is_const_value_data = false;
@@ -3153,14 +3153,15 @@
 uint32_t block_offset =
 form_value.BlockData() - de

[Lldb-commits] [PATCH] D62302: DWARF: Fix address range support in mixed 4+5 scenario

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:372
+return std::move(*expected_ranges);
+  unit.GetSymbolFileDWARF()->GetObjectFile()->GetModule()->ReportError(
+  "{0x%8.8x}: DIE has DW_AT_ranges(0x%" PRIx64 ") attribute, but "

Can we add a test that ensures we actually display this error?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62302/new/

https://reviews.llvm.org/D62302



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62425: [DWARFExpression] Remove ctor that takes just a compile unit.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

lgtm. Pavel, you good?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62425/new/

https://reviews.llvm.org/D62425



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/ELFAuxVector.cpp:18
+  lldb::offset_t saved_offset = *offset_ptr;
+  *value = data.GetMaxU64(offset_ptr, data.GetAddressByteSize());
+  return *offset_ptr != saved_offset;

aadsm wrote:
> clayborg wrote:
> > ```
> > data.GetAddress(offset_ptr);
> > ```
> I know you pointed this out but I felt weird calling `GetAddress` since 
> that's not the semantic value of the underlying data. That's why I still 
> ended up with GetMaxU64, what do you think?
I would use it since that is exactly what you are doing with 
"data.GetMaxU64(offset_ptr, data.GetAddressByteSize());".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62500/new/

https://reviews.llvm.org/D62500



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r361849 - [DWARFExpression] Remove ctor that takes just a compile unit.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue May 28 10:34:05 2019
New Revision: 361849

URL: http://llvm.org/viewvc/llvm-project?rev=361849&view=rev
Log:
[DWARFExpression] Remove ctor that takes just a compile unit.

Like many of our DWARF classes, the DWARFExpression can be initialized
in several ways. One such way was through a constructor that takes just
the compile unit. This constructor is used to initialize both empty
DWARFExpressions, and DWARFExpression that will be populated later.

To make the distinction more clear, I changed the constructor to a
default constructor and updated its call sites. Where the
DWARFExpression was being populated later, I replaced that with a call
to the copy assignment constructor.

Differential revision: https://reviews.llvm.org/D62425

Modified:
lldb/trunk/include/lldb/Expression/DWARFExpression.h
lldb/trunk/source/Expression/DWARFExpression.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
lldb/trunk/source/Symbol/Function.cpp

Modified: lldb/trunk/include/lldb/Expression/DWARFExpression.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/DWARFExpression.h?rev=361849&r1=361848&r2=361849&view=diff
==
--- lldb/trunk/include/lldb/Expression/DWARFExpression.h (original)
+++ lldb/trunk/include/lldb/Expression/DWARFExpression.h Tue May 28 10:34:05 
2019
@@ -43,8 +43,7 @@ public:
 // (.debug_loclists/.debug_loclists.dwo).
   };
 
-  /// Constructor
-  explicit DWARFExpression(DWARFUnit *dwarf_cu);
+  DWARFExpression();
 
   /// Constructor
   ///
@@ -58,7 +57,7 @@ public:
   /// \param[in] data_length
   /// The byte length of the location expression.
   DWARFExpression(lldb::ModuleSP module, const DataExtractor &data,
-  DWARFUnit *dwarf_cu, lldb::offset_t data_offset,
+  const DWARFUnit *dwarf_cu, lldb::offset_t data_offset,
   lldb::offset_t data_length);
 
   /// Destructor
@@ -132,6 +131,9 @@ public:
 
   bool Update_DW_OP_addr(lldb::addr_t file_addr);
 
+  void UpdateValue(uint64_t const_value, lldb::offset_t const_value_byte_size,
+   uint8_t addr_byte_size);
+
   void SetModule(const lldb::ModuleSP &module) { m_module_wp = module; }
 
   bool ContainsThreadLocalStorage() const;
@@ -141,66 +143,6 @@ public:
   std::function const
   &link_address_callback);
 
-  /// Make the expression parser read its location information from a given
-  /// data source.  Does not change the offset and length
-  ///
-  /// \param[in] data
-  /// A data extractor configured to read the DWARF location expression's
-  /// bytecode.
-  void SetOpcodeData(const DataExtractor &data);
-
-  /// Make the expression parser read its location information from a given
-  /// data source
-  ///
-  /// \param[in] module_sp
-  /// The module that defines the DWARF expression.
-  ///
-  /// \param[in] data
-  /// A data extractor configured to read the DWARF location expression's
-  /// bytecode.
-  ///
-  /// \param[in] data_offset
-  /// The offset of the location expression in the extractor.
-  ///
-  /// \param[in] data_length
-  /// The byte length of the location expression.
-  void SetOpcodeData(lldb::ModuleSP module_sp, const DataExtractor &data,
- lldb::offset_t data_offset, lldb::offset_t data_length);
-
-  /// Copy the DWARF location expression into a local buffer.
-  ///
-  /// It is a good idea to copy the data so we don't keep the entire object
-  /// file worth of data around just for a few bytes of location expression.
-  /// LLDB typically will mmap the entire contents of debug information files,
-  /// and if we use SetOpcodeData, it will get a shared reference to all of
-  /// this data for the and cause the object file to have to stay around. Even
-  /// worse, a very very large ".a" that contains one or more .o files could
-  /// end up being referenced. Location lists are typically small so even
-  /// though we are copying the data, it shouldn't amount to that much for the
-  /// variables we end up parsing.
-  ///
-  /// \param[in] module_sp
-  /// The module that defines the DWARF expression.
-  ///
-  /// \param[in] data
-  /// A data extractor configured to read and copy the DWARF
-  /// location expression's bytecode.
-  ///
-  /// \param[in] data_offset
-  /// The offset of the location expression in the extractor.
-  ///
-  /// \param[in] data_length
-  /// The byte length of the location expression.
-  void CopyOpcodeData(lldb::ModuleSP module_sp, const DataExtractor &data,
-

[Lldb-commits] [PATCH] D62425: [DWARFExpression] Remove ctor that takes just a compile unit.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361849: [DWARFExpression] Remove ctor that takes just a 
compile unit. (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62425?vs=201687&id=201704#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62425/new/

https://reviews.llvm.org/D62425

Files:
  lldb/trunk/include/lldb/Expression/DWARFExpression.h
  lldb/trunk/source/Expression/DWARFExpression.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/trunk/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/trunk/source/Symbol/Function.cpp

Index: lldb/trunk/source/Symbol/Function.cpp
===
--- lldb/trunk/source/Symbol/Function.cpp
+++ lldb/trunk/source/Symbol/Function.cpp
@@ -184,7 +184,7 @@
const AddressRange &range)
 : UserID(func_uid), m_comp_unit(comp_unit), m_type_uid(type_uid),
   m_type(type), m_mangled(mangled), m_block(func_uid), m_range(range),
-  m_frame_base(nullptr), m_flags(), m_prologue_byte_size(0) {
+  m_frame_base(), m_flags(), m_prologue_byte_size(0) {
   m_block.SetParentScope(this);
   assert(comp_unit != nullptr);
 }
Index: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
@@ -69,7 +69,7 @@
   is_constant = true;
 
   if (!module)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   const ArchSpec &architecture = module->GetArchitecture();
   llvm::Triple::ArchType arch_type = architecture.GetMachine();
@@ -77,7 +77,7 @@
   uint32_t address_size = architecture.GetAddressByteSize();
   uint32_t byte_size = architecture.GetDataByteSize();
   if (byte_order == eByteOrderInvalid || address_size == 0)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   RegisterKind register_kind = eRegisterKindDWARF;
   StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order);
@@ -88,13 +88,13 @@
 
 SectionList *section_list = module->GetSectionList();
 if (!section_list)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 uint32_t section_id = symbol.getAddressSection();
 
 auto section = section_list->FindSectionByID(section_id);
 if (!section)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 uint32_t offset = symbol.getAddressOffset();
 stream.PutMaxHex64(section->GetFileAddress() + offset, address_size,
@@ -129,7 +129,7 @@
   register_kind = eRegisterKindLLDB;
   reg_num = GetLLDBRegisterNumber(arch_type, reg_id);
   if (reg_num == LLDB_INVALID_REGNUM)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 }
 
 if (reg_num > 31) {
@@ -149,7 +149,7 @@
 register_kind = eRegisterKindLLDB;
 uint32_t reg_num = GetLLDBRegisterNumber(arch_type, symbol.getRegisterId());
 if (reg_num == LLDB_INVALID_REGNUM)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 if (reg_num > 31) {
   stream.PutHex8(DW_OP_regx);
@@ -168,7 +168,7 @@
 break;
   }
   default:
-return DWARFExpression(nullptr);
+return DWARFExpression();
   }
 
   DataBufferSP buffer =
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3102,7 +3102,7 @@
   Declaration decl;
   uint32_t i;
   DWARFFormValue type_die_form;
-  DWARFExpression location(die.GetCU());
+  DWARFExpression location;
   bool is_external = false;
   bool is_artificial = false;
   bool location_is_const_value_data = false;
@@ -3153,14 +3153,15 @@
 uint32_t block_offset =
 form_value.BlockData() - debug_info_data.GetDataStart();
 uint32_t block_length = form_value.Unsigned();
-location.CopyOpcodeData(module, debug_info_data, block_offset,
-block_length);
+location = DWARFExpression(module, debug_info_data, die.GetCU(),
+   block_offset, block_length);
   } else if (DWARFFormValue::IsDataForm(form_value.Form())) {
 // Retrieve the value 

[Lldb-commits] [PATCH] D62472: [CMake] LLDB.framework tools handling

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/tools/argdumper/CMakeLists.txt:9
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_add_to_framework(lldb-argdumper)
+endif()

Why do we need this in the framework? Is it only there for testing?



Comment at: lldb/tools/darwin-debug/CMakeLists.txt:6
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_add_to_framework(lldb-argdumper)
+endif()

Presumably this should be `darwin-debug`. But do we need it at all?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62472/new/

https://reviews.llvm.org/D62472



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62472: [CMake] LLDB.framework tools handling

[Frederic Riss via Phabricator via lldb-commits]

friss added inline comments.



Comment at: lldb/tools/argdumper/CMakeLists.txt:9
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_add_to_framework(lldb-argdumper)
+endif()

JDevlieghere wrote:
> Why do we need this in the framework? Is it only there for testing?
It is definitely use by our process launch code.



Comment at: lldb/tools/darwin-debug/CMakeLists.txt:6
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_add_to_framework(lldb-argdumper)
+endif()

JDevlieghere wrote:
> Presumably this should be `darwin-debug`. But do we need it at all?
I think it has come in handy a handful of times to debug some issues. It's not 
needed for LLDB to do its job though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62472/new/

https://reviews.llvm.org/D62472



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D62500#1518815 , @labath wrote:

> Definitely go for the option of refactoring the DYLD AuxVector class to make 
> it usable from lldb-server.


+1

> It doesn't look like it should be that complicated even. Instead of passing 
> it a `Process` instance, we can just pass it the data we have obtained from 
> the right process (liblldb process, or lldb-server process), along with any 
> other data it needs to interpret it (looks like it's just the pointer size 
> (?)).
> 
> The question of plugins depending on each other is just a matter of finding 
> the right place to put this new class. In my mind, Plugins/Process/Utility` 
> is not a real plugin, and I don't see much of a problem in the Posix dynamic 
> loader depending on that (it makes sense that it would need *some* data about 
> the process in order to do its job. Or, given that the new class will depend 
> on just the data extractor and nothing else, we could put it all the way up 
> to `Utility`.

I don't think this is general enough to fit in `Utility`. Looking at other 
classes in `Plugins/Process/Utility`, it seems like a better fit.

> A third option would be to create a completely new library for this. In the 
> past we've talked about a new library for "classes describing various 
> properties of a process", where we'd have MemoryRegionInfo, ProcessInfo, etc, 
> but so far it hasn't materialized. It seems like this could fit nicely into 
> this description, so we could start with the new library with this class. 
> +@JDevlieghere for any thoughts on this.

Do you mean having `AuxVector` in this library, or having it take a 
`ProcessInfo` instead of the `Process` itself?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62500/new/

https://reviews.llvm.org/D62500



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62472: [CMake] LLDB.framework tools handling

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz updated this revision to Diff 201726.
sgraenitz marked an inline comment as done.
sgraenitz added a comment.

Fix copy/paste mistake for darwin-debug


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62472/new/

https://reviews.llvm.org/D62472

Files:
  lldb/CMakeLists.txt
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/cmake/modules/LLDBFramework.cmake
  lldb/tools/argdumper/CMakeLists.txt
  lldb/tools/darwin-debug/CMakeLists.txt
  lldb/tools/debugserver/source/CMakeLists.txt
  lldb/tools/driver/CMakeLists.txt
  lldb/tools/lldb-mi/CMakeLists.txt
  lldb/tools/lldb-server/CMakeLists.txt
  lldb/tools/lldb-vscode/CMakeLists.txt

Index: lldb/tools/lldb-vscode/CMakeLists.txt
===
--- lldb/tools/lldb-vscode/CMakeLists.txt
+++ lldb/tools/lldb-vscode/CMakeLists.txt
@@ -31,5 +31,15 @@
   )
 
 if(LLDB_BUILD_FRAMEWORK)
-  lldb_setup_framework_rpaths_in_tool(lldb-vscode)
+  # In the build-tree, we know the exact path to the framework directory.
+  # The installed framework can be in different locations.
+  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
+  lldb_setup_rpaths(lldb-vscode
+BUILD_RPATH
+  "${framework_build_dir}"
+INSTALL_RPATH
+  "@loader_path/../../../SharedFrameworks"
+  "@loader_path/../../System/Library/PrivateFrameworks"
+  "@loader_path/../../Library/PrivateFrameworks"
+  )
 endif()
Index: lldb/tools/lldb-server/CMakeLists.txt
===
--- lldb/tools/lldb-server/CMakeLists.txt
+++ lldb/tools/lldb-server/CMakeLists.txt
@@ -77,3 +77,7 @@
 )
 
 target_link_libraries(lldb-server PRIVATE ${LLDB_SYSTEM_LIBS})
+
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_add_to_framework(lldb-server)
+endif()
Index: lldb/tools/lldb-mi/CMakeLists.txt
===
--- lldb/tools/lldb-mi/CMakeLists.txt
+++ lldb/tools/lldb-mi/CMakeLists.txt
@@ -95,5 +95,15 @@
   )
 
 if(LLDB_BUILD_FRAMEWORK)
-  lldb_setup_framework_rpaths_in_tool(lldb-mi)
+  # In the build-tree, we know the exact path to the framework directory.
+  # The installed framework can be in different locations.
+  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
+  lldb_setup_rpaths(lldb-mi
+BUILD_RPATH
+  "${framework_build_dir}"
+INSTALL_RPATH
+  "@loader_path/../../../SharedFrameworks"
+  "@loader_path/../../System/Library/PrivateFrameworks"
+  "@loader_path/../../Library/PrivateFrameworks"
+  )
 endif()
Index: lldb/tools/driver/CMakeLists.txt
===
--- lldb/tools/driver/CMakeLists.txt
+++ lldb/tools/driver/CMakeLists.txt
@@ -29,5 +29,15 @@
 )
 
 if(LLDB_BUILD_FRAMEWORK)
-  lldb_setup_framework_rpaths_in_tool(lldb)
+  # In the build-tree, we know the exact path to the framework directory.
+  # The installed framework can be in different locations.
+  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
+  lldb_setup_rpaths(lldb
+BUILD_RPATH
+  "${framework_build_dir}"
+INSTALL_RPATH
+  "@loader_path/../../../SharedFrameworks"
+  "@loader_path/../../System/Library/PrivateFrameworks"
+  "@loader_path/../../Library/PrivateFrameworks"
+  )
 endif()
Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -264,6 +264,10 @@
   ${entitlements}
 )
 
+  if(LLDB_BUILD_FRAMEWORK)
+lldb_add_to_framework(lldb-argdumper)
+  endif()
+
   if(IOS)
 set_property(TARGET lldbDebugserverCommon APPEND PROPERTY COMPILE_DEFINITIONS
   WITH_LOCKDOWN
Index: lldb/tools/darwin-debug/CMakeLists.txt
===
--- lldb/tools/darwin-debug/CMakeLists.txt
+++ lldb/tools/darwin-debug/CMakeLists.txt
@@ -1,3 +1,7 @@
 add_lldb_tool(darwin-debug
   darwin-debug.cpp
   )
+
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_add_to_framework(darwin-debug)
+endif()
Index: lldb/tools/argdumper/CMakeLists.txt
===
--- lldb/tools/argdumper/CMakeLists.txt
+++ lldb/tools/argdumper/CMakeLists.txt
@@ -4,3 +4,7 @@
   LINK_LIBS
 lldbUtility
   )
+
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_add_to_framework(lldb-argdumper)
+endif()
Index: lldb/cmake/modules/LLDBFramework.cmake
===
--- lldb/cmake/modules/LLDBFramework.cmake
+++ lldb/cmake/modules/LLDBFramework.cmake
@@ -36,22 +36,9 @@
 endif()
 
 # Target to capture extra steps for a fully functional framework bundle.
-add_custom_target(lldb-framework)
+add_custom_target(lldb-framework ALL)
 add_dependencies(lldb-framework liblldb)
 
-# Dependencies are defined once tools a

[Lldb-commits] [PATCH] D62472: [CMake] LLDB.framework tools handling

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

Thanks for having a look. I'd to focus on the mechanism here and keep the 
contents of the framework bundle unchanged. Happy to discuss this in separate 
reviews like D62474 .




Comment at: lldb/tools/darwin-debug/CMakeLists.txt:6
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_add_to_framework(lldb-argdumper)
+endif()

friss wrote:
> JDevlieghere wrote:
> > Presumably this should be `darwin-debug`. But do we need it at all?
> I think it has come in handy a handful of times to debug some issues. It's 
> not needed for LLDB to do its job though.
Thanks for catching that one!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62472/new/

https://reviews.llvm.org/D62472



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62472: [CMake] LLDB.framework tools handling

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

This is a much nicer solution. Thanks for doing this.




Comment at: lldb/tools/debugserver/source/CMakeLists.txt:268
+  if(LLDB_BUILD_FRAMEWORK)
+lldb_add_to_framework(lldb-argdumper)
+  endif()

This should be `debugserver`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62472/new/

https://reviews.llvm.org/D62472



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62472: [CMake] LLDB.framework tools handling

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/tools/argdumper/CMakeLists.txt:9
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_add_to_framework(lldb-argdumper)
+endif()

friss wrote:
> JDevlieghere wrote:
> > Why do we need this in the framework? Is it only there for testing?
> It is definitely use by our process launch code.
I see, apparently it's used from `Host::ShellExpandArguments` 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62472/new/

https://reviews.llvm.org/D62472



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

Thanks for doing this work! I'd love to see a faster LLDB. :)




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2697-2717
+  std::string xfer_object;
+  if (packet.GetStringBeforeChar(xfer_object, ':') == 0)
+return SendIllFormedResponse(packet, "qXfer packet missing object name");
+  // Consume ':'
+  packet.GetChar();
+  // Parse action (read/write).
+  std::string xfer_action;

labath wrote:
> I think it would be simpler to just forget about the StringExtractor here, 
> and do something like
> ```
> SmallVector fields;
> StringRef(packet.GetStringRef()).split(fields, 3);
> if (fields.size() != 4) return "Malformed packet";
> // After this, "object" is in fields[0], "action" is in fields[1], annex is 
> in fields[2], and the rest is in fields[3]
> std::string buffer_key = fields[0] + fields[2];
> ...
> ```
I agree, that would make it simpler.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2738
+  if (!memory_buffer_sp) {
+if (xfer_object == "auxv") {
+// *BSD impls should be able to do this too.

labath wrote:
> Given that this function is going to grow, it would be good to split it in 
> smaller chunks. Maybe move this code into something like 
> `ErrorOr ReadObject(StringRef object)`?
+1

You could have smaller methods like "ParseAuxvPacket" and "ReadObject"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62499/new/

https://reviews.llvm.org/D62499



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62472: [CMake] LLDB.framework tools handling

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/tools/darwin-debug/CMakeLists.txt:6
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_add_to_framework(darwin-debug)
+endif()

We need it for launching in a terminal on macOS. So it actually is needed for 
"process launch --tty" on macOS


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62472/new/

https://reviews.llvm.org/D62472



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62547: (lldb-vscode) Evaluate expressions as LLDB commands when in REPL mode.

[Ivan Hernandez via Phabricator via lldb-commits]

ivanhernandez13 created this revision.
ivanhernandez13 added a reviewer: clayborg.
ivanhernandez13 added a project: LLDB.
Herald added a subscriber: lldb-commits.

lldb-vscode can receive 'evaluate' requests in multiple modes, one of which is 
'repl' which indicates the expression to evaluate was typed into the Debug 
Console i.e. user input. This change makes 'request_evaluate' in lldb-vscode 
evaluate any user requested expression as an LLDB command if it cannot first 
find it as a local variable or a variable in the current frame. 


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D62547

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/variables/TestVSCode_variables.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -926,6 +926,7 @@
   auto arguments = request.getObject("arguments");
   lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments);
   const auto expression = GetString(arguments, "expression");
+  const auto context = GetString(arguments, "context");
 
   if (!expression.empty() && expression[0] == '`') {
 auto result = RunLLDBCommands(llvm::StringRef(),
@@ -936,25 +937,38 @@
 // Always try to get the answer from the local variables if possible. If
 // this fails, then actually evaluate an expression using the expression
 // parser. "frame variable" is more reliable than the expression parser in
-// many cases and it is faster.
+// many cases and it is faster. Run the expression as an LLDB command as a
+// last resort when in REPL context.
 lldb::SBValue value = frame.GetValueForVariablePath(
 expression.data(), lldb::eDynamicDontRunTarget);
 if (value.GetError().Fail())
   value = frame.EvaluateExpression(expression.data());
 if (value.GetError().Fail()) {
-  response["success"] = llvm::json::Value(false);
-  // This error object must live until we're done with the pointer returned
-  // by GetCString().
-  lldb::SBError error = value.GetError();
-  const char *error_cstr = error.GetCString();
-  if (error_cstr && error_cstr[0])
-EmplaceSafeString(response, "message", std::string(error_cstr));
-  else
-EmplaceSafeString(response, "message", "evaluate failed");
+  // Evaluate the expression as an LLDB command when the expression was
+  // received from the REPL console.
+  if (context == "repl") {
+if (value.GetError().Fail()) {
+  auto result = RunLLDBCommands(llvm::StringRef(),
+{expression});
+  EmplaceSafeString(body, "result", result);
+  body.try_emplace("variablesReference", (int64_t)0);
+}
+  } else {
+response["success"] = llvm::json::Value(false);
+// This error object must live until we're done with the pointer
+// returned by GetCString().
+lldb::SBError error = value.GetError();
+const char *error_cstr = error.GetCString();
+if (error_cstr && error_cstr[0])
+  EmplaceSafeString(response, "message", std::string(error_cstr));
+else
+  EmplaceSafeString(response, "message", "evaluate failed");
+  }
 } else {
   SetValueForKey(value, body, "result");
   auto value_typename = value.GetType().GetDisplayTypeName();
-  EmplaceSafeString(body, "type", value_typename ? value_typename : NO_TYPENAME);
+  EmplaceSafeString(
+body, "type", value_typename ? value_typename : NO_TYPENAME);
   if (value.MightHaveChildren()) {
 auto variablesReference = VARIDX_TO_VARREF(g_vsc.variables.GetSize());
 g_vsc.variables.Append(value);
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -519,7 +519,7 @@
 }
 return self.send_recv(command_dict)
 
-def request_evaluate(self, expression, frameIndex=0, threadId=None):
+def request_evaluate(self, expression, frameIndex=0, threadId=None, context=None):
 stackFrame = self.get_stackFrame(frameIndex=frameIndex,
  threadId=threadId)
 if stackFrame is None:
@@ -528,6 +528,8 @@
 'expression': expression,
 'frameId': stackFrame['id'],
 }
+if context is not None:
+  args_dict["context"] = context
 command_dict = {
 'command': 'evaluate',
 'type': 'request',
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/variables/TestVSCode_variables.py
===

[Lldb-commits] [PATCH] D62547: (lldb-vscode) Evaluate expressions as LLDB commands when in REPL mode.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

one major issue that still needs to be resolved is we don't know which thread 
is currently selected in the IDE UI. You can make an example program that has 
two threads, each with different variables that are available, and then type 
"foo" where "foo" is a variable that is only available in one of the thread. 
Switch between the threads and select the one where "foo" isn't available and 
then type "foo" in the debugger console. We might need to watch for packets in 
the VS DAP protocol that indicate a thread of frame are selected and set the 
selected thread and frame when they change. I am bringing this up because now 
if the user types "foo" and it didn't happen to be a valid variable in the 
current thread at frame zero, then it will try to run it as an lldb command and 
the LLDB command interpreter will happily complete a command when given a 
unique prefix for an LLDB command which might be confusing to users when they 
type "x" and it tries to run "memory read" in lldb since "x" is an alias.

This is why lldb-vscode is currently coded to requiring a backtick character in 
the debugger console to indicate we truly do want an LLDB command. Thoughts?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62547/new/

https://reviews.llvm.org/D62547



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r361886 - [SymbolFileDWARF] Remove unused member (NFC)

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue May 28 15:33:30 2019
New Revision: 361886

URL: http://llvm.org/viewvc/llvm-project?rev=361886&view=rev
Log:
[SymbolFileDWARF] Remove unused member (NFC)

Removes the unused debug line instance.

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=361886&r1=361885&r2=361886&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Tue May 28 
15:33:30 2019
@@ -358,7 +358,7 @@ SymbolFileDWARF::SymbolFileDWARF(ObjectF
   m_debug_map_module_wp(), m_debug_map_symfile(nullptr),
   m_context(objfile->GetModule()->GetSectionList(), dwo_section_list),
   m_data_debug_loc(), m_data_debug_ranges(), m_data_debug_rnglists(),
-  m_abbr(), m_info(), m_line(), m_fetched_external_modules(false),
+  m_abbr(), m_info(), m_fetched_external_modules(false),
   m_supports_DW_AT_APPLE_objc_complete_type(eLazyBoolCalculate), 
m_ranges(),
   m_unique_ast_type_map() {}
 

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h?rev=361886&r1=361885&r2=361886&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h Tue May 28 
15:33:30 2019
@@ -453,11 +453,9 @@ protected:
   DWARFDataSegment m_data_debug_rnglists;
 
   // The unique pointer items below are generated on demand if and when someone
-  // accesses
-  // them through a non const version of this class.
+  // accesses them through a non const version of this class.
   std::unique_ptr m_abbr;
   std::unique_ptr m_info;
-  std::unique_ptr m_line;
   std::unique_ptr m_global_aranges_up;
 
   typedef std::unordered_map


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62547: (lldb-vscode) Evaluate expressions as LLDB commands when in REPL mode.

[Nathan Lanza via Phabricator via lldb-commits]

lanza added a comment.

Any reason why the backtick method is insufficient? I don't like the idea of 
the behavior of typing `q` or `c` being dependent upon the frame you're in.

On a similar note, I'm arguing for a better command prompt implementation in 
the DAP & VSCode. I'm trying to get a pty to attach the `CommandInterpreter` to 
and thus get the full editline completion and history as an officially 
supported feature. 
https://github.com/microsoft/debug-adapter-protocol/issues/45. If and when that 
goes in then this would be redundant.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62547/new/

https://reviews.llvm.org/D62547



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D61833: Fix IPv6 support on lldb-server platform

[António Afonso via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB361898: Fix IPv6 support on lldb-server platform 
(authored by aadsm, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61833?vs=200562&id=201784#toc

Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61833/new/

https://reviews.llvm.org/D61833

Files:
  source/Host/posix/ConnectionFileDescriptorPosix.cpp
  unittests/Host/CMakeLists.txt
  unittests/Host/ConnectionFileDescriptorTest.cpp
  unittests/Host/SocketTest.cpp
  unittests/Host/SocketTestUtilities.cpp
  unittests/Host/SocketTestUtilities.h

Index: unittests/Host/ConnectionFileDescriptorTest.cpp
===
--- unittests/Host/ConnectionFileDescriptorTest.cpp
+++ unittests/Host/ConnectionFileDescriptorTest.cpp
@@ -0,0 +1,50 @@
+//===-- ConnectionFileDescriptorTest.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SocketTestUtilities.h"
+#include "gtest/gtest.h"
+
+#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
+#include "lldb/Utility/UriParser.h"
+
+using namespace lldb_private;
+
+class ConnectionFileDescriptorTest : public testing::Test {
+public:
+  void SetUp() override {
+ASSERT_THAT_ERROR(Socket::Initialize(), llvm::Succeeded());
+  }
+
+  void TearDown() override { Socket::Terminate(); }
+
+  void TestGetURI(std::string ip) {
+std::unique_ptr socket_a_up;
+std::unique_ptr socket_b_up;
+if (!IsAddressFamilySupported(ip)) {
+  GTEST_LOG_(WARNING) << "Skipping test due to missing IPv"
+  << (IsIPv4(ip) ? "4" : "6") << " support.";
+  return;
+}
+CreateTCPConnectedSockets(ip, &socket_a_up, &socket_b_up);
+auto socket = socket_a_up.release();
+ConnectionFileDescriptor connection_file_descriptor(socket);
+
+llvm::StringRef scheme;
+llvm::StringRef hostname;
+int port;
+llvm::StringRef path;
+std::string uri(connection_file_descriptor.GetURI());
+EXPECT_TRUE(UriParser::Parse(uri, scheme, hostname, port, path));
+EXPECT_EQ(ip, hostname);
+EXPECT_EQ(socket->GetRemotePortNumber(), port);
+  }
+};
+
+TEST_F(ConnectionFileDescriptorTest, TCPGetURIv4) { TestGetURI("127.0.0.1"); }
+
+TEST_F(ConnectionFileDescriptorTest, TCPGetURIv6) { TestGetURI("::1"); }
\ No newline at end of file
Index: unittests/Host/CMakeLists.txt
===
--- unittests/Host/CMakeLists.txt
+++ unittests/Host/CMakeLists.txt
@@ -1,4 +1,5 @@
 set (FILES
+  ConnectionFileDescriptorTest.cpp
   FileActionTest.cpp
   FileSystemTest.cpp
   HostInfoTest.cpp
@@ -8,6 +9,7 @@
   ProcessLaunchInfoTest.cpp
   SocketAddressTest.cpp
   SocketTest.cpp
+  SocketTestUtilities.cpp
   TaskPoolTest.cpp
 )
 
Index: unittests/Host/SocketTestUtilities.h
===
--- unittests/Host/SocketTestUtilities.h
+++ unittests/Host/SocketTestUtilities.h
@@ -0,0 +1,47 @@
+//===- SocketTestUtilities.h *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_UNITTESTS_HOST_SOCKETTESTUTILITIES_H
+#define LLDB_UNITTESTS_HOST_SOCKETTESTUTILITIES_H
+
+#include 
+#include 
+#include 
+
+#include "lldb/Host/Config.h"
+#include "lldb/Host/Socket.h"
+#include "lldb/Host/common/TCPSocket.h"
+#include "lldb/Host/common/UDPSocket.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Testing/Support/Error.h"
+
+#ifndef LLDB_DISABLE_POSIX
+#include "lldb/Host/posix/DomainSocket.h"
+#endif
+
+namespace lldb_private {
+template 
+void CreateConnectedSockets(
+llvm::StringRef listen_remote_address,
+const std::function &get_connect_addr,
+std::unique_ptr *a_up, std::unique_ptr *b_up);
+bool CreateTCPConnectedSockets(std::string listen_remote_ip,
+   std::unique_ptr *a_up,
+   std::unique_ptr *b_up);
+#ifndef LLDB_DISABLE_POSIX
+void CreateDomainConnectedSockets(llvm::StringRef path,
+  std::unique_ptr *a_up,
+  std::unique_ptr *b_up);
+#endif
+
+bool IsAddressFamilySupported(std::string ip);
+bool IsIPv4(std::string ip);
+} // namespace lldb_private
+
+#endif
\ No newline at end of file
Index: unittests/Host/SocketTest.cpp
=

[Lldb-commits] [PATCH] D62221: [lldb-server][LLGS] Support 'g' packets

[Guilherme Andrade via Phabricator via lldb-commits]

guiandrade updated this revision to Diff 201790.
guiandrade added a comment.

Remove redundant cast  from GDBRemoteCommunicationServerLLGS::Handle_g and add 
annotation to TestGdbRemoteGPacket::g_returns_correct_data so it gets skipped 
if the running architecture isn't x86_64


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62221/new/

https://reviews.llvm.org/D62221

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteGPacket.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/Makefile
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/main.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp

Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -377,9 +377,7 @@
 break;
 
   case 'g':
-if (packet_size == 1)
-  return eServerPacketType_g;
-break;
+return eServerPacketType_g;
 
   case 'G':
 return eServerPacketType_G;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -178,6 +178,8 @@
 
   PacketResult Handle_QPassSignals(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_g(StringExtractorGDBRemote &packet);
+
   void SetCurrentThreadID(lldb::tid_t tid);
 
   lldb::tid_t GetCurrentThreadID() const;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -187,6 +187,9 @@
   StringExtractorGDBRemote::eServerPacketType_jTraceConfigRead,
   &GDBRemoteCommunicationServerLLGS::Handle_jTraceConfigRead);
 
+  RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_g,
+&GDBRemoteCommunicationServerLLGS::Handle_g);
+
   RegisterPacketHandler(StringExtractorGDBRemote::eServerPacketType_k,
 [this](StringExtractorGDBRemote packet, Status &error,
bool &interrupt, bool &quit) {
@@ -1891,6 +1894,61 @@
   return SendPacketNoLock("l");
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_g(StringExtractorGDBRemote &packet) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
+
+  // Move past packet name.
+  packet.SetFilePos(strlen("g"));
+
+  // Get the thread to use.
+  NativeThreadProtocol *thread = GetThreadFromSuffix(packet);
+  if (!thread) {
+LLDB_LOG(log, "failed, no thread available");
+return SendErrorResponse(0x15);
+  }
+
+  // Get the thread's register context.
+  NativeRegisterContext ®_ctx = thread->GetRegisterContext();
+
+  std::vector regs_buffer;
+  for (uint32_t reg_num = 0; reg_num < reg_ctx.GetUserRegisterCount();
+   ++reg_num) {
+const RegisterInfo *reg_info = reg_ctx.GetRegisterInfoAtIndex(reg_num);
+
+if (reg_info == nullptr) {
+  LLDB_LOG(log, "failed to get register info for register index {0}",
+   reg_num);
+  return SendErrorResponse(0x15);
+}
+
+if (reg_info->value_regs != nullptr)
+  continue; // skip registers that are contained in other registers
+
+RegisterValue reg_value;
+Status error = reg_ctx.ReadRegister(reg_info, reg_value);
+if (error.Fail()) {
+  LLDB_LOG(log, "failed to read register at index {0}", reg_num);
+  return SendErrorResponse(0x15);
+}
+
+if (reg_info->byte_offset + reg_info->byte_size >= regs_buffer.size())
+  // Resize the buffer to guarantee it can store the register offsetted
+  // data.
+  regs_buffer.resize(reg_info->byte_offset + reg_info->byte_size);
+
+// Copy the register offsetted data to the buffer.
+memcpy(regs_buffer.data() + reg_info->byte_offset, reg_value.GetBytes(),
+   reg_info->byte_size);
+  }
+
+  // Write the response.
+  StreamGDBRemote response;
+  response.PutBytesAsRawHex8(regs_buffer.data(), regs_buffer.size());
+
+  return SendPacketNoLock(response.GetString());
+}
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_p(Stri

[Lldb-commits] [PATCH] D62562: [Target] Introduce Process::GetLanguageRuntimes

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: JDevlieghere, davide, jingham.

Currently there's not really a good way to iterate over the language runtimes a
process has. This is sometimes desirable (as seen in my change to Thread).
Additionally, there's not really a good reason to iterate over every available
language, but rather only over languages for which we have a plugin loaded.


https://reviews.llvm.org/D62562

Files:
  include/lldb/Target/Language.h
  include/lldb/Target/Process.h
  source/Target/Language.cpp
  source/Target/Process.cpp
  source/Target/Thread.cpp

Index: source/Target/Thread.cpp
===
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -2211,11 +2211,14 @@
 
   // NOTE: Even though this behavior is generalized, only ObjC is actually
   // supported at the moment.
-  for (unsigned lang = eLanguageTypeUnknown; lang < eNumLanguageTypes; lang++) {
-if (auto runtime = GetProcess()->GetLanguageRuntime(
-static_cast(lang)))
-  if (auto e = runtime->GetExceptionObjectForThread(shared_from_this()))
-return e;
+  std::vector language_runtimes =
+  GetProcess()->GetLanguageRuntimes();
+  for (LanguageRuntime *runtime : language_runtimes) {
+if (!runtime)
+  continue;
+
+if (auto e = runtime->GetExceptionObjectForThread(shared_from_this()))
+  return e;
   }
 
   return ValueObjectSP();
@@ -2228,11 +2231,14 @@
 
   // NOTE: Even though this behavior is generalized, only ObjC is actually
   // supported at the moment.
-  for (unsigned lang = eLanguageTypeUnknown; lang < eNumLanguageTypes; lang++) {
-if (auto runtime = GetProcess()->GetLanguageRuntime(
-static_cast(lang)))
-  if (auto bt = runtime->GetBacktraceThreadFromException(exception))
-return bt;
+  std::vector language_runtimes =
+  GetProcess()->GetLanguageRuntimes();
+  for (LanguageRuntime *runtime : language_runtimes) {
+if (!runtime)
+  continue;
+
+if (auto bt = runtime->GetBacktraceThreadFromException(exception))
+  return bt;
   }
 
   return ThreadSP();
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -44,6 +44,7 @@
 #include "lldb/Target/InstrumentationRuntime.h"
 #include "lldb/Target/JITLoader.h"
 #include "lldb/Target/JITLoaderList.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Target/MemoryHistory.h"
 #include "lldb/Target/MemoryRegionInfo.h"
@@ -1547,6 +1548,27 @@
   return m_abi_sp;
 }
 
+std::vector
+Process::GetLanguageRuntimes(bool retry_if_null) {
+  std::vector language_runtimes;
+
+  if (m_finalizing)
+return language_runtimes;
+
+  std::lock_guard guard(m_language_runtimes_mutex);
+  // Before we pass off a copy of the language runtimes, we must make sure that
+  // our collection is properly populated. It's possible that some of the
+  // language runtimes were not loaded yet, either because nobody requested it
+  // yet or the proper condition for loading wasn't yet met (e.g. libc++.so
+  // hadn't been loaded).
+  for (const lldb::LanguageType lang_type : Language::GetSupportedLanguages()) {
+if (auto runtime = GetLanguageRuntime(lang_type, retry_if_null))
+  language_runtimes.emplace_back(std::move(runtime));
+  }
+
+  return language_runtimes;
+}
+
 LanguageRuntime *Process::GetLanguageRuntime(lldb::LanguageType language,
  bool retry_if_null) {
   if (m_finalizing)
Index: source/Target/Language.cpp
===
--- source/Target/Language.cpp
+++ source/Target/Language.cpp
@@ -348,6 +348,15 @@
   }
 }
 
+std::set Language::GetSupportedLanguages() {
+  std::set supported_languages;
+  ForEach([&](Language *lang) {
+supported_languages.emplace(lang->GetLanguageType());
+return true;
+  });
+  return supported_languages;
+}
+
 void Language::GetLanguagesSupportingTypeSystems(
 std::set &languages,
 std::set &languages_for_expressions) {
Index: include/lldb/Target/Process.h
===
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -2178,6 +2178,9 @@
 
   OperatingSystem *GetOperatingSystem() { return m_os_up.get(); }
 
+  std::vector
+  GetLanguageRuntimes(bool retry_if_null = true);
+
   LanguageRuntime *GetLanguageRuntime(lldb::LanguageType language,
   bool retry_if_null = true);
 
Index: include/lldb/Target/Language.h
===
--- include/lldb/Target/Language.h
+++ include/lldb/Target/Language.h
@@ -264,6 +264,8 @@
   // etc.
   static lldb::LanguageType GetPrimaryLanguage(lldb::LanguageType language);
 
+  static std::set GetSupportedLanguages();
+
   static void GetLanguagesS

[Lldb-commits] [lldb] r361915 - build: only search for the needed python type

[Saleem Abdulrasool via lldb-commits]

Author: compnerd
Date: Tue May 28 19:26:29 2019
New Revision: 361915

URL: http://llvm.org/viewvc/llvm-project?rev=361915&view=rev
Log:
build: only search for the needed python type

Windows has different types of runtime libraries which are ABI
incompatible with one another.  This requires that the debug build of
lldb link against the debug build of python.  Adjust the python search
to search for only the required type of python.  This permits building a
release build of lldb against just the release build of python.

Modified:
lldb/trunk/cmake/modules/LLDBConfig.cmake

Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=361915&r1=361914&r2=361915&view=diff
==
--- lldb/trunk/cmake/modules/LLDBConfig.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake Tue May 28 19:26:29 2019
@@ -153,83 +153,39 @@ function(find_python_libs_windows)
   endif()
 
   file(TO_CMAKE_PATH "${PYTHON_HOME}" PYTHON_HOME)
-  file(TO_CMAKE_PATH "${PYTHON_HOME}/python_d.exe" PYTHON_DEBUG_EXE)
-  file(TO_CMAKE_PATH "${PYTHON_HOME}/libs/${PYTHONLIBS_BASE_NAME}_d.lib" 
PYTHON_DEBUG_LIB)
-  file(TO_CMAKE_PATH "${PYTHON_HOME}/${PYTHONLIBS_BASE_NAME}_d.dll" 
PYTHON_DEBUG_DLL)
-
-  file(TO_CMAKE_PATH "${PYTHON_HOME}/python.exe" PYTHON_RELEASE_EXE)
-  file(TO_CMAKE_PATH "${PYTHON_HOME}/libs/${PYTHONLIBS_BASE_NAME}.lib" 
PYTHON_RELEASE_LIB)
-  file(TO_CMAKE_PATH "${PYTHON_HOME}/${PYTHONLIBS_BASE_NAME}.dll" 
PYTHON_RELEASE_DLL)
-
-  if (NOT EXISTS ${PYTHON_DEBUG_EXE})
-message("Unable to find ${PYTHON_DEBUG_EXE}")
-unset(PYTHON_DEBUG_EXE)
-  endif()
-
-  if (NOT EXISTS ${PYTHON_RELEASE_EXE})
-message("Unable to find ${PYTHON_RELEASE_EXE}")
-unset(PYTHON_RELEASE_EXE)
-  endif()
-
-  if (NOT EXISTS ${PYTHON_DEBUG_LIB})
-message("Unable to find ${PYTHON_DEBUG_LIB}")
-unset(PYTHON_DEBUG_LIB)
-  endif()
-
-  if (NOT EXISTS ${PYTHON_RELEASE_LIB})
-message("Unable to find ${PYTHON_RELEASE_LIB}")
-unset(PYTHON_RELEASE_LIB)
-  endif()
-
-  if (NOT EXISTS ${PYTHON_DEBUG_DLL})
-message("Unable to find ${PYTHON_DEBUG_DLL}")
-unset(PYTHON_DEBUG_DLL)
-  endif()
+  # TODO(compnerd) when CMake Policy `CMP0091` is set to NEW, we should use
+  # if(CMAKE_MSVC_RUNTIME_LIBRARY MATCHES MultiThreadedDebug)
+  if(CMAKE_BUILD_TYPE STREQUAL Debug)
+file(TO_CMAKE_PATH "${PYTHON_HOME}/python_d.exe" PYTHON_EXE)
+file(TO_CMAKE_PATH "${PYTHON_HOME}/libs/${PYTHONLIBS_BASE_NAME}_d.lib" 
PYTHON_LIB)
+file(TO_CMAKE_PATH "${PYTHON_HOME}/${PYTHONLIBS_BASE_NAME}_d.dll" 
PYTHON_DLL)
+  else()
+file(TO_CMAKE_PATH "${PYTHON_HOME}/python.exe" PYTHON_EXE)
+file(TO_CMAKE_PATH "${PYTHON_HOME}/libs/${PYTHONLIBS_BASE_NAME}.lib" 
PYTHON_LIB)
+file(TO_CMAKE_PATH "${PYTHON_HOME}/${PYTHONLIBS_BASE_NAME}.dll" PYTHON_DLL)
+  endif()
+
+  foreach(component PYTHON_EXE;PYTHON_LIB;PYTHON_DLL)
+if(NOT EXISTS ${${component}})
+  message("unable to find ${component}")
+  unset(${component})
+endif()
+  endforeach()
 
-  if (NOT EXISTS ${PYTHON_RELEASE_DLL})
-message("Unable to find ${PYTHON_RELEASE_DLL}")
-unset(PYTHON_RELEASE_DLL)
-  endif()
-
-  if (NOT (PYTHON_DEBUG_EXE AND PYTHON_RELEASE_EXE AND PYTHON_DEBUG_LIB AND 
PYTHON_RELEASE_LIB AND PYTHON_DEBUG_DLL AND PYTHON_RELEASE_DLL))
-message("Python installation is corrupt. Python support will be disabled 
for this build.")
+  if (NOT PYTHON_EXE OR NOT PYTHON_LIB OR NOT PYTHON_DLL)
+message("Unable to find all Python components.  Python support will be 
disabled for this build.")
 set(LLDB_DISABLE_PYTHON 1 PARENT_SCOPE)
 return()
   endif()
 
-  # Generator expressions are evaluated in the context of each build 
configuration generated
-  # by CMake. Here we use the $:VALUE logical generator 
expression to ensure
-  # that the debug Python library, DLL, and executable are used in the Debug 
build configuration.
-  #
-  # Generator expressions can be difficult to grok at first so here's a 
breakdown of the one
-  # used for PYTHON_LIBRARY:
-  #
-  # 1. $ evaluates to 1 when the Debug configuration is being 
generated,
-  #or 0 in all other cases.
-  # 2. $<$:${PYTHON_DEBUG_LIB}> expands to ${PYTHON_DEBUG_LIB} 
when the Debug
-  #configuration is being generated, or nothing (literally) in all other 
cases.
-  # 3. $<$>:${PYTHON_RELEASE_LIB}> expands to 
${PYTHON_RELEASE_LIB} when
-  #any configuration other than Debug is being generated, or nothing in 
all other cases.
-  # 4. The conditionals in 2 & 3 are mutually exclusive.
-  # 5. A logical expression with a conditional that evaluates to 0 yields no 
value at all.
-  #
-  # Due to 4 & 5 it's possible to concatenate 2 & 3 to obtain a single value 
specific to each
-  # build configuration. In this example the value will be ${PYTHON_DEBUG_LIB} 
when generating the
-  # Debug configuration, or ${PYTHON_RELEASE_LI

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

[António Afonso via Phabricator via lldb-commits]

aadsm marked an inline comment as done.
aadsm added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2738
+  if (!memory_buffer_sp) {
+if (xfer_object == "auxv") {
+// *BSD impls should be able to do this too.

xiaobai wrote:
> labath wrote:
> > Given that this function is going to grow, it would be good to split it in 
> > smaller chunks. Maybe move this code into something like 
> > `ErrorOr ReadObject(StringRef object)`?
> +1
> 
> You could have smaller methods like "ParseAuxvPacket" and "ReadObject"
I'm actually trying to return an llvm::Expected so I can return a 
`createStringError` that will have a message and an error code (as far as I can 
see `ErrorOr`only allows to return an error code).
However, I can't figure out how to get both the error code and error message 
from the `takeError()` function. I found `llvm::errorToErrorCode` and 
`llvm::toString` but they both require to pass ownership of the error to them 
so I can only use one of them.
Is the only way to use the `handleErrors` function that will give me access to 
the underlying `ErrorInfo` where I can then call `convertToErrorCode` and 
`getMessage` on it? Sounds overly complicated for something that's probably 
simpler than this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62499/new/

https://reviews.llvm.org/D62499



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[António Afonso via Phabricator via lldb-commits]

aadsm added a comment.

> However, now that I think about it, that is nonsense, because there is no way 
> for us to say to the user that "we failed to read some initial bytes, but 
> this is the memory contents after that". So just using process_vm_readv, and 
> finishing up with ptrace sounds fine to me.

The reason why I thought reading page by page still made sense in the 
ReadMemory is to cover for the situation where we cross 3 pages and the page in 
the middle is not readable. However, that might not be realistic to happen?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62503/new/

https://reviews.llvm.org/D62503



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62562: [Target] Introduce Process::GetLanguageRuntimes

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: source/Target/Thread.cpp:2216
+  GetProcess()->GetLanguageRuntimes();
+  for (LanguageRuntime *runtime : language_runtimes) {
+if (!runtime)

if ` for (LanguageRuntime *runtime : GetProcess()->GetLanguageRuntimes()) {` 
fits on one line I would get rid of the temporary. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62562/new/

https://reviews.llvm.org/D62562



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62570: [WIP] Use LLVM's debug line parser in LLDB

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, aprantl, clayborg, grimar, friss.
Herald added subscribers: llvm-commits, jdoerfert, abidh, mgorny.
Herald added projects: LLDB, LLVM.

The line number table header was substantially revised in DWARF 5, which is not 
supported by LLDB's current debug line parser. Given the relatively limited 
contact surface between the code to parse the debug line section and the rest 
of LLDB, this seems like a good candidate to replace with LLVM's 
implementation, which does support the new standard.

In its current state this patch is mostly a proof-of-concept to show that this 
transition is possible. There are currently 8 failing tests, all related to the 
file paths from the prologue. (I know because I did the change in two parts, 
first the line table entries, then the so-called *support files*) I have yet to 
look at the failures in detail, but they are related to rnglists and split 
DWARF. Additionally, I'll need to ensure that there's minimal duplicate work 
when creating an LLVM DWARFContext and CompileUnit. We should only be parsing 
the UnitDIE, which I think is reasonable.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D62570

Files:
  lldb/include/lldb/Core/FileSpecList.h
  lldb/include/lldb/Core/Section.h
  lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
  lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

Index: llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
===
--- llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
+++ llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
@@ -216,6 +216,10 @@
 return NormalUnits[index].get();
   }
 
+  DWARFUnit *getUnitForOffset(uint32_t Offset) {
+return NormalUnits.getUnitForOffset(Offset);
+  }
+
   /// Get the unit at the specified index for the DWO units.
   DWARFUnit *getDWOUnitAtIndex(unsigned index) {
 parseDWOUnits();
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -476,6 +476,9 @@
   DIEToVariableSP m_die_to_variable_sp;
   DIEToClangType m_forward_decl_die_to_clang_type;
   ClangTypeToDIE m_forward_decl_clang_type_to_die;
+
+  llvm::DenseMap
+  m_support_files;
 };
 
 #endif // SymbolFileDWARF_SymbolFileDWARF_h_
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -57,7 +57,6 @@
 #include "DWARFDebugAbbrev.h"
 #include "DWARFDebugAranges.h"
 #include "DWARFDebugInfo.h"
-#include "DWARFDebugLine.h"
 #include "DWARFDebugMacro.h"
 #include "DWARFDebugRanges.h"
 #include "DWARFDeclContext.h"
@@ -70,6 +69,7 @@
 #include "SymbolFileDWARFDwo.h"
 #include "SymbolFileDWARFDwp.h"
 
+#include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/Support/FileSystem.h"
 
 #include 
@@ -780,23 +780,30 @@
 FileSpecList &support_files) {
   ASSERT_MODULE_LOCK(this);
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
-  if (dwarf_cu) {
-const DWARFBaseDIE cu_die = dwarf_cu->GetUnitDIEOnly();
-
-if (cu_die) {
-  const dw_offset_t stmt_list = cu_die.GetAttributeValueAsUnsigned(
-  DW_AT_stmt_list, DW_INVALID_OFFSET);
-  if (stmt_list != DW_INVALID_OFFSET) {
-// All file indexes in DWARF are one based and a file of index zero is
-// supposed to be the compile unit itself.
-support_files.Append(comp_unit);
-return DWARFDebugLine::ParseSupportFiles(
-comp_unit.GetModule(), m_context.getOrLoadLineData(), stmt_list,
-support_files, dwarf_cu);
-  }
-}
-  }
-  return false;
+  if (!dwarf_cu)
+return false;
+
+  const DWARFBaseDIE cu_die = dwarf_cu->GetUnitDIEOnly();
+  if (!cu_die)
+return false;
+
+  const dw_offset_t stmt_list =
+  cu_die.GetAttributeValueAsUnsigned(DW_AT_stmt_list, DW_INVALID_OFFSET);
+  if (stmt_list == DW_INVALID_OFFSET)
+return false;
+
+  // All file indexes in DWARF are one based and a file of index zero is
+  // supposed to be the compile unit itself.
+  // FIXME: This is false for DWARF5
+  support_files.Append(comp_unit);
+
+  if (!comp_unit.GetLineTable())
+ParseLineTable(comp_unit);
+
+  for (const FileSpec &file : m_support_files[&comp_unit])
+support_files.Append(file);
+
+  return true;
 }
 
 bool SymbolFileDWARF::ParseIsOptimized(CompileUnit &comp_unit) {
@@ -859

[Lldb-commits] [PATCH] D62547: (lldb-vscode) Evaluate expressions as LLDB commands when in REPL mode.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't know much about vscode or DAP, but this guessing of which kind of a 
command the user meant to type sounds like a bad idea to me. Having this 
specified explicitly (via backticks or whatever) feels like a better approach...


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62547/new/

https://reviews.llvm.org/D62547



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits