date:20190605

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

2019-06-05 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:2122
+  size_t phdr_num_entries = *maybe_phdr_num_entries;
+  lldb::addr_t load_base = phdr_addr - sizeof(ELF_EHDR);
+

aadsm wrote:
> labath wrote:
> > This innocent-looking line assumes that the program headers will be the 
> > very next thing coming after the elf header, and that the elf header will 
> > be contained in the lowest-located segment. Although they are usually true, 
> > neither of the two assumptions are really guaranteed.  I spent a bunch of 
> > time figuring out if there's a way to avoid that, but I haven't come up 
> > with anything... :/
> Ah, I thought that was always true. I checked gdb and they're actually using 
> the PT_PHDR entry (which gives the location of the program table itself) to 
> calculate the relocation. We should probably do the same.
Ah, of course. So, the load bias could be calculated as the difference between 
the PHDR address we got from the aux vector and the address that is contained 
in the phdr itself. That's really neat. :)



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:2136
+sizeof(phdr_entry), bytes_read);
+if (!error.Success())
+  return LLDB_INVALID_ADDRESS;

aadsm wrote:
> labath wrote:
> > Are you sure that ReadMemory doesn't return "success" on partial reads too ?
> We correctly return a non-success response if ptrace failed. However, from 
> what I could see ptrace was successfully reading 0's when the memory segment 
> was not readable for instance. I'm not concern if we're reading 0's if the 
> segment is not readable, or should I be?
> We correctly return a non-success response if ptrace failed.
Hmm... that's odd. I had a feeling most of our read-like functions return 
"success" if they got at least some data. But, anyway, that's not your problem.

> However, from what I could see ptrace was successfully reading 0's when the 
> memory segment was not readable for instance. I'm not concern if we're 
> reading 0's if the segment is not readable, or should I be?
By, "not readable", you mean mapped into process memory, but without a read 
permission (e.g., mmap(..., PROT_NONE)). In this case, I don't think it will 
return zero, but it will actually ignore the read permission and just return 
whatever is the actual contents of the memory.

However, I don't think we have to worry about that. I was mainly worried that 
ReadMemory will not fill in the `phdr_entry` struct and that we will later 
access uninitialized memory (sounding all kinds of asan warnings and stuff). 
The fact that ReadMemory may return incorrect data is something we have to be 
prepared to handle anyway, as we cannot really trust the application to provide 
the correct data here.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:2168
+  return LLDB_INVALID_ADDRESS;
+// Return the &DT_DEBUG->d_ptr which points to r_debug which contains the
+// link_map.

aadsm wrote:
> labath wrote:
> > The address of r_debug shouldn't ever change, right?
> > Wouldn't it be better return &r_debug directly, instead of returning a 
> > pointer to a pointer?
> It should not change but it might not be initialized yet. But the big reason 
> is because `GetSharedLibraryInfoAddress` should return (which I'm planning to 
> hook up to `qShlibInfoAddr`) the address of where the debug structure pointer 
> is.
> At least that's what I gather from reading the `ResolveRendezvousAddress` 
> function, the documentation is not 100% clear (imho).
Ok, sounds good then.


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] [lldb] r362586 - Ignore DIEs in the skeleton unit in a DWO scenario

2019-06-05 Thread Pavel Labath via lldb-commits

Author: labath
Date: Wed Jun  5 00:29:55 2019
New Revision: 362586

URL: http://llvm.org/viewvc/llvm-project?rev=362586&view=rev
Log:
Ignore DIEs in the skeleton unit in a DWO scenario

Summary:
r362103 exposed a bug, where we could read incorrect data if a skeleton
unit contained more than the single unit DIE. Clang emits these kinds of
units with -fsplit-dwarf-inlining (which is also the default).

Changing lldb to handle these DIEs is nontrivial, as we'd have to change
the UID encoding logic to be able to reference these DIEs, and fix up
various places which are assuming that all DIEs come from the separate
compile unit.

However, it turns out this is not necessary, as the DWO unit contains
all the information that the skeleton unit does. So, this patch just
skips parsing the extra DIEs if we have successfully found the DWO file.
This enforces the invariant that the rest of the code is already
operating under.

This patch fixes a couple of existing tests, but I've also included a
simpler test which does not depend on execution of binaries, and would
have helped us in catching this sooner.

Reviewers: clayborg, JDevlieghere, aprantl

Subscribers: probinson, dblaikie, lldb-commits

Differential Revision: https://reviews.llvm.org/D62852

Added:
lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

