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

[Pavel Labath via Phabricator via lldb-commits]

labath added subscribers: mgorny, krytarowski.
labath added a comment.

+@mgorny, @krytarowski in case they know anything interesting about dynamic 
linkers.

NativeProcessProtocol is certainly too generic for this sort of thing, but 
perhaps it would make sense to put this in a slightly more generic place? After 
all, this approach should work on all elf platforms, right? So maybe we could 
create a (for lack of a better name) NativeProcessELF class to hold these kinds 
of things. You could just make NativeProcessLinux inherit from that, and the 
NetBSD folks can migrate NativeProcessNetBSD once they test things out. IIRC 
there are other things which could be shared between NativeProcessLinux and 
NetBSD that could be moved here in the future.

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.




Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1394
 lldb::addr_t NativeProcessLinux::GetSharedLibraryInfoAddress() {
-  // punt on this for now
-  return LLDB_INVALID_ADDRESS;
+  if (GetAddressByteSize() == 8)
+return GetELFImageInfoAddress.



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);
+

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... :/



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

Are you sure that ReadMemory doesn't return "success" on partial reads too ?



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.

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?


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] D62502: Implement xfer:libraries-svr4:read packet

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D62502#1528382 , @aadsm wrote:

> @labath
>
> > Regarding the test, I am wondering whether requiring the /proc/%d/maps and 
> > the linker rendezvous structure to match isn't too strict of a requirement. 
> > Dynamic linkers (and particularly android dynamic linkers) do a lot of 
> > crazy stuff, and trying to assert this invariant exposes us to all kinds of 
> > "optimizations" that the linkers do or may do in the future. I'm wondering 
> > if it wouldn't be better to just build an executable with one or two 
> > dependent libraries, and then just assert that their entries are present in 
> > the library list. However, I'm not completely sure on this, so I'd like to 
> > hear what you think about that..
>
> Yeah, that was actually my first idea for the test but then realized we 
> already had that main.c all set up and ready to use so I decide to use proc 
> maps instead. I was not aware that things like that could happen (although I 
> had a suspicion this might be brittle) so let me revisit this.


I'm not sure whether it will be brittle, but I do know that the android linker 
does (or used to do?) some tricks where it does not really load libdl.so, but 
instead inserts a fake entry into the link map which pretends to implement that 
library (but in reality the entry points to the linker itself). That sounds 
like the kind of thing that could break if you try to correlate the linker data 
with /proc/%d/maps.

So, I think it would be better to create a separate test binary for this, 
particularly as it seems some tests with funny library names will be 
interesting too... Note that when you're creating a special-purpose executable, 
you don't have to do the whole 
wait-for-inferior-to-print-something-and-then-send-CTRL-C dance. You can just 
have the inferior dereference a null pointer (or something of the sort) to 
force a stop once it has loaded the necessary shared libraries.




Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:96-99
+  virtual Status
+  GetLoadedSVR4Libraries(std::vector &library_list) {
+return Status("Not implemented");
+  }

This should return an `Expected>`



Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteLibrariesSvr4Support.py:51-52
+
+OFFSET = 0
+LENGTH = 0x
+

Why are these all caps? TBH, you probably don't need to make these variables at 
all. You can just place them directly into the packet as strings.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:2181
+template 
+Status NativeProcessLinux::ReadSVR4LibraryInfo(lldb::addr_t link_map_addr,
+   SVR4LibraryInfo &info,

This too could return an `llvm::Error`. `Expected>` is a bit of a mouthful, but I'd consider that instead of by-ref 
returns too..



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:2197
+
+  info.name = std::string(name_buffer);
+  info.link_map = link_map_addr;

What if `name_buffer` is not null-terminated?



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h:141-147
+  template  struct ELFLinkMap {
+T l_addr;
+T l_name;
+T l_ld;
+T l_next;
+T l_prev;
+  };

It looks like this doesn't need to be in a header. You could just define the 
struct inside `ReadSVR4LibraryInfo`



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2768
 
+#if defined(__linux__)
+  if (object == "libraries-svr4") {

Now that we have removed ifdefs around the auxv, we should do that here too..



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2778
+for (auto const &library : library_list) {
+  response.Printf("https://reviews.llvm.org/D62502/new/

https://reviews.llvm.org/D62502



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D62755#1527679 , @xiaobai wrote:

> > So, what I'd do (and what I have been doing so far, but mainly as a 
> > passtime effort) was to cut the dependencies way lower that this. Ideally, 
> > I wouldn't want to include anything from the `Target` library in 
> > lldb-server, possibly by moving some stuff that lldb-server does need to 
> > use (MemoryRegionInfo) out of it.
>
> Yeah, I'd like to see this happen as well. My efforts are twofold: Remove 
> non-plugin libraries dependencies on plugin libraries, and move things out of 
> the core libraries into plugins (and possibly new non-plugin libraries, if it 
> makes sense). There are many dependency issues that I'm not yet aware of. I'm 
> hoping to uncover more as I keep decoupling.


Sounds good. I'm looking forward to seeing how this turns out.


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

https://reviews.llvm.org/D62755



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D62634#1527575 , @dblaikie wrote:

> > 1. We need to figure out whether there's a bug in clang and fix it. + 
> > @dblaikie for help with that
>
> This is intentional behavior in clang (controllable under the 
> -f[no-]split-dwarf-inlining flag, and modified by the 
> -f[no-]debug-info-for-profiling flag).
>
> This extra debug info is used for online symbolication (in the absence of 
> .dwo files) - such as for the sanitizers (accurate symbolication is necessary 
> for correctness in msan, due to msan's necessary use of blacklisting to avoid 
> certain false positives) and some forms of sample based profiling collection.
>
> In the default mode, clang includes, as you noted, just the subprograms that 
> have inlined subroutines in them in this split-dwarf-inlining data.
>  In -fdebug-info-for-profiling all subprograms are described in the skeleton 
> CU with some minimal attributes (they don't need parameters, local 
> variables/scopes, etc) necessary to do certain profile things I don't know 
> lots about.
>
> Chances are it's probably best for a debugger to ignore these entries.


Thanks for explaining David. It sounds like we can safely ignore these entries 
if we have successfully loaded the dwo file, as it will contain a superset of 
information. If we cannot find the dwo file for any reason, then this 
information might be useful as a rough approximation of debug info. However, in 
this case, there's no dwo file around, so there isn't a possibility for 
confusion. I'll prepare a patch doing something like that.

Over email, @clayborg wrote:

> > Actually, there was a subtle change of behavior here. Before this change, 
> > if we tried to parse a DIE using an abbreviation table from a different 
> > file, we would most likely fail immediately because of the fail-safe in 
> > GetAbbreviationDeclarationPtr. Now, we just do a blind read and return 
> > bogus data.
>
> This seems just like a straight up bug we need to fix in LLDB. It shouldn't 
> affect or require any changes from any compilers. Seems like a lldbassert 
> that maybe verifies that the abbreviation we use is valid for the current CU 
> in debug builds would be nice so we can trigger this bug when we run the test 
> suite or locally against an example program would be good. Depending on that 
> fact that we have a mismatch in the abbrev index seems fragile. Most 
> .debug_abbrev tables start out with a DW_TAG_compile_unit. So if the abbrev 
> index magically matches, that still doesn't mean the we will be decoding the 
> right data even if the index matches.


Yes, this part is a bug, but in order to figure out how to fix it, I had to 
know whether the compiler behavior is intentional, and whether we need to 
access that data. If it turned out we did, the fix would be more involved, 
because we'd also need to change the UID encoding so that we can reference 
these DIEs correctly, etc. Given that it seems we can just ignore these debug 
info entries, the fix will be relatively simple.


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

https://reviews.llvm.org/D62634



___
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

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, JDevlieghere, aprantl.

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
deletes any extra DIEs which are present in the skeleton unit, and
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.


https://reviews.llvm.org/D62852

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


Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -240,6 +240,15 @@
   m_die_array.shrink_to_fit();
 
   if (m_dwo_symbol_file) {
+// 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 just delete these extra DIEs
+// (if any) and reclaim some space.
+m_die_array.resize(1);
+m_die_array.shrink_to_fit();
+m_die_array[0].SetHasChildren(false);
+
 DWARFUnit *dwo_cu = m_dwo_symbol_file->GetCompileUnit();
 dwo_cu->ExtractDIEsIfNeeded();
   }
Index: lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
===
--- /dev/null
+++ 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: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -240,6 +240,15 @@
   m_die_array.shrink_to_fit();
 
   if (m_dwo_symbol_file) {
+// 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 just delete these extra DIEs
+// (if any) and reclaim some space.
+m_die_array.resize(1);
+m_die_array.shrink_to_fit();
+m_die_array[0].SetHasChildren(false);
+
 DWARFUnit *dwo_cu = m_dwo_symbol_file->GetCompileUnit();
 dwo_cu->ExtractDIEsIfNeeded();
   }
Index: lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
===
--- /dev/null
+++ 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); }
___
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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:243-251
+// 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 just delete these extra DIEs
+// (if any) and reclaim some space.
+m_die_array.resize(1);
+m_die_array.shrink_to_fit();

Can we detect and just not parse in the first place so we don't parse then 
throw away?


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

https://reviews.llvm.org/D62852



___
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 accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Nice!


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] r362500 - [CMake] Move and add settings to Apple-lldb-base cache script

