[Lldb-commits] [PATCH] D63791: [lldb] [Process/NetBSD] Fix segfault when handling watchpoint

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

mgorny added a comment.

Ping.


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

https://reviews.llvm.org/D63791



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

[Lldb-commits] [PATCH] D63792: [lldb] [Process/NetBSD] Use global enable bits for watchpoints

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

mgorny added a comment.

Ping.


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

https://reviews.llvm.org/D63792



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

[Lldb-commits] [PATCH] D63545: [lldb] [Process/NetBSD] Support reading YMM registers via PT_*XSTATE

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

mgorny added a comment.

Ping.


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

https://reviews.llvm.org/D63545



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

[Lldb-commits] [PATCH] D61233: Refactor ObjectFile::GetSDKVersion

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

looks good to me too...


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

https://reviews.llvm.org/D61233



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

[Lldb-commits] [PATCH] D63853: [Symbol] Add DeclVendor::FindTypes

[Pavel Labath via Phabricator via lldb-commits]

labath resigned from this revision.
labath added a comment.

Besides picking at code style details, I don't believe I have anything useful 
to add here. :)




Comment at: source/Plugins/Language/ObjC/ObjCLanguage.cpp:939-941
+auto types =
+decl_vendor->FindTypes(name, /*max_matches*/ UINT32_MAX);
+for (auto type : types) {

I don't think using auto is helpful here -- we have many classes to represent 
"types", and it's not clear that this is talking about CompilerType. If you 
want to shorten this, I'd suggest folding the FindTypes call into the for loop:
`for (const CompilerType &type : decl_vendor->FindTypes(...))`.


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

https://reviews.llvm.org/D63853



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

Re: [Lldb-commits] [lldb] r364494 - [Reproducers] Fix flakiness and off-by-one during replay.

[Pavel Labath via lldb-commits]

Do we have a test which records us interrupting a process, and then 
makes sure this replays correctly ?




On 27/06/2019 04:03, Jonas Devlieghere via lldb-commits wrote:

Author: jdevlieghere
Date: Wed Jun 26 19:03:34 2019
New Revision: 364494

URL: http://llvm.org/viewvc/llvm-project?rev=364494&view=rev
Log:
[Reproducers] Fix flakiness and off-by-one during replay.

This fixes two replay issues that caused the tests to behave
erratically:

1. It fixes an off-by-one error, where all replies where shifted by 1
because of a `+` packet that should've been ignored.

2. It fixes another off-by-one-error, where an asynchronous ^C was
offsetting all subsequent packets. The reason is that we
'synchronize' requests and replies. In reality, a stop reply is only
sent when the process halt. During replay however, we instantly
report the stop, as the reply to packets like continue (vCont).

Both packets should be ignored, and indeed, checking the gdb-remote log,
no unexpected packets are received anymore.

Additionally, be more pedantic when it comes to unexpected packets and
return an failure form the replay server. This way we should be able to
catch these things faster in the future.

Modified:
 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp?rev=364494&r1=364493&r2=364494&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 Wed Jun 26 19:03:34 2019
@@ -30,6 +30,7 @@ using namespace lldb;
  using namespace lldb_private;
  using namespace lldb_private::process_gdb_remote;
  
+/// Check if the given expected packet matches the actual packet.

  static bool unexpected(llvm::StringRef expected, llvm::StringRef actual) {
// The 'expected' string contains the raw data, including the leading $ and
// trailing checksum. The 'actual' string contains only the packet's 
content.
@@ -48,6 +49,25 @@ static bool unexpected(llvm::StringRef e
return true;
  }
  
+/// Check if we should reply to the given packet.

+static bool skip(llvm::StringRef data) {
+  assert(!data.empty() && "Empty packet?");
+
+  // We've already acknowledge the '+' packet so we're done here.
+  if (data == "+")
+return true;
+
+  /// Don't 't reply to ^C. We need this because of stop reply packets, which
+  /// are only returned when the target halts. Reproducers synchronize these
+  /// 'asynchronous' replies, by recording them as a regular replies to the
+  /// previous packet (e.g. vCont). As a result, we should ignore real
+  /// asynchronous requests.
+  if (data.data()[0] == 0x03)
+return true;
+
+  return false;
+}
+
  GDBRemoteCommunicationReplayServer::GDBRemoteCommunicationReplayServer()
  : GDBRemoteCommunication("gdb-replay", "gdb-replay.rx_packet"),
m_async_broadcaster(nullptr, "lldb.gdb-replay.async-broadcaster"),
@@ -89,9 +109,8 @@ GDBRemoteCommunicationReplayServer::GetP
  
m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue);
  
-  // If m_send_acks is true, we're before the handshake phase. We've already

-  // acknowledge the '+' packet so we're done here.
-  if (m_send_acks && packet.GetStringRef() == "+")
+  // Check if we should reply to this packet.
+  if (skip(packet.GetStringRef()))
  return PacketResult::Success;
  
// This completes the handshake. Since m_send_acks was true, we can unset it

@@ -102,9 +121,8 @@ GDBRemoteCommunicationReplayServer::GetP
// A QEnvironment packet is sent for every environment variable. If the
// number of environment variables is different during replay, the replies
// become out of sync.
-  if (packet.GetStringRef().find("QEnvironment") == 0) {
+  if (packet.GetStringRef().find("QEnvironment") == 0)
  return SendRawPacketNoLock("$OK#9a");
-  }
  
Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));

while (!m_packet_history.empty()) {
@@ -112,7 +130,7 @@ GDBRemoteCommunicationReplayServer::GetP
  GDBRemoteCommunicationHistory::Entry entry = m_packet_history.back();
  m_packet_history.pop_back();
  
-// We're handled the handshake implicitly before. Skip the packet and move

+// We've handled the handshake implicitly before. Skip the packet and move
  // on.
  if (entry.packet.data == "+")
continue;
@@ -124,6 +142,7 @@ GDBRemoteCommunicationReplayServer::GetP
   entry.packet.data);
  LLDB_LOG(log, "GDBRemoteCommunicationReplayServer actual packet: 
'{0}'",
   packet.GetStringRef());
+return PacketResult::ErrorSendFailed;
 

[Lldb-commits] [PATCH] D63868: Unify+fix remote XML libraries handling with the legacy one

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

@jankratochvil wrote:

> That is because it fixed failing ReadMemory (due to PAGE_SIZE size reading 
> shared library name overlapping to a next page which is accidentally unmapped)


Does this mean that there is a bug in lldb-server, where some memory read 
requests fail if they span an unreadable page. Can this also be triggered by a 
$m packet? Would you be interested in distilling that into a test?

In D63868#1560990 , @jankratochvil 
wrote:

> In D63868#1560872 , @aadsm wrote:
>
> > I don't think we can stop loading/unloading the libraries in 
> > `ProcessGDBRemote::LoadModules`. The windows dynamic loader relies on this
>
>
> Then it could be Windows-specific.


I also think it would be good to move this stuff into the windows loader. Right 
now we have a lot of confusion about who is responsible for "loading" modules 
into a target. Some processes (I think mostly post-mortem plugins) do the 
loading themselves. Others create dynamic loader plugins to do that. And in the 
case of svr4, we have ProcessGDBRemote creating a DynamicLoader plugin, which 
then calls back into the process to do the actual loading. That seems too 
convoluted to me...

I think it would be better to do the loading inside the DynamicLoader plugin, 
and have `ProcessGDBRemote::LoadModules` be responsible only for sending the 
appropriate qXfer packet and parsing the response. The fact that loading the 
libraries from a `LoadedModuleInfoList` is largely similar for posix&windows 
can be handled (at a later date) by factoring the common code into a utility 
function, a common base class or something else...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63868



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

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

[Pavel Labath via Phabricator via lldb-commits]

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

Looks good to me, but I'd suggest waiting a while to give @clayborg a chance to 
respond...




Comment at: lldb/lit/SymbolFile/sizeless-symbol.test:4
+
+image lookup --address 1
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful

jankratochvil wrote:
> labath wrote:
> > As you're fixing this bug by changing the way symbol size is computed, it 
> > would be nice to also put (and check it's output) a "image dump symtab" 
> > command here. That would also make it clear what are the assumptions the 
> > following "lookup" commands are operating under.
> I only hope the `image dump symtab` output ordering is stable.
I'm pretty sure these are printed in the same order as present in .symtab. I'd 
be more worried about the order in which the symbols get emitted into the 
symtab in the first place. Theoretically this might change due to some patches 
to the assembler, but it should not change spuriously...



Comment at: lldb/lit/SymbolFile/sizeless-symbol.test:10
+image dump symtab
+# CHECK:Index   UserID DSX TypeFile Address/Value Load Address 
  Size   Flags  Name
+# CHECK-NEXT:--- -- --- --- -- 
-- -- -- 
--

Nit: Please line up this table by inserting a couple of spaces between ":" and 
"Index".



Comment at: lldb/source/Symbol/Symtab.cpp:904
+// same address. So do not set size of a non-last zero-sized symbol.
+if (i == num_entries - 1 || 
m_file_addr_to_index.GetMutableEntryAtIndex(i + 1)->GetRangeBase() != 
curr_base_addr) {
   const RangeVector::Entry *containing_section =

Please clang-format this.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63540



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

[Lldb-commits] [PATCH] D63868: Unify+fix remote XML libraries handling with the legacy one

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D63868#1563802 , @labath wrote:

> Does this mean that there is a bug in lldb-server, where some memory read 
> requests fail if they span an unreadable page. Can this also be triggered by 
> a $m packet? Would you be interested in distilling that into a test?


OK, I will try to reproduce it - you are right GDB documentation 
 says 
for the `m` packet: "//The reply may contain fewer addressable memory units 
than requested if the server was able to read only part of the region of 
memory. //"

And LLDB is using `ReadMemory` in 
`NativeProcessProtocol::ReadMemoryWithoutTrap` from 
`GDBRemoteCommunicationServerLLGS::Handle_memory_read`.

>   I think it would be better to do the loading inside the DynamicLoader 
> plugin, and have `ProcessGDBRemote::LoadModules` be responsible only for 
> sending the appropriate qXfer packet and parsing the response. The fact that 
> loading the libraries from a `LoadedModuleInfoList` is largely similar for 
> posix&windows can be handled (at a later date) by factoring the common code 
> into a utility function, a common base class or something else...

IIUC you believe this patch should be more refactored? I do not have much 
overview of this code of LLDB (+Windows libraries in general). IIUC you propose:

- The current libraries loading code from `LoadedModuleInfoList` of 
`ProcessGDBRemote::LoadModules` moved to `lldb/source/Plugins/Platform/Windows/`
- later: `DynamicLoaderPOSIXDYLD::RefreshModules` and 
`DynamicLoaderPOSIXDYLD::LoadAllCurrentModules` somehow reusing that code 
(after it is moved back to some common library)? But that looks to me as too 
little code to make it worth it.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63868



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

[Lldb-commits] [PATCH] D63545: [lldb] [Process/NetBSD] Support reading YMM registers via PT_*XSTATE

[Pavel Labath via Phabricator via lldb-commits]

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

I don't know whether the code is correct for NetBSD, but it looks ok from a 
general readability perspective.


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

https://reviews.llvm.org/D63545



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

[Lldb-commits] [PATCH] D63792: [lldb] [Process/NetBSD] Use global enable bits for watchpoints

[Pavel Labath via Phabricator via lldb-commits]

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

Sorry about the delay, I was OOO last week. BTW, the usual rate of pinging on a 
patch is one week https://llvm.org/docs/DeveloperPolicy.html#code-reviews.

If the NetBSD kernel really sets the global-enable flag in the dr7 register, 
I'd be tempted to try to make it crash/misbehave by setting a hardware 
breakpoint somewhere in the early entry code (before it gets a chance to clear 
it, if it even does that), ideally in the breakpoint handler so it triggers an 
infinite loop :). However, that is not relevant to this patch...




Comment at: 
lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp:733
   // for watchpoints 0, 1, 2, or 3, respectively, set bits 1, 3, 5, or 7
-  uint64_t enable_bit = 1 << (2 * wp_index);
+  uint64_t enable_bit = 2 << (2 * wp_index);
 

Not a big deal, but I'd find `1 << (2*wp_index + 1)` less surprising..


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

https://reviews.llvm.org/D63792



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

[Lldb-commits] [PATCH] D63791: [lldb] [Process/NetBSD] Fix segfault when handling watchpoint

[Pavel Labath via Phabricator via lldb-commits]

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

LGTM. I don't know what Kamil is up to these days. It might be better to have 
him review patches like this, but if he's unable to do that, I'm happy to stamp 
them too...


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

https://reviews.llvm.org/D63791



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

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil marked 5 inline comments as done.
jankratochvil added inline comments.



Comment at: lldb/lit/SymbolFile/sizeless-symbol.test:10
+image dump symtab
+# CHECK:Index   UserID DSX TypeFile Address/Value Load Address 
  Size   Flags  Name
+# CHECK-NEXT:--- -- --- --- -- 
-- -- -- 
--

labath wrote:
> Nit: Please line up this table by inserting a couple of spaces between ":" 
> and "Index".
Great, I did not expect it will still match with the added whitespaces.



Comment at: lldb/source/Symbol/Symtab.cpp:904
+// same address. So do not set size of a non-last zero-sized symbol.
+if (i == num_entries - 1 || 
m_file_addr_to_index.GetMutableEntryAtIndex(i + 1)->GetRangeBase() != 
curr_base_addr) {
   const RangeVector::Entry *containing_section =

labath wrote:
> Please clang-format this.
Sorry I forgot to run the clang-format.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63540



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

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 207252.
jankratochvil marked 2 inline comments as done.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63540

Files:
  lldb/lit/SymbolFile/Inputs/sizeless-symbol.s
  lldb/lit/SymbolFile/sizeless-symbol.test
  lldb/source/Symbol/Symtab.cpp


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -896,8 +896,14 @@
   for (size_t i = 0; i < num_entries; i++) {
 FileRangeToIndexMap::Entry *entry =
 m_file_addr_to_index.GetMutableEntryAtIndex(i);
-if (entry->GetByteSize() == 0) {
-  addr_t curr_base_addr = entry->GetRangeBase();
+if (entry->GetByteSize() > 0)
+  continue;
+addr_t curr_base_addr = entry->GetRangeBase();
+// Symbols with non-zero size will show after zero-sized symbols on the
+// same address. So do not set size of a non-last zero-sized symbol.
+if (i == num_entries - 1 ||
+m_file_addr_to_index.GetMutableEntryAtIndex(i + 1)
+->GetRangeBase() != curr_base_addr) {
   const RangeVector::Entry *containing_section =
   section_ranges.FindEntryThatContains(curr_base_addr);
 
Index: lldb/lit/SymbolFile/sizeless-symbol.test
===
--- /dev/null
+++ lldb/lit/SymbolFile/sizeless-symbol.test
@@ -0,0 +1,14 @@
+# Some targets do not have the .size directive.
+# RUN: %clang -target x86_64-unknown-unknown-elf %S/Inputs/sizeless-symbol.s 
-c -o %t.o
+# RUN: %lldb %t.o -s %s -o quit | FileCheck %s
+
+image lookup --address 1
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful
+image lookup --address 2
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful + 1
+image dump symtab
+# CHECK: Index   UserID DSX TypeFile Address/Value Load 
Address   Size   Flags  Name
+# CHECK-NEXT:--- -- --- --- -- 
-- -- -- 
--
+# CHECK-NEXT:[0]  1 Code0x0003 
   0x 0x sizeend
+# CHECK-NEXT:[1]  2 Code0x0001 
   0x0002 0x sizeful
+# CHECK-NEXT:[2]  3 Code0x0001 
   0x 0x sizeless
Index: lldb/lit/SymbolFile/Inputs/sizeless-symbol.s
===
--- /dev/null
+++ lldb/lit/SymbolFile/Inputs/sizeless-symbol.s
@@ -0,0 +1,8 @@
+.text
+.byte   0
+sizeless:
+sizeful:
+.byte   0
+.byte   0
+sizeend:
+.size   sizeful, sizeend - sizeful


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -896,8 +896,14 @@
   for (size_t i = 0; i < num_entries; i++) {
 FileRangeToIndexMap::Entry *entry =
 m_file_addr_to_index.GetMutableEntryAtIndex(i);
-if (entry->GetByteSize() == 0) {
-  addr_t curr_base_addr = entry->GetRangeBase();
+if (entry->GetByteSize() > 0)
+  continue;
+addr_t curr_base_addr = entry->GetRangeBase();
+// Symbols with non-zero size will show after zero-sized symbols on the
+// same address. So do not set size of a non-last zero-sized symbol.
+if (i == num_entries - 1 ||
+m_file_addr_to_index.GetMutableEntryAtIndex(i + 1)
+->GetRangeBase() != curr_base_addr) {
   const RangeVector::Entry *containing_section =
   section_ranges.FindEntryThatContains(curr_base_addr);
 
Index: lldb/lit/SymbolFile/sizeless-symbol.test
===
--- /dev/null
+++ lldb/lit/SymbolFile/sizeless-symbol.test
@@ -0,0 +1,14 @@
+# Some targets do not have the .size directive.
+# RUN: %clang -target x86_64-unknown-unknown-elf %S/Inputs/sizeless-symbol.s -c -o %t.o
+# RUN: %lldb %t.o -s %s -o quit | FileCheck %s
+
+image lookup --address 1
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful
+image lookup --address 2
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful + 1
+image dump symtab
+# CHECK: Index   UserID DSX TypeFile Address/Value Load Address   Size   Flags  Name
+# CHECK-NEXT:--- -- --- --- -- -- -- -- --
+# CHECK-NEXT:[0]  1 Code0x00030x 0x sizeend
+# CHECK-NEXT:[1]  2 Code0x0001 

[Lldb-commits] [PATCH] D63792: [lldb] [Process/NetBSD] Use global enable bits for watchpoints

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

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

That's probably one of the reasons why NetBSD normally prevents unprivileged 
users from setting DRs.




Comment at: 
lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp:733
   // for watchpoints 0, 1, 2, or 3, respectively, set bits 1, 3, 5, or 7
-  uint64_t enable_bit = 1 << (2 * wp_index);
+  uint64_t enable_bit = 2 << (2 * wp_index);
 

labath wrote:
> Not a big deal, but I'd find `1 << (2*wp_index + 1)` less surprising..
Will do.


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

https://reviews.llvm.org/D63792



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

[Lldb-commits] [PATCH] D63791: [lldb] [Process/NetBSD] Fix segfault when handling watchpoint

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

mgorny added a comment.

I'm going to ask Kamil to stamp those patches anyway but I always appreciate 
your advice wrt LLDB coding style and general integration. After all, we all 
want LLDB codebase to be more unified.


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

https://reviews.llvm.org/D63791



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

[Lldb-commits] [PATCH] D63868: Unify+fix remote XML libraries handling with the legacy one

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D63868#1563873 , @jankratochvil 
wrote:

> In D63868#1563802 , @labath wrote:
>
> > Does this mean that there is a bug in lldb-server, where some memory read 
> > requests fail if they span an unreadable page. Can this also be triggered 
> > by a $m packet? Would you be interested in distilling that into a test?
>
>
> OK, I will try to reproduce it - you are right GDB documentation 
>  says 
> for the `m` packet: "//The reply may contain fewer addressable memory units 
> than requested if the server was able to read only part of the region of 
> memory. //"
>
> And LLDB is using `ReadMemory` in 
> `NativeProcessProtocol::ReadMemoryWithoutTrap` from 
> `GDBRemoteCommunicationServerLLGS::Handle_memory_read`.


Thanks. To deterministically reproduce an unmapped page of memory, the best 
approach I can think of is to first mmap a larger block of memory, and then 
munmap a part of it.

> 
> 
>>   I think it would be better to do the loading inside the DynamicLoader 
>> plugin, and have `ProcessGDBRemote::LoadModules` be responsible only for 
>> sending the appropriate qXfer packet and parsing the response. The fact that 
>> loading the libraries from a `LoadedModuleInfoList` is largely similar for 
>> posix&windows can be handled (at a later date) by factoring the common code 
>> into a utility function, a common base class or something else...
> 
> IIUC you believe this patch should be more refactored? I do not have much 
> overview of this code of LLDB (+Windows libraries in general). IIUC you 
> propose:
> 
> - The current libraries loading code from `LoadedModuleInfoList` of 
> `ProcessGDBRemote::LoadModules` moved to 
> `lldb/source/Plugins/Platform/Windows/`

Yes, that's more or less it. Though I am open to being convinced otherwise...

> - later: `DynamicLoaderPOSIXDYLD::RefreshModules` and 
> `DynamicLoaderPOSIXDYLD::LoadAllCurrentModules` somehow reusing that code 
> (after it is moved back to some common library)? But that looks to me as too 
> little code to make it worth it.

Possibly. If the same code is useful for both windows and posix, then it might 
make sense to put it into the base DynamicLoader class, which means we wouldn't 
have to create new plugins or whatnot. Or we might actually want to create a 
DynamicLoaderSVR4, which only works with SVR4, and have these delegate to it. 
But that's a discussion for another day...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63868



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

[Lldb-commits] [PATCH] D63792: [lldb] [Process/NetBSD] Use global enable bits for watchpoints

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

mgorny updated this revision to Diff 207255.
mgorny added a comment.

Updated per review.


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

https://reviews.llvm.org/D63792

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchlocation/TestWatchLocation.py
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchpoint/TestMyFirstWatchpoint.py
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_hits/TestMultipleHits.py
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_threads/TestWatchpointMultipleThreads.py
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/TestWatchpointCommands.py
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/condition/TestWatchpointConditionCmd.py
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/TestWatchpointDisable.py
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
  
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -691,7 +691,7 @@
 
   uint64_t control_bits = reg_value.GetAsUInt64();
 
-  is_vacant = !(control_bits & (1 << (2 * wp_index)));
+  is_vacant = !(control_bits & (1 << (2 * wp_index + 1)));
 
   return error;
 }
@@ -730,7 +730,7 @@
 return error;
 
   // for watchpoints 0, 1, 2, or 3, respectively, set bits 1, 3, 5, or 7
-  uint64_t enable_bit = 1 << (2 * wp_index);
+  uint64_t enable_bit = 1 << (2 * wp_index + 1);
 
   // set bits 16-17, 20-21, 24-25, or 28-29
   // with 0b01 for write, and 0b11 for read/write
Index: lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py
@@ -36,7 +36,6 @@
 bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
 # Read-write watchpoints not supported on SystemZ
 @expectedFailureAll(archs=['s390x'])
-@expectedFailureNetBSD
 def test_byte_size_watchpoints_with_byte_selection(self):
 """Test to selectively watch different bytes in a 8-byte array."""
 self.run_watchpoint_size_test('byteArray', 8, '1')
@@ -46,7 +45,6 @@
 bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
 # Read-write watchpoints not supported on SystemZ
 @expectedFailureAll(archs=['s390x'])
-@expectedFailureNetBSD
 def test_two_byte_watchpoints_with_word_selection(self):
 """Test to selectively watch different words in an 8-byte word array."""
 self.run_watchpoint_size_test('wordArray', 4, '2')
@@ -56,7 +54,6 @@
 bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
 # Read-write watchpoints not supported on SystemZ
 @expectedFailureAll(archs=['s390x'])
-@expectedFailureNetBSD
 def test_four_byte_watchpoints_with_dword_selection(self):
 """Test to selectively watch two double words in an 8-byte dword array."""
 self.run_watchpoint_size_test('dwordArray', 2, '4')
Index: lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
@@ -39,7 +39,6 @@
 @expectedFailureAll(
 oslist=["windows"],
 bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
-@expectedFailureNetBSD
 def test_watchlocation_using_watchpoint_set(self):
 """Test watching a location with 'watchpoint set expression -w write -s size' option."""

[Lldb-commits] [lldb] r364744 - Remove null checks of results of new expressions

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Jul  1 04:09:15 2019
New Revision: 364744

URL: http://llvm.org/viewvc/llvm-project?rev=364744&view=rev
Log:
Remove null checks of results of new expressions

operator new doesn't return a null pointer, even if one turns off
exceptions (it calls std::terminate instead). Therefore, all of this is
dead code.

Modified:
lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp

lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm.cpp

lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp

lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_mips64.cpp

lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_powerpc.cpp

lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp

lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_i386.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_x86_64.cpp

Modified: lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp?rev=364744&r1=364743&r2=364744&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp Mon Jul  1 
04:09:15 2019
@@ -246,14 +246,12 @@ size_t ObjectFileJIT::ReadSectionData(
   if (section->GetFileSize()) {
 const void *src = (void *)(uintptr_t)section->GetFileOffset();
 
-DataBufferSP data_sp(
-new lldb_private::DataBufferHeap(src, section->GetFileSize()));
-if (data_sp) {
-  section_data.SetData(data_sp, 0, data_sp->GetByteSize());
-  section_data.SetByteOrder(GetByteOrder());
-  section_data.SetAddressByteSize(GetAddressByteSize());
-  return section_data.GetByteSize();
-}
+DataBufferSP data_sp =
+std::make_shared(src, section->GetFileSize());
+section_data.SetData(data_sp, 0, data_sp->GetByteSize());
+section_data.SetByteOrder(GetByteOrder());
+section_data.SetAddressByteSize(GetAddressByteSize());
+return section_data.GetByteSize();
   }
   section_data.Clear();
   return 0;

Modified: 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm.cpp?rev=364744&r1=364743&r2=364744&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm.cpp
 Mon Jul  1 04:09:15 2019
@@ -155,7 +155,7 @@ bool RegisterContextPOSIXProcessMonitor_
 DataBufferSP &data_sp) {
   bool success = false;
   data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
-  if (data_sp && ReadGPR() && ReadFPR()) {
+  if (ReadGPR() && ReadFPR()) {
 uint8_t *dst = data_sp->GetBytes();
 success = dst != 0;
 

Modified: 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp?rev=364744&r1=364743&r2=364744&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp
 Mon Jul  1 04:09:15 2019
@@ -164,7 +164,7 @@ bool RegisterContextPOSIXProcessMonitor_
 lldb::DataBufferSP &data_sp) {
   bool success = false;
   data_sp.reset(new lldb_private::DataBufferHeap(REG_CONTEXT_SIZE, 0));
-  if (data_sp && ReadGPR() && ReadFPR()) {
+  if (ReadGPR() && ReadFPR()) {
 uint8_t *dst = data_sp->GetBytes();
 success = dst != 0;
 

Modified: 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_mips64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_mips64.cpp?rev=364744&r1=364743&r2=364744&view=diff
=

[Lldb-commits] [lldb] r364748 - Fix TestGdbRemoteLibrariesSvr4Support

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Jul  1 05:00:25 2019
New Revision: 364748

URL: http://llvm.org/viewvc/llvm-project?rev=364748&view=rev
Log:
Fix TestGdbRemoteLibrariesSvr4Support

D62502 had a bug (visible only with D62503 reverted), where it would
error out if attempting to read a string from memory and the memory page
after the string happened to be unmapped.

This fixes the problem by checking for whether ReadMemory read *any*
bytes, instead of checking whether it returned an error. A greater
question is whether ReadMemory should even return an error if it read at
least one byte, but I'm leaving that for a separate patch.

Modified:
lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp

Modified: lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp?rev=364748&r1=364747&r2=364748&view=diff
==
--- lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp (original)
+++ lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp Mon Jul  1 
05:00:25 2019
@@ -120,7 +120,7 @@ NativeProcessELF::ReadSVR4LibraryInfo(ll
   char name_buffer[PATH_MAX];
   error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
  bytes_read);
-  if (!error.Success())
+  if (bytes_read == 0)
 return error.ToError();
   name_buffer[PATH_MAX - 1] = '\0';
 
@@ -176,4 +176,4 @@ NativeProcessELF::GetLoadedSVR4Libraries
   return library_list;
 }
 
-} // namespace lldb_private
\ No newline at end of file
+} // namespace lldb_private


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

[Lldb-commits] [lldb] r364751 - Revert "Implement xfer:libraries-svr4:read packet"

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Jul  1 05:41:20 2019
New Revision: 364751

URL: http://llvm.org/viewvc/llvm-project?rev=364751&view=rev
Log:
Revert "Implement xfer:libraries-svr4:read packet"

D62502, together with D62503 have broken the builds which have XML
support enabled. Reverting D62503 (r364355) fixed that, but has broken
has left some of the tests introduced by D62502 broken more or less
nondeternimistically (it depended on whether the system happens to place
the library list near unreadable pages of memory). I attempted to make a
partial fix for this in r364748, but Jan Kratochvil pointed out that
this reintroduces the problem which reverting D62503 was trying to
solve.

So instead, I back out the whole thing so we can get back to a clean
slate that works for everyone. We can figure out a way forward from
there.

This reverts r364748, r363772 and r363707.

Removed:

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/Makefile

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/main.cpp

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_a.cpp

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_a.mk

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_b_quote.cpp

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_b_quote.mk
Modified:
lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp
lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.h

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Modified: lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h?rev=364751&r1=364750&r2=364751&view=diff
==
--- lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h Mon Jul  1 
05:41:20 2019
@@ -32,14 +32,6 @@ namespace lldb_private {
 class MemoryRegionInfo;
 class ResumeActionList;
 
-struct SVR4LibraryInfo {
-  std::string name;
-  lldb::addr_t link_map;
-  lldb::addr_t base_addr;
-  lldb::addr_t ld_addr;
-  lldb::addr_t next;
-};
-
 // NativeProcessProtocol
 class NativeProcessProtocol {
 public:
@@ -94,12 +86,6 @@ public:
 
   virtual lldb::addr_t GetSharedLibraryInfoAddress() = 0;
 
-  virtual llvm::Expected>
-  GetLoadedSVR4Libraries() {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Not implemented");
-  }
-
   virtual bool IsAlive() const;
 
   virtual size_t UpdateThreads() = 0;

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py?rev=364751&r1=364750&r2=364751&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
 Mon Jul  1 05:41:20 2019
@@ -513,8 +513,7 @@ class GdbRemoteTestCaseBase(TestBase):
 self,
 inferior_args=None,
 inferior_sleep_seconds=3,
-inferior_exe_path=None,
-inferior_env=None):
+inferior_exe_path=None):
 """Prep the debug monitor, the inferior, and the expected packet 
stream.
 
 Handle the separate cases of using the debug monitor in 
attach-to-inferior mode
@@ -577,9 +576,6 @@ class GdbRemoteTestCaseBase(TestBase):
 
 # Build the expected protocol stream
 self.add_no_ack_remote_stream()
-if inferior_env:
-for name, value in inferior_env.items():
-self.add_set_environment_packets(name, value)
 if self._inferior_startup == self._STARTUP_LAUNCH:
 self.add_verified_launch_packets(launch_args)
 
@@ -660,12 +656,6 @@ class GdbRemoteTestCaseBase(TestBase):
  {"direction": "send", "regex": r"^\$(.+)#[0-9a-fA-F]{2}$", 
"capture": {1: "process_info_raw"}}],
 True)
 
-def add_set_environment_packets(self

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I've also reverted the preceeding patch in this series as reverting this one 
has caused one of the other tests to start failing (I've tried to make a more 
surgical fix first, but @jankratochvil pointed out that this basically 
reintroduced the problem that we were trying  to solve by reverting this).

If I understand correctly the problem correctly, the issue is that the svr4 
support on the client side is broken right now, and enabling svr4 on the server 
exposes that problem. If so, then it seems to me that the best course of action 
here is:

- introduce a setting which controls whether the client uses svr4 
(`plugin.process.gdb-remote.use.svr4` ?). Have it default to false
- reapply the preceeding patch with all fixes (i.e., revert rL364751 
)
- reapply this patch
- fix svr4 clientside (either D63868  or 
D62504 ). This patch can also flip the new 
setting to true, or it can be done as a followup

Originally, I was going to suggest hard-disabling the client support as step 1, 
but now I think a setting is a better option because:

- it will enable people to work around svr4 issues if they happen to encounter 
them in the future
- (my favourite) it will enable to test the non-svr4 code path without needing 
to rebuild with xml support disabled
- it will enable people to *enable* svr4 support if it happened to work for 
them (in conjunction with a custom server, I guess) while steps 2--4 are 
underway

WDYT?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62503



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

[Lldb-commits] [lldb] r364753 - @skipIfXmlSupportMissing TestRecognizeBreakpoint

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Jul  1 06:12:29 2019
New Revision: 364753

URL: http://llvm.org/viewvc/llvm-project?rev=364753&view=rev
Log:
@skipIfXmlSupportMissing TestRecognizeBreakpoint

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py?rev=364753&r1=364752&r2=364753&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py
 Mon Jul  1 06:12:29 2019
@@ -16,6 +16,7 @@ class TestRecognizeBreakpoint(GDBRemoteT
 we would be able to reconstruct it from the thread info, but not if the
 stub doesn't support it """
  
+@skipIfXmlSupportMissing
 def test(self):
 class MyResponder(MockGDBServerResponder):
 def __init__(self):


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

[Lldb-commits] [lldb] r364754 - Don't check the validity of newly contructed data buffers

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Jul  1 06:18:19 2019
New Revision: 364754

URL: http://llvm.org/viewvc/llvm-project?rev=364754&view=rev
Log:
Don't check the validity of newly contructed data buffers

A bunch of places were checking that DataBufferHeap::GetBytes returns a
non-null pointer right after constructing it. The only time when
GetBytes returns a null pointer is if it is empty (and I'm not sure that
even this is a good idea), but that is clearly not the case here, as the
buffer was constructed with a non-zero size just a couple of lines back.

Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp

lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

Modified: 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp?rev=364754&r1=364753&r2=364754&view=diff
==
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp 
Mon Jul  1 06:18:19 2019
@@ -285,13 +285,6 @@ Status NativeRegisterContextLinux_arm::R
 return error;
 
   uint8_t *dst = data_sp->GetBytes();
-  if (dst == nullptr) {
-error.SetErrorStringWithFormat("DataBufferHeap instance of size %" PRIu64
-   " returned a null pointer",
-   (uint64_t)REG_CONTEXT_SIZE);
-return error;
-  }
-
   ::memcpy(dst, &m_gpr_arm, GetGPRSize());
   dst += GetGPRSize();
   ::memcpy(dst, &m_fpr, sizeof(m_fpr));

Modified: 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp?rev=364754&r1=364753&r2=364754&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
Mon Jul  1 06:18:19 2019
@@ -286,13 +286,6 @@ Status NativeRegisterContextLinux_arm64:
 return error;
 
   uint8_t *dst = data_sp->GetBytes();
-  if (dst == nullptr) {
-error.SetErrorStringWithFormat("DataBufferHeap instance of size %" PRIu64
-   " returned a null pointer",
-   REG_CONTEXT_SIZE);
-return error;
-  }
-
   ::memcpy(dst, &m_gpr_arm64, GetGPRSize());
   dst += GetGPRSize();
   ::memcpy(dst, &m_fpr, sizeof(m_fpr));

Modified: 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp?rev=364754&r1=364753&r2=364754&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp 
Mon Jul  1 06:18:19 2019
@@ -394,13 +394,6 @@ Status NativeRegisterContextLinux_mips64
   }
 
   uint8_t *dst = data_sp->GetBytes();
-  if (dst == nullptr) {
-error.SetErrorStringWithFormat("DataBufferHeap instance of size %" PRIu64
-   " returned a null pointer",
-   REG_CONTEXT_SIZE);
-return error;
-  }
-
   ::memcpy(dst, &m_gpr, GetRegisterInfoInterface().GetGPRSize());
   dst += GetRegisterInfoInterface().GetGPRSize();
 

Modified: 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp?rev=364754&r1=364753&r2=364754&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp 
Mon Jul  1 06:18:19 2019
@@ -371,13 +371,6 @@ Status NativeRegisterContextLinux_ppc64l
 return error;
 
   uint8_t *dst = data_sp->GetBytes();
-  if (dst == nullptr) {
-error.SetErrorStringWithFormat("DataBufferHeap instance of size %" PRIu64
-   " returned a null pointer",
-   REG_CONTEXT_SIZE);
-return error;
-  }
-
   :

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.cpp:31-36
+  if (!data_sp) {
+error.SetErrorStringWithFormat(
+"failed to allocate DataBufferHeap instance of size %" PRIu64,
+data_size);
+return error;
+  }

asmith wrote:
> labath wrote:
> > `new` doesn't fail. This is dead code.
> The code is copied from/following the other targets. Maybe all the targets 
> could be cleaned up in another PR. For example Arm64,
> 
> NativeRegisterContextLinux_arm64::ReadAllRegisterValues()
>   data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
>   if (!data_sp)
Done in r364744.



Comment at: 
lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.cpp:52-57
+  if (dst == nullptr) {
+error.SetErrorStringWithFormat("DataBufferHeap instance of size %" PRIu64
+   " returned a null pointer",
+   data_size);
+return error;
+  }

asmith wrote:
> labath wrote:
> > This can't ever be true.
> Copied from Arm64
Done in r364754.


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

https://reviews.llvm.org/D63165



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think it will be pretty hard for a single person to gather all of these stats 
for all targets. So perhaps the best solution is to have the setting for the 
`g` packet, defaulting to true. After a while, if nobody notices a regression, 
we can just remove it...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62931



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

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added inline comments.



Comment at: lldb/lit/SymbolFile/sizeless-symbol.test:5
+
+image lookup --address 1
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful

Symbols are ordered in the same order as they appear in the symbol table. On 
Darwin we coalesce symbols, so not all symbol are always in the symbol table, 
but we don't do this for ELF.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63540



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

[Lldb-commits] [lldb] r364773 - Fix lookup of symbols at the same address with no size vs. size

[Jan Kratochvil via lldb-commits]

Author: jankratochvil
Date: Mon Jul  1 07:31:26 2019
New Revision: 364773

URL: http://llvm.org/viewvc/llvm-project?rev=364773&view=rev
Log:
Fix lookup of symbols at the same address with no size vs. size

This fixes a failing testcase on Fedora 30 x86_64 (regression Fedora 29->30):

PASS:
./bin/lldb 
./lldb-test-build.noindex/functionalities/unwind/noreturn/TestNoreturnUnwind.test_dwarf/a.out
 -o 'settings set symbols.enable-external-lookup false' -o r -o bt -o quit
  * frame #0: 0x77aa6e75 libc.so.6`__GI_raise + 325
frame #1: 0x77a91895 libc.so.6`__GI_abort + 295
frame #2: 0x00401140 a.out`func_c at main.c:12:2
frame #3: 0x0040113a a.out`func_b at main.c:18:2
frame #4: 0x00401134 a.out`func_a at main.c:26:2
frame #5: 0x0040112e a.out`main(argc=, 
argv=) at main.c:32:2
frame #6: 0x77a92f33 libc.so.6`__libc_start_main + 243
frame #7: 0x0040106e a.out`_start + 46

vs.

FAIL - unrecognized abort() function:
./bin/lldb 
./lldb-test-build.noindex/functionalities/unwind/noreturn/TestNoreturnUnwind.test_dwarf/a.out
 -o 'settings set symbols.enable-external-lookup false' -o r -o bt -o quit
  * frame #0: 0x77aa6e75 libc.so.6`.annobin_raise.c + 325
frame #1: 0x77a91895 libc.so.6`.annobin_loadmsgcat.c_end.unlikely + 
295
frame #2: 0x00401140 a.out`func_c at main.c:12:2
frame #3: 0x0040113a a.out`func_b at main.c:18:2
frame #4: 0x00401134 a.out`func_a at main.c:26:2
frame #5: 0x0040112e a.out`main(argc=, 
argv=) at main.c:32:2
frame #6: 0x77a92f33 libc.so.6`.annobin_libc_start.c + 243
frame #7: 0x0040106e a.out`.annobin_init.c.hot + 46

The extra ELF symbols are there due to Annobin (I did not investigate why this 
problem happened specifically since F-30 and not since F-28).
It is due to:

Symbol table '.dynsym' contains 2361 entries:
Valu e  Size Type   Bind   Vis Name
00022769   5 FUNC   LOCAL  DEFAULT _nl_load_domain.cold
0002276e   0 NOTYPE LOCAL  HIDDEN  .annobin_abort.c.unlikely
...
0002276e   0 NOTYPE LOCAL  HIDDEN  .annobin_loadmsgcat.c_end.unlikely
...
0002276e   0 NOTYPE LOCAL  HIDDEN  .annobin_textdomain.c_end.unlikely
0002276e 548 FUNC   GLOBAL DEFAULT abort
0002276e 548 FUNC   GLOBAL DEFAULT abort@@GLIBC_2.2.5
0002276e 548 FUNC   LOCAL  DEFAULT __GI_abort
00022992   0 NOTYPE LOCAL  HIDDEN  .annobin_abort.c_end.unlikely

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

Added:
lldb/trunk/lit/SymbolFile/Inputs/sizeless-symbol.s
lldb/trunk/lit/SymbolFile/sizeless-symbol.test
Modified:
lldb/trunk/source/Symbol/Symtab.cpp

Added: lldb/trunk/lit/SymbolFile/Inputs/sizeless-symbol.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/Inputs/sizeless-symbol.s?rev=364773&view=auto
==
--- lldb/trunk/lit/SymbolFile/Inputs/sizeless-symbol.s (added)
+++ lldb/trunk/lit/SymbolFile/Inputs/sizeless-symbol.s Mon Jul  1 07:31:26 2019
@@ -0,0 +1,8 @@
+.text
+.byte   0
+sizeless:
+sizeful:
+.byte   0
+.byte   0
+sizeend:
+.size   sizeful, sizeend - sizeful

Added: lldb/trunk/lit/SymbolFile/sizeless-symbol.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/sizeless-symbol.test?rev=364773&view=auto
==
--- lldb/trunk/lit/SymbolFile/sizeless-symbol.test (added)
+++ lldb/trunk/lit/SymbolFile/sizeless-symbol.test Mon Jul  1 07:31:26 2019
@@ -0,0 +1,14 @@
+# Some targets do not have the .size directive.
+# RUN: %clang -target x86_64-unknown-unknown-elf %S/Inputs/sizeless-symbol.s 
-c -o %t.o
+# RUN: %lldb %t.o -s %s -o quit | FileCheck %s
+
+image lookup --address 1
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful
+image lookup --address 2
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful + 1
+image dump symtab
+# CHECK: Index   UserID DSX TypeFile Address/Value Load 
Address   Size   Flags  Name
+# CHECK-NEXT:--- -- --- --- -- 
-- -- -- 
--
+# CHECK-NEXT:[0]  1 Code0x0003 
   0x 0x sizeend
+# CHECK-NEXT:[1]  2 Code0x0001 
   0x0002 0x sizeful
+# CHECK-NEXT:[2]  3 Code0x0001 
   0x 0x sizeless

Modified: lldb/trunk/source/Symbol/Symtab.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Symtab.cpp?rev=364773&r1=364772&r2=364773&view=diff
==
--- lldb/trunk/source/Symbol/S

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

[Jan Kratochvil via Phabricator via lldb-commits]

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



Comment at: lldb/lit/SymbolFile/sizeless-symbol.test:5
+
+image lookup --address 1
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful

clayborg wrote:
> Symbols are ordered in the same order as they appear in the symbol table. On 
> Darwin we coalesce symbols, so not all symbol are always in the symbol table, 
> but we don't do this for ELF.
OK, great; thanks for the review.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63540



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

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

[Jan Kratochvil via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
jankratochvil marked an inline comment as done.
Closed by commit rL364773: Fix lookup of symbols at the same address with no 
size vs. size (authored by jankratochvil, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63540?vs=207252&id=207304#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63540

Files:
  lldb/trunk/lit/SymbolFile/Inputs/sizeless-symbol.s
  lldb/trunk/lit/SymbolFile/sizeless-symbol.test
  lldb/trunk/source/Symbol/Symtab.cpp


Index: lldb/trunk/source/Symbol/Symtab.cpp
===
--- lldb/trunk/source/Symbol/Symtab.cpp
+++ lldb/trunk/source/Symbol/Symtab.cpp
@@ -896,8 +896,14 @@
   for (size_t i = 0; i < num_entries; i++) {
 FileRangeToIndexMap::Entry *entry =
 m_file_addr_to_index.GetMutableEntryAtIndex(i);
-if (entry->GetByteSize() == 0) {
-  addr_t curr_base_addr = entry->GetRangeBase();
+if (entry->GetByteSize() > 0)
+  continue;
+addr_t curr_base_addr = entry->GetRangeBase();
+// Symbols with non-zero size will show after zero-sized symbols on the
+// same address. So do not set size of a non-last zero-sized symbol.
+if (i == num_entries - 1 ||
+m_file_addr_to_index.GetMutableEntryAtIndex(i + 1)
+->GetRangeBase() != curr_base_addr) {
   const RangeVector::Entry *containing_section =
   section_ranges.FindEntryThatContains(curr_base_addr);
 
Index: lldb/trunk/lit/SymbolFile/sizeless-symbol.test
===
--- lldb/trunk/lit/SymbolFile/sizeless-symbol.test
+++ lldb/trunk/lit/SymbolFile/sizeless-symbol.test
@@ -0,0 +1,14 @@
+# Some targets do not have the .size directive.
+# RUN: %clang -target x86_64-unknown-unknown-elf %S/Inputs/sizeless-symbol.s 
-c -o %t.o
+# RUN: %lldb %t.o -s %s -o quit | FileCheck %s
+
+image lookup --address 1
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful
+image lookup --address 2
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful + 1
+image dump symtab
+# CHECK: Index   UserID DSX TypeFile Address/Value Load 
Address   Size   Flags  Name
+# CHECK-NEXT:--- -- --- --- -- 
-- -- -- 
--
+# CHECK-NEXT:[0]  1 Code0x0003 
   0x 0x sizeend
+# CHECK-NEXT:[1]  2 Code0x0001 
   0x0002 0x sizeful
+# CHECK-NEXT:[2]  3 Code0x0001 
   0x 0x sizeless
Index: lldb/trunk/lit/SymbolFile/Inputs/sizeless-symbol.s
===
--- lldb/trunk/lit/SymbolFile/Inputs/sizeless-symbol.s
+++ lldb/trunk/lit/SymbolFile/Inputs/sizeless-symbol.s
@@ -0,0 +1,8 @@
+.text
+.byte   0
+sizeless:
+sizeful:
+.byte   0
+.byte   0
+sizeend:
+.size   sizeful, sizeend - sizeful


Index: lldb/trunk/source/Symbol/Symtab.cpp
===
--- lldb/trunk/source/Symbol/Symtab.cpp
+++ lldb/trunk/source/Symbol/Symtab.cpp
@@ -896,8 +896,14 @@
   for (size_t i = 0; i < num_entries; i++) {
 FileRangeToIndexMap::Entry *entry =
 m_file_addr_to_index.GetMutableEntryAtIndex(i);
-if (entry->GetByteSize() == 0) {
-  addr_t curr_base_addr = entry->GetRangeBase();
+if (entry->GetByteSize() > 0)
+  continue;
+addr_t curr_base_addr = entry->GetRangeBase();
+// Symbols with non-zero size will show after zero-sized symbols on the
+// same address. So do not set size of a non-last zero-sized symbol.
+if (i == num_entries - 1 ||
+m_file_addr_to_index.GetMutableEntryAtIndex(i + 1)
+->GetRangeBase() != curr_base_addr) {
   const RangeVector::Entry *containing_section =
   section_ranges.FindEntryThatContains(curr_base_addr);
 
Index: lldb/trunk/lit/SymbolFile/sizeless-symbol.test
===
--- lldb/trunk/lit/SymbolFile/sizeless-symbol.test
+++ lldb/trunk/lit/SymbolFile/sizeless-symbol.test
@@ -0,0 +1,14 @@
+# Some targets do not have the .size directive.
+# RUN: %clang -target x86_64-unknown-unknown-elf %S/Inputs/sizeless-symbol.s -c -o %t.o
+# RUN: %lldb %t.o -s %s -o quit | FileCheck %s
+
+image lookup --address 1
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful
+image lookup --address 2
+# CHECK: Summary: sizeless-symbol.test.tmp.o

[Lldb-commits] [lldb] r364776 - Revert "[lldb] [Process/NetBSD] Fix constructor after r363707"

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Mon Jul  1 07:38:47 2019
New Revision: 364776

URL: http://llvm.org/viewvc/llvm-project?rev=364776&view=rev
Log:
Revert "[lldb] [Process/NetBSD] Fix constructor after r363707"

Now that r364751 has been reverted, we need to revert this fixup
as well.

Modified:
lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

Modified: lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp?rev=364776&r1=364775&r2=364776&view=diff
==
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp (original)
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp Mon Jul  1 
07:38:47 2019
@@ -140,7 +140,7 @@ NativeProcessNetBSD::NativeProcessNetBSD
  NativeDelegate &delegate,
  const ArchSpec &arch,
  MainLoop &mainloop)
-: NativeProcessELF(pid, terminal_fd, delegate), m_arch(arch) {
+: NativeProcessProtocol(pid, terminal_fd, delegate), m_arch(arch) {
   if (m_terminal_fd != -1) {
 Status status = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
 assert(status.Success());


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

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

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

aadsm added a comment.

Yap, that makes a lot of sense to me. I looked into this over the weekend and 
figured out the reason why it was broken (tl;dr: we can't use LoadModules) and 
created a tidier version of D63868  that, like 
D62504  avoids fetching the svr4 packet twice.

Will put it up in a few minutes. Yesterday the git server (or phabricator's 
mysql) was down or something, it seemed like there was not enough disk space.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62503



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

[Lldb-commits] [PATCH] D63791: [lldb] [Process/NetBSD] Fix segfault when handling watchpoint

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:250
+if (!thread) {
+  LLDB_LOG(log,
+   "thread not found in m_threads, pid = {0}, LWP = {1}",

We should always have a fallback and whenever we register an event for an 
unknown thread, we shall create it in place. There are potential races in the 
kernel that might lead to this order of reporting events. This is less likely 
for DB Registers, but still we shall be prepared for it just in case.

I'm fine to leave this for later once we will handle debuggees with multiple 
threads.


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

https://reviews.llvm.org/D63791



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

[Lldb-commits] [PATCH] D63792: [lldb] [Process/NetBSD] Use global enable bits for watchpoints

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

I would follow the same kernel behavior here as Linux, but that can be done 
independently.


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

https://reviews.llvm.org/D63792



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

[Lldb-commits] [PATCH] D62504: Avoid calling LoadModules twice when modules change

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

aadsm abandoned this revision.
aadsm added a comment.

Replaced by D62503 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62504



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

[Lldb-commits] [PATCH] D64013: Correctly use GetLoadedModuleList to take advantage of libraries-svr4

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

aadsm created this revision.
aadsm added reviewers: labath, jankratochvil, clayborg.
Herald added subscribers: lldb-commits, JDevlieghere, srhines.
Herald added a project: LLDB.
aadsm added a reviewer: xiaobai.

Here's a replacement for D62504 . I thought I 
could use LoadModules to implement this but in reality I can't because there 
are at few issues with it:

- The LoadModules assumes that the list returned by GetLoadedModuleList is 
comprehensive in the sense that reflects all the mapped segments, however, this 
is not true, for instance VDSO entry is not there since it's loaded manually by 
LoadVDSO using GetMemoryRegionInfo and it doesn't represent a specific shared 
object in disk. Because of this LoadModules will unload the VDSO module.
- The loader (interpreter) module might have also been loaded using 
GetMemoryRegionInfo, this is true when we launch the process and the rendezvous 
structure is not yet available (done through LoadInterpreterModule()). The 
problem here is that this entry will point to the same file name as the one 
found in /proc/pid/maps, however, when we read the same module from the 
r_debug.link_map structure it might be under a different name. This is true at 
least on CentOS where the loader is a symlink. Because of this LoadModules will 
unload and load the module in a way where the rendezvous breakpoint is 
unresolved but not resolved again (because we add the new module first and 
remove the old one after).

The symlink issue might be fixable by first unloading the old and loading the 
news (but sounds super brittle), however, I'm not sure how to fix the VDSO 
issue.
Since I can't trust it I'm just going to use GetLoadedModuleList directly with 
the same logic that we use today for when we read the linked list in lldb. The 
only safe thing to do here is to only calculate differences between different 
snapshots of the svr4 packet itself. This will also cut the dependency this 
plugin has from LoadModules.

I separated the 2 logics into 2 different functions (remote and not remote) 
because I don't like mixing 2 different logics in the same function with 
if/else's. Two different functions makes it easier to reason with I believe. 
However, I did abstract away the logic that decides if we should take a 
snapshot or add/remove modules so both functions could reuse it.

The other difference between the two is that on the UpdateSOEntriesFromRemote I 
take the snapshot only once when state = Consistent because I didn't find a 
good reason to always update that, as we already got the list from state = Add 
| Remove. I probably should use the same logic on UpdateSOEntries though I 
don't see a reason not to since it's really using the same data, just read in 
different places. Any thoughts here?

It might also be worthwhile to add a test to make sure we don't unload modules 
that were not actually "unloaded" like the vdso. I haven't done this yet though.
This diff is also missing the option for svr4 like proposed in 
https://reviews.llvm.org/D62503#1564296, I'll start working on this but wanted 
to have this up first.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64013

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -200,9 +200,9 @@
 
   llvm::VersionTuple GetHostOSVersion() override;
 
-  size_t LoadModules(LoadedModuleInfoList &module_list) override;
+  Status LoadModules() override;
 
-  size_t LoadModules() override;
+  llvm::Expected GetLoadedModuleList() override;
 
   Status GetFileLoadAddress(const FileSpec &file, bool &is_loaded,
 lldb::addr_t &load_addr) override;
@@ -391,9 +391,6 @@
   // Query remote GDBServer for register information
   bool GetGDBServerRegisterInfo(ArchSpec &arch);
 
-  // Query remote GDBServer for a detailed loaded library list
-  Status GetLoadedModuleList(LoadedModuleInfoList &);
-
   lldb::ModuleSP LoadModuleAtAddress(const FileSpec &file,
  lldb::addr_t link_map,
  lldb::addr_t base_addr,
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2728,9 +2728,9 @@
 
   // the loaded module list can also provides a link map address
   if (addr == LLDB_INVALID_ADDRESS) {
-LoadedModule

[Lldb-commits] [lldb] r364779 - [lldb] [Process/NetBSD] Support reading YMM registers via PT_*XSTATE

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Mon Jul  1 08:11:04 2019
New Revision: 364779

URL: http://llvm.org/viewvc/llvm-project?rev=364779&view=rev
Log:
[lldb] [Process/NetBSD] Support reading YMM registers via PT_*XSTATE

Provide a (conditional) support for the new PT_GETXSTATE
and PT_SETXSTATE ptrace() requests, and use them to implement getting
and setting YMM registers.  The functions used for splitting
and recombining YMM register data are based on matching functions
in FreeBSD plugin, with some simplification and updates to match NetBSD
structures.

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

Modified:

lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h

Modified: 
lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp?rev=364779&r1=364778&r2=364779&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp 
Mon Jul  1 08:11:04 2019
@@ -22,7 +22,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -148,7 +151,7 @@ int NativeRegisterContextNetBSD_x86_64::
   else if (reg_num <= k_last_fpr_x86_64)
 return FPRegSet;
   else if (reg_num <= k_last_avx_x86_64)
-return -1; // AVX
+return XStateRegSet; // AVX
   else if (reg_num <= k_last_mpxr_x86_64)
 return -1; // MPXR
   else if (reg_num <= k_last_mpxc_x86_64)
@@ -167,6 +170,15 @@ Status NativeRegisterContextNetBSD_x86_6
 return DoRegisterSet(PT_GETFPREGS, &m_fpr_x86_64);
   case DBRegSet:
 return DoRegisterSet(PT_GETDBREGS, &m_dbr_x86_64);
+  case XStateRegSet:
+#ifdef HAVE_XSTATE
+{
+  struct iovec iov = {&m_xstate_x86_64, sizeof(m_xstate_x86_64)};
+  return DoRegisterSet(PT_GETXSTATE, &iov);
+}
+#else
+return Status("XState is not supported by the kernel");
+#endif
   }
   llvm_unreachable("NativeRegisterContextNetBSD_x86_64::ReadRegisterSet");
 }
@@ -179,6 +191,15 @@ Status NativeRegisterContextNetBSD_x86_6
 return DoRegisterSet(PT_SETFPREGS, &m_fpr_x86_64);
   case DBRegSet:
 return DoRegisterSet(PT_SETDBREGS, &m_dbr_x86_64);
+  case XStateRegSet:
+#ifdef HAVE_XSTATE
+{
+  struct iovec iov = {&m_xstate_x86_64, sizeof(m_xstate_x86_64)};
+  return DoRegisterSet(PT_SETXSTATE, &iov);
+}
+#else
+return Status("XState is not supported by the kernel");
+#endif
   }
   llvm_unreachable("NativeRegisterContextNetBSD_x86_64::WriteRegisterSet");
 }
@@ -360,6 +381,39 @@ NativeRegisterContextNetBSD_x86_64::Read
 reg_value.SetBytes(&m_fpr_x86_64.fxstate.fx_xmm[reg - lldb_xmm0_x86_64],
reg_info->byte_size, endian::InlHostByteOrder());
 break;
+  case lldb_ymm0_x86_64:
+  case lldb_ymm1_x86_64:
+  case lldb_ymm2_x86_64:
+  case lldb_ymm3_x86_64:
+  case lldb_ymm4_x86_64:
+  case lldb_ymm5_x86_64:
+  case lldb_ymm6_x86_64:
+  case lldb_ymm7_x86_64:
+  case lldb_ymm8_x86_64:
+  case lldb_ymm9_x86_64:
+  case lldb_ymm10_x86_64:
+  case lldb_ymm11_x86_64:
+  case lldb_ymm12_x86_64:
+  case lldb_ymm13_x86_64:
+  case lldb_ymm14_x86_64:
+  case lldb_ymm15_x86_64:
+#ifdef HAVE_XSTATE
+if (!(m_xstate_x86_64.xs_rfbm & XCR0_SSE) ||
+!(m_xstate_x86_64.xs_rfbm & XCR0_YMM_Hi128)) {
+  error.SetErrorStringWithFormat("register \"%s\" not supported by 
CPU/kernel",
+ reg_info->name);
+} else {
+  uint32_t reg_index = reg - lldb_ymm0_x86_64;
+  YMMReg ymm = XStateToYMM(
+  m_xstate_x86_64.xs_fxsave.fx_xmm[reg_index].xmm_bytes,
+  m_xstate_x86_64.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes);
+  reg_value.SetBytes(ymm.bytes, reg_info->byte_size,
+ endian::InlHostByteOrder());
+}
+#else
+error.SetErrorString("XState queries not supported by the kernel");
+#endif
+break;
   case lldb_dr0_x86_64:
   case lldb_dr1_x86_64:
   case lldb_dr2_x86_64:
@@ -552,6 +606,39 @@ Status NativeRegisterContextNetBSD_x86_6
 ::memcpy(&m_fpr_x86_64.fxstate.fx_xmm[reg - lldb_xmm0_x86_64],
  reg_value.GetBytes(), reg_value.GetByteSize());
 break;
+  case lldb_ymm0_x86_64:
+  case lldb_ymm1_x86_64:
+  case lldb_ymm2_x86_64:
+  case lldb_ymm3_x86_64:
+  case lldb_ymm4_x86_64:
+  case lldb_ymm5_x86_64:
+  case lldb_ymm6_x86_64:
+  case lldb_ymm7_x86_64:
+  case lldb_ymm8_x86_64:
+  case lldb_ymm9_x86_64:
+  case lldb_ymm10_x86_64:
+  case lldb_ymm11_x86_64:
+  case lldb_ymm12_x86_64:
+  case lldb_ymm13_x86_64:
+  case lldb_ymm14_x86_64:
+  case lldb_ymm15_x86_64:
+#ifdef HAVE_XSTATE
+if (!(m_xstate_x86_64.xs_rfbm & XCR0_SSE) ||
+!(m_xstate_

[Lldb-commits] [lldb] r364780 - [lldb] [Process/NetBSD] Fix segfault when handling watchpoint

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Mon Jul  1 08:11:10 2019
New Revision: 364780

URL: http://llvm.org/viewvc/llvm-project?rev=364780&view=rev
Log:
[lldb] [Process/NetBSD] Fix segfault when handling watchpoint

Fix the watchpoint/breakpoint code to search for matching thread entry
in m_threads explicitly rather than assuming that it will be present
at specified index.  The previous code segfault since it wrongly assumed
that the index will match LWP ID which was incorrect even for a single
thread (where index was 0 and LWP ID was 1).

While fixing that off-by-one error would help for this specific task,
I believe it is better to be explicit in what we are searching for.

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

Modified:
lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

Modified: lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp?rev=364780&r1=364779&r2=364780&view=diff
==
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp (original)
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp Mon Jul  1 
08:11:10 2019
@@ -238,12 +238,25 @@ void NativeProcessNetBSD::MonitorSIGTRAP
 SetState(StateType::eStateStopped, true);
   } break;
   case TRAP_DBREG: {
+// Find the thread.
+NativeThreadNetBSD* thread = nullptr;
+for (const auto &t : m_threads) {
+  if (t->GetID() == info.psi_lwpid) {
+thread = static_cast(t.get());
+break;
+  }
+}
+if (!thread) {
+  LLDB_LOG(log,
+   "thread not found in m_threads, pid = {0}, LWP = {1}",
+   GetID(), info.psi_lwpid);
+  break;
+}
+
 // If a watchpoint was hit, report it
-uint32_t wp_index;
-Status error = static_cast(*m_threads[info.psi_lwpid])
-   .GetRegisterContext()
-   .GetWatchpointHitIndex(
-   wp_index, (uintptr_t)info.psi_siginfo.si_addr);
+uint32_t wp_index = LLDB_INVALID_INDEX32;
+Status error = thread->GetRegisterContext().GetWatchpointHitIndex(
+wp_index, (uintptr_t)info.psi_siginfo.si_addr);
 if (error.Fail())
   LLDB_LOG(log,
"received error while checking for watchpoint hits, pid = "
@@ -258,11 +271,9 @@ void NativeProcessNetBSD::MonitorSIGTRAP
 }
 
 // If a breakpoint was hit, report it
-uint32_t bp_index;
-error = static_cast(*m_threads[info.psi_lwpid])
-.GetRegisterContext()
-.GetHardwareBreakHitIndex(bp_index,
-   
(uintptr_t)info.psi_siginfo.si_addr);
+uint32_t bp_index = LLDB_INVALID_INDEX32;
+error = thread->GetRegisterContext().GetHardwareBreakHitIndex(
+bp_index, (uintptr_t)info.psi_siginfo.si_addr);
 if (error.Fail())
   LLDB_LOG(log,
"received error while checking for hardware "


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

[Lldb-commits] [lldb] r364781 - [lldb] [Process/NetBSD] Use global enable bits for watchpoints

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Mon Jul  1 08:11:42 2019
New Revision: 364781

URL: http://llvm.org/viewvc/llvm-project?rev=364781&view=rev
Log:
[lldb] [Process/NetBSD] Use global enable bits for watchpoints

Set global enable bits (i.e. bits 1, 3, 5, 7) to enable watchpoints
on NetBSD rather than the local enable bits (0, 2, 4, 6).  The former
are necessary for watchpoints to be correctly recognized by the NetBSD
kernel.  The latter cause them to be reported as trace points.

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchlocation/TestWatchLocation.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchpoint/TestMyFirstWatchpoint.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_hits/TestMultipleHits.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_threads/TestWatchpointMultipleThreads.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/TestWatchpointCommands.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/condition/TestWatchpointConditionCmd.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/TestWatchpointDisable.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py

lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchlocation/TestWatchLocation.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchlocation/TestWatchLocation.py?rev=364781&r1=364780&r2=364781&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchlocation/TestWatchLocation.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchlocation/TestWatchLocation.py
 Mon Jul  1 08:11:42 2019
@@ -42,7 +42,6 @@ class HelloWatchLocationTestCase(TestBas
 # SystemZ and PowerPC also currently supports only one H/W watchpoint
 @expectedFailureAll(archs=['powerpc64le', 's390x'])
 @skipIfDarwin
-@expectedFailureNetBSD
 def test_hello_watchlocation(self):
 """Test watching a location with '-s size' option."""
 self.build(dictionary=self.d)

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchpoint/TestMyFirstWatchpoint.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchpoint/TestMyFirstWatchpoint.py?rev=364781&r1=364780&r2=364781&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchpoint/TestMyFirstWatchpoint.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchpoint/TestMyFirstWatchpoint.py
 Mon Jul  1 08:11:42 2019
@@ -34,7 +34,6 @@ class HelloWatchpointTestCase(TestBase):
 @expectedFailureAll(
 oslist=["windows"],
 bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not 
supported on Windows")
-@expectedFailureNetBSD
 @add_test_categories(["basic_process"])
 def test_hello_watchpoint_using_watchpoint_set(self):
 """Test a simple sequence of watchpoint creation and watchpoint hit."""

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_hits/TestMultipleHits.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_hits/TestMultipleHits.py?rev=364781&r1=364780&r2=364781&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_hits/TestMultipleHits.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_hits/TestMultipleHits.py
 Mon Jul  1 08:11:42 2019
@@ -23,7 +23,6 @@ class MultipleHitsTestCase(TestBase):
 bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not 
supported

[Lldb-commits] [PATCH] D63545: [lldb] [Process/NetBSD] Support reading YMM registers via PT_*XSTATE

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364779: [lldb] [Process/NetBSD] Support reading YMM 
registers via PT_*XSTATE (authored by mgorny, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63545?vs=205999&id=207318#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63545

Files:
  
lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h

Index: lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
===
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
@@ -14,6 +14,7 @@
 // clang-format off
 #include 
 #include 
+#include 
 #include 
 // clang-format on
 
@@ -21,6 +22,10 @@
 #include "Plugins/Process/Utility/RegisterContext_x86.h"
 #include "Plugins/Process/Utility/lldb-x86-register-enums.h"
 
+#if defined(PT_GETXSTATE) && defined(PT_SETXSTATE)
+#define HAVE_XSTATE
+#endif
+
 namespace lldb_private {
 namespace process_netbsd {
 
@@ -68,12 +73,15 @@
 
 private:
   // Private member types.
-  enum { GPRegSet, FPRegSet, DBRegSet };
+  enum { GPRegSet, FPRegSet, DBRegSet, XStateRegSet };
 
   // Private member variables.
   struct reg m_gpr_x86_64;
   struct fpreg m_fpr_x86_64;
   struct dbreg m_dbr_x86_64;
+#ifdef HAVE_XSTATE
+  struct xstate m_xstate_x86_64;
+#endif
 
   int GetSetForNativeRegNum(int reg_num) const;
 
Index: lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -22,7 +22,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -148,7 +151,7 @@
   else if (reg_num <= k_last_fpr_x86_64)
 return FPRegSet;
   else if (reg_num <= k_last_avx_x86_64)
-return -1; // AVX
+return XStateRegSet; // AVX
   else if (reg_num <= k_last_mpxr_x86_64)
 return -1; // MPXR
   else if (reg_num <= k_last_mpxc_x86_64)
@@ -167,6 +170,15 @@
 return DoRegisterSet(PT_GETFPREGS, &m_fpr_x86_64);
   case DBRegSet:
 return DoRegisterSet(PT_GETDBREGS, &m_dbr_x86_64);
+  case XStateRegSet:
+#ifdef HAVE_XSTATE
+{
+  struct iovec iov = {&m_xstate_x86_64, sizeof(m_xstate_x86_64)};
+  return DoRegisterSet(PT_GETXSTATE, &iov);
+}
+#else
+return Status("XState is not supported by the kernel");
+#endif
   }
   llvm_unreachable("NativeRegisterContextNetBSD_x86_64::ReadRegisterSet");
 }
@@ -179,6 +191,15 @@
 return DoRegisterSet(PT_SETFPREGS, &m_fpr_x86_64);
   case DBRegSet:
 return DoRegisterSet(PT_SETDBREGS, &m_dbr_x86_64);
+  case XStateRegSet:
+#ifdef HAVE_XSTATE
+{
+  struct iovec iov = {&m_xstate_x86_64, sizeof(m_xstate_x86_64)};
+  return DoRegisterSet(PT_SETXSTATE, &iov);
+}
+#else
+return Status("XState is not supported by the kernel");
+#endif
   }
   llvm_unreachable("NativeRegisterContextNetBSD_x86_64::WriteRegisterSet");
 }
@@ -360,6 +381,39 @@
 reg_value.SetBytes(&m_fpr_x86_64.fxstate.fx_xmm[reg - lldb_xmm0_x86_64],
reg_info->byte_size, endian::InlHostByteOrder());
 break;
+  case lldb_ymm0_x86_64:
+  case lldb_ymm1_x86_64:
+  case lldb_ymm2_x86_64:
+  case lldb_ymm3_x86_64:
+  case lldb_ymm4_x86_64:
+  case lldb_ymm5_x86_64:
+  case lldb_ymm6_x86_64:
+  case lldb_ymm7_x86_64:
+  case lldb_ymm8_x86_64:
+  case lldb_ymm9_x86_64:
+  case lldb_ymm10_x86_64:
+  case lldb_ymm11_x86_64:
+  case lldb_ymm12_x86_64:
+  case lldb_ymm13_x86_64:
+  case lldb_ymm14_x86_64:
+  case lldb_ymm15_x86_64:
+#ifdef HAVE_XSTATE
+if (!(m_xstate_x86_64.xs_rfbm & XCR0_SSE) ||
+!(m_xstate_x86_64.xs_rfbm & XCR0_YMM_Hi128)) {
+  error.SetErrorStringWithFormat("register \"%s\" not supported by CPU/kernel",
+ reg_info->name);
+} else {
+  uint32_t reg_index = reg - lldb_ymm0_x86_64;
+  YMMReg ymm = XStateToYMM(
+  m_xstate_x86_64.xs_fxsave.fx_xmm[reg_index].xmm_bytes,
+  m_xstate_x86_64.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes);
+  reg_value.SetBytes(ymm.bytes, reg_info->byte_size,
+ endian::InlHostByteOrder());
+}
+#else
+error.SetErrorString("XState queries not supported by the kernel");
+#endif
+break;
   case lldb_dr0_x86_64:
   case lldb_dr1_x86_64:
   case lldb_dr2_x86_64:
@@ -552,6 +606,39 @@
 ::memcpy(&m_fpr_x86_64.fxstate.fx_xmm[reg - lldb_xmm0_x86_64],
  reg_value.GetBytes()

[Lldb-commits] [PATCH] D63791: [lldb] [Process/NetBSD] Fix segfault when handling watchpoint

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364780: [lldb] [Process/NetBSD] Fix segfault when handling 
watchpoint (authored by mgorny, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63791?vs=206528&id=207319#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63791

Files:
  lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp


Index: lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -238,12 +238,25 @@
 SetState(StateType::eStateStopped, true);
   } break;
   case TRAP_DBREG: {
+// Find the thread.
+NativeThreadNetBSD* thread = nullptr;
+for (const auto &t : m_threads) {
+  if (t->GetID() == info.psi_lwpid) {
+thread = static_cast(t.get());
+break;
+  }
+}
+if (!thread) {
+  LLDB_LOG(log,
+   "thread not found in m_threads, pid = {0}, LWP = {1}",
+   GetID(), info.psi_lwpid);
+  break;
+}
+
 // If a watchpoint was hit, report it
-uint32_t wp_index;
-Status error = static_cast(*m_threads[info.psi_lwpid])
-   .GetRegisterContext()
-   .GetWatchpointHitIndex(
-   wp_index, (uintptr_t)info.psi_siginfo.si_addr);
+uint32_t wp_index = LLDB_INVALID_INDEX32;
+Status error = thread->GetRegisterContext().GetWatchpointHitIndex(
+wp_index, (uintptr_t)info.psi_siginfo.si_addr);
 if (error.Fail())
   LLDB_LOG(log,
"received error while checking for watchpoint hits, pid = "
@@ -258,11 +271,9 @@
 }
 
 // If a breakpoint was hit, report it
-uint32_t bp_index;
-error = static_cast(*m_threads[info.psi_lwpid])
-.GetRegisterContext()
-.GetHardwareBreakHitIndex(bp_index,
-   
(uintptr_t)info.psi_siginfo.si_addr);
+uint32_t bp_index = LLDB_INVALID_INDEX32;
+error = thread->GetRegisterContext().GetHardwareBreakHitIndex(
+bp_index, (uintptr_t)info.psi_siginfo.si_addr);
 if (error.Fail())
   LLDB_LOG(log,
"received error while checking for hardware "


Index: lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -238,12 +238,25 @@
 SetState(StateType::eStateStopped, true);
   } break;
   case TRAP_DBREG: {
+// Find the thread.
+NativeThreadNetBSD* thread = nullptr;
+for (const auto &t : m_threads) {
+  if (t->GetID() == info.psi_lwpid) {
+thread = static_cast(t.get());
+break;
+  }
+}
+if (!thread) {
+  LLDB_LOG(log,
+   "thread not found in m_threads, pid = {0}, LWP = {1}",
+   GetID(), info.psi_lwpid);
+  break;
+}
+
 // If a watchpoint was hit, report it
-uint32_t wp_index;
-Status error = static_cast(*m_threads[info.psi_lwpid])
-   .GetRegisterContext()
-   .GetWatchpointHitIndex(
-   wp_index, (uintptr_t)info.psi_siginfo.si_addr);
+uint32_t wp_index = LLDB_INVALID_INDEX32;
+Status error = thread->GetRegisterContext().GetWatchpointHitIndex(
+wp_index, (uintptr_t)info.psi_siginfo.si_addr);
 if (error.Fail())
   LLDB_LOG(log,
"received error while checking for watchpoint hits, pid = "
@@ -258,11 +271,9 @@
 }
 
 // If a breakpoint was hit, report it
-uint32_t bp_index;
-error = static_cast(*m_threads[info.psi_lwpid])
-.GetRegisterContext()
-.GetHardwareBreakHitIndex(bp_index,
-   (uintptr_t)info.psi_siginfo.si_addr);
+uint32_t bp_index = LLDB_INVALID_INDEX32;
+error = thread->GetRegisterContext().GetHardwareBreakHitIndex(
+bp_index, (uintptr_t)info.psi_siginfo.si_addr);
 if (error.Fail())
   LLDB_LOG(log,
"received error while checking for hardware "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63792: [lldb] [Process/NetBSD] Use global enable bits for watchpoints

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364781: [lldb] [Process/NetBSD] Use global enable bits for 
watchpoints (authored by mgorny, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63792?vs=207255&id=207320#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63792

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchlocation/TestWatchLocation.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchpoint/TestMyFirstWatchpoint.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_hits/TestMultipleHits.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_threads/TestWatchpointMultipleThreads.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/TestWatchpointCommands.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/condition/TestWatchpointConditionCmd.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/TestWatchpointDisable.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py
  
lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py
@@ -36,7 +36,6 @@
 bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
 # Read-write watchpoints not supported on SystemZ
 @expectedFailureAll(archs=['s390x'])
-@expectedFailureNetBSD
 def test_byte_size_watchpoints_with_byte_selection(self):
 """Test to selectively watch different bytes in a 8-byte array."""
 self.run_watchpoint_size_test('byteArray', 8, '1')
@@ -46,7 +45,6 @@
 bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
 # Read-write watchpoints not supported on SystemZ
 @expectedFailureAll(archs=['s390x'])
-@expectedFailureNetBSD
 def test_two_byte_watchpoints_with_word_selection(self):
 """Test to selectively watch different words in an 8-byte word array."""
 self.run_watchpoint_size_test('wordArray', 4, '2')
@@ -56,7 +54,6 @@
 bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
 # Read-write watchpoints not supported on SystemZ
 @expectedFailureAll(archs=['s390x'])
-@expectedFailureNetBSD
 def test_four_byte_watchpoints_with_dword_selection(self):
 """Test to selectively watch two double words in an 8-byte dword array."""
 self.run_watchpoint_size_test('dwordArray', 2, '4')
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
@@ -39,7 +39,6 @@
 @expectedFailureAll(
 oslist=["windows"],
 bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
-@expectedFailureNetBSD
 def test_watchlocation_using_watchpoint_set(self):
 """Test watching a location with 'watchpoint set expression -w write -s size' option."""
 self.build()
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/T

[Lldb-commits] [PATCH] D64013: Correctly use GetLoadedModuleList to take advantage of libraries-svr4

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't have time to do a full review of this today, but this seems pretty good 
at a first glance...




Comment at: lldb/include/lldb/Target/Process.h:684
+  ///A status object indicating if the operation was sucessful or not.
+  virtual Status LoadModules() { return Status("Not implemented"); }
 

Since you're already changing the signature, let's make this return an 
llvm::Error.



Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:184-206
+bool DYLDRendezvous::StateIsTakeSnapshot() {
+  return (
+  // When the previous and current states are consistent this is the first
+  // time we have been asked to update.  Just take a snapshot of the
+  // currently loaded modules.
+  (m_previous.state == eConsistent && m_current.state == eConsistent) ||
+  // If we are about to add or remove a shared object clear out the current

I like that you've factored out the decoding of the states, but I think it 
would be better to make this a separate function returning an enum (`GetAction` 
?). This will make it more obvious which combinations of flags are covered. 
Also, `StateIsTakeSnapshot` reads weirdly...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64013



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

[Lldb-commits] [PATCH] D64013: Correctly use GetLoadedModuleList to take advantage of libraries-svr4

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

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



Comment at: lldb/include/lldb/Target/Process.h:684
+  ///A status object indicating if the operation was sucessful or not.
+  virtual Status LoadModules() { return Status("Not implemented"); }
 

labath wrote:
> Since you're already changing the signature, let's make this return an 
> llvm::Error.
Ah yeah ok. It still weirds me out that an Error object might not be an error. 
In this regard I actually much prefer the Status object as it much better 
conveys the meaning of the return object. Is the plan to move from Status to 
Error in lldb?



Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:184-206
+bool DYLDRendezvous::StateIsTakeSnapshot() {
+  return (
+  // When the previous and current states are consistent this is the first
+  // time we have been asked to update.  Just take a snapshot of the
+  // currently loaded modules.
+  (m_previous.state == eConsistent && m_current.state == eConsistent) ||
+  // If we are about to add or remove a shared object clear out the current

labath wrote:
> I like that you've factored out the decoding of the states, but I think it 
> would be better to make this a separate function returning an enum 
> (`GetAction` ?). This will make it more obvious which combinations of flags 
> are covered. Also, `StateIsTakeSnapshot` reads weirdly...
Sounds good. I didn't want to use the word consistent, I wanted to match rest 
of the language used in the code. Originally I had ShouldX (e.g.: 
ShouldTakeSnapshot) but I didn't think it was clear when to call it (or that it 
was derived from the state). I like the GetAction idea, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64013



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

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

Centralizing it means changing the classes that inherit from 
SymbolContextScope, so I will refactor that in a seaparate change and stack 
this change on top of that one.


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

https://reviews.llvm.org/D63240



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

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Jim, you ok with doing the symbol context scope refactoring as a separate diff?


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

https://reviews.llvm.org/D63240



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

[Lldb-commits] [lldb] r364826 - [lldb] [lldbsuite] Use a unique class name for TestBacktraceAll

[Stella Stamenova via lldb-commits]

Author: stella.stamenova
Date: Mon Jul  1 11:13:20 2019
New Revision: 364826

URL: http://llvm.org/viewvc/llvm-project?rev=364826&view=rev
Log:
[lldb] [lldbsuite] Use a unique class name for TestBacktraceAll

It looks like when this test was added, it was based on TestBreakAfterJoin and 
it ended up with the same class name. This is an issue because the logs 
associated with the tests use the class name as the identifier for the file and 
if two tests have the same name their logs overwrite each other. On 
non-windows, this just means we lose one of the logs, but on Windows this means 
that one of the tests will fail occasionally because the file are locked by the 
other test.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/TestBacktraceAll.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/TestBacktraceAll.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/TestBacktraceAll.py?rev=364826&r1=364825&r2=364826&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/TestBacktraceAll.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/TestBacktraceAll.py
 Mon Jul  1 11:13:20 2019
@@ -11,7 +11,7 @@ from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
-class BreakpointAfterJoinTestCase(TestBase):
+class BacktraceAllTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 


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

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.

Sure, NP.


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

https://reviews.llvm.org/D63240



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

[Lldb-commits] [PATCH] D64013: Correctly use GetLoadedModuleList to take advantage of libraries-svr4

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Target/Process.h:684
+  ///A status object indicating if the operation was sucessful or not.
+  virtual Status LoadModules() { return Status("Not implemented"); }
 

aadsm wrote:
> labath wrote:
> > Since you're already changing the signature, let's make this return an 
> > llvm::Error.
> Ah yeah ok. It still weirds me out that an Error object might not be an 
> error. In this regard I actually much prefer the Status object as it much 
> better conveys the meaning of the return object. Is the plan to move from 
> Status to Error in lldb?
Yep. lldb_private::Status used to be called lldb_private::Error, but it was 
renamed to avoid confusing with the llvm version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64013



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

[Lldb-commits] [lldb] r364845 - [Core] Generalize ValueObject::IsRuntimeSupportValue

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Mon Jul  1 13:36:33 2019
New Revision: 364845

URL: http://llvm.org/viewvc/llvm-project?rev=364845&view=rev
Log:
[Core] Generalize ValueObject::IsRuntimeSupportValue

Summary:
Instead of falling back to ObjCLanguageRuntime, we should be falling
back to every loaded language runtime. This makes ValueObject more
language agnostic.

Reviewers: labath, compnerd, JDevlieghere, davide

Subscribers: lldb-commits

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

Modified:
lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
lldb/trunk/include/lldb/Target/LanguageRuntime.h
lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h
lldb/trunk/source/Core/ValueObject.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Symbol/Function.cpp
lldb/trunk/source/Target/CPPLanguageRuntime.cpp
lldb/trunk/source/Target/ObjCLanguageRuntime.cpp

Modified: lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h?rev=364845&r1=364844&r2=364845&view=diff
==
--- lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h (original)
+++ lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h Mon Jul  1 13:36:33 2019
@@ -78,7 +78,7 @@ public:
   lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
   bool stop_others) override;
 
-  bool IsRuntimeSupportValue(ValueObject &valobj) override;
+  bool IsWhitelistedRuntimeValue(ConstString name) override;
 protected:
   // Classes that inherit from CPPLanguageRuntime can see and modify these
   CPPLanguageRuntime(Process *process);

Modified: lldb/trunk/include/lldb/Target/LanguageRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/LanguageRuntime.h?rev=364845&r1=364844&r2=364845&view=diff
==
--- lldb/trunk/include/lldb/Target/LanguageRuntime.h (original)
+++ lldb/trunk/include/lldb/Target/LanguageRuntime.h Mon Jul  1 13:36:33 2019
@@ -152,9 +152,9 @@ public:
   virtual lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
   bool stop_others) = 
0;
 
-  /// Identify whether a value is a language implementation detaul
-  /// that should be hidden from the user interface by default.
-  virtual bool IsRuntimeSupportValue(ValueObject &valobj) { return false; }
+  /// Identify whether a name is a runtime value that should not be hidden by
+  /// from the user interface.
+  virtual bool IsWhitelistedRuntimeValue(ConstString name) { return false; }
 
   virtual void ModulesDidLoad(const ModuleList &module_list) {}
 

Modified: lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h?rev=364845&r1=364844&r2=364845&view=diff
==
--- lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h (original)
+++ lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h Mon Jul  1 13:36:33 
2019
@@ -301,8 +301,7 @@ public:
 
   /// Check whether the name is "self" or "_cmd" and should show up in
   /// "frame variable".
-  static bool IsWhitelistedRuntimeValue(ConstString name);
-  bool IsRuntimeSupportValue(ValueObject &valobj) override;
+  bool IsWhitelistedRuntimeValue(ConstString name) override;
 
 protected:
   // Classes that inherit from ObjCLanguageRuntime can see and modify these

Modified: lldb/trunk/source/Core/ValueObject.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObject.cpp?rev=364845&r1=364844&r2=364845&view=diff
==
--- lldb/trunk/source/Core/ValueObject.cpp (original)
+++ lldb/trunk/source/Core/ValueObject.cpp Mon Jul  1 13:36:33 2019
@@ -1695,18 +1695,28 @@ bool ValueObject::IsPossibleDynamicType(
 
 bool ValueObject::IsRuntimeSupportValue() {
   Process *process(GetProcessSP().get());
-  if (process) {
-LanguageRuntime *runtime =
-process->GetLanguageRuntime(GetObjectRuntimeLanguage());
-if (!runtime)
-  runtime = ObjCLanguageRuntime::Get(*process);
-if (runtime)
-  return runtime->IsRuntimeSupportValue(*this);
-// If there is no language runtime, trust the compiler to mark all
-// runtime support variables as artificial.
-return GetVariable() && GetVariable()->IsArtificial();
+  if (!process)
+return false;
+
+  // We trust the the compiler did the right thing and marked runtime support
+  // values as artificial.
+  if (!GetVariable() || !GetVariable()->IsArtificial())
+return false;
+
+  LanguageType lang = eLanguageTypeUnknown;
+  if (auto *sym_ctx_scope = GetSymbolContextScope()) {
+if (auto *func = sym_ctx_s

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

[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 rL364845: [Core] Generalize ValueObject::IsRuntimeSupportValue 
(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/D63240/new/

https://reviews.llvm.org/D63240

Files:
  lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
  lldb/trunk/include/lldb/Target/LanguageRuntime.h
  lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h
  lldb/trunk/source/Core/ValueObject.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/trunk/source/Symbol/Function.cpp
  lldb/trunk/source/Target/CPPLanguageRuntime.cpp
  lldb/trunk/source/Target/ObjCLanguageRuntime.cpp

Index: lldb/trunk/source/Core/ValueObject.cpp
===
--- lldb/trunk/source/Core/ValueObject.cpp
+++ lldb/trunk/source/Core/ValueObject.cpp
@@ -1695,18 +1695,28 @@
 
 bool ValueObject::IsRuntimeSupportValue() {
   Process *process(GetProcessSP().get());
-  if (process) {
-LanguageRuntime *runtime =
-process->GetLanguageRuntime(GetObjectRuntimeLanguage());
-if (!runtime)
-  runtime = ObjCLanguageRuntime::Get(*process);
-if (runtime)
-  return runtime->IsRuntimeSupportValue(*this);
-// If there is no language runtime, trust the compiler to mark all
-// runtime support variables as artificial.
-return GetVariable() && GetVariable()->IsArtificial();
+  if (!process)
+return false;
+
+  // We trust the the compiler did the right thing and marked runtime support
+  // values as artificial.
+  if (!GetVariable() || !GetVariable()->IsArtificial())
+return false;
+
+  LanguageType lang = eLanguageTypeUnknown;
+  if (auto *sym_ctx_scope = GetSymbolContextScope()) {
+if (auto *func = sym_ctx_scope->CalculateSymbolContextFunction())
+  lang = func->GetLanguage();
+else if (auto *comp_unit =
+ sym_ctx_scope->CalculateSymbolContextCompileUnit())
+  lang = comp_unit->GetLanguage();
   }
-  return false;
+
+  if (auto *runtime = process->GetLanguageRuntime(lang))
+if (runtime->IsWhitelistedRuntimeValue(GetName()))
+  return false;
+
+  return true;
 }
 
 bool ValueObject::IsNilReference() {
Index: lldb/trunk/source/Target/ObjCLanguageRuntime.cpp
===
--- lldb/trunk/source/Target/ObjCLanguageRuntime.cpp
+++ lldb/trunk/source/Target/ObjCLanguageRuntime.cpp
@@ -46,19 +46,6 @@
   return name == g_self || name == g_cmd;
 }
 
-bool ObjCLanguageRuntime::IsRuntimeSupportValue(ValueObject &valobj) {
-  // All runtime support values have to be marked as artificial by the
-  // compiler. But not all artificial variables should be hidden from
-  // the user.
-  if (!valobj.GetVariable())
-return false;
-  if (!valobj.GetVariable()->IsArtificial())
-return false;
-
-  // Whitelist "self" and "_cmd".
-  return !IsWhitelistedRuntimeValue(valobj.GetName());
-}
-
 bool ObjCLanguageRuntime::AddClass(ObjCISA isa,
const ClassDescriptorSP &descriptor_sp,
const char *class_name) {
Index: lldb/trunk/source/Target/CPPLanguageRuntime.cpp
===
--- lldb/trunk/source/Target/CPPLanguageRuntime.cpp
+++ lldb/trunk/source/Target/CPPLanguageRuntime.cpp
@@ -43,20 +43,8 @@
 CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
 : LanguageRuntime(process) {}
 
-bool CPPLanguageRuntime::IsRuntimeSupportValue(ValueObject &valobj) {
-  // All runtime support values have to be marked as artificial by the
-  // compiler. But not all artificial variables should be hidden from
-  // the user.
-  if (!valobj.GetVariable())
-return false;
-  if (!valobj.GetVariable()->IsArtificial())
-return false;
-
-  // Whitelist "this" and since there is no ObjC++ runtime, any ObjC names.
-  ConstString name = valobj.GetName();
-  if (name == g_this)
-return false;
-  return !ObjCLanguageRuntime::IsWhitelistedRuntimeValue(name);
+bool CPPLanguageRuntime::IsWhitelistedRuntimeValue(ConstString name) {
+  return name == g_this;
 }
 
 bool CPPLanguageRuntime::GetObjectDescription(Stream &str,
Index: lldb/trunk/source/Symbol/Function.cpp
===
--- lldb/trunk/source/Symbol/Function.cpp
+++ lldb/trunk/source/Symbol/Function.cpp
@@ -589,10 +589,14 @@
 }
 
 lldb::LanguageType Function::GetLanguage() const {
+  lldb::LanguageType lang = m_mangled.GuessLanguage();
+  if (lang != lldb::eLanguageTypeUnknown)
+return lang;
+
   if (m_comp_unit)
 return m_comp_unit->GetLanguage();
-  else
-return lldb::eLanguageTypeUnknown;
+
+  return lldb::eLanguageTypeUnknown;
 }
 
 ConstString Fu

[Lldb-commits] [lldb] r364852 - [Reproducer] Assert on unexpected packet

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Jul  1 14:25:34 2019
New Revision: 364852

URL: http://llvm.org/viewvc/llvm-project?rev=364852&view=rev
Log:
[Reproducer] Assert on unexpected packet

I'm not able to reproduce the reproducer flakiness we're seeing on
GreenDragon. I want to add this assert to find out if the GDB remote
packets are somehow getting out of sync when this happens.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp?rev=364852&r1=364851&r2=364852&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 Mon Jul  1 14:25:34 2019
@@ -142,6 +142,7 @@ GDBRemoteCommunicationReplayServer::GetP
  entry.packet.data);
 LLDB_LOG(log, "GDBRemoteCommunicationReplayServer actual packet: 
'{0}'",
  packet.GetStringRef());
+assert(false && "Encountered unexpected packet during replay");
 return PacketResult::ErrorSendFailed;
   }
 


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

[Lldb-commits] [lldb] r364860 - [lldb] [lldbsuite] Use a unique class name for TestValueVarUpdate

[Stella Stamenova via lldb-commits]

Author: stella.stamenova
Date: Mon Jul  1 15:12:55 2019
New Revision: 364860

URL: http://llvm.org/viewvc/llvm-project?rev=364860&view=rev
Log:
[lldb] [lldbsuite] Use a unique class name for TestValueVarUpdate

It looks like when this test was added, it was based on TestHelloWorld and it 
ended up with the same class name. This is an issue because the logs associated 
with the tests use the class name as the identifier for the file and if two 
tests have the same name their logs overwrite each other. On non-windows, this 
just means we lose one of the logs, but on Windows this means that one of the 
tests will fail occasionally because the file are locked by the other test.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py?rev=364860&r1=364859&r2=364860&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
 Mon Jul  1 15:12:55 2019
@@ -13,7 +13,7 @@ from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
-class HelloWorldTestCase(TestBase):
+class ValueVarUpdateTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 


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

[Lldb-commits] [PATCH] D64042: [Symbol] Improve Variable::GetLanguage

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: jingham, clayborg.
Herald added a subscriber: jdoerfert.

When trying to ascertain what language a variable belongs to, just
checking the compilation unit is often not enough. In r364845 I added a way to
check for a variable's language type, but didn't put it in Variable itself.
Let's go ahead and put it in Variable.


https://reviews.llvm.org/D64042

Files:
  source/Core/ValueObject.cpp
  source/Symbol/Variable.cpp


Index: source/Symbol/Variable.cpp
===
--- source/Symbol/Variable.cpp
+++ source/Symbol/Variable.cpp
@@ -54,10 +54,18 @@
 Variable::~Variable() {}
 
 lldb::LanguageType Variable::GetLanguage() const {
-  SymbolContext variable_sc;
-  m_owner_scope->CalculateSymbolContext(&variable_sc);
-  if (variable_sc.comp_unit)
-return variable_sc.comp_unit->GetLanguage();
+  lldb::LanguageType lang = m_mangled.GuessLanguage();
+  if (lang != lldb::eLanguageTypeUnknown)
+return lang;
+
+  if (auto *func = m_owner_scope->CalculateSymbolContextFunction())
+if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
+  return lang;
+else if (auto *comp_unit =
+ m_owner_scope->CalculateSymbolContextCompileUnit())
+  if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
+return lang;
+
   return lldb::eLanguageTypeUnknown;
 }
 
Index: source/Core/ValueObject.cpp
===
--- source/Core/ValueObject.cpp
+++ source/Core/ValueObject.cpp
@@ -1703,16 +1703,7 @@
   if (!GetVariable() || !GetVariable()->IsArtificial())
 return false;
 
-  LanguageType lang = eLanguageTypeUnknown;
-  if (auto *sym_ctx_scope = GetSymbolContextScope()) {
-if (auto *func = sym_ctx_scope->CalculateSymbolContextFunction())
-  lang = func->GetLanguage();
-else if (auto *comp_unit =
- sym_ctx_scope->CalculateSymbolContextCompileUnit())
-  lang = comp_unit->GetLanguage();
-  }
-
-  if (auto *runtime = process->GetLanguageRuntime(lang))
+  if (auto *runtime = 
process->GetLanguageRuntime(GetVariable()->GetLanguage()))
 if (runtime->IsWhitelistedRuntimeValue(GetName()))
   return false;
 


Index: source/Symbol/Variable.cpp
===
--- source/Symbol/Variable.cpp
+++ source/Symbol/Variable.cpp
@@ -54,10 +54,18 @@
 Variable::~Variable() {}
 
 lldb::LanguageType Variable::GetLanguage() const {
-  SymbolContext variable_sc;
-  m_owner_scope->CalculateSymbolContext(&variable_sc);
-  if (variable_sc.comp_unit)
-return variable_sc.comp_unit->GetLanguage();
+  lldb::LanguageType lang = m_mangled.GuessLanguage();
+  if (lang != lldb::eLanguageTypeUnknown)
+return lang;
+
+  if (auto *func = m_owner_scope->CalculateSymbolContextFunction())
+if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
+  return lang;
+else if (auto *comp_unit =
+ m_owner_scope->CalculateSymbolContextCompileUnit())
+  if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
+return lang;
+
   return lldb::eLanguageTypeUnknown;
 }
 
Index: source/Core/ValueObject.cpp
===
--- source/Core/ValueObject.cpp
+++ source/Core/ValueObject.cpp
@@ -1703,16 +1703,7 @@
   if (!GetVariable() || !GetVariable()->IsArtificial())
 return false;
 
-  LanguageType lang = eLanguageTypeUnknown;
-  if (auto *sym_ctx_scope = GetSymbolContextScope()) {
-if (auto *func = sym_ctx_scope->CalculateSymbolContextFunction())
-  lang = func->GetLanguage();
-else if (auto *comp_unit =
- sym_ctx_scope->CalculateSymbolContextCompileUnit())
-  lang = comp_unit->GetLanguage();
-  }
-
-  if (auto *runtime = process->GetLanguageRuntime(lang))
+  if (auto *runtime = process->GetLanguageRuntime(GetVariable()->GetLanguage()))
 if (runtime->IsWhitelistedRuntimeValue(GetName()))
   return false;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63853: [Symbol] Add DeclVendor::FindTypes

[Alex Langford via Phabricator via lldb-commits]

xiaobai updated this revision to Diff 207425.
xiaobai added a comment.

Pavel's suggestion


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

https://reviews.llvm.org/D63853

Files:
  include/lldb/Symbol/DeclVendor.h
  source/API/SBTarget.cpp
  source/Plugins/Language/ObjC/ObjCLanguage.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
  source/Symbol/CMakeLists.txt
  source/Symbol/DeclVendor.cpp

Index: source/Symbol/DeclVendor.cpp
===
--- /dev/null
+++ source/Symbol/DeclVendor.cpp
@@ -0,0 +1,29 @@
+//===-- DeclVendor.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Symbol/DeclVendor.h"
+
+#include "lldb/Symbol/ClangASTContext.h"
+
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+
+std::vector DeclVendor::FindTypes(ConstString name,
+uint32_t max_matches) {
+  // FIXME: This depends on clang, but should be able to support any
+  // TypeSystem.
+  std::vector ret;
+  std::vector decls;
+  if (FindDecls(name, /*append*/ true, max_matches, decls))
+for (auto *decl : decls)
+  if (auto type = ClangASTContext::GetTypeForDecl(decl))
+ret.push_back(type);
+  return ret;
+}
Index: source/Symbol/CMakeLists.txt
===
--- source/Symbol/CMakeLists.txt
+++ source/Symbol/CMakeLists.txt
@@ -21,6 +21,7 @@
   DWARFCallFrameInfo.cpp
   DebugMacros.cpp
   Declaration.cpp
+  DeclVendor.cpp
   FuncUnwinders.cpp
   Function.cpp
   LineEntry.cpp
Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
===
--- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
+++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
@@ -245,25 +245,19 @@
 if (!decl_vendor)
   return clang::QualType();
 
-const bool append = false;
-const uint32_t max_matches = 1;
-std::vector decls;
-
-uint32_t num_types =
-decl_vendor->FindDecls(ConstString(name), append, max_matches, decls);
+auto types = decl_vendor->FindTypes(ConstString(name), /*max_matches*/ 1);
 
 // The user can forward-declare something that has no definition.  The runtime
 // doesn't prohibit this at all. This is a rare and very weird case.  We keep
 // this assert in debug builds so we catch other weird cases.
 #ifdef LLDB_CONFIGURATION_DEBUG
-assert(num_types);
+assert(!types.empty());
 #else
-if (!num_types)
+if (types.empty())
   return ast_ctx.getObjCIdType();
 #endif
 
-return ClangUtil::GetQualType(
-ClangASTContext::GetTypeForDecl(decls[0]).GetPointerType());
+return ClangUtil::GetQualType(types.front().GetPointerType());
   } else {
 // We're going to resolve this dynamically anyway, so just smile and wave.
 return ast_ctx.getObjCIdType();
Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -474,12 +474,10 @@
   class_type_or_name.SetTypeSP(type_sp);
 } else {
   // try to go for a CompilerType at least
-  DeclVendor *vendor = GetDeclVendor();
-  if (vendor) {
-std::vector decls;
-if (vendor->FindDecls(class_name, false, 1, decls) && decls.size())
-  class_type_or_name.SetCompilerType(
-  ClangASTContext::GetTypeForDecl(decls[0]));
+  if (auto *vendor = GetDeclVendor()) {
+auto types = vendor->FindTypes(class_name, /*max_matches*/ 1);
+if (!types.empty())
+  class_type_or_name.SetCompilerType(types.front());
   }
 }
   }
Index: source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -932,25 +932,16 @@
ResultSet &results) override {
   bool result = false;
 
-  Process *process = exe_scope->CalculateProcess().get();
-  if (process) {
-auto objc_runtime = ObjCLanguageRuntime::Get(*process);
-if (objc_runtime) {
-  auto decl_vendor = objc_runtime->GetDeclVendo

[Lldb-commits] [PATCH] D63853: [Symbol] Add DeclVendor::FindTypes

[Jim Ingham via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D63853



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

[Lldb-commits] [PATCH] D64013: Correctly use GetLoadedModuleList to take advantage of libraries-svr4

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

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

Address comments, add GetAction that returns an enum with what to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64013

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -200,9 +200,9 @@
 
   llvm::VersionTuple GetHostOSVersion() override;
 
-  size_t LoadModules(LoadedModuleInfoList &module_list) override;
+  llvm::Error LoadModules() override;
 
-  size_t LoadModules() override;
+  llvm::Expected GetLoadedModuleList() override;
 
   Status GetFileLoadAddress(const FileSpec &file, bool &is_loaded,
 lldb::addr_t &load_addr) override;
@@ -391,9 +391,6 @@
   // Query remote GDBServer for register information
   bool GetGDBServerRegisterInfo(ArchSpec &arch);
 
-  // Query remote GDBServer for a detailed loaded library list
-  Status GetLoadedModuleList(LoadedModuleInfoList &);
-
   lldb::ModuleSP LoadModuleAtAddress(const FileSpec &file,
  lldb::addr_t link_map,
  lldb::addr_t base_addr,
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2728,9 +2728,9 @@
 
   // the loaded module list can also provides a link map address
   if (addr == LLDB_INVALID_ADDRESS) {
-LoadedModuleInfoList list;
-if (GetLoadedModuleList(list).Success())
-  addr = list.m_link_map;
+llvm::Expected list = GetLoadedModuleList();
+if (list)
+  addr = list->m_link_map;
   }
 
   return addr;
@@ -4671,28 +4671,29 @@
   return m_register_info.GetNumRegisters() > 0;
 }
 
-Status ProcessGDBRemote::GetLoadedModuleList(LoadedModuleInfoList &list) {
+llvm::Expected ProcessGDBRemote::GetLoadedModuleList() {
   // Make sure LLDB has an XML parser it can use first
   if (!XMLDocument::XMLEnabled())
-return Status(0, ErrorType::eErrorTypeGeneric);
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "XML parsing not available");
 
   Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS);
   if (log)
 log->Printf("ProcessGDBRemote::%s", __FUNCTION__);
 
+  LoadedModuleInfoList list;
   GDBRemoteCommunicationClient &comm = m_gdb_comm;
 
   // check that we have extended feature read support
   if (comm.GetQXferLibrariesSVR4ReadSupported()) {
-list.clear();
-
 // request the loaded library list
 std::string raw;
 lldb_private::Status lldberr;
 
 if (!comm.ReadExtFeature(ConstString("libraries-svr4"), ConstString(""),
  raw, lldberr))
-  return Status(0, ErrorType::eErrorTypeGeneric);
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Error in libraries-svr4 packet");
 
 // parse the xml file in memory
 if (log)
@@ -4700,11 +4701,14 @@
 XMLDocument doc;
 
 if (!doc.ParseMemory(raw.c_str(), raw.size(), "noname.xml"))
-  return Status(0, ErrorType::eErrorTypeGeneric);
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Error reading noname.xml");
 
 XMLNode root_element = doc.GetRootElement("library-list-svr4");
 if (!root_element)
-  return Status();
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Error finding library-list-svr4 xml element");
 
 // main link map structure
 llvm::StringRef main_lm = root_element.GetAttributeValue("main-lm");
@@ -4770,27 +4774,29 @@
 if (log)
   log->Printf("found %" PRId32 " modules in total",
   (int)list.m_list.size());
+return list;
   } else if (comm.GetQXferLibrariesReadSupported()) {
-list.clear();
-
 // request the loaded library list
 std::string raw;
 lldb_private::Status lldberr;
 
 if (!comm.ReadExtFeature(ConstString("libraries"), ConstString(""), raw,
  lldberr))
-  return Status(0, ErrorType::eErrorTypeGeneric);
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Error in libraries packet");
 
 if (log)
   log->Printf("parsing: %s", raw.c_str());
 XMLDocument doc;