Added: lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp?rev=362586&view=auto
==
--- lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp (added)
+++ lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp Wed Jun  5 
00:29:55 2019
@@ -0,0 +1,8 @@
+// RUN: %clangxx -target x86_64-pc-linux -gsplit-dwarf -fsplit-dwarf-inlining \
+// RUN:   -c %s -o %t
+// RUN: %lldb %t -o "breakpoint set -n foo" -b | FileCheck %s
+
+// CHECK: Breakpoint 1: 2 locations
+
+__attribute__((always_inline)) int foo(int x) { return x; }
+int bar(int x) { return foo(x); }

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp?rev=362586&r1=362585&r2=362586&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp Wed Jun  5 
00:29:55 2019
@@ -183,6 +183,17 @@ void DWARFUnit::ExtractDIEsRWLocked() {
 
   if (!m_first_die)
 AddUnitDIE(m_die_array.front());
+
+  // With -fsplit-dwarf-inlining, clang will emit non-empty skeleton 
compile
+  // units. We are not able to access these DIE *and* the dwo file
+  // simultaneously. We also don't need to do that as the dwo file will
+  // contain a superset of information. So, we don't even attempt to parse
+  // any remaining DIEs.
+  if (m_dwo_symbol_file) {
+m_die_array.front().SetHasChildren(false);
+break;
+  }
+
 } else {
   if (null_die) {
 if (prev_die_had_children) {


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

[Lldb-commits] [PATCH] D62852: Ignore DIEs in the skeleton unit in a DWO scenario

2019-06-05 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362586: Ignore DIEs in the skeleton unit in a DWO scenario 
(authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62852

Files:
  lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp


Index: lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
===
--- lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
+++ lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
@@ -0,0 +1,8 @@
+// RUN: %clangxx -target x86_64-pc-linux -gsplit-dwarf -fsplit-dwarf-inlining \
+// RUN:   -c %s -o %t
+// RUN: %lldb %t -o "breakpoint set -n foo" -b | FileCheck %s
+
+// CHECK: Breakpoint 1: 2 locations
+
+__attribute__((always_inline)) int foo(int x) { return x; }
+int bar(int x) { return foo(x); }
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -183,6 +183,17 @@
 
   if (!m_first_die)
 AddUnitDIE(m_die_array.front());
+
+  // With -fsplit-dwarf-inlining, clang will emit non-empty skeleton 
compile
+  // units. We are not able to access these DIE *and* the dwo file
+  // simultaneously. We also don't need to do that as the dwo file will
+  // contain a superset of information. So, we don't even attempt to parse
+  // any remaining DIEs.
+  if (m_dwo_symbol_file) {
+m_die_array.front().SetHasChildren(false);
+break;
+  }
+
 } else {
   if (null_die) {
 if (prev_die_had_children) {


Index: lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
===
--- lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
+++ lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
@@ -0,0 +1,8 @@
+// RUN: %clangxx -target x86_64-pc-linux -gsplit-dwarf -fsplit-dwarf-inlining \
+// RUN:   -c %s -o %t
+// RUN: %lldb %t -o "breakpoint set -n foo" -b | FileCheck %s
+
+// CHECK: Breakpoint 1: 2 locations
+
+__attribute__((always_inline)) int foo(int x) { return x; }
+int bar(int x) { return foo(x); }
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -183,6 +183,17 @@
 
   if (!m_first_die)
 AddUnitDIE(m_die_array.front());
+
+  // With -fsplit-dwarf-inlining, clang will emit non-empty skeleton compile
+  // units. We are not able to access these DIE *and* the dwo file
+  // simultaneously. We also don't need to do that as the dwo file will
+  // contain a superset of information. So, we don't even attempt to parse
+  // any remaining DIEs.
+  if (m_dwo_symbol_file) {
+m_die_array.front().SetHasChildren(false);
+break;
+  }
+
 } else {
   if (null_die) {
 if (prev_die_had_children) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62878: [CMake] Export CMAKE_CONFIGURATION_TYPES for the LLVM build-tree

2019-06-05 Thread Phabricator via Phabricator via lldb-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362588: [CMake] Export CMAKE_CONFIGURATION_TYPES for the 
LLVM build-tree (authored by stefan.graenitz, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D62878?vs=203013&id=203100#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62878

Files:
  llvm/trunk/cmake/modules/LLVMConfig.cmake.in


Index: llvm/trunk/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/trunk/cmake/modules/LLVMConfig.cmake.in
+++ llvm/trunk/cmake/modules/LLVMConfig.cmake.in
@@ -83,6 +83,7 @@
 set(LLVM_TOOLS_BINARY_DIR "@LLVM_CONFIG_TOOLS_BINARY_DIR@")
 set(LLVM_TOOLS_INSTALL_DIR "@LLVM_TOOLS_INSTALL_DIR@")
 set(LLVM_HAVE_OPT_VIEWER_MODULES @LLVM_HAVE_OPT_VIEWER_MODULES@)
+set(LLVM_CONFIGURATION_TYPES @CMAKE_CONFIGURATION_TYPES@)
 
 if(NOT TARGET LLVMSupport)
   set(LLVM_EXPORTED_TARGETS "@LLVM_CONFIG_EXPORTS@")


Index: llvm/trunk/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/trunk/cmake/modules/LLVMConfig.cmake.in
+++ llvm/trunk/cmake/modules/LLVMConfig.cmake.in
@@ -83,6 +83,7 @@
 set(LLVM_TOOLS_BINARY_DIR "@LLVM_CONFIG_TOOLS_BINARY_DIR@")
 set(LLVM_TOOLS_INSTALL_DIR "@LLVM_TOOLS_INSTALL_DIR@")
 set(LLVM_HAVE_OPT_VIEWER_MODULES @LLVM_HAVE_OPT_VIEWER_MODULES@)
+set(LLVM_CONFIGURATION_TYPES @CMAKE_CONFIGURATION_TYPES@)
 
 if(NOT TARGET LLVMSupport)
   set(LLVM_EXPORTED_TARGETS "@LLVM_CONFIG_EXPORTS@")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62879: [CMake] Add configuration dirs as potential locations for llvm-lit and llvm-tblgen in standalone builds

2019-06-05 Thread Phabricator via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362589: [CMake] Add configuration dirs as potential 
locations for llvm-lit and llvm… (authored by stefan.graenitz, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62879?vs=203030&id=203102#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62879

Files:
  lldb/trunk/cmake/modules/LLDBStandalone.cmake


Index: lldb/trunk/cmake/modules/LLDBStandalone.cmake
===
--- lldb/trunk/cmake/modules/LLDBStandalone.cmake
+++ lldb/trunk/cmake/modules/LLDBStandalone.cmake
@@ -1,3 +1,12 @@
+function(append_configuration_directories input_dir output_dirs)
+  set(dirs_list ${input_dir})
+  foreach(config_type ${LLVM_CONFIGURATION_TYPES})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} dir ${input_dir})
+list(APPEND dirs_list ${dir})
+  endforeach()
+  set(${output_dirs} ${dirs_list} PARENT_SCOPE)
+endfunction()
+
 # If we are not building as a part of LLVM, build LLDB as an
 # standalone project, using LLVM as an external library:
 if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
@@ -27,7 +36,10 @@
   if(CMAKE_HOST_WIN32 AND NOT CYGWIN)
 set(lit_file_name "${lit_file_name}.py")
   endif()
-  set(LLVM_DEFAULT_EXTERNAL_LIT "${LLVM_TOOLS_BINARY_DIR}/${lit_file_name}" 
CACHE PATH "Path to llvm-lit")
+
+  append_configuration_directories(${LLVM_TOOLS_BINARY_DIR} config_dirs)
+  find_program(lit_full_path ${lit_file_name} ${config_dirs} NO_DEFAULT_PATH)
+  set(LLVM_DEFAULT_EXTERNAL_LIT ${lit_full_path} CACHE PATH "Path to llvm-lit")
 
   if(CMAKE_CROSSCOMPILING)
 set(LLVM_NATIVE_BUILD "${LLDB_PATH_TO_LLVM_BUILD}/NATIVE")
@@ -51,8 +63,9 @@
 
"${LLVM_NATIVE_BUILD}/Release/bin/llvm-tblgen${HOST_EXECUTABLE_SUFFIX}")
 endif()
   else()
-find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
-  NO_DEFAULT_PATH)
+set(tblgen_file_name "llvm-tblgen${CMAKE_EXECUTABLE_SUFFIX}")
+append_configuration_directories(${LLVM_TOOLS_BINARY_DIR} config_dirs)
+find_program(LLVM_TABLEGEN_EXE ${tblgen_file_name} ${config_dirs} 
NO_DEFAULT_PATH)
   endif()
 
   # They are used as destination of target generators.


Index: lldb/trunk/cmake/modules/LLDBStandalone.cmake
===
--- lldb/trunk/cmake/modules/LLDBStandalone.cmake
+++ lldb/trunk/cmake/modules/LLDBStandalone.cmake
@@ -1,3 +1,12 @@
+function(append_configuration_directories input_dir output_dirs)
+  set(dirs_list ${input_dir})
+  foreach(config_type ${LLVM_CONFIGURATION_TYPES})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} dir ${input_dir})
+list(APPEND dirs_list ${dir})
+  endforeach()
+  set(${output_dirs} ${dirs_list} PARENT_SCOPE)
+endfunction()
+
 # If we are not building as a part of LLVM, build LLDB as an
 # standalone project, using LLVM as an external library:
 if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
@@ -27,7 +36,10 @@
   if(CMAKE_HOST_WIN32 AND NOT CYGWIN)
 set(lit_file_name "${lit_file_name}.py")
   endif()
-  set(LLVM_DEFAULT_EXTERNAL_LIT "${LLVM_TOOLS_BINARY_DIR}/${lit_file_name}" CACHE PATH "Path to llvm-lit")
+
+  append_configuration_directories(${LLVM_TOOLS_BINARY_DIR} config_dirs)
+  find_program(lit_full_path ${lit_file_name} ${config_dirs} NO_DEFAULT_PATH)
+  set(LLVM_DEFAULT_EXTERNAL_LIT ${lit_full_path} CACHE PATH "Path to llvm-lit")
 
   if(CMAKE_CROSSCOMPILING)
 set(LLVM_NATIVE_BUILD "${LLDB_PATH_TO_LLVM_BUILD}/NATIVE")
@@ -51,8 +63,9 @@
 "${LLVM_NATIVE_BUILD}/Release/bin/llvm-tblgen${HOST_EXECUTABLE_SUFFIX}")
 endif()
   else()
-find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
-  NO_DEFAULT_PATH)
+set(tblgen_file_name "llvm-tblgen${CMAKE_EXECUTABLE_SUFFIX}")
+append_configuration_directories(${LLVM_TOOLS_BINARY_DIR} config_dirs)
+find_program(LLVM_TABLEGEN_EXE ${tblgen_file_name} ${config_dirs} NO_DEFAULT_PATH)
   endif()
 
   # They are used as destination of target generators.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r362589 - [CMake] Add configuration dirs as potential locations for llvm-lit and llvm-tblgen in standalone builds

2019-06-05 Thread Stefan Granitz via lldb-commits

Author: stefan.graenitz
Date: Wed Jun  5 01:31:50 2019
New Revision: 362589

URL: http://llvm.org/viewvc/llvm-project?rev=362589&view=rev
Log:
[CMake] Add configuration dirs as potential locations for llvm-lit and 
llvm-tblgen in standalone builds

Summary:
If the provided LLVM build-tree used a multi-configuration generator like 
Xcode, `LLVM_TOOLS_BINARY_DIR` will have a generator-specific placeholder to 
express `CMAKE_CFG_INTDIR`. Thus `llvm-lit` and `llvm-tblgen` won't be found.
D62878 exports the actual configuration types so we can fix the path and add 
them to the search paths for `find_program()`.

Reviewers: xiaobai, labath, stella.stamenova

Reviewed By: xiaobai, stella.stamenova

Subscribers: mgorny, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D62879

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

Modified: lldb/trunk/cmake/modules/LLDBStandalone.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBStandalone.cmake?rev=362589&r1=362588&r2=362589&view=diff
==
--- lldb/trunk/cmake/modules/LLDBStandalone.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBStandalone.cmake Wed Jun  5 01:31:50 2019
@@ -1,3 +1,12 @@
+function(append_configuration_directories input_dir output_dirs)
+  set(dirs_list ${input_dir})
+  foreach(config_type ${LLVM_CONFIGURATION_TYPES})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} dir ${input_dir})
+list(APPEND dirs_list ${dir})
+  endforeach()
+  set(${output_dirs} ${dirs_list} PARENT_SCOPE)
+endfunction()
+
 # If we are not building as a part of LLVM, build LLDB as an
 # standalone project, using LLVM as an external library:
 if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