[Stefan Granitz via lldb-commits]

Author: stefan.graenitz
Date: Tue Jun  4 07:21:48 2019
New Revision: 362500

URL: http://llvm.org/viewvc/llvm-project?rev=362500&view=rev
Log:
[CMake] Move and add settings to Apple-lldb-base cache script

Modified:
lldb/trunk/cmake/caches/Apple-lldb-base.cmake
lldb/trunk/cmake/caches/Apple-lldb-macOS.cmake

Modified: lldb/trunk/cmake/caches/Apple-lldb-base.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/caches/Apple-lldb-base.cmake?rev=362500&r1=362499&r2=362500&view=diff
==
--- lldb/trunk/cmake/caches/Apple-lldb-base.cmake (original)
+++ lldb/trunk/cmake/caches/Apple-lldb-base.cmake Tue Jun  4 07:21:48 2019
@@ -1,5 +1,10 @@
+set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
+set(CMAKE_EXPORT_COMPILE_COMMANDS ON CACHE BOOL "")
+
 set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(LLVM_ENABLE_MODULES ON CACHE BOOL "")
 
 # Release builds set these explicitly:
 #set(LLDB_VERSION_MAJOR  CACHE STRING "")

Modified: lldb/trunk/cmake/caches/Apple-lldb-macOS.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/caches/Apple-lldb-macOS.cmake?rev=362500&r1=362499&r2=362500&view=diff
==
--- lldb/trunk/cmake/caches/Apple-lldb-macOS.cmake (original)
+++ lldb/trunk/cmake/caches/Apple-lldb-macOS.cmake Tue Jun  4 07:21:48 2019
@@ -17,6 +17,3 @@ set(LLDB_FRAMEWORK_INSTALL_DIR /Applicat
 set(CMAKE_OSX_DEPLOYMENT_TARGET 10.11 CACHE STRING "")
 set(LLDB_USE_SYSTEM_DEBUGSERVER ON CACHE BOOL "")
 set(LLVM_EXTERNALIZE_DEBUGINFO OFF CACHE BOOL "")
-
-set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
-set(LLVM_ENABLE_MODULES ON CACHE BOOL "")


___
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

[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/Linux/NativeProcessLinux.cpp:2136
+sizeof(phdr_entry), bytes_read);
+if (!error.Success())
+  return LLDB_INVALID_ADDRESS;

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?



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.

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).


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] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

[David Blaikie via lldb-commits]

On Tue, Jun 4, 2019 at 4:15 AM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:

> labath added a comment.
>
> In D62634#1527575 , @dblaikie
> wrote:
>
> > > 1. We need to figure out whether there's a bug in clang and fix it. +
> @dblaikie for help with that
> >
> > This is intentional behavior in clang (controllable under the
> -f[no-]split-dwarf-inlining flag, and modified by the
> -f[no-]debug-info-for-profiling flag).
> >
> > This extra debug info is used for online symbolication (in the absence
> of .dwo files) - such as for the sanitizers (accurate symbolication is
> necessary for correctness in msan, due to msan's necessary use of
> blacklisting to avoid certain false positives) and some forms of sample
> based profiling collection.
> >
> > In the default mode, clang includes, as you noted, just the subprograms
> that have inlined subroutines in them in this split-dwarf-inlining data.
> >  In -fdebug-info-for-profiling all subprograms are described in the
> skeleton CU with some minimal attributes (they don't need parameters, local
> variables/scopes, etc) necessary to do certain profile things I don't know
> lots about.
> >
> > Chances are it's probably best for a debugger to ignore these entries.
>
>
> Thanks for explaining David. It sounds like we can safely ignore these
> entries if we have successfully loaded the dwo file, as it will contain a
> superset of information. If we cannot find the dwo file for any reason,
> then this information might be useful as a rough approximation of debug
> info. However, in this case, there's no dwo file around, so there isn't a
> possibility for confusion. I'll prepare a patch doing something like that.
>

Yep - safe to ignore. "nice to have" if the .dwo file can't be loaded, but
not a huge motivation/I wouldn't bend over backwards to make that work if
it's a significant burden.


>
> Over email, @clayborg wrote:
>
> > > Actually, there was a subtle change of behavior here. Before this
> change, if we tried to parse a DIE using an abbreviation table from a
> different file, we would most likely fail immediately because of the
> fail-safe in GetAbbreviationDeclarationPtr. Now, we just do a blind read
> and return bogus data.
> >
> > This seems just like a straight up bug we need to fix in LLDB. It
> shouldn't affect or require any changes from any compilers. Seems like a
> lldbassert that maybe verifies that the abbreviation we use is valid for
> the current CU in debug builds would be nice so we can trigger this bug
> when we run the test suite or locally against an example program would be
> good. Depending on that fact that we have a mismatch in the abbrev index
> seems fragile. Most .debug_abbrev tables start out with a
> DW_TAG_compile_unit. So if the abbrev index magically matches, that still
> doesn't mean the we will be decoding the right data even if the index
> matches.
>
>
> Yes, this part is a bug, but in order to figure out how to fix it, I had
> to know whether the compiler behavior is intentional, and whether we need
> to access that data. If it turned out we did, the fix would be more
> involved, because we'd also need to change the UID encoding so that we can
> reference these DIEs correctly, etc. Given that it seems we can just ignore
> these debug info entries, the fix will be relatively simple.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D62634/new/
>
> https://reviews.llvm.org/D62634
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

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

aadsm marked 3 inline comments as done.
aadsm added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteLibrariesSvr4Support.py:51-52
+
+OFFSET = 0
+LENGTH = 0x
+

labath wrote:
> Why are these all caps? TBH, you probably don't need to make these variables 
> at all. You can just place them directly into the packet as strings.
tbh I mostly copy pasted the auxv test :D but yeah it can be made simpler.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:2197
+
+  info.name = std::string(name_buffer);
+  info.link_map = link_map_addr;

labath wrote:
> What if `name_buffer` is not null-terminated?
Nicely spotted. I should add the size but this made me realize I'm not ensuring 
a `\0` on my next diff where I implement ReadCStringFromMemory.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2778
+for (auto const &library : library_list) {
+  response.Printf(" You're completely ignoring escaping here, which is a pretty complex topic, 
> and one of the reasons why constructing this via the DOM api is so 
> complicated. There are a couple of considerations here:
> - `"`, `&`, and other special xml characters: these will definitely need to 
> be xml-escaped properly
> - non-7-bit characters: how these will come out depends on whether the 
> client&server agree on the encoding used. I'm not sure what's the best thing 
> to do here
> - low ascii (nonprintable) characters: it looks like these cannot be 
> represented in xml (1.0) at all (?)
> 
> It might be good to check what gdb does in these situations
Oh yeah, this is a path so of course that will happen. (we probably should 
update ds2 for that as well).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502



___
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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:243-251
+// 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 just delete these extra DIEs
+// (if any) and reclaim some space.
+m_die_array.resize(1);
+m_die_array.shrink_to_fit();

clayborg wrote:
> Can we detect and just not parse in the first place so we don't parse then 
> throw away?
Also, what if we have one DWO file that isn't there where we don't delete these 
DIEs, followed by one that is. Won't we run into lldb::user_id_t collisions in 
this case? Should we just never parse DWO DIEs other than the first one to 
avoid this?


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

https://reviews.llvm.org/D62852



___
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

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

sgraenitz created this revision.
sgraenitz added reviewers: JDevlieghere, jingham, xiaobai, stella.stamenova.
Herald added a subscriber: mgorny.
Herald added a project: LLDB.

Allow to run the test suite when building LLDB Standalone with Xcode against a 
provided LLVM build-tree that used a single-configuration generator like Ninja.

So far both test drivers, lit-based `check-lldb` as well as `lldb-dotest`, were 
looking for test dependencies (clang/dsymutil/etc.) in a subdirectory with the 
configuration name (Debug/Release/etc.). It was implicitly assuming that the 
provided LLVM build-tree was also built with a multi-configuration generator 
(Xcode/VisualStudio). In practice, however, the opposite is more common: build 
the dependencies with Ninja and LLDB with Xcode for development. With this 
patch it becomes the default.

I think this will also work with Visual Studio, but kept the Xcode restriction 
because I didn't test it. I am happy to relax the condition, once someone can 
confirm it works.

If necessary I can add an option to enable this feature, like 
`LLDB_PROVIDED_NINJA_BUILD_TREE=On`. What do you think?
Otherwise, is there any other use-case I missed or you want to be considered?

Once this is sound, I'm planning the following steps:

- add stage to the lldb-cmake-standalone bot 
 to build 
and test it
- update the `Building LLDB with Xcode` section in the docs
- bring the same mechanism to swift-lldb
- fade out the manually maintained Xcode project

On macOS build and test like this:

  $ git clone https://github.com/llvm/llvm-project.git /path/to/llvm-project
  
  $ cd /path/to/lldb-dev-deps
  $ cmake -GNinja 
-C/path/to/llvm-project/lldb/cmake/caches/Apple-lldb-macOS.cmake 
-DLLVM_ENABLE_PROJECTS="clang;libcxx;libcxxabi" /path/to/llvm-project/llvm
  $ ninja
  
  $ cd /path/to/lldb-dev-xcode
  $ cmake -GXcode 
-C/path/to/llvm-project/lldb/cmake/caches/Apple-lldb-macOS.cmake 
-DLLVM_DIR=/path/to/lldb-dev-deps/lib/cmake/llvm 
-DClang_DIR=/path/to/lldb-dev-deps/lib/cmake/clang /path/to/llvm-project/lldb
  $ xcodebuild -configuration Debug -target check-lldb
  $ xcodebuild -configuration Debug -target lldb-dotest
  $ ./Debug/bin/lldb-dotest


Repository:
  rG LLVM Github Monorepo

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,23 @@
 
 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 AND ${CMAKE_GENERATOR} STREQUAL "Xcode")
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# For standalone 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}")
+
+# Replace the remaining CMAKE_CFG_INTDIR with ".", assuming the provided
+# LLVM build-tree used a single-configuration generator like Ninja.
+string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+
+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,36 @@
 # 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 AND ${CMAKE_GENERATOR} STREQUAL "Xcode")
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# For standalone 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}")
+
+# Replace the remaining CMAKE_CFG_INTDIR with ".", assuming the provided
+# LLVM build-tree used a

[Lldb-commits] [lldb] r362510 - [lldb] Fix out-of-bounds read after c3ea7c66fec021867e005ad1b02f3c7e80feaa85

[James Y Knight via lldb-commits]

Author: jyknight
Date: Tue Jun  4 08:27:19 2019
New Revision: 362510

URL: http://llvm.org/viewvc/llvm-project?rev=362510&view=rev
Log:
[lldb] Fix out-of-bounds read after c3ea7c66fec021867e005ad1b02f3c7e80feaa85
"Add support for mid-function epilogues on x86 that end in a non-local jump."

Detected by asan.

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=362510&r1=362509&r2=362510&view=diff
==
--- 
lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp 
Tue Jun  4 08:27:19 2019
@@ -736,7 +736,6 @@ bool x86AssemblyInspectionEngine::pc_rel
   int opcode_size = 0;
 
   uint8_t b1 = m_cur_insn[0];
-  uint8_t b2 = m_cur_insn[1];
 
   switch (b1) {
 case 0x77: // JA/JNBE rel8
@@ -764,6 +763,7 @@ bool x86AssemblyInspectionEngine::pc_rel
   break;
   }
   if (b1 == 0x0f && opcode_size == 0) {
+uint8_t b2 = m_cur_insn[1];
 switch (b2) {
   case 0x87: // JA/JNBE rel16/rel32
   case 0x86: // JBE/JNA rel16/rel32


___
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

[Stella Stamenova via Phabricator via lldb-commits]

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

I don't think that this would apply very well to Visual Studio with the current 
change because it is likely that if you are using VS to build LLDB, you also 
used VS to build the dependencies. Even for xcode, I am sure that there are 
people who build the dependencies standalone with xcode and then are building 
LLDB standalone with xcode, so any change that absolutely treats the scenario 
as one or the other is going to break someone.

I think a better change would be to allow a parameter to be passed like your 
suggested `LLDB_PROVIDED_NINJA_BUILD_TREE=On` (except don't use Ninja in the 
name, how the tree came about is not important and it could also be make or a 
tree copied from another build) and then use that and LLDB_BUILT_STANDALONE for 
the pivot. Then if someone provides the new parameter, they know what they are 
doing and we can expect it to work for Xcode and VS both.


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] D62812: [llvm] [CodeView] Move Triple::ArchType → CPUType mapping from LLDB

[Michał Górny via Phabricator via lldb-commits]

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



Comment at: llvm/include/llvm/DebugInfo/CodeView/CodeView.h:147
+return CPUType::ARM64;
+  default:
+return CPUType::X64;

compnerd wrote:
> This is still wrong.  `x86` and `armv7` are totally legitimate values.  
> Furthermore, x86 is still far more prevalent on Windows.
Could you be more specific, please? Are you asking me to cover more input 
values?

Which option do you suggest mapping x86 into? Pentium3?


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

https://reviews.llvm.org/D62812



___
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

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 202986.
labath added a comment.

- avoid parsing the extra DIEs in the first place


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

https://reviews.llvm.org/D62852

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


Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ 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: lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
===
--- /dev/null
+++ 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: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ 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: lit/SymbolFile/DWARF/split-dwarf-inlining.cpp
===
--- /dev/null
+++ 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); }
___
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

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:243-251
+// 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 just delete these extra DIEs
+// (if any) and reclaim some space.
+m_die_array.resize(1);
+m_die_array.shrink_to_fit();

clayborg wrote:
> clayborg wrote:
> > Can we detect and just not parse in the first place so we don't parse then 
> > throw away?
> Also, what if we have one DWO file that isn't there where we don't delete 
> these DIEs, followed by one that is. Won't we run into lldb::user_id_t 
> collisions in this case? Should we just never parse DWO DIEs other than the 
> first one to avoid this?
I've changed the parsing logic to bail out earlier.

There won't be any UID collisions if only some of the dwo files are missing, 
because we will, for all intents and purposes, treat a compile with a missing 
dwo file the same way as a regular (non-dwo) compile unit. And we already 
support mixing dwo and non-dwo compile units in a single file.


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

https://reviews.llvm.org/D62852



___
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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

> LLDB_PROVIDED_NINJA_BUILD_TREE=On

An even better option would be to get the llvm build to export the fact whether 
it was built with a multi-config generator or not. Then the user wouldn't have 
to provide anything, and things would "just work". :)


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] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

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

sgraenitz added a comment.

Thanks for your feedback. I should have mentioned that this is pretty much a 
draft. For now I was happy it works at all.

In D62859#1529498 , @stella.stamenova 
wrote:

> I don't think that this would apply very well to Visual Studio with the 
> current change because it is likely that if you are using VS to build LLDB, 
> you also used VS to build the dependencies.


Good point

> I think a better change would be to allow a parameter to be passed like your 
> suggested `LLDB_PROVIDED_NINJA_BUILD_TREE=On` (except don't use Ninja in the 
> name, how the tree came about is not important and it could also be make or a 
> tree copied from another build) and then use that and LLDB_BUILT_STANDALONE 
> for the pivot. Then if someone provides the new parameter, they know what 
> they are doing and we can expect it to work for Xcode and VS both.

Yes, then it's either that or what Pavel proposed.

In D62859#1529666 , @labath wrote:

> > LLDB_PROVIDED_NINJA_BUILD_TREE=On
>
> An even better option would be to get the llvm build to export the fact 
> whether it was built with a multi-config generator or not. Then the user 
> wouldn't have to provide anything, and things would "just work". :)