@@ -27,7 +36,10 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
   if(CMAKE_HOST_WIN32 AND NOT CYGWIN)
 set(lit_file_name "${lit_file_name}.py")
   endif()
-  set(LLVM_DEFAULT_EXTERNAL_LIT "${LLVM_TOOLS_BINARY_DIR}/${lit_file_name}" 
CACHE PATH "Path to llvm-lit")
+
+  append_configuration_directories(${LLVM_TOOLS_BINARY_DIR} config_dirs)
+  find_program(lit_full_path ${lit_file_name} ${config_dirs} NO_DEFAULT_PATH)
+  set(LLVM_DEFAULT_EXTERNAL_LIT ${lit_full_path} CACHE PATH "Path to llvm-lit")
 
   if(CMAKE_CROSSCOMPILING)
 set(LLVM_NATIVE_BUILD "${LLDB_PATH_TO_LLVM_BUILD}/NATIVE")
@@ -51,8 +63,9 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
 
"${LLVM_NATIVE_BUILD}/Release/bin/llvm-tblgen${HOST_EXECUTABLE_SUFFIX}")
 endif()
   else()
-find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
-  NO_DEFAULT_PATH)
+set(tblgen_file_name "llvm-tblgen${CMAKE_EXECUTABLE_SUFFIX}")
+append_configuration_directories(${LLVM_TOOLS_BINARY_DIR} config_dirs)
+find_program(LLVM_TABLEGEN_EXE ${tblgen_file_name} ${config_dirs} 
NO_DEFAULT_PATH)
   endif()
 
   # They are used as destination of target generators.


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

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

2019-06-05 Thread Pavel Labath via lldb-commits

The "overlapping sections" thread reminding me I should reply here.. It
also reminded me of another possible use case for multiply-mapped
sections. Thread-local sections (.tbss, .tdata in elf) contain data
which is somehow (the exact mechanisms are still quite opaque to me)
mapped memory for each thread. I am not sure what we do about these
things now, but this also sounds like a thing that could/should be
modelled as a single section being loaded multiple times.

On 31/05/2019 00:23, Jim Ingham wrote:

I agree that the platform path has to be held by the target, not by the module.
In your example lldb would love to have a single local copy of the executable
so it can do reads locally, which makes it clear that there are real cases
where it would be obvious two targets should share the same Module, but would
need to have different remote names for the it.

I don't see how extending the Platform Path in the module to be a vector can handle
this situation. We have tried to keep Modules from knowing about Targets, but the
only way to make sense of all the platform paths associated with the Module is to
know which one is used by a given Target. So you'd really have to have a pair of
Name & Target. That argues that the Module was the wrong place for this
information to begin with.

You could imagine doing this by having a map of platform path -> module in the
target, but then there are all sorts of places you'd have to be careful to consult
this, and that seems fallible.

Yes, this sounds pretty sub-optimal.

It really sounds like the Target should be dealing with TargetModules (a pair of Module &
PlatformPath) and not straight Modules. Probably for the sake of reducing changes we could
have "CacheModules" which are what Modules are today, and Modules that are the pair
of PlatformPath and Module. I think this is what Greg was talking about with his BaseModule
class.

I think I like the direction of this. This additional separation might
also help with another I've been having with how modules work. Right now
if we call Module::SetSymbolFileSpec, it will construct a new
SymbolFile, but then still keep the old one around just in case somebody
uses it. It would be cleaner if "TargetModule::SetSymbolFileSpec" could
just create a new instace of "CacheModule" with the new symbol file, and
just drop the reference to the old CacheModule.

And then we could avoid storing a vector of "platform paths" in the
TargetModule by just creating two TargetModules -- since they would be
sharing the same CacheModule, this wouldn't cost much.

Then if you want to support the same CacheModule being reused in a given
target, the SectionLoadList would have to hold a pair of Section/TargetModule
- a TargetSection? - so it would know which instance of the module was meant.
I think you also have to do the same thing with Address. It currently holds a
Section, but that isn't fully specified in the case where the Module can appear
twice. It would have to have a TargetSection instead. Except that you can
pull Address's out of Modules w/o going through a Target. So again you might
have to make a distinction between Address and TargetAddress, which might get
messy.

This might get messy, but it also might enable us to clean some things
up and/or strengthen some invariants. For instance, if we call the
TargetSections "LoadedSections" and create them only when they are
loaded into a target, then we can maintain the invariant that a
LoadedSection can always resolve itself to a "load address" (right now
we can do that only if a section happens to be loaded), and to a
"CacheSection".

The Address class can now hold two kinds of addresses: a load address
and a section+offset combo. For converting between the two one has to go
through a target. I think it would make sense for the conversion process
to return something of a different type, as then one would always know
which kind of address he is talking about. This might also be
interesting for the address space folks, as the address space may need
to be represented differently for the two address kinds. for a
"sectioned address" the address space can probably be given implicitly
via the section, but a "load address" would need to store the address
space explicitly.

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

[Lldb-commits] [PATCH] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

2019-06-05 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz updated this revision to Diff 203106.
sgraenitz added a comment.

Remove Xcode restriction and polish


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62859

Files:
  lldb/lit/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt


Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -5,8 +5,28 @@
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
-# Generate wrapper for each build mode.
-if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+# Generate lldb-dotest Python driver script for each build mode.
+if(LLDB_BUILT_STANDALONE)
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# In paths to our build-tree, replace CMAKE_CFG_INTDIR with our actual 
configuration names.
+string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} 
config_runtime_output_dir ${LLVM_RUNTIME_OUTPUT_INTDIR})
+string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} 
LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
+
+# Remaining ones must be paths to the provided LLVM build-tree.
+if(${config_type} IN_LIST LLVM_CONFIGURATION_TYPES)
+  # Multi-configuration generator like Xcode (with a matching config).
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+else()
+  # Single-configuration generator like Ninja.
+  string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+endif()
+
+configure_file(
+  lldb-dotest.in
+  ${config_runtime_output_dir}/lldb-dotest @ONLY
+)
+  endforeach()
+elseif(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
   foreach(LLVM_BUILD_MODE ${CMAKE_CONFIGURATION_TYPES})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
Index: lldb/lit/CMakeLists.txt
===
--- lldb/lit/CMakeLists.txt
+++ lldb/lit/CMakeLists.txt
@@ -1,21 +1,41 @@
 # Test runner infrastructure for LLDB. This configures the LLDB test trees
 # for use by Lit, and delegates to LLVM's lit test handlers.
 
-if (CMAKE_CFG_INTDIR STREQUAL ".")
-  set(LLVM_BUILD_MODE ".")
-else ()
+get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
+
+if(LLDB_BUILT_STANDALONE)
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# In paths to our build-tree, replace CMAKE_CFG_INTDIR with our actual 
configuration names.
+string(REPLACE ${CMAKE_CFG_INTDIR} "%(build_mode)s" 
config_runtime_output_dir ${LLVM_RUNTIME_OUTPUT_INTDIR})
+string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} 
LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
+
+# Remaining ones must be paths to the provided LLVM build-tree.
+if(${config_type} IN_LIST LLVM_CONFIGURATION_TYPES)
+  # Multi-configuration generator like Xcode (with a matching config).
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+else()
+  # Single-configuration generator like Ninja.
+  string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+endif()
+  endforeach()
+
   set(LLVM_BUILD_MODE "%(build_mode)s")