Yes, I will check if I can store it in LLVMConfig.cmake


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] [lldb] r362543 - [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Tue Jun  4 12:29:59 2019
New Revision: 362543

URL: http://llvm.org/viewvc/llvm-project?rev=362543&view=rev
Log:
[ABI] Fix SystemV ABI to handle nested aggregate type returned in register

Add a function to flatten the nested aggregate type

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

Patch by Wanyi Ye 

Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/call-func.cpp
  - copied, changed from r362510, 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/call-func.c
Removed:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/call-func.c
Modified:
lldb/trunk/include/lldb/Symbol/ClangASTContext.h
lldb/trunk/include/lldb/Symbol/TypeSystem.h

lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/Makefile

lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/trivial_abi/TestTrivialABI.py
lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=362543&r1=362542&r2=362543&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Tue Jun  4 12:29:59 2019
@@ -598,6 +598,8 @@ public:
 
   bool IsVoidType(lldb::opaque_compiler_type_t type) override;
 
+  bool CanPassInRegisters(const CompilerType &type) override;
+
   bool SupportsLanguage(lldb::LanguageType language) override;
 
   static bool GetCXXClassName(const CompilerType &type,

Modified: lldb/trunk/include/lldb/Symbol/TypeSystem.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/TypeSystem.h?rev=362543&r1=362542&r2=362543&view=diff
==
--- lldb/trunk/include/lldb/Symbol/TypeSystem.h (original)
+++ lldb/trunk/include/lldb/Symbol/TypeSystem.h Tue Jun  4 12:29:59 2019
@@ -181,6 +181,8 @@ public:
 
   virtual bool IsVoidType(lldb::opaque_compiler_type_t type) = 0;
 
+  virtual bool CanPassInRegisters(const CompilerType &type) = 0;
+
   // TypeSystems can support more than one language
   virtual bool SupportsLanguage(lldb::LanguageType language) = 0;
 

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/Makefile?rev=362543&r1=362542&r2=362543&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/Makefile 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/Makefile 
Tue Jun  4 12:29:59 2019
@@ -1,5 +1,5 @@
 LEVEL = ../../make
 
-C_SOURCES := call-func.c
+CXX_SOURCES := call-func.cpp
 
 include $(LEVEL)/Makefile.rules

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py?rev=362543&r1=362542&r2=362543&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
 Tue Jun  4 12:29:59 2019
@@ -57,7 +57,7 @@ class ReturnValueTestCase(TestBase):
 
 frame = thread.GetFrameAtIndex(0)
 fun_name = frame.GetFunctionName()
-self.assertTrue(fun_name == "outer_sint")
+self.assertTrue(fun_name == "outer_sint(int)")
 
 return_value = thread.GetStopReturnValue()
 self.assertTrue(return_value.IsValid())
@@ -78,7 +78,7 @@ class ReturnValueTestCase(TestBase):
 
 frame = thread.GetFrameAtIndex(1)
 fun_name = frame.GetFunctionName()
-self.assertTrue(fun_name == "outer_sint")
+self.assertTrue(fun_name == "outer_sint(int)")
 in_int = frame.FindVariable("value").GetValueAsSigned(error)
 self.assertTrue(error.Success())
 
@@ -98,7 +98,7 @@ class ReturnValueTestCase(TestBase):
 
 # Now try some simple returns that have different types:
 inner_float_bkpt = self.target.BreakpointCreateByName(
-"inner_float", exe)
+"inner_float(float)", exe)
 self.assertTrue(inner_float_bkpt, VALID_BREAKPOINT)
 self.process.Continue()
 thread_list = lldbutil.get_threads_stopped_at_breakpoint(
@@ -118,7 +118,7 @@ class ReturnValueTestCase(TestBase):
 
 frame = thre

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

[Alex Langford 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 rL362543: [ABI] Fix SystemV ABI to handle nested aggregate 
type returned in register (authored by xiaobai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62702?vs=202824&id=202997#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62702

Files:
  lldb/trunk/include/lldb/Symbol/ClangASTContext.h
  lldb/trunk/include/lldb/Symbol/TypeSystem.h
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/call-func.c
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/call-func.cpp
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/trivial_abi/TestTrivialABI.py
  lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
  lldb/trunk/source/Symbol/ClangASTContext.cpp

Index: lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
===
--- lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
+++ lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
@@ -30,6 +30,8 @@
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Status.h"
 
+#include 
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -1558,6 +1560,55 @@
   return return_valobj_sp;
 }
 
+// The compiler will flatten the nested aggregate type into single
+// layer and push the value to stack
+// This helper function will flatten an aggregate type
+// and return true if it can be returned in register(s) by value
+// return false if the aggregate is in memory
+static bool FlattenAggregateType(
+Thread &thread, ExecutionContext &exe_ctx,
+CompilerType &return_compiler_type,
+uint32_t data_byte_offset,
+std::vector &aggregate_field_offsets,
+std::vector &aggregate_compiler_types) {
+
+  const uint32_t num_children = return_compiler_type.GetNumFields();
+  for (uint32_t idx = 0; idx < num_children; ++idx) {
+std::string name;
+bool is_signed;
+uint32_t count;
+bool is_complex;
+
+uint64_t field_bit_offset = 0;
+CompilerType field_compiler_type = return_compiler_type.GetFieldAtIndex(
+idx, name, &field_bit_offset, nullptr, nullptr);
+llvm::Optional field_bit_width =
+  field_compiler_type.GetBitSize(&thread);
+
+// if we don't know the size of the field (e.g. invalid type), exit
+if (!field_bit_width || *field_bit_width == 0) {
+  return false;
+}
+
+uint32_t field_byte_offset = field_bit_offset / 8 + data_byte_offset;
+
+const uint32_t field_type_flags = field_compiler_type.GetTypeInfo();
+if (field_compiler_type.IsIntegerOrEnumerationType(is_signed) ||
+field_compiler_type.IsPointerType() ||
+field_compiler_type.IsFloatingPointType(count, is_complex)) {
+  aggregate_field_offsets.push_back(field_byte_offset);
+  aggregate_compiler_types.push_back(field_compiler_type);
+} else if (field_type_flags & eTypeHasChildren) {
+  if (!FlattenAggregateType(thread, exe_ctx, field_compiler_type,
+field_byte_offset, aggregate_field_offsets,
+aggregate_compiler_types)) {
+return false;
+  }
+}
+  }
+  return true;
+}
+
 ValueObjectSP ABISysV_x86_64::GetReturnValueObjectImpl(
 Thread &thread, CompilerType &return_compiler_type) const {
   ValueObjectSP return_valobj_sp;
@@ -1580,10 +1631,17 @@
   if (return_compiler_type.IsAggregateType()) {
 Target *target = exe_ctx.GetTargetPtr();
 bool is_memory = true;
-if (*bit_width <= 128) {
-  ByteOrder target_byte_order = target->GetArchitecture().GetByteOrder();
+std::vector aggregate_field_offsets;
+std::vector aggregate_compiler_types;
+if (return_compiler_type.GetTypeSystem()->CanPassInRegisters(
+  return_compiler_type) &&
+  *bit_width <= 128 &&
+  FlattenAggregateType(thread, exe_ctx, return_compiler_type,
+  0, aggregate_field_offsets,
+  aggregate_compiler_types)) {
+  ByteOrder byte_order = target->GetArchitecture().GetByteOrder();
   DataBufferSP data_sp(new DataBufferHeap(16, 0));
-  DataExtractor return_ext(data_sp, target_byte_order,
+  DataExtractor return_ext(data_sp, byte_order,
target->GetArchitecture().GetAddressByteSize());
 
   const RegisterInfo *rax_info =
@@ -1613,36 +1671,27 @@
   uint32_t integer_bytes =
   0; // Tracks how much of the rax/rds registers we've consumed so far
 
-  const uint32_t num_chi

[Lldb-commits] [lldb] r362544 - [Target] Remove Process::GetCPPLanguageRuntime

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Tue Jun  4 13:14:33 2019
New Revision: 362544

URL: http://llvm.org/viewvc/llvm-project?rev=362544&view=rev
Log:
[Target] Remove Process::GetCPPLanguageRuntime

Summary:
I want to remove this method because I think that Process should be
language agnostic, or at least, not have knowledge about specific language
runtimes. There is "GetLanguageRuntime()" which should be used instead. If the
caller a CPPLanguageRuntime, they should cast it as needed. Ideally, this
should only happen in plugins that need C++ specific knowledge.

The next step I would like to do is remove "GetObjCLanguageRuntime()" as well.
There are a lot more instances of that function being used, so I wanted to
upload this one first to get the general reception to this idea.

Reviewers: compnerd, davide, JDevlieghere, jingham, clayborg, labath, aprantl

Subscribers: lldb-commits

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

Modified:
lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/include/lldb/lldb-forward.h
lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp

lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h?rev=362544&r1=362543&r2=362544&view=diff
==
--- lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h (original)
+++ lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h Tue Jun  4 13:14:33 2019
@@ -43,6 +43,11 @@ public:
 return lldb::eLanguageTypeC_plus_plus;
   }
 
+  static CPPLanguageRuntime *GetCPPLanguageRuntime(Process &process) {
+return static_cast(
+process.GetLanguageRuntime(lldb::eLanguageTypeC_plus_plus));
+  }
+
   virtual bool IsVTableName(const char *name) = 0;
 
   bool GetObjectDescription(Stream &str, ValueObject &object) override;

Modified: lldb/trunk/include/lldb/Target/Process.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=362544&r1=362543&r2=362544&view=diff
==
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Tue Jun  4 13:14:33 2019
@@ -2184,8 +2184,6 @@ public:
   LanguageRuntime *GetLanguageRuntime(lldb::LanguageType language,
   bool retry_if_null = true);
 
-  CPPLanguageRuntime *GetCPPLanguageRuntime(bool retry_if_null = true);
-
   ObjCLanguageRuntime *GetObjCLanguageRuntime(bool retry_if_null = true);
 
   bool IsPossibleDynamicValue(ValueObject &in_value);

Modified: lldb/trunk/include/lldb/lldb-forward.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb-forward.h?rev=362544&r1=362543&r2=362544&view=diff
==
--- lldb/trunk/include/lldb/lldb-forward.h (original)
+++ lldb/trunk/include/lldb/lldb-forward.h Tue Jun  4 13:14:33 2019
@@ -43,7 +43,6 @@ class BreakpointSiteList;
 class BroadcastEventSpec;
 class Broadcaster;
 class BroadcasterManager;
-class CPPLanguageRuntime;
 class ClangASTContext;
 class ClangASTImporter;
 class ClangASTMetadata;

Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp?rev=362544&r1=362543&r2=362544&view=diff
==
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp (original)
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp Tue Jun  4 13:14:33 
2019
@@ -67,7 +67,8 @@ bool lldb_private::formatters::LibcxxFun
   if (process == nullptr)
 return false;
 
-  CPPLanguageRuntime *cpp_runtime = process->GetCPPLanguageRuntime();
+  CPPLanguageRuntime *cpp_runtime =
+  CPPLanguageRuntime::GetCPPLanguageRuntime(*process);
 
   if (!cpp_runtime)
 return false;

Modified: 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp?rev=362544&r1=362543&r2=362544&view=diff
==
--- 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
 Tue Jun  4 13:14:33 2019
@@ -462,7 +462,7 @@ lldb::SearchFilterSP AppleObjCRuntime::C
 
 ValueObjectSP AppleObjCRuntime::GetExceptionObjectForThread(
 ThreadSP thread_sp) {
-  auto cpp_runtime = m_process->GetCPPLanguageRuntime();
+  auto *cpp_runtime = m_process->Get

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

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362544: [Target] Remove Process::GetCPPLanguageRuntime 
(authored by xiaobai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62755

Files:
  lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
  lldb/trunk/include/lldb/Target/Process.h
  lldb/trunk/include/lldb/lldb-forward.h
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  lldb/trunk/source/Target/Process.cpp


Index: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -67,7 +67,8 @@
   if (process == nullptr)
 return false;
 
-  CPPLanguageRuntime *cpp_runtime = process->GetCPPLanguageRuntime();
+  CPPLanguageRuntime *cpp_runtime =
+  CPPLanguageRuntime::GetCPPLanguageRuntime(*process);
 
   if (!cpp_runtime)
 return false;
Index: 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
===
--- 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -462,7 +462,7 @@
 
 ValueObjectSP AppleObjCRuntime::GetExceptionObjectForThread(
 ThreadSP thread_sp) {
-  auto cpp_runtime = m_process->GetCPPLanguageRuntime();
+  auto *cpp_runtime = m_process->GetLanguageRuntime(eLanguageTypeC_plus_plus);
   if (!cpp_runtime) return ValueObjectSP();
   auto cpp_exception = cpp_runtime->GetExceptionObjectForThread(thread_sp);
   if (!cpp_exception) return ValueObjectSP();
Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -1598,16 +1598,6 @@
   return runtime;
 }
 
-CPPLanguageRuntime *Process::GetCPPLanguageRuntime(bool retry_if_null) {
-  std::lock_guard guard(m_language_runtimes_mutex);
-  LanguageRuntime *runtime =
-  GetLanguageRuntime(eLanguageTypeC_plus_plus, retry_if_null);
-  if (!runtime)
-return nullptr;
-
-  return static_cast(runtime);
-}
-
 ObjCLanguageRuntime *Process::GetObjCLanguageRuntime(bool retry_if_null) {
   std::lock_guard guard(m_language_runtimes_mutex);
   LanguageRuntime *runtime =
Index: lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
===
--- lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
+++ lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
@@ -43,6 +43,11 @@
 return lldb::eLanguageTypeC_plus_plus;
   }
 
+  static CPPLanguageRuntime *GetCPPLanguageRuntime(Process &process) {
+return static_cast(
+process.GetLanguageRuntime(lldb::eLanguageTypeC_plus_plus));
+  }
+
   virtual bool IsVTableName(const char *name) = 0;
 
   bool GetObjectDescription(Stream &str, ValueObject &object) override;
Index: lldb/trunk/include/lldb/Target/Process.h
===
--- lldb/trunk/include/lldb/Target/Process.h
+++ lldb/trunk/include/lldb/Target/Process.h
@@ -2184,8 +2184,6 @@
   LanguageRuntime *GetLanguageRuntime(lldb::LanguageType language,
   bool retry_if_null = true);
 
-  CPPLanguageRuntime *GetCPPLanguageRuntime(bool retry_if_null = true);
-
   ObjCLanguageRuntime *GetObjCLanguageRuntime(bool retry_if_null = true);
 
   bool IsPossibleDynamicValue(ValueObject &in_value);
Index: lldb/trunk/include/lldb/lldb-forward.h
===
--- lldb/trunk/include/lldb/lldb-forward.h
+++ lldb/trunk/include/lldb/lldb-forward.h
@@ -43,7 +43,6 @@
 class BroadcastEventSpec;
 class Broadcaster;
 class BroadcasterManager;
-class CPPLanguageRuntime;
 class ClangASTContext;
 class ClangASTImporter;
 class ClangASTMetadata;


Index: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -67,7 +67,8 @@
   if (process == nullptr)
 return false;
 
-  CPPLanguageRuntime *cpp_runtime = process->GetCPPLanguageRuntime();
+  CPPLanguageRuntime *cpp_runtime =
+  CPPLanguageRuntime::GetCPPLanguageRuntime(*process);
 
   if (!cpp_runtime)
 return false;
Index: lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
===
--- lldb/trunk/source/Plugins/Lang

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

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

sgraenitz created this revision.
sgraenitz added reviewers: labath, beanz, mgorny.
Herald added a project: LLVM.

Useful info for standalone builds of subprojects. If a multi-configuration 
generator was used for the provided LLVM build-tree, standalone builds should 
consider actual subdirectories per configuration in `find_program()` (e.g. 
looking for `llvm-lit` or `llvm-tblgen`).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62878

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


Index: llvm/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/cmake/modules/LLVMConfig.cmake.in
+++ llvm/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/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/cmake/modules/LLVMConfig.cmake.in
+++ llvm/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

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

sgraenitz created this revision.
sgraenitz added reviewers: xiaobai, labath, stella.stamenova.
Herald added a subscriber: mgorny.
Herald added a project: LLDB.

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()`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62879

Files:
  lldb/cmake/modules/LLDBStandalone.cmake


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/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,8 @@
 
"${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)
+append_configuration_directories(${LLVM_TOOLS_BINARY_DIR} config_dirs)
+find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${config_dirs} 
NO_DEFAULT_PATH)
   endif()
 
   # They are used as destination of target generators.


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/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,8 @@
 "${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)
+append_configuration_directories(${LLVM_TOOLS_BINARY_DIR} config_dirs)
+find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${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] [PATCH] D62879: [CMake] Add configuration dirs as potential locations for llvm-lit and llvm-tblgen in standalone builds

[Stella Stamenova via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62879



___
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

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

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

Chose the replacement for remaining matches of CMAKE_CFG_INTDIR depdending on 
LLVM_CONFIGURATION_TYPES


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 AND ${CMAKE_GENERATOR} STREQUAL "Xcode")
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# For standalone 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}")
+
+# Replace remaining ones matching the generator used for the provided LLVM 
build-tree.
+if(config_type IN_LIST LLVM_CONFIGURATION_TYPES)
+  # Multi-configuration generator like Xcode with a matching configuration.
+  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,36 @@
 # 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 AND ${CMAKE_GENERATOR} STREQUAL "Xcode")
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# For standalone 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}")
+
+# Replace the remaining CMAKE_CFG_INTDIR with ".", assuming the provided
+# LLVM build-tree used a single-configuration generator like Ninja.
+string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+  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 AND ${CMAKE_GENERATOR} STREQUAL "Xcode")
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# For standalone build-tree: replace CMAKE_CFG_INTDIR with our actual configuration names
+string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} config_runtime_output_dir ${LLVM_RUNTIME_OUTP

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

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added inline comments.



Comment at: lldb/lit/CMakeLists.txt:13
+# Replace the remaining CMAKE_CFG_INTDIR with ".", assuming the provided
+# LLVM build-tree used a single-configuration generator like Ninja.
+string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")

It looks like you need the update here also. Is there a way to share the logic 
between the two cmake files?

You probably no longer need the xcode pivot either, right?


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] D62879: [CMake] Add configuration dirs as potential locations for llvm-lit and llvm-tblgen in standalone builds

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

sgraenitz added a comment.

IIUC Clang uses the same mechanism. Moving `append_configuration_directories()` 
to AddLLVM.cmake could help there too. We may consider it for the future once 
D62859  is through.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62879



___
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

[Alex Langford via Phabricator via lldb-commits]

xiaobai accepted this revision.
xiaobai added a comment.

Seems fine to me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62879



___
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

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

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

Patch the replacement logic also in lit/CMakeLists.txt


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 AND ${CMAKE_GENERATOR} STREQUAL "Xcode")
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# For standalone 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}")
+
+# Replace remaining ones matching the generator used for the provided LLVM 
build-tree.
+if(config_type IN_LIST LLVM_CONFIGURATION_TYPES)
+  # Multi-configuration generator like Xcode with a matching configuration.
+  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 AND ${CMAKE_GENERATOR} STREQUAL "Xcode")
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# For standalone 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}")
+
+# Replace remaining ones matching the generator used for the provided LLVM 
build-tree.
+if(config_type IN_LIST LLVM_CONFIGURATION_TYPES)
+  # Multi-configuration generator like Xcode with a matching configuration.
+  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 AND ${CMAKE_GENERATOR} STREQUAL "Xcode")
+  foreach(config_type ${CMAKE_CONFIGURATIO

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

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

sgraenitz marked an inline comment as done.
sgraenitz added a comment.

After figuring out D62879  that came up on the 
way, this wasn't too complicated indeed. I put the patch for exporting 
`LLVM_CONFIGURATION_TYPES` here: D62878 




Comment at: lldb/lit/CMakeLists.txt:13
+# Replace the remaining CMAKE_CFG_INTDIR with ".", assuming the provided
+# LLVM build-tree used a single-configuration generator like Ninja.
+string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")

stella.stamenova wrote:
> It looks like you need the update here also. Is there a way to share the 
> logic between the two cmake files?
> 
> You probably no longer need the xcode pivot either, right?
Thanks for catching this one! Ok deal, I remove the Xcode restriction and you 
run a test with VS? ;)


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] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

[Stella Stamenova via Phabricator via lldb-commits]

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



Comment at: lldb/lit/CMakeLists.txt:13
+# Replace the remaining CMAKE_CFG_INTDIR with ".", assuming the provided
+# LLVM build-tree used a single-configuration generator like Ninja.
+string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")

sgraenitz wrote:
> stella.stamenova wrote:
> > It looks like you need the update here also. Is there a way to share the 
> > logic between the two cmake files?
> > 
> > You probably no longer need the xcode pivot either, right?
> Thanks for catching this one! Ok deal, I remove the Xcode restriction and you 
> run a test with VS? ;)
I can run some tests with VS, probably tomorrow though.


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] D62879: [CMake] Add configuration dirs as potential locations for llvm-lit and llvm-tblgen in standalone builds

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

sgraenitz added inline comments.



Comment at: lldb/cmake/modules/LLDBStandalone.cmake:67
+append_configuration_directories(${LLVM_TOOLS_BINARY_DIR} config_dirs)
+find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${config_dirs} 
NO_DEFAULT_PATH)
   endif()

How is this working on Windows anyway? It should be `llvm-tblgen.exe` there 
right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62879



___
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

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

sgraenitz added a comment.

That will be fine with post-commit review. Will submit tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62878



___
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

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added inline comments.



Comment at: lldb/cmake/modules/LLDBStandalone.cmake:67
+append_configuration_directories(${LLVM_TOOLS_BINARY_DIR} config_dirs)
+find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${config_dirs} 
NO_DEFAULT_PATH)
   endif()

sgraenitz wrote:
> How is this working on Windows anyway? It should be `llvm-tblgen.exe` there 
> right?
Ah, good point. You should use `llvm-tblgen${HOST_EXECUTABLE_SUFFIX}` as the 
name. That will add the suffix when required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62879



___
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

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

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

Add CMAKE_EXECUTABLE_SUFFIX when searching for llvm-tblgen


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62879

Files:
  lldb/cmake/modules/LLDBStandalone.cmake


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/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/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/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] [PATCH] D62879: [CMake] Add configuration dirs as potential locations for llvm-lit and llvm-tblgen in standalone builds

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

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



Comment at: lldb/cmake/modules/LLDBStandalone.cmake:67
+append_configuration_directories(${LLVM_TOOLS_BINARY_DIR} config_dirs)
+find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${config_dirs} 
NO_DEFAULT_PATH)
   endif()

stella.stamenova wrote:
> sgraenitz wrote:
> > How is this working on Windows anyway? It should be `llvm-tblgen.exe` there 
> > right?
> Ah, good point. You should use `llvm-tblgen${HOST_EXECUTABLE_SUFFIX}` as the 
> name. That will add the suffix when required.
Ok added the suffix. I can imaging CMake would do it automatically if 
necessary, but it's not documented.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62879



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

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

[Nathan Lanza via Phabricator via lldb-commits]

lanza added a comment.

@clayborg Seems like this still steps into the `call` if the call is the last 
instruction in the range. `ThreadPlanStepRange::SetNextBranchBreakpoint` checks 
`if (last_index - pc_index > 1)` before setting the breakpoint. So if 
`last_index == pc_index` and `pc` points to `call` then the thread plan will 
resort to single stepping and thus go through all the same machinery. 
Obviously, this isn't a problem as this just leads to using the same 
functionality that it used prior to this patch, but you miss out on the 
optimization you're aiming for.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58678



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

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

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D62797#1528483 , @jingham wrote:

> I have no problem with the change in general.  However, you've introduced the 
> possibility of name collision between convenience type definition in various 
> languages.  So it would be good to run through the persistent DECL's for ALL 
> the supported languages and report collisions.  Probably also need to add a 
> -l flag to pick a language?


Hmm, yes that's true. Would you like me to implement that in this patch as well 
or would you be okay with a follow up patch?


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

https://reviews.llvm.org/D62797



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

[Lldb-commits] [lldb] r362557 - Call abs to avoid signed/unsigned comparison warning.

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Tue Jun  4 15:46:20 2019
New Revision: 362557

URL: http://llvm.org/viewvc/llvm-project?rev=362557&view=rev
Log:
Call abs to avoid signed/unsigned comparison warning.

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=362557&r1=362556&r2=362557&view=diff
==
--- 
lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp 
Tue Jun  4 15:46:20 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 && -offset > current_func_text_offset) {
+if (offset < 0 && abs (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] [PATCH] D62797: [Expression] Add PersistentExpressionState::GetCompilerTypeFromPersistentDecl

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Pretty much the only actual effect of this patch (besides its changes to the 
dependency graph) are introducing the possibility for ambiguity here.  It seems 
more logical to do it all as one patch.


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

https://reviews.llvm.org/D62797



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

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

[Fangrui Song via lldb-commits]

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] [PATCH] D62887: Update the thread list before setting stop reasons with an OS plugin

[Jim Ingham via Phabricator via lldb-commits]

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

When talking to a gdb-remote server that didn't support the newer thread info 
packets, we noticed that lldb would hit a breakpoint in target, but then 
continue on without stopping.  This was because the only indication of why you 
stopped in this scenario was the initial "T05thread:01;" packet.  
Unfortunately, lldb read the stop reasons from the stop packet and created the 
new thread & set its stop reason BEFORE calling UpdateThreadListIfNeeded.  So 
when you finally got around to calling UpdateThreadListIfNeeded, we would 
notice that the ThreadList StopID was out of date, wipe the stop reasons, and 
then update.  Normally some other thread info packet would tell us the stop 
reason again, and we we'd get back on track.  But if they weren't supported, 
we'd be left thinking that the process had stopped for no reason, and 
auto-continue it.

The main part of the patch just adjusts the call to UpdateThreadListIfNeeded so 
it happens before we try to set StopInfo's.

I also added a test that captures this failure.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D62887

Files:
  
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py
  
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
  
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/operating_system_2.py
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  source/Target/Process.cpp

Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -3033,6 +3033,13 @@
 
   if (!m_os_up)
 LoadOperatingSystemPlugin(false);
+  
+  // Somebody might have gotten threads before now, but we need to force the
+  // update after we've loaded the OperatingSystem plugin or it won't get a
+  // chance to process the threads.
+  m_thread_list.Clear();
+  UpdateThreadListIfNeeded();
+  
   // Figure out which one is the executable, and set that in our target:
   const ModuleList &target_modules = GetTarget().GetImages();
   std::lock_guard guard(target_modules.GetMutex());
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2426,6 +2426,15 @@
 
   // Scope for the lock
   {
+// Check to see if SetThreadStopInfo() filled in m_thread_ids?
+if (m_thread_ids.empty()) {
+// No, we need to fetch the thread list manually
+UpdateThreadIDList();
+}
+// We might set some stop info's so make sure the thread list is up to
+// date before we do that or we might overwrite what was computed here.
+UpdateThreadListIfNeeded();
+
 // Lock the thread stack while we access it
 std::lock_guard guard(m_last_stop_packet_mutex);
 // Get the number of stop packets on the stack
@@ -2440,13 +2449,7 @@
 // Clear the thread stop stack
 m_stop_packet_stack.clear();
   }
-
-  // Check to see if SetThreadStopInfo() filled in m_thread_ids?
-  if (m_thread_ids.empty()) {
-// No, we need to fetch the thread list manually
-UpdateThreadIDList();
-  }
-
+  
   // If we have queried for a default thread id
   if (m_initial_tid != LLDB_INVALID_THREAD_ID) {
 m_thread_list.SetSelectedThreadByID(m_initial_tid);
Index: packages/Python/lldbsuite/test/functionalities/gdb_remote_client/operating_system_2.py
===
--- packages/Python/lldbsuite/test/functionalities/gdb_remote_client/operating_system_2.py
+++ packages/Python/lldbsuite/test/functionalities/gdb_remote_client/operating_system_2.py
@@ -0,0 +1,62 @@
+import lldb
+import struct
+
+
+class OperatingSystemPlugIn(object):
+"""Class that provides data for an instance of a LLDB 'OperatingSystemPython' plug-in class
+   This version stops once with threads 0x111 and 0x222, then stops a second time with threads
+   0x111 and 0x333."""
+
+def __init__(self, process):
+'''Initialization needs a valid.SBProcess object.
+
+This plug-in will get created after a live process is valid and has stopped for the first time.
+'''
+self.process = None
+self.registers = None
+self.threads = None
+self.times_called = 0
+if isinstance(process, lldb.SBProcess) and process.IsValid():
+self.process = process
+self.threads = None  # Will be an dictionary containing info for each thread
+
+def get_target(self):
+return self.process.target
+
+def get_thread_info(self):
+self.times_called += 1
+
+if self.times_called == 1:
+self.threads = [{
+'tid': 0x111,
+  

[Lldb-commits] [PATCH] D62213: [ABI] Implement Windows ABI for x86_64

[Wanyi Ye via Phabricator via lldb-commits]

kusmour updated this revision to Diff 203061.
kusmour added a comment.

Update the `GetReturnValueObjectImpl` using function `CanPassInRegisters` to 
explicitly check

NOTE: There's NO register context info about registers beyond general purpose 
registers
So floating point return type is not supported since it uses XMM0 to return
need to update register context for windows x64


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62213

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ABI/CMakeLists.txt
  lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
  lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp
  lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h
  lldb/source/Plugins/ABI/Windows-x86_64/CMakeLists.txt
  lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp

Index: lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
+++ lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
@@ -133,14 +133,14 @@
  nullptr,
  0},
 {DEFINE_GPR(r10, nullptr),
- {dwarf_r10_x86_64, dwarf_r10_x86_64, LLDB_REGNUM_GENERIC_ARG5,
+ {dwarf_r10_x86_64, dwarf_r10_x86_64, LLDB_INVALID_REGNUM,
   LLDB_INVALID_REGNUM, lldb_r10_x86_64},
  nullptr,
  nullptr,
  nullptr,
  0},
 {DEFINE_GPR(r11, nullptr),
- {dwarf_r11_x86_64, dwarf_r11_x86_64, LLDB_REGNUM_GENERIC_ARG6,
+ {dwarf_r11_x86_64, dwarf_r11_x86_64, LLDB_INVALID_REGNUM,
   LLDB_INVALID_REGNUM, lldb_r11_x86_64},
  nullptr,
  nullptr,
Index: lldb/source/Plugins/ABI/Windows-x86_64/CMakeLists.txt
===
--- /dev/null
+++ lldb/source/Plugins/ABI/Windows-x86_64/CMakeLists.txt
@@ -0,0 +1,10 @@
+add_lldb_library(lldbPluginABIWindows_x86_64 PLUGIN
+  ABIWindows_x86_64.cpp
+
+  LINK_LIBS
+lldbCore
+lldbSymbol
+lldbTarget
+  LINK_COMPONENTS
+Support
+  )
Index: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h
===
--- /dev/null
+++ lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h
@@ -0,0 +1,99 @@
+//===-- ABIWindows_x86_64.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 liblldb_ABIWindows_x86_64_h_
+#define liblldb_ABIWindows_x86_64_h_
+
+#include "lldb/Target/ABI.h"
+#include "lldb/lldb-private.h"
+
+class ABIWindows_x86_64 : public lldb_private::ABI {
+public:
+  ~ABIWindows_x86_64() override = default;
+
+  size_t GetRedZoneSize() const override;
+
+  bool PrepareTrivialCall(lldb_private::Thread &thread, lldb::addr_t sp,
+  lldb::addr_t functionAddress,
+  lldb::addr_t returnAddress,
+  llvm::ArrayRef args) const override;
+
+  bool GetArgumentValues(lldb_private::Thread &thread,
+ lldb_private::ValueList &values) const override;
+
+  lldb_private::Status
+  SetReturnValueObject(lldb::StackFrameSP &frame_sp,
+   lldb::ValueObjectSP &new_value) override;
+
+  lldb::ValueObjectSP
+  GetReturnValueObjectImpl(lldb_private::Thread &thread,
+   lldb_private::CompilerType &type) const override;
+
+  bool
+  CreateFunctionEntryUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool CreateDefaultUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
+
+  // In Windows_x86_64 ABI, stack will always be maintained 16-byte aligned
+  bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
+	  if (cfa & (16ull - 1ull))
+  return false; // Not 16 byte aligned
+if (cfa == 0)
+  return false; // Zero is not a valid stack address
+return true;
+  }
+
+  bool CodeAddressIsValid(lldb::addr_t pc) override {
+// We have a 64 bit address space, so anything is valid as opcodes
+// aren't fixed width...
+return true;
+  }
+
+  const lldb_private::RegisterInfo *
+  GetRegisterInfoArray(uint32_t &count) override;
+
+  bool GetPointerReturnRegister(const char *&name) override;
+
+  //--
+  // Static Functions
+  //--
+
+  static void Initialize();
+
+  static void Terminate();
+
+  static lldb::ABISP CreateInstance(lldb::ProcessSP process_sp, const lldb_priv

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

[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/Linux/NativeProcessLinux.cpp:2122
+  size_t phdr_num_entries = *maybe_phdr_num_entries;
+  lldb::addr_t load_base = phdr_addr - sizeof(ELF_EHDR);
+

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.


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] D62502: Implement xfer:libraries-svr4:read packet

[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:2778
+for (auto const &library : library_list) {
+  response.Printf(" labath wrote:
> > You're completely ignoring escaping here, which is a pretty complex topic, 
> > and one of the reasons why constructing this via the DOM api is so 
> > complicated. There are a couple of considerations here:
> > - `"`, `&`, and other special xml characters: these will definitely need to 
> > be xml-escaped properly
> > - non-7-bit characters: how these will come out depends on whether the 
> > client&server agree on the encoding used. I'm not sure what's the best 
> > thing to do here
> > - low ascii (nonprintable) characters: it looks like these cannot be 
> > represented in xml (1.0) at all (?)
> > 
> > It might be good to check what gdb does in these situations
> Oh yeah, this is a path so of course that will happen. (we probably should 
> update ds2 for that as well).
gdb manually escapes < > " and & into their respective &code; but doesn't seem 
to handle characters like \r, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502



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

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

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

aadsm updated this revision to Diff 203083.
aadsm added a comment.

- Improved the tests by using a binary that is specificly linked to other 
shared libs.
- USe libxml2 to generate the name attribute (I've talked with Greg about this 
and he recommended to create a new class for this)
- Address other comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502

Files:
  lldb/include/lldb/Host/XML.h
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/Makefile
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/main.cpp
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_a.cpp
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_a.mk
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_b_quote.cpp
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_b_quote.mk
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

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
@@ -2765,6 +2765,27 @@
 return std::move(*buffer_or_error);
   }
 
+  if (object == "libraries-svr4") {
+if (!XMLDocument::XMLEnabled())
+  return llvm::make_error(
+  "Xfer object not supported");
+auto library_list = m_debugged_process_up->GetLoadedSVR4Libraries();
+if (!library_list)
+  return library_list.takeError();
+
+StreamString response;
+response.Printf("");
+for (auto const &library : *library_list) {
+  response.Printf("", library.ld_addr);
+}
+response.Printf("");
+return MemoryBuffer::getMemBufferCopy(response.GetString(), __FUNCTION__);
+  }
+
   return llvm::make_error(
   "Xfer object not supported");
 }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -826,6 +826,9 @@
   response.PutCString(";QPassSignals+");
   response.PutCString(";qXfer:auxv:read+");
 #endif
+#if defined(__linux__)
+  response.PutCString(";qXfer:libraries-svr4:read+");
+#endif
 
   return SendPacketNoLock(response.GetString());
 }
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -79,6 +79,9 @@
 
   lldb::addr_t GetSharedLibraryInfoAddress() override;
 
+  llvm::Expected>
+  GetLoadedSVR4Libraries() override;
+
   size_t UpdateThreads() override;
 
   const ArchSpec &GetArchitecture() const override { return m_arch; }
@@ -135,6 +138,17 @@
   template 
   lldb::addr_t GetELFImageInfoAddress();
 
+  template  struct ELFLinkMap {
+T l_addr;
+T l_name;
+T l_ld;
+T l_next;
+T l_prev;
+  };
+  template 
+  Status ReadSVR4LibraryInfo(lldb::addr_t link_map_addr, SVR4LibraryInfo &info,
+ lldb::addr_t &next);
+
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -2176,3 +2176,74 @@
 
   return LLDB_INVALID_ADDRESS;
 }
+
+template 
+Status NativeProcessLinux::ReadSVR4LibraryInfo(lldb::addr_t link_map_addr,
+   SVR4LibraryInfo &info,
+   lldb::addr_t &next) {
+  ELFLinkMap link_map;
+  size_t bytes_read;
+  auto error =
+  ReadMemory(link_map_addr, &link_map, sizeof(link_map), bytes_read);
+  if (!error.Success())
+return error;
+
+  char name_buffer[PATH_MAX];
+  error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
+ bytes_read);
+  if (!error.Success())
+return error;
+  name_buffer[PATH_MAX - 1] = '\0';
+  info.name = std::string(name_buffer);
+ 

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

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

aadsm updated this revision to Diff 203084.
aadsm added a comment.

Make sure we always return a null terminated string


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62503

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp


Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -2189,11 +2189,12 @@
 return error;
 
   char name_buffer[PATH_MAX];
-  error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
- bytes_read);
+  error = ReadCStringFromMemory(link_map.l_name,
+reinterpret_cast(&name_buffer),
+sizeof(name_buffer), bytes_read);
   if (!error.Success())
 return error;
-  name_buffer[PATH_MAX - 1] = '\0';
+
   info.name = std::string(name_buffer);
   info.link_map = link_map_addr;
   info.base_addr = link_map.l_addr;
Index: lldb/source/Host/common/NativeProcessProtocol.cpp
===
--- lldb/source/Host/common/NativeProcessProtocol.cpp
+++ lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -16,6 +16,8 @@
 #include "lldb/Utility/State.h"
 #include "lldb/lldb-enumerations.h"
 
+#include "llvm/Support/Process.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -659,6 +661,51 @@
   return Status();
 }
 
+Status NativeProcessProtocol::ReadCStringFromMemory(lldb::addr_t addr,
+char *buffer,
+size_t max_size,
+size_t &total_bytes_read) {
+  const size_t cache_line_size = llvm::sys::Process::getPageSizeEstimate();
+  size_t bytes_read = 0;
+  size_t bytes_left = max_size;
+  addr_t curr_addr = addr;
+  char *curr_buffer = buffer;
+  total_bytes_read = 0;
+  Status error;
+
+  while (bytes_left > 0 && error.Success()) {
+addr_t cache_line_bytes_left =
+cache_line_size - (curr_addr % cache_line_size);
+addr_t bytes_to_read = std::min(bytes_left, cache_line_bytes_left);
+error = ReadMemory(curr_addr, reinterpret_cast(curr_buffer),
+   bytes_to_read, bytes_read);
+
+if (bytes_read == 0)
+  break;
+
+auto str_end = std::memchr(curr_buffer, '\0', bytes_read);
+if (str_end != NULL) {
+  total_bytes_read = (size_t)(reinterpret_cast(str_end) - buffer);
+  error.Clear();
+  break;
+}
+
+total_bytes_read += bytes_read;
+curr_buffer += bytes_read;
+curr_addr = reinterpret_cast(reinterpret_cast(curr_addr) +
+ bytes_read);
+bytes_left -= bytes_read;
+  }
+
+  // Make sure we return a null terminated string.
+  if (bytes_left == 0 && buffer[max_size - 1] != '\0') {
+buffer[max_size - 1] = '\0';
+total_bytes_read--;
+  }
+
+  return error;
+}
+
 lldb::StateType NativeProcessProtocol::GetState() const {
   std::lock_guard guard(m_state_mutex);
   return m_state;
Index: lldb/include/lldb/Host/common/NativeProcessProtocol.h
===
--- lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -83,6 +83,9 @@
   Status ReadMemoryWithoutTrap(lldb::addr_t addr, void *buf, size_t size,
size_t &bytes_read);
 
+  Status ReadCStringFromMemory(lldb::addr_t addr, char *buffer, size_t 
max_size,
+   size_t &total_bytes_read);
+
   virtual Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size,
  size_t &bytes_written) = 0;
 


Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -2189,11 +2189,12 @@
 return error;
 
   char name_buffer[PATH_MAX];
-  error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
- bytes_read);
+  error = ReadCStringFromMemory(link_map.l_name,
+reinterpret_cast(&name_buffer),
+sizeof(name_buffer), bytes_read);
   if (!error.Success())
 return error;
-  name_buffer[PATH_MAX - 1] = '\0';
+
   info.name = std::string(name_buffer);
   info.link_map = link_map_addr;
   info.base_addr = link_map.l_addr;
Index: lldb/source/Host/common/NativeProcessProtocol.cpp
===
--- lldb/

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Using the libxml function for the escaping is fine, but there are a couple of 
caveats. I believe people expect to be able to build lldb without libxml. At 
least that what all the existing xml-related code assumes (you'll see it has 
some #ifdefs and stuff). If we don't intend to change that, then we'll need to 
do something similar here. It's fine if some features (like this one) don't 
work without libxml, but they should degrade gracefully.

The svr reading support in the client depends on having libxml around, so 
depending on it here would be consistent. OTOH, lldb-server is used also in 
embedded(ish) environments where libxml is hard to come by (or just big), so it 
may make sense to try to avoid it here (particularly as you're still 
constructing the xml elements by hand).

So overall, it's up to you how you want to proceed here. However, if you do use 
libxml, I think you'll need to do something like:

- make sure the changes in XML.h compile without libxml (move stuff into an 
ifdef, provide a no-op implementation for the other case, and similar)
- only say we support the svr extension if `XMLDocument::XMLEnabled()` is true
- guard the tests with `@skipIfXmlSupportMissing`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502



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