-endif ()
+else()
+  if (CMAKE_CFG_INTDIR STREQUAL ".")
+set(LLVM_BUILD_MODE ".")
+  else ()
+set(LLVM_BUILD_MODE "%(build_mode)s")
+  endif ()
 
-if (CMAKE_SIZEOF_VOID_P EQUAL 8)
-  set(LLDB_IS_64_BITS 1)
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
 endif()
 
-get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
-
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
-string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+
+if (CMAKE_SIZEOF_VOID_P EQUAL 8)
+  set(LLDB_IS_64_BITS 1)
+endif()
 
 list(APPEND LLDB_TEST_DEPS
   LLDBUnitTests


Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -5,8 +5,28 @@
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
-# Generate wrapper for each build mode.
-if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+# Generate lldb-dotest Python driver script for each build mode.
+if(LLDB_BUILT_STANDALONE)
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# In paths to our build-tree, replace CMAKE_CFG_INTDIR with our actual configuration names.
+string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} config_runtime_output_dir

[Lldb-commits] [PATCH] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

2019-06-05 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz marked 2 inline comments as done.
sgraenitz added inline comments.



Comment at: lldb/lit/CMakeLists.txt:7
+if(LLDB_BUILT_STANDALONE)
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# In paths to our build-tree, replace CMAKE_CFG_INTDIR with our actual 
configuration names.

I will have to fix this. We only need one match here.

> Is there a way to share the logic between the two cmake files?

Unfortunately they're slightly different. lldb-dotest generates the actual 
driver scripts. Here, we only write the (one) cfg file and it relies on 
replacements of `%(build_mode)s` at runtime in ( see 
https://github.com/llvm/llvm-project/blob/6fc4c1cc54/lldb/lit/Suite/lit.site.cfg.in#L35).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62859



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

[Lldb-commits] [PATCH] D62894: DWARF: Share line tables of type units

2019-06-05 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: clayborg, aprantl, JDevlieghere.
Herald added a subscriber: jdoerfert.

This patch creates a cache of file lists in line tables referenced by
type units.  This cache is used to avoid parsing a line table twice
(since a file list will generally be shared by many type units).

It also sets things up in a way that parsing of DW_AT_decl_file
attributes will keep working even when we stop creating lldb compile
units for dwarf type units, but it stops short of actually doing that.
This means that the request for files now go directly to SymbolFileDWARF
instead of being routed there indirectly via the
lldb_private::CompileUnit class.

As a result of this, a number of occurences of SymbolContext variables
in DWARFASTParserClang have become unused, so I remove them.

This patch reduces the number of times a file list is being parsed, but
the situation is still suboptimal, as the parsed list is being copied
multiple times. This will be fixed when we stop creating CompileUnits
for DWARF type units.


https://reviews.llvm.org/D62894

Files:
  lit/SymbolFile/DWARF/debug-types-line-tables.s
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -49,6 +49,7 @@
 class DWARFDebugRangesBase;
 class DWARFDeclContext;
 class DWARFFormValue;
+class DWARFTypeUnit;
 class SymbolFileDWARFDebugMap;
 class SymbolFileDWARFDwo;
 class SymbolFileDWARFDwp;
@@ -299,6 +300,8 @@
 
   lldb_private::DWARFContext &GetDWARFContext() { return m_context; }
 
+  lldb_private::FileSpec GetSupportFile(DWARFUnit &unit, size_t file_idx);
+
 protected:
   typedef llvm::DenseMap
   DIEToTypePtr;
@@ -438,6 +441,8 @@
 
   SymbolFileDWARFDwp *GetDwpSymbolFile();
 
+  const lldb_private::FileSpecList &GetTypeUnitSupportFiles(DWARFTypeUnit &tu);
+
   lldb::ModuleWP m_debug_map_module_wp;
   SymbolFileDWARFDebugMap *m_debug_map_symfile;
 
@@ -476,6 +481,8 @@
   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_type_unit_support_files;
 };
 
 #endif // SymbolFileDWARF_SymbolFileDWARF_h_
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -54,6 +54,7 @@
 #include "AppleDWARFIndex.h"
 #include "DWARFASTParser.h"
 #include "DWARFASTParserClang.h"
+#include "DWARFCompileUnit.h"
 #include "DWARFDebugAbbrev.h"
 #include "DWARFDebugAranges.h"
 #include "DWARFDebugInfo.h"
@@ -62,6 +63,7 @@
 #include "DWARFDebugRanges.h"
 #include "DWARFDeclContext.h"
 #include "DWARFFormValue.h"
+#include "DWARFTypeUnit.h"
 #include "DWARFUnit.h"
 #include "DebugNamesDWARFIndex.h"
 #include "LogChannelDWARF.h"
@@ -775,26 +777,55 @@
 bool SymbolFileDWARF::ParseSupportFiles(CompileUnit &comp_unit,
 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);
-  }
+  DWARFUnit *unit = GetDWARFCompileUnit(&comp_unit);
+  if (auto *tu = llvm::dyn_cast_or_null(unit)) {
+support_files = GetTypeUnitSupportFiles(*tu);
+return true;
+  }
+
+  if (unit) {
+const dw_offset_t stmt_list = unit->GetLineTableOffset();
+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_c

[Lldb-commits] [PATCH] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

2019-06-05 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz updated this revision to Diff 203118.
sgraenitz added a comment.

Unfortunately lit is a little more complicated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62859

Files:
  lldb/lit/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt


Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -5,8 +5,28 @@
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
-# Generate wrapper for each build mode.
-if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+# Generate lldb-dotest Python driver script for each build mode.
+if(LLDB_BUILT_STANDALONE)
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# In paths to our build-tree, replace CMAKE_CFG_INTDIR with our actual 
configuration names.
+string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} 
config_runtime_output_dir ${LLVM_RUNTIME_OUTPUT_INTDIR})
+string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} 
LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
+
+# Remaining ones must be paths to the provided LLVM build-tree.
+if(${config_type} IN_LIST LLVM_CONFIGURATION_TYPES)
+  # Multi-configuration generator like Xcode (with a matching config).
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+else()
+  # Single-configuration generator like Ninja.
+  string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+endif()
+
+configure_file(
+  lldb-dotest.in
+  ${config_runtime_output_dir}/lldb-dotest @ONLY
+)
+  endforeach()
+elseif(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
   foreach(LLVM_BUILD_MODE ${CMAKE_CONFIGURATION_TYPES})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
Index: lldb/lit/CMakeLists.txt
===
--- lldb/lit/CMakeLists.txt
+++ lldb/lit/CMakeLists.txt
@@ -1,21 +1,47 @@
 # Test runner infrastructure for LLDB. This configures the LLDB test trees
 # for use by Lit, and delegates to LLVM's lit test handlers.
 
-if (CMAKE_CFG_INTDIR STREQUAL ".")
+# LLVM_BUILD_MODE is used in lit.site.cfg
+if(CMAKE_CFG_INTDIR STREQUAL ".")
   set(LLVM_BUILD_MODE ".")
-else ()
+else()
   set(LLVM_BUILD_MODE "%(build_mode)s")
-endif ()
-
-if (CMAKE_SIZEOF_VOID_P EQUAL 8)
-  set(LLDB_IS_64_BITS 1)
 endif()
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
+set(dotest_args_replacement ${LLVM_BUILD_MODE})
+
+if(LLDB_BUILT_STANDALONE)
+  # In paths to our build-tree, replace CMAKE_CFG_INTDIR with our 
configuration name placeholder.
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} 
config_runtime_output_dir ${LLVM_RUNTIME_OUTPUT_INTDIR})
+  string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} 
LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
+
+  # Remaining ones must be paths to the provided LLVM build-tree.
+  if(LLVM_CONFIGURATION_TYPES)
+# LLDB uses single-config; LLVM multi-config; pick one and prefer Release 
types.
+# Otherwise, if both use multi-config the default is fine.
+if(NOT CMAKE_CONFIGURATION_TYPES)
+  if(RelWithDebInfo IN_LIST LLVM_CONFIGURATION_TYPES)
+set(dotest_args_replacement RelWithDebInfo)
+  elseif(Release IN_LIST LLVM_CONFIGURATION_TYPES)
+set(dotest_args_replacement Release)
+  else()
+list(GET LLVM_CONFIGURATION_TYPES 0 dotest_args_replacement)
+  endif()
+endif()
+  else()
+# Common case: LLVM used a single-configuration generator like Ninja.
+set(dotest_args_replacement ".")
+  endif()
+endif()
 
+string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
-string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+
+if (CMAKE_SIZEOF_VOID_P EQUAL 8)
+  set(LLDB_IS_64_BITS 1)
+endif()
 
 list(APPEND LLDB_TEST_DEPS
   LLDBUnitTests


Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -5,8 +5,28 @@
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
-# Generate wrapper for each build mode.
-if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+# Generate lldb-dotest Python driver script for each build mode.
+if(LLDB_BUILT_STANDALONE)
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# In paths to

[Lldb-commits] [PATCH] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

2019-06-05 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz updated this revision to Diff 203121.
sgraenitz added a comment.

Polishing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62859

Files:
  lldb/lit/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt


Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -5,8 +5,28 @@
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
-# Generate wrapper for each build mode.
-if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+# Generate lldb-dotest Python driver script for each build mode.
+if(LLDB_BUILT_STANDALONE)
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# In paths to our build-tree, replace CMAKE_CFG_INTDIR with our actual 
configuration names.
+string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} 
config_runtime_output_dir ${LLVM_RUNTIME_OUTPUT_INTDIR})
+string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} 
LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
+
+# Remaining ones must be paths to the provided LLVM build-tree.
+if(${config_type} IN_LIST LLVM_CONFIGURATION_TYPES)
+  # Multi-configuration generator like Xcode (with a matching config).
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+else()
+  # Single-configuration generator like Ninja.
+  string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+endif()
+
+configure_file(
+  lldb-dotest.in
+  ${config_runtime_output_dir}/lldb-dotest @ONLY
+)
+  endforeach()
+elseif(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
   foreach(LLVM_BUILD_MODE ${CMAKE_CONFIGURATION_TYPES})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
Index: lldb/lit/CMakeLists.txt
===
--- lldb/lit/CMakeLists.txt
+++ lldb/lit/CMakeLists.txt
@@ -1,6 +1,7 @@
 # Test runner infrastructure for LLDB. This configures the LLDB test trees
 # for use by Lit, and delegates to LLVM's lit test handlers.
 
+# LLVM_BUILD_MODE is used in lit.site.cfg
 if (CMAKE_CFG_INTDIR STREQUAL ".")
   set(LLVM_BUILD_MODE ".")
 else ()
@@ -12,10 +13,35 @@
 endif()
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
+set(dotest_args_replacement ${LLVM_BUILD_MODE})
 
+if(LLDB_BUILT_STANDALONE)
+  # In paths to our build-tree, replace CMAKE_CFG_INTDIR with our 
configuration name placeholder.
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} 
config_runtime_output_dir ${LLVM_RUNTIME_OUTPUT_INTDIR})
+  string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} 
LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
+
+  # Remaining ones must be paths to the provided LLVM build-tree.
+  if(LLVM_CONFIGURATION_TYPES)
+# LLDB uses single-config; LLVM multi-config; pick one and prefer Release 
types.
+# Otherwise, if both use multi-config the default is fine.
+if(NOT CMAKE_CONFIGURATION_TYPES)
+  if(RelWithDebInfo IN_LIST LLVM_CONFIGURATION_TYPES)
+set(dotest_args_replacement RelWithDebInfo)
+  elseif(Release IN_LIST LLVM_CONFIGURATION_TYPES)
+set(dotest_args_replacement Release)
+  else()
+list(GET LLVM_CONFIGURATION_TYPES 0 dotest_args_replacement)
+  endif()
+endif()
+  else()
+# Common case: LLVM used a single-configuration generator like Ninja.
+set(dotest_args_replacement ".")
+  endif()
+endif()
+
+string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
-string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
 
 list(APPEND LLDB_TEST_DEPS
   LLDBUnitTests


Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -5,8 +5,28 @@
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
-# Generate wrapper for each build mode.
-if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+# Generate lldb-dotest Python driver script for each build mode.
+if(LLDB_BUILT_STANDALONE)
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# In paths to our build-tree, replace CMAKE_CFG_INTDIR with our actual configuration names.
+string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} config_runtime_output_dir ${LLVM_RUNTIME_OUTPUT_INTDIR})
+string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${con

[Lldb-commits] [PATCH] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

2019-06-05 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz added a comment.

`./Debug/bin/lldb-dotest` now failing with:

  OSError: [Errno 2] No such file or directory
  Config=x86_64-/path/to/lldb-dev-deps-relwithdebinfo/bin/clang-9

Investigating


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62859



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

[Lldb-commits] [PATCH] D62168: [DynamicLoader] Make sure we always set the rendezvous breakpoint

2019-06-05 Thread António Afonso via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362619: [DynamicLoader] Make sure we always set the 
rendezvous breakpoint (authored by aadsm, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62168?vs=200383&id=203179#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62168

Files:
  lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp


Index: 
lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- 
lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ 
lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -150,11 +150,6 @@
  true);
 
 LoadAllCurrentModules();
-if (!SetRendezvousBreakpoint()) {
-  // If we cannot establish rendezvous breakpoint right now we'll try again
-  // at entry point.
-  ProbeEntry();
-}
 
 m_process->GetTarget().ModulesDidLoad(module_list);
 if (log) {
@@ -169,6 +164,14 @@
   }
 }
   }
+
+  if (executable_sp.get()) {
+if (!SetRendezvousBreakpoint()) {
+  // If we cannot establish rendezvous breakpoint right now we'll try again
+  // at entry point.
+  ProbeEntry();
+}
+  }
 }
 
 void DynamicLoaderPOSIXDYLD::DidLaunch() {


Index: lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -150,11 +150,6 @@
  true);
 
 LoadAllCurrentModules();
-if (!SetRendezvousBreakpoint()) {
-  // If we cannot establish rendezvous breakpoint right now we'll try again
-  // at entry point.
-  ProbeEntry();
-}
 
 m_process->GetTarget().ModulesDidLoad(module_list);
 if (log) {
@@ -169,6 +164,14 @@
   }
 }
   }
+
+  if (executable_sp.get()) {
+if (!SetRendezvousBreakpoint()) {
+  // If we cannot establish rendezvous breakpoint right now we'll try again
+  // at entry point.
+  ProbeEntry();
+}
+  }
 }
 
 void DynamicLoaderPOSIXDYLD::DidLaunch() {
___
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

2019-06-05 Thread António Afonso via Phabricator via lldb-commits

aadsm added a comment.

> Another advantage of having this in an abstract class is that you could test 
> this in isolation, as NativeProcessProtocol is already setup to mock memory 
> accesses: 
> https://github.com/llvm-mirror/lldb/blob/master/unittests/Host/NativeProcessProtocolTest.cpp.

I might be missing something here, I'm not sure how having this in a 
`NativeProcessELF` class instead of `NativeProcessLinux` would make things 
easier for testing. Like you said, `NativeProcessProtocol` is the one set up to 
mock memory access. I still need to create my own `MockProcessELF`, which makes 
me think if there's a way to somehow reuse `MockProcess` to create 
`MockProcessELF`?


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] D62501: Implement GetSharedLibraryInfoAddress

2019-06-05 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D62501#1531202 , @aadsm wrote:

> > Another advantage of having this in an abstract class is that you could 
> > test this in isolation, as NativeProcessProtocol is already setup to mock 
> > memory accesses: 
> > https://github.com/llvm-mirror/lldb/blob/master/unittests/Host/NativeProcessProtocolTest.cpp.
>
> I might be missing something here, I'm not sure how having this in a 
> `NativeProcessELF` class instead of `NativeProcessLinux` would make things 
> easier for testing. Like you said, `NativeProcessProtocol` is the one set up 
> to mock memory access. I still need to create my own `MockProcessELF`, which 
> makes me think if there's a way to somehow reuse `MockProcess` to create 
> `MockProcessELF`?


Yeah, sorry, I guess that came out more optimistic then what I meant. What I 
was trying to say, that it is possible to create NativeProcessProtocol in a 
unit test, which means it would be also possible for the NativeProcessELF.

I haven't given this much thought, but it may be possible to reuse the stuff in 
MockProcess by making it a template (so you'd have a 
MockProcess, and a MockProcess)


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

Re: [Lldb-commits] [lldb] r362570 - Fix -Wsign-compare by explicit cast after r362557

2019-06-05 Thread Shafik Yaghmour via lldb-commits

abs(offset) is not the same as addr_t(-offset) AFAICT offset is positive so we 
are casting a small negative number to an unsigned type which will result in a 
really large value e.g. https://godbolt.org/z/B2Z7Ok

> On Jun 4, 2019, at 6:49 PM, Fangrui Song via lldb-commits 
>  wrote:
> 
> Author: maskray
> Date: Tue Jun  4 18:49:06 2019
> New Revision: 362570
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=362570&view=rev
> Log:
> Fix -Wsign-compare by explicit cast after r362557
> 
> Modified:
>
> lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
> 
> Modified: 
> lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp?rev=362570&r1=362569&r2=362570&view=diff
> ==
> --- 
> lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp 
> (original)
> +++ 
> lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp 
> Tue Jun  4 18:49:06 2019
> @@ -819,7 +819,7 @@ bool x86AssemblyInspectionEngine::local_
>   int offset;
>   if (pc_rel_branch_or_jump_p (instruction_length, offset) && offset != 0) {
> addr_t next_pc_value = current_func_text_offset + instruction_length;
> -if (offset < 0 && abs (offset) > current_func_text_offset) {
> +if (offset < 0 && addr_t(-offset) > current_func_text_offset) {
>   // Branch target is before the start of this function
>   return false;
> }
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
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

2019-06-05 Thread António Afonso via Phabricator via lldb-commits

aadsm added a comment.

> I haven't given this much thought, but it may be possible to reuse the stuff 
> in MockProcess by making it a template (so you'd have a 
> MockProcess, and a MockProcess)

Ah interesting, will explore that instead. I was actually thinking if it would 
be possible to extract the common stuff into a MockProcessCommon and then use 
multiple inheritance like `MockProcess : MockProcessCommon, 
NativeProcessProtocol` and `MockProcessELF : MockProcessCommon, 
NativeProcessELF` but I don't know much about C++ to know if it's feasible so 
I'll just need to try, it will be a good learning experience.


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

Re: [Lldb-commits] [lldb] r362570 - Fix -Wsign-compare by explicit cast after r362557

2019-06-05 Thread Pavel Labath via lldb-commits


On 05/06/2019 21:23, Shafik Yaghmour via lldb-commits wrote:

abs(offset) is not the same as addr_t(-offset)


It is, if you assume that offset is negative, which is what this code is 
doing:



if (offset < 0 && addr_t(-offset) > current_func_text_offset)


So, we will only ever convert -offset into an addr_t if offset is 
negative (i.e. -offset is positive).

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

Re: [Lldb-commits] [lldb] r362570 - Fix -Wsign-compare by explicit cast after r362557

2019-06-05 Thread Shafik Yaghmour via lldb-commits

Apologies, I somehow missed the offset < 0.

> On Jun 5, 2019, at 12:38 PM, Pavel Labath  wrote:
> 
> On 05/06/2019 21:23, Shafik Yaghmour via lldb-commits wrote:
>> abs(offset) is not the same as addr_t(-offset)
> 
> It is, if you assume that offset is negative, which is what this code is 
> doing:
> 
>>> if (offset < 0 && addr_t(-offset) > current_func_text_offset)
> 
> So, we will only ever convert -offset into an addr_t if offset is negative 
> (i.e. -offset is positive).

___
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

2019-06-05 Thread António Afonso via Phabricator via lldb-commits

aadsm updated this revision to Diff 203235.
aadsm marked an inline comment as done.
aadsm added a comment.

Address final 2 comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62499

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/unittests/Process/gdb-remote/CMakeLists.txt
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerTest.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteTestUtils.h

Index: lldb/unittests/Process/gdb-remote/GDBRemoteTestUtils.h
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteTestUtils.h
+++ lldb/unittests/Process/gdb-remote/GDBRemoteTestUtils.h
@@ -8,9 +8,11 @@
 #ifndef lldb_unittests_Process_gdb_remote_GDBRemoteTestUtils_h
 #define lldb_unittests_Process_gdb_remote_GDBRemoteTestUtils_h
 
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+#include "lldb/Utility/Connection.h"
 
 namespace lldb_private {
 namespace process_gdb_remote {
@@ -21,10 +23,38 @@
   static void TearDownTestCase();
 };
 
-struct MockServer : public GDBRemoteCommunicationServer {
+class MockConnection : public lldb_private::Connection {
+public:
+  MockConnection(std::vector &packets) { m_packets = &packets; };
+
+  MOCK_METHOD2(Connect,
+   lldb::ConnectionStatus(llvm::StringRef url, Status *error_ptr));
+  MOCK_METHOD5(Read, size_t(void *dst, size_t dst_len,
+const Timeout &timeout,
+lldb::ConnectionStatus &status, Status *error_ptr));
+  MOCK_METHOD0(GetURI, std::string());
+  MOCK_METHOD0(InterruptRead, bool());
+
+  lldb::ConnectionStatus Disconnect(Status *error_ptr) {
+return lldb::eConnectionStatusSuccess;
+  };
+
+  bool IsConnected() const { return true; };
+  size_t Write(const void *dst, size_t dst_len, lldb::ConnectionStatus &status,
+   Status *error_ptr) {
+m_packets->emplace_back(static_cast(dst), dst_len);
+return dst_len;
+  };
+
+  std::vector *m_packets;
+};
+
+class MockServer : public GDBRemoteCommunicationServer {
+public:
   MockServer()
   : GDBRemoteCommunicationServer("mock-server", "mock-server.listener") {
 m_send_acks = false;
+m_send_error_strings = true;
   }
 
   PacketResult SendPacket(llvm::StringRef payload) {
@@ -37,10 +67,22 @@
sync_on_timeout);
   }
 
+  using GDBRemoteCommunicationServer::SendErrorResponse;
   using GDBRemoteCommunicationServer::SendOKResponse;
   using GDBRemoteCommunicationServer::SendUnimplementedResponse;
 };
 
+class MockServerWithMockConnection : public MockServer {
+public:
+  MockServerWithMockConnection() : MockServer() {
+SetConnection(new MockConnection(m_packets));
+  }
+
+  llvm::ArrayRef GetPackets() { return m_packets; };
+
+  std::vector m_packets;
+};
+
 } // namespace process_gdb_remote
 } // namespace lldb_private
 
Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerTest.cpp
@@ -0,0 +1,73 @@
+//===-- GDBRemoteCommunicationServerTest.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 "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "GDBRemoteTestUtils.h"
+
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+#include "lldb/Utility/Connection.h"
+
+namespace lldb_private {
+namespace process_gdb_remote {
+
+TEST(GDBRemoteCommunicationServerTest, SendErrorResponse_ErrorNumber) {
+  MockServerWithMockConnection server;
+  server.SendErrorResponse(0x42);
+
+  EXPECT_THAT(server.GetPackets(), testing::ElementsAre("$E42#ab"));
+}
+
+TEST(GDBRemoteCommunicationServerTest, SendErrorResponse_Status) {
+  MockServerWithMockConnection server;
+  Status status;
+
+  status.SetError(0x42, lldb::eErrorTypeGeneric);
+  status.SetErrorString("Test error message");
+  server.SendErrorResponse(status);
+
+  EXPECT_THAT(
+  server.GetPackets(),
+  testing::ElementsAre("$E42;54657374206572726f72206d657373616765#ad"));
+}
+
+TEST(GDBRemoteCommunicationServerTest, SendErrorResponse_UnimplementedError) {
+  MockServerWithMockConne

[Lldb-commits] [lldb] r362639 - [NativeProcessDarwin] Remove dead code. NFCI.

2019-06-05 Thread Davide Italiano via lldb-commits

Author: davide
Date: Wed Jun  5 13:23:03 2019
New Revision: 362639

URL: http://llvm.org/viewvc/llvm-project?rev=362639&view=rev
Log:
[NativeProcessDarwin] Remove dead code. NFCI.

Modified:
lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.cpp

Modified: lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.cpp?rev=362639&r1=362638&r2=362639&view=diff
==
--- lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.cpp Wed Jun  5 
13:23:03 2019
@@ -171,14 +171,6 @@ Status NativeProcessDarwin::FinalizeLaun
   Status error;
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
 
-#if 0
-m_path = path;
-size_t i;
-char const *arg;
-for (i=0; (arg = argv[i]) != NULL; i++)
-m_args.push_back(arg);
-#endif
-
   error = StartExceptionThread();
   if (!error.Success()) {
 if (log)


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

[Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

2019-06-05 Thread Guilherme Andrade via Phabricator via lldb-commits

guiandrade created this revision.
guiandrade added a reviewer: labath.
guiandrade added projects: LLDB, LLVM.
Herald added a subscriber: lldb-commits.

Following up on https://reviews.llvm.org/D62221 
, this change introduces
the setting plugin.process.gdb-remote.try-to-use-g-packet. When that is on
and the server supports the use of 'g' packets, those will be used regardless
of whether 'p' packets are supported.

Using 'g' packets can improve performance by reducing the number of packets
exchanged between client and server when a large number of registers
needs to be fetched.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D62931

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -128,6 +128,12 @@
   ASSERT_EQ(0,
 memcmp(buffer_sp->GetBytes(), one_register, sizeof one_register));
 
+  async_result = std::async(std::launch::async,
+[&] { return client.GetgPacketSupported(tid); });
+  Handle_QThreadSuffixSupported(server, true);
+  HandlePacket(server, "g;thread:0047;", all_registers_hex);
+  ASSERT_TRUE(async_result.get());
+
   read_result = std::async(std::launch::async,
[&] { return client.ReadAllRegisters(tid); });
   HandlePacket(server, "g;thread:0047;", all_registers_hex);
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
@@ -25,7 +25,7 @@
 
 class ThreadGDBRemote : public Thread {
 public:
-  ThreadGDBRemote(Process &process, lldb::tid_t tid);
+  ThreadGDBRemote(Process &process, lldb::tid_t tid, bool try_to_use_g_packet);
 
   ~ThreadGDBRemote() override;
 
@@ -100,6 +100,7 @@
   uint64_t
   m_queue_serial_number; // Queue info from stop reply/stop info for thread
   lldb_private::LazyBool m_associated_with_libdispatch_queue;
+  bool m_try_to_use_g_packet;
 
   bool PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef data);
 
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -33,12 +33,14 @@
 
 // Thread Registers
 
-ThreadGDBRemote::ThreadGDBRemote(Process &process, lldb::tid_t tid)
+ThreadGDBRemote::ThreadGDBRemote(Process &process, lldb::tid_t tid,
+ bool try_to_use_g_packet)
 : Thread(process, tid), m_thread_name(), m_dispatch_queue_name(),
   m_thread_dispatch_qaddr(LLDB_INVALID_ADDRESS),
   m_dispatch_queue_t(LLDB_INVALID_ADDRESS), m_queue_kind(eQueueKindUnknown),
   m_queue_serial_number(LLDB_INVALID_QUEUE_ID),
-  m_associated_with_libdispatch_queue(eLazyBoolCalculate) {
+  m_associated_with_libdispatch_queue(eLazyBoolCalculate),
+  m_try_to_use_g_packet(try_to_use_g_packet) {
   Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_THREAD));
   LLDB_LOG(log, "this = {0}, pid = {1}, tid = {2}", this, process.GetID(),
GetID());
@@ -303,9 +305,12 @@
 if (process_sp) {
   ProcessGDBRemote *gdb_process =
   static_cast(process_sp.get());
-  // read_all_registers_at_once will be true if 'p' packet is not
-  // supported.
+  // read_all_registers_at_once will be true if either the flag
+  // m_try_to_use_g_packet is true and the server supports 'g' packet
+  // or if 'p' packet is not supported.
   bool read_all_registers_at_once =
+  (m_try_to_use_g_packet &&
+   gdb_process->GetGDBRemote().GetgPacketSupported(GetID())) ||
   !gdb_process->GetGDBRemote().GetpPacketSupported(GetID());
   reg_ctx_sp = std::make_shared(
   *this, concrete_frame_idx, gdb_process->m_register_info,
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -120,9 +120,15 @@
  , nullptr, {},
  "Specify the default packet timeout in seconds."},
 {"target-definition-file", OptionValue::eTypeFileSpec, true, 0,

[Lldb-commits] [PATCH] D62934: [LanguageRuntime] Introdce LLVM-style casts

2019-06-05 Thread Alex Langford via Phabricator via lldb-commits

xiaobai created this revision.
xiaobai added reviewers: labath, JDevlieghere.
Herald added a subscriber: arphaman.

Using llvm-style rtti gives us stronger guarantees around casting
LanguageRuntimes.

As discussed in D62755 


https://reviews.llvm.org/D62934

Files:
  include/lldb/Target/CPPLanguageRuntime.h
  include/lldb/Target/LanguageRuntime.h
  include/lldb/Target/ObjCLanguageRuntime.h
  source/Plugins/Language/ObjC/CF.cpp
  source/Plugins/Language/ObjC/Cocoa.cpp
  source/Plugins/Language/ObjC/NSArray.cpp
  source/Plugins/Language/ObjC/NSDictionary.cpp
  source/Plugins/Language/ObjC/NSError.cpp
  source/Plugins/Language/ObjC/NSException.cpp
  source/Plugins/Language/ObjC/NSIndexPath.cpp
  source/Plugins/Language/ObjC/NSSet.cpp
  source/Plugins/Language/ObjC/NSString.cpp
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.h
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.h
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
  source/Target/CPPLanguageRuntime.cpp
  source/Target/LanguageRuntime.cpp
  source/Target/ObjCLanguageRuntime.cpp
  source/Target/Process.cpp

Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1602,10 +1602,7 @@
   std::lock_guard guard(m_language_runtimes_mutex);
   LanguageRuntime *runtime =
   GetLanguageRuntime(eLanguageTypeObjC, retry_if_null);
-  if (!runtime)
-return nullptr;
-
-  return static_cast(runtime);
+  return llvm::cast_or_null(runtime);
 }
 
 bool Process::IsPossibleDynamicValue(ValueObject &in_value) {
Index: source/Target/ObjCLanguageRuntime.cpp
===
--- source/Target/ObjCLanguageRuntime.cpp
+++ source/Target/ObjCLanguageRuntime.cpp
@@ -31,8 +31,9 @@
 // Destructor
 ObjCLanguageRuntime::~ObjCLanguageRuntime() {}
 
-ObjCLanguageRuntime::ObjCLanguageRuntime(Process *process)
-: LanguageRuntime(process), m_impl_cache(),
+ObjCLanguageRuntime::ObjCLanguageRuntime(Process *process,
+ LanguageRuntimeKind kind)
+: LanguageRuntime(process, kind), m_impl_cache(),
   m_has_new_literals_and_indexing(eLazyBoolCalculate),
   m_isa_to_descriptor(), m_hash_to_isa_map(), m_type_size_cache(),
   m_isa_to_descriptor_stop_id(UINT32_MAX), m_complete_class_cache(),
Index: source/Target/LanguageRuntime.cpp
===
--- source/Target/LanguageRuntime.cpp
+++ source/Target/LanguageRuntime.cpp
@@ -218,7 +218,8 @@
   return nullptr;
 }
 
-LanguageRuntime::LanguageRuntime(Process *process) : m_process(process) {}
+LanguageRuntime::LanguageRuntime(Process *process, LanguageRuntimeKind kind)
+: m_process(process), m_kind(kind) {}
 
 LanguageRuntime::~LanguageRuntime() = default;
 
Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -38,8 +38,9 @@
 // Destructor
 CPPLanguageRuntime::~CPPLanguageRuntime() {}
 
-CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
-: LanguageRuntime(process) {}
+CPPLanguageRuntime::CPPLanguageRuntime(Process *process,
+   LanguageRuntimeKind kind)
+: LanguageRuntime(process, kind) {}
 
 bool CPPLanguageRuntime::IsRuntimeSupportValue(ValueObject &valobj) {
   // All runtime support values have to be marked as artificial by the
Index: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
===
--- source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
+++ source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
@@ -314,6 +314,10 @@
 
   static lldb_private::ConstString GetPluginNameStatic();
 
+  static bool classof(const LanguageRuntime *runtime) {
+return runtime->GetKind() == eRenderScriptLanguageRuntime;
+  }
+
   static bool IsRenderScriptModule(const lldb::ModuleSP &module_sp);
 
   static ModuleKind GetModuleKind(const lldb::ModuleSP &module_sp);
Index: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===
--- source/Plugin

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

2019-06-05 Thread António Afonso via Phabricator via lldb-commits

aadsm added a comment.

@labath while working on this I had to also move the GetAuxValue function into 
the NativeProcessELF, which I think it's fine. However, this means this class 
now depends on the AuxValue class defined in the PluginUtility. Initially I was 
going to put the NativeProcessELF in the lldbHost but it feels weird that this 
will now depend on a plugin, even if it's the utility one. Should I create a 
new ELF plugin just for this?


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] D62931: [lldb-server] Add setting to force 'g' packet use

2019-06-05 Thread Pavel Labath via Phabricator via lldb-commits

labath added reviewers: clayborg, jasonmolenda.
labath added a comment.

Personally, I'm wondering whether we even need a setting for this. Running the 
speed test command locally (i.e. the lowest latency we can possibly get), I get 
results like:

  (lldb) process plugin packet speed-test --count 10 --max-receive 2048
  Testing sending 10 packets of various sizes:
  ...
  qSpeedTest(send=  4, recv=  0) in 0.788656771 s for 126797.88 
packets/s (0.007886 ms per packet) with standard deviation of 0.001604 ms
  qSpeedTest(send=  4, recv=  4) in 0.770005465 s for 129869.21 
packets/s (0.007700 ms per packet) with standard deviation of 0.002120 ms
  qSpeedTest(send=  4, recv=  8) in 0.895460367 s for 111674.40 
packets/s (0.008954 ms per packet) with standard deviation of 0.002048 ms
  qSpeedTest(send=  4, recv= 16) in 0.910367966 s for 109845.70 
packets/s (0.009103 ms per packet) with standard deviation of 0.001886 ms
  qSpeedTest(send=  4, recv= 32) in 0.889442623 s for 112429.96 
packets/s (0.008894 ms per packet) with standard deviation of 0.001705 ms
  qSpeedTest(send=  4, recv= 64) in 0.945124865 s for 105806.12 
packets/s (0.009451 ms per packet) with standard deviation of 0.002349 ms
  qSpeedTest(send=  4, recv=128) in 0.759995580 s for 131579.72 
packets/s (0.007599 ms per packet) with standard deviation of 0.002971 ms
  qSpeedTest(send=  4, recv=256) in 0.847535312 s for 117989.19 
packets/s (0.008475 ms per packet) with standard deviation of 0.002629 ms
  qSpeedTest(send=  4, recv=512) in 1.022377729 s for  97811.21 
packets/s (0.010223 ms per packet) with standard deviation of 0.003248 ms
  qSpeedTest(send=  4, recv=   1024) in 1.436389208 s for  69619.02 
packets/s (0.014363 ms per packet) with standard deviation of 0.000688 ms
  qSpeedTest(send=  4, recv=   2048) in 1.910557270 s for  52340.75 
packets/s (0.019105 ms per packet) with standard deviation of 0.001194 ms
  ...

Now, if we assume that the `p` resonse is about 16 bytes long, and `g` response 
is 2048, we find that the `g` packet is worth approximately two `p` packets. 
And this is a local link, where it pretty much doesn't matter what we do, as 
we're pumping 50k packets per second even in the slowest case. On links with a 
non-negligible latency, the difference between the two packets should be even 
smaller (though it might be nice to verify that).

Combining this with the fact that we normally expedite all general purpose 
registers (and so there won't be *any* packets if one is only reading those), I 
would say that enabling this unconditionally is a pretty safe thing to do.

Greg, Jason, what do you think?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62931



___
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

2019-06-05 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D62501#1531959 , @aadsm wrote:

> @labath while working on this I had to also move the GetAuxValue function 
> into the NativeProcessELF, which I think it's fine. However, this means this 
> class now depends on the AuxValue class defined in the PluginUtility. 
> Initially I was going to put the NativeProcessELF in the lldbHost but it 
> feels weird that this will now depend on a plugin, even if it's the utility 
> one. Should I create a new ELF plugin just for this?


Yeah, having Host depend on Process/Utility would be suboptimal, as we've just 
spend a lot of time making sure Host does not depend on anything. I would be 
fine with creating a new plugin for this. A couple of other possible solutions 
I see are:

- put this in Plugins/Process/POSIX: These days, the folder is mostly empty, so 
we could start repurposing it for this. Having an "ELF" process inside "POSIX" 
folder looks a bit weird, but my reasoning behind that is that we will probably 
one day need a NativeProcessPOSIX to share the common code between non-elf 
posix platform (i.e. darwin). Then NativeProcessELF would be a refinement 
(subclass) of NativeProcessPOSIX
- move all Native***Protocol classes out of Host into some new place. This is 
pretty specialized code, and I don't think it needs to live in a relatively 
general place. At that point it becomes less important to have to outgoing 
dependencies here.
- make the rendezvous structure parser a "utility" too. All this needs to 
function is the aux vector and something to read the memory with. You could 
provide these as arguments to its constructor. This way we won't need a 
NativeProcessELF, side-stepping the issue of where to place it. Additionally, 
this would open the door for using this code on the client side. Right now, 
DynamicLoaderPOSIXDYLD plugin has it's own code for parsing this structure, but 
I don't see a reason why this code couldn't be used there too. It would also 
solve the testing problems, as you'd just need to mock these two things (auxv, 
memory), and not the full NativeProcessProtocol API.

When I spell things out this way, I think I'm starting to prefer the last 
option. Let me know what you think.


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] D62501: Implement GetSharedLibraryInfoAddress

[Lldb-commits] [lldb] r362586 - Ignore DIEs in the skeleton unit in a DWO scenario

[Lldb-commits] [PATCH] D62852: Ignore DIEs in the skeleton unit in a DWO scenario

[Lldb-commits] [PATCH] D62878: [CMake] Export CMAKE_CONFIGURATION_TYPES for the LLVM build-tree

[Lldb-commits] [PATCH] D62879: [CMake] Add configuration dirs as potential locations for llvm-lit and llvm-tblgen in standalone builds

[Lldb-commits] [lldb] r362589 - [CMake] Add configuration dirs as potential locations for llvm-lit and llvm-tblgen in standalone builds

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

[Lldb-commits] [PATCH] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

[Lldb-commits] [PATCH] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

[Lldb-commits] [PATCH] D62894: DWARF: Share line tables of type units

[Lldb-commits] [PATCH] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

[Lldb-commits] [PATCH] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

[Lldb-commits] [PATCH] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

[Lldb-commits] [PATCH] D62168: [DynamicLoader] Make sure we always set the rendezvous breakpoint

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

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

Re: [Lldb-commits] [lldb] r362570 - Fix -Wsign-compare by explicit cast after r362557

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

Re: [Lldb-commits] [lldb] r362570 - Fix -Wsign-compare by explicit cast after r362557

Re: [Lldb-commits] [lldb] r362570 - Fix -Wsign-compare by explicit cast after r362557

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

[Lldb-commits] [lldb] r362639 - [NativeProcessDarwin] Remove dead code. NFCI.

[Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

[Lldb-commits] [PATCH] D62934: [LanguageRuntime] Introdce LLVM-style casts

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

[Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

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

27 matches

Site Navigation

Mail list logo

Footer information