[Lldb-commits] [PATCH] D77765: Fix incorrect L1 inferior memory cache flushing

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I am not a pro at the gtest stuff, but this was very hard to follow and figure 
out what these tests are doing.




Comment at: lldb/source/Target/Memory.cpp:23-24
 // MemoryCache constructor
-MemoryCache::MemoryCache(Process )
+MemoryCache::MemoryCache(MemoryFromInferiorReader ,
+ uint64_t cache_line_size)
 : m_mutex(), m_L1_cache(), m_L2_cache(), m_invalid_ranges(),

Might be cleaner to add getting the cache line size from the 
MemoryFromInferiorReader as a second virtual function. This would avoid having 
to add the extra parameter here and to the clear method.



Comment at: lldb/source/Target/Memory.cpp:31
 
-void MemoryCache::Clear(bool clear_invalid_ranges) {
+void MemoryCache::Clear(uint64_t cache_line_size, bool clear_invalid_ranges) {
   std::lock_guard guard(m_mutex);

remove "cache_line_size" if we add a new virtual function to 
MemoryFromInferiorReader



Comment at: lldb/source/Target/Memory.cpp:37
 m_invalid_ranges.Clear();
-  m_L2_cache_line_byte_size = m_process.GetMemoryCacheLineSize();
+  m_L2_cache_line_byte_size = cache_line_size;
 }

Change to:
```
m_L2_cache_line_byte_size = m_reader.GetCacheLineSize();
```
if we add virtual function to MemoryFromInferiorReader




Comment at: lldb/source/Target/Memory.cpp:65-66
+  // intersecting.
+  BlockMap::iterator previous = pos;
+  previous--;
+  AddrRange previous_range(previous->first,

This can just be:

```
BlockMap::iterator previous = pos - 1;
```



Comment at: lldb/source/Target/Memory.cpp:67-68
+  previous--;
+  AddrRange previous_range(previous->first,
+   previous->second->GetByteSize());
+  if (previous_range.DoesIntersect(flush_range))

name "chunk_range" like in the while loop below for consistency?



Comment at: lldb/source/Target/Process.cpp:488
   m_profile_data_comm_mutex(), m_profile_data(), m_iohandler_sync(0),
-  m_memory_cache(*this), m_allocated_memory_cache(*this),
-  m_should_detach(false), m_next_event_action_up(), m_public_run_lock(),
-  m_private_run_lock(), m_finalizing(false), m_finalize_called(false),
+  m_memory_cache(*this, GetMemoryCacheLineSize()),
+  m_allocated_memory_cache(*this), m_should_detach(false),

remove GetMemoryCacheLineSize() if we add virtual method to 
MemoryFromInferiorReader



Comment at: lldb/source/Target/Process.cpp:612
   m_image_tokens.clear();
-  m_memory_cache.Clear();
+  m_memory_cache.Clear(GetMemoryCacheLineSize());
   m_allocated_memory_cache.Clear();

remove GetMemoryCacheLineSize() if we add virtual method to 
MemoryFromInferiorReader



Comment at: lldb/source/Target/Process.cpp:1456
 m_mod_id.SetStopEventForLastNaturalStopID(event_sp);
-  m_memory_cache.Clear();
+  m_memory_cache.Clear(GetMemoryCacheLineSize());
   LLDB_LOGF(log, "Process::SetPrivateState (%s) stop_id = %u",

remove GetMemoryCacheLineSize() if we add virtual method to 
MemoryFromInferiorReader



Comment at: lldb/source/Target/Process.cpp:5575
   m_thread_list.DiscardThreadPlans();
-  m_memory_cache.Clear(true);
+  m_memory_cache.Clear(GetMemoryCacheLineSize(), true);
   DoDidExec();

remove GetMemoryCacheLineSize() if we add virtual method to 
MemoryFromInferiorReader



Comment at: lldb/unittests/Target/MemoryCacheTest.cpp:32-69
+MemoryCacheTest::MemoryCacheTest()
+: m_cache(*this, k_cache_line_size), m_inferior_read_count(0) {
+  for (size_t i = 0; i < k_memory_size; i++)
+m_memory.push_back(static_cast(i));
+}
+
+void MemoryCacheTest::AddRangeToL1Cache(lldb::addr_t addr, size_t len) {

Just inline these to avoid having to have a declaration inside the class and 
then these functions outside of the class?



Comment at: lldb/unittests/Target/MemoryCacheTest.cpp:34-35
+: m_cache(*this, k_cache_line_size), m_inferior_read_count(0) {
+  for (size_t i = 0; i < k_memory_size; i++)
+m_memory.push_back(static_cast(i));
+}

Comment maybe saying something like:

```
// Fill memory from [0x0 - 0x256) with byte values that match the index. We 
will use this memory in each test and each test will start with a fresh copy.
```



Comment at: lldb/unittests/Target/MemoryCacheTest.cpp:75-76
+
+  m_cache.Read(10, result.data(), result.size(), error);
+  ASSERT_EQ(10, result[0]);
+

So we are reading 10 bytes, but only verifying the first byte in the 10 bytes 
we read? Why not verify all of them to be sure it worked with a for loop?

```
for (int i=0; i p : cached)
+AddRangeToL1Cache(p.first, p.second);
+

seems weird to use AddRangeToL1Cache instead of 

[Lldb-commits] [PATCH] D77588: [lldb/Reproducers] Make it possible to capture reproducers for the Python test suite.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D77588#1968700 , @labath wrote:

> I think the code looks ok now, though I have to say I am still somewhat fuzzy 
> on what exactly will this patch enable. I think I understand the capture 
> part, but what happens during replay? Will that already be smart enough to 
> match the SB calls made by the test harness during replay to the captured 
> calls that were made during recording, or is there some more work needed 
> there?


That's implemented in https://reviews.llvm.org/D77602

> Could you add a brief documentation on this somewhere?

Sure thing: https://reviews.llvm.org/D1


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D77588



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

[Lldb-commits] [PATCH] D77771: [lldb/Docs] Document driver and inline replay.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: labath.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D1

Files:
  lldb/docs/resources/reproducers.rst


Index: lldb/docs/resources/reproducers.rst
===
--- lldb/docs/resources/reproducers.rst
+++ lldb/docs/resources/reproducers.rst
@@ -93,6 +93,68 @@
 
 Coming soon.
 
+Driver Replay
+`
+
+No matter how a reproducer was captured, they can always be replayed with the
+command line driver. When a reproducer is passed with the `--replay` flag, the
+driver short-circuits and passes off control to the reproducer infrastructure,
+effectively bypassing its normal operation. This works because the driver is
+implemented using the SB API and is therefore nothing more than a sequence of
+SB API calls.
+
+Replay is driven by the ``Registry::Replay``. As long as there's data in the
+buffer holding the API data, the next SB API function call is deserialized.
+Once the function is known, the registry can retrieve its signature, and use
+that to deserialize its arguments. The function can then be invoked, most
+commonly through the synthesized default replayer, or potentially using a
+custom defined replay function. This process continues, until more data is
+available or a replay error is encountered.
+
+During replay only a function's side effects matter. The result returned by the
+replayed function is ignored because it cannot be observed beyond the driver.
+This is sound, because anything that is passed into a subsequent API call will
+have been serialized as an input argument. This also works for SB API objects
+because the reproducers know about every object that has crossed the API
+boundary, which is true by definition for object return values.
+
+Driver replay can be considered active, because the reproducers are replaying
+the SB API functions one after each other.
+
+Inline Replay
+`
+
+Inline replay was added to the reproducers to support running the API test
+suite against a reproducer. The API test suite is written in Python and tests
+the debugger by calling into its API from Python. To make this work, the API
+must transparently replay itself when called. This is what makes inline replay
+different from driver replay. For the latter, it is lldb itself that's driving
+replay. For inline replay, the driving factor is external, e.g. the test suite.
+
+In order to replay API calls, the reproducers need a way to intercept them.
+Every API call is already instrumented with an ``LLDB_RECORD_*`` macro that
+captures its input arguments. Furthermore, it also contains the necessary logic
+to detect which calls cross the API boundary and should be intercepted. We were
+able to reuse all of this to implement inline replay.
+
+When inline replay is enabled, nothing happens until an API is called. Inside
+that API function, the macro will detect whether this call should be replayed
+(i.e. crossed the API boundary). If the answer is yes, the next function is
+deserialized from the SB API data and compared to the current function. If the
+signature matches, we deserialize its input arguments and reinvoke the current
+function with the deserialized arguments. We don't need to do anything special
+to prevent us from recursively calling the replayed version again, as the API
+boundary crossing logic knows that we're still behind the API boundary when we
+re-invoked the current function.
+
+Another big difference with driver replay is the return value. While this
+didn't matter for driver replay, it's key for inline replay, because that's
+what gets checked by the test suite. Luckily, the ``LLDB_RECORD_*`` macros
+contained sufficient type information to derive the result type.
+
+Inline replay can be considered passive, because a function is only replayed
+once it is called through the API.
+
 Testing
 ---
 


Index: lldb/docs/resources/reproducers.rst
===
--- lldb/docs/resources/reproducers.rst
+++ lldb/docs/resources/reproducers.rst
@@ -93,6 +93,68 @@
 
 Coming soon.
 
+Driver Replay
+`
+
+No matter how a reproducer was captured, they can always be replayed with the
+command line driver. When a reproducer is passed with the `--replay` flag, the
+driver short-circuits and passes off control to the reproducer infrastructure,
+effectively bypassing its normal operation. This works because the driver is
+implemented using the SB API and is therefore nothing more than a sequence of
+SB API calls.
+
+Replay is driven by the ``Registry::Replay``. As long as there's data in the
+buffer holding the API data, the next SB API function call is deserialized.
+Once the function is known, the registry can retrieve its signature, and use
+that to deserialize its arguments. The function can then be invoked, most
+commonly through the synthesized default replayer, or potentially using a
+custom 

[Lldb-commits] [lldb] e7db1ae - [lldb/Docs] Elaborate on reproducer testing

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-08T20:10:52-07:00
New Revision: e7db1aec3bdb833832056894f5eba2f359a7c384

URL: 
https://github.com/llvm/llvm-project/commit/e7db1aec3bdb833832056894f5eba2f359a7c384
DIFF: 
https://github.com/llvm/llvm-project/commit/e7db1aec3bdb833832056894f5eba2f359a7c384.diff

LOG: [lldb/Docs] Elaborate on reproducer testing

Added: 


Modified: 
lldb/docs/resources/reproducers.rst

Removed: 




diff  --git a/lldb/docs/resources/reproducers.rst 
b/lldb/docs/resources/reproducers.rst
index c20c29892978..3475ad5e7197 100644
--- a/lldb/docs/resources/reproducers.rst
+++ b/lldb/docs/resources/reproducers.rst
@@ -98,25 +98,28 @@ Testing
 
 Reproducers are tested in the following ways:
 
- - Unit tests to cover the generic reproducer infrastructure.
- - Focused test cases in the ``test/Shell/Reproducer`` directory. These tests
-   serve as integration tests for the reproducers infrastructure, as well as
-   doing some sanity checking for basic debugger functionality.
-
-Additional testing is possible:
-
- - It's possible to unconditionally capture reproducers while running the
-   entire test suite by setting the ``LLDB_CAPTURE_REPRODUCER`` environment
-   variable. Assuming the reproducers behave correctly, this can also help to
-   reproduce test failures.
- - It's possible to run the shell tests from a reproducer replay. The
+ - Unit tests to cover the reproducer infrastructure. There are tests for the
+   provider, loader and for the reproducer instrumentation.
+ - Feature specific end-to-end test cases in the ``test/Shell/Reproducer``
+   directory. These tests serve as integration and regression tests for the
+   reproducers infrastructure, as well as doing some sanity checking for basic
+   debugger functionality.
+ - The shell tests can be run against a reproducer replay. The
``check-lldb-repro`` target will run the shell test suite twice. First it
runs the test suite and captures a reproducer for every lldb invocation and
saves it to a known location based off lldb's arguments and  working
directory. Then it runs the test suite again, this time replaying the
reproducers. Certain tests do not fit this paradigm (for example test that
-   check the output of the binary being debugged) and can be skipped by setting
-   ``UNSUPPORTED: lldb-repro`` at the top of the test.
+   check the output of the binary being debugged) and are skipped by marking
+   them as unsupported by adding ``UNSUPPORTED: lldb-repro`` to the top of the
+   test.
+
+Additional testing is possible:
+
+ - It's possible to unconditionally capture reproducers while running the
+   entire test suite by setting the ``LLDB_CAPTURE_REPRODUCER`` environment
+   variable. Assuming no bugs in reproducers, this can also help to reproduce
+   and investigate test failures.
 
 Knows Issues
 



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

[Lldb-commits] [PATCH] D77765: Fix incorrect L1 inferior memory cache flushing

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin created this revision.
jarin added reviewers: labath, clayborg.
jarin added a project: LLDB.
Herald added subscribers: lldb-commits, mgorny.
jarin retitled this revision from "Fix incorrect L1 cache flushing" to "Fix 
incorrect L1 inferior memory cache flushing".

As discussed in https://reviews.llvm.org/D74398, the L1 
 memory cache flushing is incorrect.

For instance, if the L1  cache contains two chunks 
(10, 10) and (30, 10) and we call MemoryCache::Flush(25, 10), the current code 
does not flush anything (because it just tries to flush the previous range (10, 
10) and if that is not intersecting, it will bail out).

With this patch, if the previous chunk is not overlapping, we still try the 
next chunk, and only if that one is not overlapping, we bail out.

This also adds some unit tests for the cache (some of the tests fail with the 
current code). The unit tests required some changes for testability.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77765

Files:
  lldb/include/lldb/Target/Memory.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Target/Memory.cpp
  lldb/source/Target/Process.cpp
  lldb/unittests/Target/CMakeLists.txt
  lldb/unittests/Target/MemoryCacheTest.cpp

Index: lldb/unittests/Target/MemoryCacheTest.cpp
===
--- /dev/null
+++ lldb/unittests/Target/MemoryCacheTest.cpp
@@ -0,0 +1,144 @@
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "lldb/Target/Memory.h"
+#include "lldb/Utility/Status.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+namespace {
+
+class MemoryCacheTest : public MemoryFromInferiorReader, public testing::Test {
+  static const size_t k_memory_size = 256;
+  static const uint64_t k_cache_line_size = 16;
+
+  size_t ReadMemoryFromInferior(lldb::addr_t vm_addr, void *buf, size_t size,
+Status ) override;
+
+public:
+  MemoryCacheTest();
+
+  void AddRangeToL1Cache(lldb::addr_t addr, size_t len);
+  void IncrementAndFlushRange(lldb::addr_t addr, size_t len);
+  std::vector ReadByBytes(lldb::addr_t addr, size_t len);
+
+protected:
+  MemoryCache m_cache;
+  std::vector m_memory;
+  size_t m_inferior_read_count;
+};
+
+MemoryCacheTest::MemoryCacheTest()
+: m_cache(*this, k_cache_line_size), m_inferior_read_count(0) {
+  for (size_t i = 0; i < k_memory_size; i++)
+m_memory.push_back(static_cast(i));
+}
+
+void MemoryCacheTest::AddRangeToL1Cache(lldb::addr_t addr, size_t len) {
+  m_cache.AddL1CacheData(addr, m_memory.data() + addr, len);
+}
+
+size_t MemoryCacheTest::ReadMemoryFromInferior(lldb::addr_t vm_addr, void *buf,
+   size_t size, Status ) {
+  m_inferior_read_count++;
+
+  if (vm_addr >= m_memory.size())
+return 0;
+
+  size = std::min(size, m_memory.size() - vm_addr);
+  memcpy(buf, m_memory.data() + vm_addr, size);
+  return size;
+}
+
+void MemoryCacheTest::IncrementAndFlushRange(lldb::addr_t addr, size_t len) {
+  for (size_t i = 0; i < len; i++)
+m_memory[addr + i]++;
+  m_cache.Flush(addr, len);
+}
+
+std::vector MemoryCacheTest::ReadByBytes(lldb::addr_t addr,
+  size_t len) {
+  std::vector result(len, 0);
+  for (size_t i = 0; i < len; i++) {
+Status error;
+m_cache.Read(addr + i, &(result[i]), 1, error);
+EXPECT_TRUE(error.Success());
+  }
+  return result;
+}
+
+TEST_F(MemoryCacheTest, L1FlushSimple) {
+  Status error;
+  std::vector result(10);
+
+  m_cache.Read(10, result.data(), result.size(), error);
+  ASSERT_EQ(10, result[0]);
+
+  IncrementAndFlushRange(10, 1);
+
+  m_cache.Read(10, result.data(), result.size(), error);
+  ASSERT_EQ(m_memory[10], result[0]);
+}
+
+class MemoryCacheOverlapTest
+: public MemoryCacheTest,
+  public testing::WithParamInterface> {};
+
+TEST_P(MemoryCacheOverlapTest, L1FlushFromTwoWithOverlap) {
+  // Add two ranges to the cache.
+  std::vector> cached = {{10, 10}, {30, 10}};
+  for (std::pair p : cached)
+AddRangeToL1Cache(p.first, p.second);
+
+  // Flush the range given by the parameter.
+  IncrementAndFlushRange(GetParam().first, GetParam().second);
+
+  // Check the memory.
+  size_t check_size = 50;
+  std::vector result = ReadByBytes(0, check_size);
+  EXPECT_THAT(result, ::testing::ElementsAreArray(m_memory.data(), check_size));
+}
+
+INSTANTIATE_TEST_CASE_P(
+AllMemoryCacheOverlapTest, MemoryCacheOverlapTest,
+::testing::Values(
+std::make_pair(5, 6), std::make_pair(5, 10), std::make_pair(5, 20),
+std::make_pair(5, 30), std::make_pair(5, 40), std::make_pair(10, 1),
+std::make_pair(10, 21), std::make_pair(19, 1), std::make_pair(19, 11),
+std::make_pair(19, 12), std::make_pair(20, 11), std::make_pair(20, 25),
+std::make_pair(29, 2), std::make_pair(29, 12), std::make_pair(30, 1),
+std::make_pair(39, 1), 

[Lldb-commits] [PATCH] D77662: [lldb/test] Make TestLoadUnload compatible with windows

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/make/Makefile.rules:477
 ifeq (1,$(USE_LIBDL))
-   ifneq ($(OS),NetBSD)
+   ifeq (,$(filter $(OS), NetBSD Windows_NT))
LDFLAGS += -ldl

I'm no expert in makefile syntax (especially not for Unix-y versions), but this 
looks weird, with the leading comma and the matching parenthesis for `$(filter` 
sitting near the end of the expression.  Is this right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77662



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

[Lldb-commits] [PATCH] D77661: [lldb/Python] Add lldbconfig Python module to make the lldb module configurable.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb0bdaf9ba2bf: [lldb/Python] Add lldbconfig module to make 
the lldb module configurable (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D77661?vs=255792=256140#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77661

Files:
  lldb/bindings/python.swig
  lldb/packages/Python/lldbconfig/__init__.py
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -954,7 +954,9 @@
 
 setupSysPath()
 
+import lldbconfig
 import lldb
+
 # Use host platform by default.
 lldb.selected_platform = lldb.SBPlatform.GetHostPlatform()
 
Index: lldb/packages/Python/lldbconfig/__init__.py
===
--- /dev/null
+++ lldb/packages/Python/lldbconfig/__init__.py
@@ -0,0 +1 @@
+INITIALIZE = True
Index: lldb/bindings/python.swig
===
--- lldb/bindings/python.swig
+++ lldb/bindings/python.swig
@@ -128,8 +128,15 @@
 %include "./python/python-wrapper.swig"
 
 %pythoncode%{
+_initialize = True
+try:
+   import lldbconfig
+   _initialize = lldbconfig.INITIALIZE
+except ImportError:
+   pass
 debugger_unique_id = 0
-SBDebugger.Initialize()
+if _initialize:
+   SBDebugger.Initialize()
 debugger = None
 target = None
 process = None


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -954,7 +954,9 @@
 
 setupSysPath()
 
+import lldbconfig
 import lldb
+
 # Use host platform by default.
 lldb.selected_platform = lldb.SBPlatform.GetHostPlatform()
 
Index: lldb/packages/Python/lldbconfig/__init__.py
===
--- /dev/null
+++ lldb/packages/Python/lldbconfig/__init__.py
@@ -0,0 +1 @@
+INITIALIZE = True
Index: lldb/bindings/python.swig
===
--- lldb/bindings/python.swig
+++ lldb/bindings/python.swig
@@ -128,8 +128,15 @@
 %include "./python/python-wrapper.swig"
 
 %pythoncode%{
+_initialize = True
+try:
+   import lldbconfig
+   _initialize = lldbconfig.INITIALIZE
+except ImportError:
+   pass
 debugger_unique_id = 0
-SBDebugger.Initialize()
+if _initialize:
+   SBDebugger.Initialize()
 debugger = None
 target = None
 process = None
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] b0bdaf9 - [lldb/Python] Add lldbconfig module to make the lldb module configurable

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-08T15:59:24-07:00
New Revision: b0bdaf9ba2bfa9e099c7cb650650133f6ea2024f

URL: 
https://github.com/llvm/llvm-project/commit/b0bdaf9ba2bfa9e099c7cb650650133f6ea2024f
DIFF: 
https://github.com/llvm/llvm-project/commit/b0bdaf9ba2bfa9e099c7cb650650133f6ea2024f.diff

LOG: [lldb/Python] Add lldbconfig module to make the lldb module configurable

Using the approach suggested by Pavel in D77588, this patch introduces a
new lldbconfig module that lives next to the lldb module. It makes it
possible to make the lldb module configurable before importing it. More
specifically it makes it possible to delay initializing the debugger,
which is needed for testing the reproducer.

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

Added: 
lldb/packages/Python/lldbconfig/__init__.py

Modified: 
lldb/bindings/python.swig
lldb/packages/Python/lldbsuite/test/dotest.py

Removed: 




diff  --git a/lldb/bindings/python.swig b/lldb/bindings/python.swig
index b086d436e57b..5b1269878dac 100644
--- a/lldb/bindings/python.swig
+++ b/lldb/bindings/python.swig
@@ -128,8 +128,15 @@ using namespace lldb;
 %include "./python/python-wrapper.swig"
 
 %pythoncode%{
+_initialize = True
+try:
+   import lldbconfig
+   _initialize = lldbconfig.INITIALIZE
+except ImportError:
+   pass
 debugger_unique_id = 0
-SBDebugger.Initialize()
+if _initialize:
+   SBDebugger.Initialize()
 debugger = None
 target = None
 process = None

diff  --git a/lldb/packages/Python/lldbconfig/__init__.py 
b/lldb/packages/Python/lldbconfig/__init__.py
new file mode 100644
index ..6c43d709df71
--- /dev/null
+++ b/lldb/packages/Python/lldbconfig/__init__.py
@@ -0,0 +1 @@
+INITIALIZE = True

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 4e86d1a59322..31c617c1f311 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -954,7 +954,9 @@ def run_suite():
 
 setupSysPath()
 
+import lldbconfig
 import lldb
+
 # Use host platform by default.
 lldb.selected_platform = lldb.SBPlatform.GetHostPlatform()
 



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

[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix inline replay for functions returning string as (char*, size_t).

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: labath.
Herald added a subscriber: abidh.
JDevlieghere added a parent revision: D77602: [lldb/Reproducers] Support inline 
replay.

Several SB API functions return strings a `(char*, size_t)` pair. During 
capture, we serialize an empty string for the `char*` because the memory can be 
uninitialized.  During replay, we have custom replay redirects that ensure that 
we don't override the buffer from which we're reading, but rather a buffer on 
the heap with the given length. This is sufficient for the "regular" reproducer 
use case, where we only care about the side effects of the API calls, not the 
actual return values.

This is not sufficient for inline replay. For inline replay, we ignore all the 
incoming arguments, and re-execute the current function with the arguments 
serialized in the reproducer. This means that these function will update the 
deserialized copy of the arguments, rather than whatever was passed in by the 
SWIG wrapper. To solve this problem, I've extended the reproducer 
instrumentation with special case replayers and corresponding macros. They 
ignore the replayer in the registry and the incoming char pointer, and instead 
reinvoke the current method on the deserialized class, and populate the output 
argument. It's unfortunate that this needs to be special cased, but I don't see 
a better solution.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D77759

Files:
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBFileSpec.cpp
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBStructuredData.cpp
  lldb/source/API/SBThread.cpp

Index: lldb/source/API/SBThread.cpp
===
--- lldb/source/API/SBThread.cpp
+++ lldb/source/API/SBThread.cpp
@@ -312,8 +312,8 @@
 }
 
 size_t SBThread::GetStopDescription(char *dst, size_t dst_len) {
-  LLDB_RECORD_METHOD(size_t, SBThread, GetStopDescription, (char *, size_t), "",
- dst_len);
+  LLDB_RECORD_CHAR_PTR_METHOD(size_t, SBThread, GetStopDescription,
+  (char *, size_t), dst, "", dst_len);
 
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
Index: lldb/source/API/SBStructuredData.cpp
===
--- lldb/source/API/SBStructuredData.cpp
+++ lldb/source/API/SBStructuredData.cpp
@@ -196,8 +196,8 @@
 }
 
 size_t SBStructuredData::GetStringValue(char *dst, size_t dst_len) const {
-  LLDB_RECORD_METHOD_CONST(size_t, SBStructuredData, GetStringValue,
-   (char *, size_t), "", dst_len);
+  LLDB_RECORD_CHAR_PTR_METHOD_CONST(size_t, SBStructuredData, GetStringValue,
+(char *, size_t), dst, "", dst_len);
 
   return (m_impl_up ? m_impl_up->GetStringValue(dst, dst_len) : 0);
 }
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -271,8 +271,8 @@
 }
 
 size_t SBProcess::GetSTDOUT(char *dst, size_t dst_len) const {
-  LLDB_RECORD_METHOD_CONST(size_t, SBProcess, GetSTDOUT, (char *, size_t), "",
-   dst_len);
+  LLDB_RECORD_CHAR_PTR_METHOD_CONST(size_t, SBProcess, GetSTDOUT,
+(char *, size_t), dst, "", dst_len);
 
   size_t bytes_read = 0;
   ProcessSP process_sp(GetSP());
@@ -285,8 +285,8 @@
 }
 
 size_t SBProcess::GetSTDERR(char *dst, size_t dst_len) const {
-  LLDB_RECORD_METHOD_CONST(size_t, SBProcess, GetSTDERR, (char *, size_t), "",
-   dst_len);
+  LLDB_RECORD_CHAR_PTR_METHOD_CONST(size_t, SBProcess, GetSTDERR,
+(char *, size_t), dst, "", dst_len);
 
   size_t bytes_read = 0;
   ProcessSP process_sp(GetSP());
@@ -299,8 +299,8 @@
 }
 
 size_t SBProcess::GetAsyncProfileData(char *dst, size_t dst_len) const {
-  LLDB_RECORD_METHOD_CONST(size_t, SBProcess, GetAsyncProfileData,
-   (char *, size_t), "", dst_len);
+  LLDB_RECORD_CHAR_PTR_METHOD_CONST(size_t, SBProcess, GetAsyncProfileData,
+(char *, size_t), dst, "", dst_len);
 
   size_t bytes_read = 0;
   ProcessSP process_sp(GetSP());
Index: lldb/source/API/SBFileSpec.cpp
===
--- lldb/source/API/SBFileSpec.cpp
+++ lldb/source/API/SBFileSpec.cpp
@@ -143,8 +143,8 @@
 }
 
 uint32_t SBFileSpec::GetPath(char *dst_path, size_t dst_len) const {
-  LLDB_RECORD_METHOD_CONST(uint32_t, SBFileSpec, GetPath, (char *, size_t), "",
-   dst_len);
+  LLDB_RECORD_CHAR_PTR_METHOD_CONST(uint32_t, SBFileSpec, GetPath,
+(char *, size_t), dst_path, "", dst_len);
 
   uint32_t result = m_opaque_up->GetPath(dst_path, dst_len);
 
Index: 

[Lldb-commits] [lldb] ff1658b - Fix -Wdeprecated-copy warning in XcodeSDK.

[Eric Christopher via lldb-commits]

Author: Eric Christopher
Date: 2020-04-08T15:12:53-07:00
New Revision: ff1658b167c835ca55f554a3ad5aac444a6f9c9c

URL: 
https://github.com/llvm/llvm-project/commit/ff1658b167c835ca55f554a3ad5aac444a6f9c9c
DIFF: 
https://github.com/llvm/llvm-project/commit/ff1658b167c835ca55f554a3ad5aac444a6f9c9c.diff

LOG: Fix -Wdeprecated-copy warning in XcodeSDK.

Added: 


Modified: 
lldb/include/lldb/Utility/XcodeSDK.h

Removed: 




diff  --git a/lldb/include/lldb/Utility/XcodeSDK.h 
b/lldb/include/lldb/Utility/XcodeSDK.h
index 9b9f1d226bf0..b186ab4a7091 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -45,6 +45,7 @@ class XcodeSDK {
   void Merge(XcodeSDK other);
 
   XcodeSDK =(XcodeSDK other);
+  XcodeSDK(const XcodeSDK&) = default;
   bool operator==(XcodeSDK other);
 
   /// Return parsed SDK number, and SDK version number.



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

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

I don't have commit access, can someone with access commit all three approved 
CLs in this stack?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806



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

[Lldb-commits] [PATCH] D77587: [SVE] Add new VectorType subclasses

[Christopher Tetreault via Phabricator via lldb-commits]

ctetreau updated this revision to Diff 256100.
ctetreau added a comment.
Herald added subscribers: lldb-commits, frgossen, grosul1, Joonsoo, kerbowa, 
liufengdb, lucyrfox, mgester, arpith-jacob, csigg, nicolasvasilache, 
antiagainst, shauheen, burmako, jpienaar, rriddle, mehdi_amini, nhaehnle, 
jvesely, arsenm, jholewinski.
Herald added a project: LLDB.

adddress code review issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77587

Files:
  lldb/source/Expression/IRInterpreter.cpp
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/DerivedTypes.h
  llvm/include/llvm/IR/Type.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/ExecutionEngine/ExecutionEngine.cpp
  llvm/lib/ExecutionEngine/Interpreter/Execution.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/IR/Type.cpp
  llvm/lib/Linker/IRMover.cpp
  llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
  llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
  llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
  llvm/lib/Transforms/IPO/GlobalOpt.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

Index: mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
===
--- mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -168,12 +168,12 @@
   return nullptr;
 return LLVMType::getArrayTy(elementType, type->getArrayNumElements());
   }
-  case llvm::Type::VectorTyID: {
-auto *typeVTy = llvm::cast(type);
-if (typeVTy->isScalable()) {
-  emitError(unknownLoc) << "scalable vector types not supported";
-  return nullptr;
-}
+  case llvm::Type::ScalableVectorTyID: {
+emitError(unknownLoc) << "scalable vector types not supported";
+return nullptr;
+  }
+  case llvm::Type::FixedVectorTyID: {
+auto *typeVTy = llvm::cast(type);
 LLVMType elementType = processType(typeVTy->getElementType());
 if (!elementType)
   return nullptr;
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -483,7 +483,8 @@
   return cmpNumbers(STyL->getNumElements(), STyR->getNumElements());
 return cmpTypes(STyL->getElementType(), STyR->getElementType());
   }
-  case Type::VectorTyID: {
+  case Type::FixedVectorTyID:
+  case Type::ScalableVectorTyID: {
 auto *STyL = cast(TyL);
 auto *STyR = cast(TyR);
 if (STyL->getElementCount().Scalable != STyR->getElementCount().Scalable)
Index: llvm/lib/Transforms/IPO/GlobalOpt.cpp
===
--- llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -130,7 +130,8 @@
   default: break;
   case Type::PointerTyID:
 return true;
-  case Type::VectorTyID:
+  case Type::FixedVectorTyID:
+  case Type::ScalableVectorTyID:
 if (cast(Ty)->getElementType()->isPointerTy())
   return true;
 break;
Index: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
===
--- llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
+++ llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
@@ -1184,7 +1184,7 @@
 case Type::IntegerTyID: // Integers larger than 64 bits
 case Type::StructTyID:
 case Type::ArrayTyID:
-case Type::VectorTyID:
+case Type::FixedVectorTyID:
   ElementSize = DL.getTypeStoreSize(ETy);
   // Ptx allows variable initilization only for constant and
   // global state spaces.
@@ -1358,7 +1358,7 @@
   switch (ETy->getTypeID()) {
   case Type::StructTyID:
   case Type::ArrayTyID:
-  case Type::VectorTyID:
+  case Type::FixedVectorTyID:
 ElementSize = DL.getTypeStoreSize(ETy);
 O << " .b8 ";
 getSymbol(GVar)->print(O, MAI);
@@ -1892,7 +1892,7 @@
   }
 
   case Type::ArrayTyID:
-  case Type::VectorTyID:
+  case Type::FixedVectorTyID:
   case Type::StructTyID: {
 if (isa(CPV) || isa(CPV)) {
   int ElementSize = DL.getTypeAllocSize(CPV->getType());
Index: llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp
===
--- llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp
+++ llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp
@@ -307,7 +307,7 @@
 const ArrayType *ATy = cast(Ty);
 return getSmallestAddressableSize(ATy->getElementType(), GV, TM);
   }
-  case Type::VectorTyID: {
+  case Type::FixedVectorTyID: {
 const VectorType *PTy = cast(Ty);
 return getSmallestAddressableSize(PTy->getElementType(), GV, TM);
   }

[Lldb-commits] [PATCH] D77587: [SVE] Add new VectorType subclasses

[Christopher Tetreault via Phabricator via lldb-commits]

ctetreau updated this revision to Diff 256101.
ctetreau added a comment.

Fix permissions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77587

Files:
  lldb/source/Expression/IRInterpreter.cpp
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/DerivedTypes.h
  llvm/include/llvm/IR/Type.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/ExecutionEngine/ExecutionEngine.cpp
  llvm/lib/ExecutionEngine/Interpreter/Execution.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/IR/Type.cpp
  llvm/lib/Linker/IRMover.cpp
  llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
  llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
  llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
  llvm/lib/Transforms/IPO/GlobalOpt.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

Index: mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
===
--- mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -168,12 +168,12 @@
   return nullptr;
 return LLVMType::getArrayTy(elementType, type->getArrayNumElements());
   }
-  case llvm::Type::VectorTyID: {
-auto *typeVTy = llvm::cast(type);
-if (typeVTy->isScalable()) {
-  emitError(unknownLoc) << "scalable vector types not supported";
-  return nullptr;
-}
+  case llvm::Type::ScalableVectorTyID: {
+emitError(unknownLoc) << "scalable vector types not supported";
+return nullptr;
+  }
+  case llvm::Type::FixedVectorTyID: {
+auto *typeVTy = llvm::cast(type);
 LLVMType elementType = processType(typeVTy->getElementType());
 if (!elementType)
   return nullptr;
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -483,7 +483,8 @@
   return cmpNumbers(STyL->getNumElements(), STyR->getNumElements());
 return cmpTypes(STyL->getElementType(), STyR->getElementType());
   }
-  case Type::VectorTyID: {
+  case Type::FixedVectorTyID:
+  case Type::ScalableVectorTyID: {
 auto *STyL = cast(TyL);
 auto *STyR = cast(TyR);
 if (STyL->getElementCount().Scalable != STyR->getElementCount().Scalable)
Index: llvm/lib/Transforms/IPO/GlobalOpt.cpp
===
--- llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -130,7 +130,8 @@
   default: break;
   case Type::PointerTyID:
 return true;
-  case Type::VectorTyID:
+  case Type::FixedVectorTyID:
+  case Type::ScalableVectorTyID:
 if (cast(Ty)->getElementType()->isPointerTy())
   return true;
 break;
Index: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
===
--- llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
+++ llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
@@ -1184,7 +1184,7 @@
 case Type::IntegerTyID: // Integers larger than 64 bits
 case Type::StructTyID:
 case Type::ArrayTyID:
-case Type::VectorTyID:
+case Type::FixedVectorTyID:
   ElementSize = DL.getTypeStoreSize(ETy);
   // Ptx allows variable initilization only for constant and
   // global state spaces.
@@ -1358,7 +1358,7 @@
   switch (ETy->getTypeID()) {
   case Type::StructTyID:
   case Type::ArrayTyID:
-  case Type::VectorTyID:
+  case Type::FixedVectorTyID:
 ElementSize = DL.getTypeStoreSize(ETy);
 O << " .b8 ";
 getSymbol(GVar)->print(O, MAI);
@@ -1892,7 +1892,7 @@
   }
 
   case Type::ArrayTyID:
-  case Type::VectorTyID:
+  case Type::FixedVectorTyID:
   case Type::StructTyID: {
 if (isa(CPV) || isa(CPV)) {
   int ElementSize = DL.getTypeAllocSize(CPV->getType());
Index: llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp
===
--- llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp
+++ llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp
@@ -307,7 +307,7 @@
 const ArrayType *ATy = cast(Ty);
 return getSmallestAddressableSize(ATy->getElementType(), GV, TM);
   }
-  case Type::VectorTyID: {
+  case Type::FixedVectorTyID: {
 const VectorType *PTy = cast(Ty);
 return getSmallestAddressableSize(PTy->getElementType(), GV, TM);
   }
Index: llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
@@ -409,7 +409,7 @@
 Type *ArgType = 

[Lldb-commits] [PATCH] D77587: [SVE] Add new VectorType subclasses

[Christopher Tetreault via Phabricator via lldb-commits]

ctetreau added a comment.

In D77587#1965604 , @efriedma wrote:

> If I'm following correctly, this should apply on its own.  Then you're 
> expecting the following API changes:
>
> 1. getNumElements() will move from VectorType to FixedVectorType.  Existing 
> code will be changed to either cast to FixedVectorType, or switch to using 
> getElementCount().
> 2. Maybe remove the default argument from VectorType::get()
>
>   Does that sound right?


This is basically the plan:

1. Add the new types and functions
2. Fix up usages of getNumElements()
3. move getNumElements() and getMinNumElements() into the derived classes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77587



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

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbe3f8a8e1b95: [commands] Support autorepeat in SBCommands 
(authored by Walter Erquinigo wall...@fb.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77444

Files:
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/unittests/API/CMakeLists.txt
  lldb/unittests/API/TestSBCommandInterpreterTest.cpp
  lldb/unittests/CMakeLists.txt

Index: lldb/unittests/CMakeLists.txt
===
--- lldb/unittests/CMakeLists.txt
+++ lldb/unittests/CMakeLists.txt
@@ -59,6 +59,7 @@
 endfunction()
 
 add_subdirectory(TestingSupport)
+add_subdirectory(API)
 add_subdirectory(Breakpoint)
 add_subdirectory(Core)
 add_subdirectory(DataFormatter)
Index: lldb/unittests/API/TestSBCommandInterpreterTest.cpp
===
--- /dev/null
+++ lldb/unittests/API/TestSBCommandInterpreterTest.cpp
@@ -0,0 +1,138 @@
+//===-- TestSBCommandInterpreterTest.cpp --===//
+//
+// 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 "gtest/gtest.h"
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include 
+#include 
+
+using namespace lldb;
+
+class TestSBCommandInterpreterTest : public testing::Test {
+protected:
+  void SetUp() override {
+SBDebugger::Initialize();
+m_dbg = SBDebugger::Create(/*source_init_files=*/false);
+m_interp = m_dbg.GetCommandInterpreter();
+  }
+
+  SBDebugger m_dbg;
+  SBCommandInterpreter m_interp;
+};
+
+class DummyCommand : public SBCommandPluginInterface {
+public:
+  DummyCommand(const char *message) : m_message(message) {}
+
+  bool DoExecute(SBDebugger dbg, char **command,
+ SBCommandReturnObject ) {
+result.PutCString(m_message.c_str());
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+
+private:
+  std::string m_message;
+};
+
+TEST_F(TestSBCommandInterpreterTest, SingleWordCommand) {
+  // We first test a command without autorepeat
+  DummyCommand dummy("It worked");
+  m_interp.AddCommand("dummy", , /*help=*/nullptr);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("dummy", result, /*add_to_history=*/true);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+EXPECT_FALSE(result.Succeeded());
+EXPECT_STREQ(result.GetError(), "error: No auto repeat.\n");
+  }
+
+  // Now we test a command with autorepeat
+  m_interp.AddCommand("dummy_with_autorepeat", , /*help=*/nullptr,
+  /*syntax=*/nullptr, /*auto_repeat_command=*/nullptr);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("dummy_with_autorepeat", result,
+   /*add_to_history=*/true);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+}
+
+TEST_F(TestSBCommandInterpreterTest, MultiWordCommand) {
+  auto command = m_interp.AddMultiwordCommand("multicommand", /*help=*/nullptr);
+  // We first test a subcommand without autorepeat
+  DummyCommand subcommand("It worked again");
+  command.AddCommand("subcommand", , /*help=*/nullptr);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("multicommand subcommand", result,
+   /*add_to_history=*/true);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked again\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+EXPECT_FALSE(result.Succeeded());
+EXPECT_STREQ(result.GetError(), "error: No auto repeat.\n");
+  }
+
+  // We first test a subcommand with autorepeat
+  command.AddCommand("subcommand_with_autorepeat", ,
+ /*help=*/nullptr, /*syntax=*/nullptr,
+ /*auto_repeat_command=*/nullptr);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("multicommand subcommand_with_autorepeat", result,
+   /*add_to_history=*/true);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked again\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

[Davide Italiano via Phabricator via lldb-commits]

davide closed this revision.
davide added a comment.

commit d51b38f1b3a34c2a8e1869af6434ebd743ce7a5e 
 (HEAD -> 
master, origin/master, origin/HEAD)
Author: Davide Italiano 
Date:   Wed Apr 8 11:06:00 2020 -0700

  [DWARF] Not all the constant variables are "static".
  
  Fixes rdar://problem/61402307
  
  Differential Revision: https://reviews.llvm.org/D77698


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

https://reviews.llvm.org/D77698



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

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

[Jim Ingham via Phabricator via lldb-commits]

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

The code looks good to me.  I'll defer to Adrian as to whether the test is 
robust, though it also seems reasonable to deal with a failure caused by llvm 
changes when it happens.


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

https://reviews.llvm.org/D77698



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

[Lldb-commits] [lldb] d51b38f - [DWARF] Not all the constant variables are "static".

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-04-08T11:07:19-07:00
New Revision: d51b38f1b3a34c2a8e1869af6434ebd743ce7a5e

URL: 
https://github.com/llvm/llvm-project/commit/d51b38f1b3a34c2a8e1869af6434ebd743ce7a5e
DIFF: 
https://github.com/llvm/llvm-project/commit/d51b38f1b3a34c2a8e1869af6434ebd743ce7a5e.diff

LOG: [DWARF] Not all the constant variables are "static".

Fixes rdar://problem/61402307

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/static_scope.s

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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index 746be69a3e12..b089c4e1f04a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -1016,6 +1016,29 @@ DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(const 
DWARFUnit *cu) const {
   return nullptr;
 }
 
+bool DWARFDebugInfoEntry::IsGlobalOrStaticVariable() const {
+  if (Tag() != DW_TAG_variable)
+return false;
+  const DWARFDebugInfoEntry *parent_die = GetParent();
+  while (parent_die != nullptr) {
+switch (parent_die->Tag()) {
+case DW_TAG_subprogram:
+case DW_TAG_lexical_block:
+case DW_TAG_inlined_subroutine:
+  return false;
+
+case DW_TAG_compile_unit:
+case DW_TAG_partial_unit:
+  return true;
+
+default:
+  break;
+}
+parent_die = parent_die->GetParent();
+  }
+  return false;
+}
+
 bool DWARFDebugInfoEntry::operator==(const DWARFDebugInfoEntry ) const {
   return m_offset == rhs.m_offset && m_parent_idx == rhs.m_parent_idx &&
  m_sibling_idx == rhs.m_sibling_idx &&

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
index 3a962ad8e9b3..c05d79c01817 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -167,6 +167,8 @@ class DWARFDebugInfoEntry {
   void SetSiblingIndex(uint32_t idx) { m_sibling_idx = idx; }
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
+  bool IsGlobalOrStaticVariable() const;
+
 protected:
   static DWARFDeclContext
   GetDWARFDeclContextStatic(const DWARFDebugInfoEntry *die, DWARFUnit *cu);

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 3951a2a32a1b..36f8aa2bfc13 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -204,35 +204,8 @@ void ManualDWARFIndex::IndexUnitImpl(DWARFUnit ,
 case DW_AT_location:
 case DW_AT_const_value:
   has_location_or_const_value = true;
-  if (tag == DW_TAG_variable) {
-const DWARFDebugInfoEntry *parent_die = die.GetParent();
-while (parent_die != nullptr) {
-  switch (parent_die->Tag()) {
-  case DW_TAG_subprogram:
-  case DW_TAG_lexical_block:
-  case DW_TAG_inlined_subroutine:
-// Even if this is a function level static, we don't add it. We
-// could theoretically add these if we wanted to by
-// introspecting into the DW_AT_location and seeing if the
-// location describes a hard coded address, but we don't want
-// the performance penalty of that right now.
-is_global_or_static_variable = false;
-parent_die = nullptr; // Terminate the while loop.
-break;
-
-  case DW_TAG_compile_unit:
-  case DW_TAG_partial_unit:
-is_global_or_static_variable = true;
-parent_die = nullptr; // Terminate the while loop.
-break;
-
-  default:
-parent_die =
-parent_die->GetParent(); // Keep going in the while loop.
-break;
-  }
-}
-  }
+  is_global_or_static_variable = die.IsGlobalOrStaticVariable();
+
   break;
 
 case DW_AT_specification:

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 6d43f957d362..5c14d3b52ac5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3442,7 +3442,7 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const 
SymbolContext ,
 }
  

[Lldb-commits] [lldb] be3f8a8 - [commands] Support autorepeat in SBCommands

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-04-08T10:54:14-07:00
New Revision: be3f8a8e1b95db1a8bdcc7a66ba27f8a6ea65469

URL: 
https://github.com/llvm/llvm-project/commit/be3f8a8e1b95db1a8bdcc7a66ba27f8a6ea65469
DIFF: 
https://github.com/llvm/llvm-project/commit/be3f8a8e1b95db1a8bdcc7a66ba27f8a6ea65469.diff

LOG: [commands] Support autorepeat in SBCommands

Summary:
This adds support for commands created through the API to support autorepeat.
This covers the case of single word and multiword commands.

Comprehensive tests are included as well.

Reviewers: labath, clayborg

Subscribers: lldb-commits

Tags: #lldb

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

Added: 
lldb/unittests/API/CMakeLists.txt
lldb/unittests/API/TestSBCommandInterpreterTest.cpp

Modified: 
lldb/include/lldb/API/SBCommandInterpreter.h
lldb/source/API/SBCommandInterpreter.cpp
lldb/unittests/CMakeLists.txt

Removed: 




diff  --git a/lldb/include/lldb/API/SBCommandInterpreter.h 
b/lldb/include/lldb/API/SBCommandInterpreter.h
index 485d4ca58de0..e07eeb58bf6a 100644
--- a/lldb/include/lldb/API/SBCommandInterpreter.h
+++ b/lldb/include/lldb/API/SBCommandInterpreter.h
@@ -111,14 +111,86 @@ class SBCommandInterpreter {
 
   lldb::SBCommand AddMultiwordCommand(const char *name, const char *help);
 
+  /// Add a new command to the lldb::CommandInterpreter.
+  ///
+  /// The new command won't support autorepeat. If you need this functionality,
+  /// use the override of this function that accepts the \a auto_repeat_command
+  /// parameter.
+  ///
+  /// \param[in] name
+  /// The name of the command.
+  ///
+  /// \param[in] impl
+  /// The handler of this command.
+  ///
+  /// \param[in] help
+  /// The general description to show as part of the help message of this
+  /// command.
+  ///
+  /// \return
+  /// A lldb::SBCommand representing the newly created command.
   lldb::SBCommand AddCommand(const char *name,
  lldb::SBCommandPluginInterface *impl,
  const char *help);
 
+  /// Add a new command to the lldb::CommandInterpreter.
+  ///
+  /// The new command won't support autorepeat. If you need this functionality,
+  /// use the override of this function that accepts the \a auto_repeat_command
+  /// parameter.
+  ///
+  /// \param[in] name
+  /// The name of the command.
+  ///
+  /// \param[in] impl
+  /// The handler of this command.
+  ///
+  /// \param[in] help
+  /// The general description to show as part of the help message of this
+  /// command.
+  ///
+  /// \param[in] syntax
+  /// The syntax to show as part of the help message of this command. This
+  /// could include a description of the 
diff erent arguments and flags this
+  /// command accepts.
+  ///
+  /// \return
+  /// A lldb::SBCommand representing the newly created command.
   lldb::SBCommand AddCommand(const char *name,
  lldb::SBCommandPluginInterface *impl,
  const char *help, const char *syntax);
 
+  /// Add a new command to the lldb::CommandInterpreter.
+  ///
+  /// \param[in] name
+  /// The name of the command.
+  ///
+  /// \param[in] impl
+  /// The handler of this command.
+  ///
+  /// \param[in] help
+  /// The general description to show as part of the help message of this
+  /// command.
+  ///
+  /// \param[in] syntax
+  /// The syntax to show as part of the help message of this command. This
+  /// could include a description of the 
diff erent arguments and flags this
+  /// command accepts.
+  ///
+  /// \param[in] auto_repeat_command
+  /// Autorepeating is triggered when the user presses Enter successively
+  /// after executing a command. If \b nullptr is provided, the previous
+  /// exact command will be repeated. If \b "" is provided, autorepeating
+  /// is disabled. Otherwise, the provided string is used as a repeat
+  /// command.
+  ///
+  /// \return
+  /// A lldb::SBCommand representing the newly created command.
+  lldb::SBCommand AddCommand(const char *name,
+ lldb::SBCommandPluginInterface *impl,
+ const char *help, const char *syntax,
+ const char *auto_repeat_command);
+
   void SourceInitFileInHomeDirectory(lldb::SBCommandReturnObject );
 
   void
@@ -283,14 +355,90 @@ class SBCommand {
   lldb::SBCommand AddMultiwordCommand(const char *name,
   const char *help = nullptr);
 
+  /// Add a new subcommand to the lldb::SBCommand.
+  ///
+  /// The new command won't support autorepeat. If you need this functionality,
+  /// use the override of this function that accepts the \a auto_repeat
+  /// parameter.
+  ///
+  /// \param[in] name
+  /// The name of the command.
+  ///
+  /// 

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

[Davide Italiano via Phabricator via lldb-commits]

davide updated this revision to Diff 256063.
davide added a comment.
Herald added a reviewer: jdoerfert.

Added test, updated comment.


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

https://reviews.llvm.org/D77698

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/static_scope.s

Index: lldb/test/Shell/SymbolFile/DWARF/static_scope.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/static_scope.s
@@ -0,0 +1,312 @@
+# Test that the DWARF parser assigns the right scope to the
+# variable `b`, which is `local` and not `static`.
+
+# REQUIRES: x86
+# UNSUPPORTED: lldb-repro
+
+# RUN: llvm-mc -triple=x86_64-apple-macosx10.15.0 -filetype=obj %s > %t.o
+# RUN: lldb-test symbols %t.o | FileCheck %s
+
+# CHECK: Variable{{.*}}, name = "b", type = {{.*}} (int), scope = local
+
+	.section	__TEXT,__text,regular,pure_instructions
+	.macosx_version_min 10, 15
+	.file	1 "/Users/davide/work/build/bin" "a.c"
+	.globl	_main   ## -- Begin function main
+	.p2align	4, 0x90
+_main:  ## @main
+Lfunc_begin0:
+	.loc	1 2 0   ## a.c:2:0
+	.cfi_startproc
+## %bb.0:
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+Ltmp0:
+	##DEBUG_VALUE: b <- 3
+	.loc	1 5 9 prologue_end  ## a.c:5:9
+	movl	_a(%rip), %eax
+Ltmp1:
+	.loc	1 7 5   ## a.c:7:5
+	xorl	%eax, %eax
+	popq	%rbp
+	retq
+Ltmp2:
+Lfunc_end0:
+	.cfi_endproc
+## -- End function
+	.globl	_a  ## @a
+.zerofill __DATA,__common,_a,4,2
+	.section	__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+	.byte	1   ## Abbreviation Code
+	.byte	17  ## DW_TAG_compile_unit
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	37  ## DW_AT_producer
+	.byte	14  ## DW_FORM_strp
+	.byte	19  ## DW_AT_language
+	.byte	5   ## DW_FORM_data2
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.byte	17  ## DW_AT_low_pc
+	.byte	1   ## DW_FORM_addr
+	.byte	18  ## DW_AT_high_pc
+	.byte	6   ## DW_FORM_data4
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	2   ## Abbreviation Code
+	.byte	52  ## DW_TAG_variable
+	.byte	0   ## DW_CHILDREN_no
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	63  ## DW_AT_external
+	.byte	25  ## DW_FORM_flag_present
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	2   ## DW_AT_location
+	.byte	24  ## DW_FORM_exprloc
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	3   ## Abbreviation Code
+	.byte	53  ## DW_TAG_volatile_type
+	.byte	0   ## DW_CHILDREN_no
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	4   ## Abbreviation Code
+	.byte	36  ## DW_TAG_base_type
+	.byte	0   ## DW_CHILDREN_no
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	62  ## DW_AT_encoding
+	.byte	11  ## DW_FORM_data1
+	.byte	11  ## DW_AT_byte_size
+	.byte	11  ## DW_FORM_data1
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	5   ## Abbreviation Code
+	.byte	46  ## DW_TAG_subprogram
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	17

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

[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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77444



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

[Lldb-commits] [lldb] 8e40987 - Fix e796c77b26acab0b530ac6516f1dda21b8494733

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-04-08T10:48:01-07:00
New Revision: 8e40987e1897276308728d28484fbe93f0a7a1e0

URL: 
https://github.com/llvm/llvm-project/commit/8e40987e1897276308728d28484fbe93f0a7a1e0
DIFF: 
https://github.com/llvm/llvm-project/commit/8e40987e1897276308728d28484fbe93f0a7a1e0.diff

LOG: Fix e796c77b26acab0b530ac6516f1dda21b8494733

Failure:
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/14556/testReport/junit/lldb-api/tools_lldb-vscode_breakpoint-events/TestVSCode_breakpointEvents_py/

Diff:
[lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints 
request
https://reviews.llvm.org/D76968

It failed a test TestVSCode_breakpointEvents that only runs Darwin that needed 
to be updated to match the current logic.
The change is simple.

Added: 


Modified: 

lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointEvents.py

Removed: 




diff  --git 
a/lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointEvents.py
 
b/lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointEvents.py
index 3a221cd0478c..aae9cc8cb101 100644
--- 
a/lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointEvents.py
+++ 
b/lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointEvents.py
@@ -114,5 +114,5 @@ def test_breakpoint_events(self):
 "breakpoint event is for breakpoint %i" % (foo_bp_id))
 self.assertTrue('line' in breakpoint and breakpoint['line'] > 0,
 "breakpoint event is has a line number")
-self.assertTrue("foo.cpp" in breakpoint['source']['path'],
-"breakpoint event path contains foo.cpp")
+self.assertNotIn("source", breakpoint,
+"breakpoint event should not return a source object")



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

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe796c77b26ac: [lldb-vscode] Correctly return source mapped 
breakpoints for setBreakpoints… (authored by Walter Erquinigo 
walterme...@fb.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/breakpoint/Makefile
  lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
  lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp
  lldb/test/API/tools/lldb-vscode/breakpoint/other.c
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -418,7 +418,16 @@
   bp.MatchesName(BreakpointBase::GetBreakpointLabel())) {
 auto bp_event = CreateEventObject("breakpoint");
 llvm::json::Object body;
-body.try_emplace("breakpoint", CreateBreakpoint(bp));
+// As VSCode already knows the path of this breakpoint, we don't
+// need to send it back as part of a "changed" event. This
+// prevent us from sending to VSCode paths that should be source
+// mapped. Note that CreateBreakpoint doesn't apply source mapping.
+// Besides, the current implementation of VSCode ignores the
+// "source" element of breakpoint events.
+llvm::json::Value source_bp = CreateBreakpoint(bp);
+source_bp.getAsObject()->erase("source");
+
+body.try_emplace("breakpoint", source_bp);
 body.try_emplace("reason", "changed");
 bp_event.try_emplace("body", std::move(body));
 g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
@@ -1364,13 +1373,13 @@
 llvm::sys::fs::set_current_path(debuggerRoot.data());
   }
 
-  SetSourceMapFromArguments(*arguments);
-
   // Run any initialize LLDB commands the user specified in the launch.json.
   // This is run before target is created, so commands can't do anything with
   // the targets - preRunCommands are run with the target.
   g_vsc.RunInitCommands();
 
+  SetSourceMapFromArguments(*arguments);
+
   lldb::SBError status;
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
   if (status.Fail()) {
@@ -1766,13 +1775,14 @@
 const auto _bp = existing_source_bps->second.find(src_bp.line);
 if (existing_bp != existing_source_bps->second.end()) {
   existing_bp->second.UpdateBreakpoint(src_bp);
-  AppendBreakpoint(existing_bp->second.bp, response_breakpoints);
+  AppendBreakpoint(existing_bp->second.bp, response_breakpoints, path,
+   src_bp.line);
   continue;
 }
   }
   // At this point the breakpoint is new
   src_bp.SetBreakpoint(path.data());
-  AppendBreakpoint(src_bp.bp, response_breakpoints);
+  AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
   g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
 }
   }
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -184,28 +184,58 @@
 void SetValueForKey(lldb::SBValue , llvm::json::Object ,
 llvm::StringRef key);
 
-/// Converts \a bp to a JSON value and appends all locations to the
+/// Converts \a bp to a JSON value and appends the first valid location to the
 /// \a breakpoints array.
 ///
 /// \param[in] bp
-/// A LLDB breakpoint object which will get all locations extracted
-/// and converted into a JSON objects in the \a breakpoints array
+/// A LLDB breakpoint object which will get the first valid location
+/// extracted and converted into a JSON object in the \a breakpoints array
 ///
 /// \param[in] breakpoints
 /// A JSON array that will get a llvm::json::Value for \a bp
 /// appended to it.
-void AppendBreakpoint(lldb::SBBreakpoint , llvm::json::Array );
+///
+/// \param[in] request_path
+/// An optional source path to use when creating the "Source" object of this
+/// breakpoint. If not specified, the "Source" object is created from the
+/// breakpoint's address' LineEntry. It is useful to ensure the same source
+/// paths provided by the setBreakpoints request are returned to the IDE.
+///
+/// \param[in] request_line
+/// An optional line to use when creating the "Breakpoint" object to append.
+/// It is used if the breakpoint has no valid locations.
+/// It is 

[Lldb-commits] [lldb] e796c77 - [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-04-08T09:52:59-07:00
New Revision: e796c77b26acab0b530ac6516f1dda21b8494733

URL: 
https://github.com/llvm/llvm-project/commit/e796c77b26acab0b530ac6516f1dda21b8494733
DIFF: 
https://github.com/llvm/llvm-project/commit/e796c77b26acab0b530ac6516f1dda21b8494733.diff

LOG: [lldb-vscode] Correctly return source mapped breakpoints for 
setBreakpoints request

Summary:
When using source maps for a breakpoint, in order to find the actual source 
that breakpoint has resolved, we
need to use a query similar to what 
CommandObjectSource::DumpLinesInSymbolContexts does, which is the logic
used by the CLI to display the source line at a given breakpoint. That's 
necessary because from a breakpoint
location you only have an address, which points to the original source 
location, not the source mapped one.

in the setBreakpoints request handler, we haven't been doing such query and we 
were returning the original
source location, which was breaking the UX of VSCode, as many breakpoints were 
being set but not displayed
in the source file next to each line. Besides, clicking the source path of a 
breakpoint in the breakpoints
view in the debug tab was not working for this case, as VSCode was trying to 
open a non-existent file, thus
showing an error to the user.

Ideally, we should do the query mentioned above to find the actual source 
location to respond to the IDE,
however, that query is expensive and users can have an arbitrary number of 
breakpoints set. As a simpler fix,
the request sent by VSCode already contains the full source path, which exists 
because the user set it from
the IDE itself, so we can simply reuse it instead of querying from the SB API.

I wrote a test for this, and found out that I had to move 
SetSourceMapFromArguments after RunInitCommands in
lldb-vscode.cpp, because an init command used in all tests is `settings clear 
-all`, which would clear the
source map unless specified after initCommands. And in any case, I think it 
makes sense to have initCommands
run before anything the debugger would do.

Reviewers: clayborg, kusmour, labath, aadsm

Subscribers: lldb-commits

Tags: #lldb

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

Added: 
lldb/test/API/tools/lldb-vscode/breakpoint/other.c

Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/breakpoint/Makefile
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/JSONUtils.h
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index 54f09e2cdbee..3b06fa07d048 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -266,8 +266,8 @@ def launch(self, program=None, args=None, cwd=None, 
env=None,
stopOnEntry=False, disableASLR=True,
disableSTDIO=False, shellExpandArguments=False,
trace=False, initCommands=None, preRunCommands=None,
-   stopCommands=None, exitCommands=None,sourcePath= None,
-   debuggerRoot=None, launchCommands=None):
+   stopCommands=None, exitCommands=None,sourcePath=None,
+   debuggerRoot=None, launchCommands=None, sourceMap=None):
 '''Sending launch request to vscode
 '''
 
@@ -298,7 +298,8 @@ def cleanup():
 exitCommands=exitCommands,
 sourcePath=sourcePath,
 debuggerRoot=debuggerRoot,
-launchCommands=launchCommands)
+launchCommands=launchCommands,
+sourceMap=sourceMap)
 if not (response and response['success']):
 self.assertTrue(response['success'],
 'launch failed (%s)' % (response['message']))

diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 7c966bcc04ee..805de43f25f3 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -570,7 +570,7 @@ def request_launch(self, program, args=None, cwd=None, 
env=None,
disableSTDIO=False, shellExpandArguments=False,
trace=False, initCommands=None, preRunCommands=None,
stopCommands=None, exitCommands=None, sourcePath=None,
-   debuggerRoot=None, launchCommands=None):
+   

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 256027.
wallace added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77444

Files:
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/unittests/API/CMakeLists.txt
  lldb/unittests/API/TestSBCommandInterpreterTest.cpp
  lldb/unittests/CMakeLists.txt

Index: lldb/unittests/CMakeLists.txt
===
--- lldb/unittests/CMakeLists.txt
+++ lldb/unittests/CMakeLists.txt
@@ -59,6 +59,7 @@
 endfunction()
 
 add_subdirectory(TestingSupport)
+add_subdirectory(API)
 add_subdirectory(Breakpoint)
 add_subdirectory(Core)
 add_subdirectory(DataFormatter)
Index: lldb/unittests/API/TestSBCommandInterpreterTest.cpp
===
--- /dev/null
+++ lldb/unittests/API/TestSBCommandInterpreterTest.cpp
@@ -0,0 +1,138 @@
+//===-- TestSBCommandInterpreterTest.cpp --===//
+//
+// 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 "gtest/gtest.h"
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include 
+#include 
+
+using namespace lldb;
+
+class TestSBCommandInterpreterTest : public testing::Test {
+protected:
+  void SetUp() override {
+SBDebugger::Initialize();
+m_dbg = SBDebugger::Create(/*source_init_files=*/false);
+m_interp = m_dbg.GetCommandInterpreter();
+  }
+
+  SBDebugger m_dbg;
+  SBCommandInterpreter m_interp;
+};
+
+class DummyCommand : public SBCommandPluginInterface {
+public:
+  DummyCommand(const char *message) : m_message(message) {}
+
+  bool DoExecute(SBDebugger dbg, char **command,
+ SBCommandReturnObject ) {
+result.PutCString(m_message.c_str());
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+
+private:
+  std::string m_message;
+};
+
+TEST_F(TestSBCommandInterpreterTest, SingleWordCommand) {
+  // We first test a command without autorepeat
+  DummyCommand dummy("It worked");
+  m_interp.AddCommand("dummy", , /*help=*/nullptr);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("dummy", result, /*add_to_history=*/true);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+EXPECT_FALSE(result.Succeeded());
+EXPECT_STREQ(result.GetError(), "error: No auto repeat.\n");
+  }
+
+  // Now we test a command with autorepeat
+  m_interp.AddCommand("dummy_with_autorepeat", , /*help=*/nullptr,
+  /*syntax=*/nullptr, /*auto_repeat_command=*/nullptr);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("dummy_with_autorepeat", result,
+   /*add_to_history=*/true);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+}
+
+TEST_F(TestSBCommandInterpreterTest, MultiWordCommand) {
+  auto command = m_interp.AddMultiwordCommand("multicommand", /*help=*/nullptr);
+  // We first test a subcommand without autorepeat
+  DummyCommand subcommand("It worked again");
+  command.AddCommand("subcommand", , /*help=*/nullptr);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("multicommand subcommand", result,
+   /*add_to_history=*/true);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked again\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+EXPECT_FALSE(result.Succeeded());
+EXPECT_STREQ(result.GetError(), "error: No auto repeat.\n");
+  }
+
+  // We first test a subcommand with autorepeat
+  command.AddCommand("subcommand_with_autorepeat", ,
+ /*help=*/nullptr, /*syntax=*/nullptr,
+ /*auto_repeat_command=*/nullptr);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("multicommand subcommand_with_autorepeat", result,
+   /*add_to_history=*/true);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked again\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked again\n");
+  }
+
+  DummyCommand subcommand2("It worked 

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 256022.
wallace added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/breakpoint/Makefile
  lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
  lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp
  lldb/test/API/tools/lldb-vscode/breakpoint/other.c
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -418,7 +418,16 @@
   bp.MatchesName(BreakpointBase::GetBreakpointLabel())) {
 auto bp_event = CreateEventObject("breakpoint");
 llvm::json::Object body;
-body.try_emplace("breakpoint", CreateBreakpoint(bp));
+// As VSCode already knows the path of this breakpoint, we don't
+// need to send it back as part of a "changed" event. This
+// prevent us from sending to VSCode paths that should be source
+// mapped. Note that CreateBreakpoint doesn't apply source mapping.
+// Besides, the current implementation of VSCode ignores the
+// "source" element of breakpoint events.
+llvm::json::Value source_bp = CreateBreakpoint(bp);
+source_bp.getAsObject()->erase("source");
+
+body.try_emplace("breakpoint", source_bp);
 body.try_emplace("reason", "changed");
 bp_event.try_emplace("body", std::move(body));
 g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
@@ -1364,13 +1373,13 @@
 llvm::sys::fs::set_current_path(debuggerRoot.data());
   }
 
-  SetSourceMapFromArguments(*arguments);
-
   // Run any initialize LLDB commands the user specified in the launch.json.
   // This is run before target is created, so commands can't do anything with
   // the targets - preRunCommands are run with the target.
   g_vsc.RunInitCommands();
 
+  SetSourceMapFromArguments(*arguments);
+
   lldb::SBError status;
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
   if (status.Fail()) {
@@ -1766,13 +1775,14 @@
 const auto _bp = existing_source_bps->second.find(src_bp.line);
 if (existing_bp != existing_source_bps->second.end()) {
   existing_bp->second.UpdateBreakpoint(src_bp);
-  AppendBreakpoint(existing_bp->second.bp, response_breakpoints);
+  AppendBreakpoint(existing_bp->second.bp, response_breakpoints, path,
+   src_bp.line);
   continue;
 }
   }
   // At this point the breakpoint is new
   src_bp.SetBreakpoint(path.data());
-  AppendBreakpoint(src_bp.bp, response_breakpoints);
+  AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
   g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
 }
   }
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -184,28 +184,58 @@
 void SetValueForKey(lldb::SBValue , llvm::json::Object ,
 llvm::StringRef key);
 
-/// Converts \a bp to a JSON value and appends all locations to the
+/// Converts \a bp to a JSON value and appends the first valid location to the
 /// \a breakpoints array.
 ///
 /// \param[in] bp
-/// A LLDB breakpoint object which will get all locations extracted
-/// and converted into a JSON objects in the \a breakpoints array
+/// A LLDB breakpoint object which will get the first valid location
+/// extracted and converted into a JSON object in the \a breakpoints array
 ///
 /// \param[in] breakpoints
 /// A JSON array that will get a llvm::json::Value for \a bp
 /// appended to it.
-void AppendBreakpoint(lldb::SBBreakpoint , llvm::json::Array );
+///
+/// \param[in] request_path
+/// An optional source path to use when creating the "Source" object of this
+/// breakpoint. If not specified, the "Source" object is created from the
+/// breakpoint's address' LineEntry. It is useful to ensure the same source
+/// paths provided by the setBreakpoints request are returned to the IDE.
+///
+/// \param[in] request_line
+/// An optional line to use when creating the "Breakpoint" object to append.
+/// It is used if the breakpoint has no valid locations.
+/// It is useful to ensure the same line
+/// provided by the setBreakpoints request are returned to the IDE as a
+/// fallback.
+void 

[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 255923.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327

Files:
  lldb/include/lldb/Core/UniqueCStringMap.h
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -31,8 +31,8 @@
 
   DWARFCompileUnit *GetDWOCompileUnitForHash(uint64_t hash);
 
-  size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray _die_offsets) override;
+  bool GetObjCMethods(lldb_private::ConstString class_name,
+  llvm::function_ref callback) override;
 
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -95,10 +95,10 @@
   return GetBaseSymbolFile().GetForwardDeclClangTypeToDie();
 }
 
-size_t SymbolFileDWARFDwo::GetObjCMethodDIEOffsets(
-lldb_private::ConstString class_name, DIEArray _die_offsets) {
-  return GetBaseSymbolFile().GetObjCMethodDIEOffsets(class_name,
- method_die_offsets);
+bool SymbolFileDWARFDwo::GetObjCMethods(
+lldb_private::ConstString class_name,
+llvm::function_ref callback) {
+  return GetBaseSymbolFile().GetObjCMethods(class_name, callback);
 }
 
 UniqueDWARFASTTypeMap ::GetUniqueDWARFASTTypeMap() {
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -237,8 +237,8 @@
   lldb_private::CompileUnit *
   GetCompUnitForDWARFCompUnit(DWARFCompileUnit _cu);
 
-  virtual size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray _die_offsets);
+  virtual bool GetObjCMethods(lldb_private::ConstString class_name,
+  llvm::function_ref callback);
 
   bool Supports_DW_AT_APPLE_objc_complete_type(DWARFUnit *cu);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1463,11 +1463,9 @@
   return static_cast(non_dwo_cu->GetUserData());
 }
 
-size_t SymbolFileDWARF::GetObjCMethodDIEOffsets(ConstString class_name,
-DIEArray _die_offsets) {
-  method_die_offsets.clear();
-  m_index->GetObjCMethods(class_name, method_die_offsets);
-  return method_die_offsets.size();
+bool SymbolFileDWARF::GetObjCMethods(
+ConstString class_name, llvm::function_ref callback) {
+  return m_index->GetObjCMethods(class_name, callback);
 }
 
 bool SymbolFileDWARF::GetFunction(const DWARFDIE , SymbolContext ) {
@@ -2048,32 +2046,28 @@
   context, basename))
 basename = name.GetStringRef();
 
-  DIEArray die_offsets;
-  m_index->GetGlobalVariables(ConstString(basename), die_offsets);
-  const size_t num_die_matches = die_offsets.size();
-
-  SymbolContext sc;
-  sc.module_sp = m_objfile_sp->GetModule();
-  assert(sc.module_sp);
-
   // Loop invariant: Variables up to this index have been checked for context
   // matches.
   uint32_t pruned_idx = original_size;
 
-  for (size_t i = 0; i < num_die_matches; ++i) {
-const DIERef _ref = die_offsets[i];
+  SymbolContext sc;
+  m_index->GetGlobalVariables(ConstString(basename), 

[Lldb-commits] [PATCH] D77662: [lldb/test] Make TestLoadUnload compatible with windows

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 255921.
labath edited the summary of this revision.
labath added a comment.

Remove bogus runCmd. Everything should check out now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77662

Files:
  lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  lldb/packages/Python/lldbsuite/test/make/dylib.h
  lldb/packages/Python/lldbsuite/test/make/test_common.h
  lldb/test/API/functionalities/load_unload/Makefile
  lldb/test/API/functionalities/load_unload/TestLoadUnload.py
  lldb/test/API/functionalities/load_unload/a.cpp
  lldb/test/API/functionalities/load_unload/b.cpp
  lldb/test/API/functionalities/load_unload/d.cpp
  lldb/test/API/functionalities/load_unload/main.cpp

Index: lldb/test/API/functionalities/load_unload/main.cpp
===
--- lldb/test/API/functionalities/load_unload/main.cpp
+++ lldb/test/API/functionalities/load_unload/main.cpp
@@ -1,72 +1,57 @@
-#include 
-#include 
+#include "dylib.h"
 #include 
-#include 
-#include 
-#include 
+#include 
 #include 
+#include 
 
-int
-main (int argc, char const *argv[])
-{
-#if defined (__APPLE__)
-const char *a_name = "@executable_path/libloadunload_a.dylib";
-const char *c_name = "@executable_path/libloadunload_c.dylib";
-#else
-const char *a_name = "libloadunload_a.so";
-const char *c_name = "libloadunload_c.so";
-#endif
-void *a_dylib_handle = NULL;
-void *c_dylib_handle = NULL;
-int (*a_function) (void);
-
-a_dylib_handle = dlopen (a_name, RTLD_NOW); // Set break point at this line for test_lldb_process_load_and_unload_commands().
-if (a_dylib_handle == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (1);
-}
-
-a_function = (int (*) ()) dlsym (a_dylib_handle, "a_function");
-if (a_function == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (2);
-}
-printf ("First time around, got: %d\n", a_function ());
-dlclose (a_dylib_handle);
-
-c_dylib_handle = dlopen (c_name, RTLD_NOW);
-if (c_dylib_handle == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (3);
-}
-a_function = (int (*) ()) dlsym (c_dylib_handle, "c_function");
-if (a_function == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (4);
-}
-
-a_dylib_handle = dlopen (a_name, RTLD_NOW);
-if (a_dylib_handle == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (5);
-}
-
-a_function = (int (*) ()) dlsym (a_dylib_handle, "a_function");
-if (a_function == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (6);
-}
-printf ("Second time around, got: %d\n", a_function ());
-dlclose (a_dylib_handle);
-
-int d_function(void);
-printf ("d_function returns: %d\n", d_function());
-
-return 0;
+int main(int argc, char const *argv[]) {
+  const char *a_name = "loadunload_a";
+  const char *c_name = "loadunload_c";
+  void *a_dylib_handle = NULL;
+  void *c_dylib_handle = NULL; // Set break point at this line for test_lldb_process_load_and_unload_commands().
+  int (*a_function)(void);
+
+  a_dylib_handle = dylib_open(a_name);
+  if (a_dylib_handle == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(1);
+  }
+
+  a_function = (int (*)())dylib_get_symbol(a_dylib_handle, "a_function");
+  if (a_function == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(2);
+  }
+  printf("First time around, got: %d\n", a_function());
+  dylib_close(a_dylib_handle);
+
+  c_dylib_handle = dylib_open(c_name);
+  if (c_dylib_handle == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(3);
+  }
+  a_function = (int (*)())dylib_get_symbol(c_dylib_handle, "c_function");
+  if (a_function == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(4);
+  }
+
+  a_dylib_handle = dylib_open(a_name);
+  if (a_dylib_handle == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(5);
+  }
+
+  a_function = (int (*)())dylib_get_symbol(a_dylib_handle, "a_function");
+  if (a_function == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(6);
+  }
+  printf("Second time around, got: %d\n", a_function());
+  dylib_close(a_dylib_handle);
+
+  int LLDB_DYLIB_IMPORT d_function(void);
+  printf("d_function returns: %d\n", d_function());
+
+  return 0;
 }
Index: lldb/test/API/functionalities/load_unload/d.cpp
===
--- lldb/test/API/functionalities/load_unload/d.cpp
+++ lldb/test/API/functionalities/load_unload/d.cpp
@@ -5,8 +5,6 @@
 
 int d_global = d_init();
 
-int
-d_function ()
-{ // Find this line number within d_dunction().
-return 700;
+int LLDB_DYLIB_EXPORT d_function() {
+  return 

[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

[Jan Kratochvil via Phabricator via lldb-commits]

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



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:190-195
+const_iterator left = llvm::lower_bound(m_map, unique_cstr, Compare());
+if (left != m_map.end() && left->cstring != unique_cstr)
+  left = m_map.end();
+const_iterator right =
+std::upper_bound(left, m_map.end(), unique_cstr, Compare());
+return llvm::make_range(left, right);

labath wrote:
> Looks better, but I have a feeling it could be simplified even further. 
> Wouldn't a plain `return llvm::make_range(std::equal_range(m_map.begin(), 
> m_map.end(), Compare());` work just as well? (Sorry for taking you down the 
> wrong path with the lower/upper_bound comment -- equal_range is basically a 
> combination of lower_bound and upper_bound calls.)
Yes, sorry, I was never using these functions before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327



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

[Lldb-commits] [PATCH] D77662: [WIP][lldb/test] Make TestLoadUnload compatible with windows

[Pavel Labath via Phabricator via lldb-commits]

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

Cleaning up the patch. It should be ready now, I'm just going to give it one 
more spin on windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77662

Files:
  lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  lldb/packages/Python/lldbsuite/test/make/dylib.h
  lldb/packages/Python/lldbsuite/test/make/test_common.h
  lldb/test/API/functionalities/load_unload/Makefile
  lldb/test/API/functionalities/load_unload/TestLoadUnload.py
  lldb/test/API/functionalities/load_unload/a.cpp
  lldb/test/API/functionalities/load_unload/b.cpp
  lldb/test/API/functionalities/load_unload/d.cpp
  lldb/test/API/functionalities/load_unload/main.cpp

Index: lldb/test/API/functionalities/load_unload/main.cpp
===
--- lldb/test/API/functionalities/load_unload/main.cpp
+++ lldb/test/API/functionalities/load_unload/main.cpp
@@ -1,72 +1,57 @@
-#include 
-#include 
+#include "dylib.h"
 #include 
-#include 
-#include 
-#include 
+#include 
 #include 
+#include 
 
-int
-main (int argc, char const *argv[])
-{
-#if defined (__APPLE__)
-const char *a_name = "@executable_path/libloadunload_a.dylib";
-const char *c_name = "@executable_path/libloadunload_c.dylib";
-#else
-const char *a_name = "libloadunload_a.so";
-const char *c_name = "libloadunload_c.so";
-#endif
-void *a_dylib_handle = NULL;
-void *c_dylib_handle = NULL;
-int (*a_function) (void);
-
-a_dylib_handle = dlopen (a_name, RTLD_NOW); // Set break point at this line for test_lldb_process_load_and_unload_commands().
-if (a_dylib_handle == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (1);
-}
-
-a_function = (int (*) ()) dlsym (a_dylib_handle, "a_function");
-if (a_function == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (2);
-}
-printf ("First time around, got: %d\n", a_function ());
-dlclose (a_dylib_handle);
-
-c_dylib_handle = dlopen (c_name, RTLD_NOW);
-if (c_dylib_handle == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (3);
-}
-a_function = (int (*) ()) dlsym (c_dylib_handle, "c_function");
-if (a_function == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (4);
-}
-
-a_dylib_handle = dlopen (a_name, RTLD_NOW);
-if (a_dylib_handle == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (5);
-}
-
-a_function = (int (*) ()) dlsym (a_dylib_handle, "a_function");
-if (a_function == NULL)
-{
-fprintf (stderr, "%s\n", dlerror());
-exit (6);
-}
-printf ("Second time around, got: %d\n", a_function ());
-dlclose (a_dylib_handle);
-
-int d_function(void);
-printf ("d_function returns: %d\n", d_function());
-
-return 0;
+int main(int argc, char const *argv[]) {
+  const char *a_name = "loadunload_a";
+  const char *c_name = "loadunload_c";
+  void *a_dylib_handle = NULL;
+  void *c_dylib_handle = NULL; // Set break point at this line for test_lldb_process_load_and_unload_commands().
+  int (*a_function)(void);
+
+  a_dylib_handle = dylib_open(a_name);
+  if (a_dylib_handle == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(1);
+  }
+
+  a_function = (int (*)())dylib_get_symbol(a_dylib_handle, "a_function");
+  if (a_function == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(2);
+  }
+  printf("First time around, got: %d\n", a_function());
+  dylib_close(a_dylib_handle);
+
+  c_dylib_handle = dylib_open(c_name);
+  if (c_dylib_handle == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(3);
+  }
+  a_function = (int (*)())dylib_get_symbol(c_dylib_handle, "c_function");
+  if (a_function == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(4);
+  }
+
+  a_dylib_handle = dylib_open(a_name);
+  if (a_dylib_handle == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(5);
+  }
+
+  a_function = (int (*)())dylib_get_symbol(a_dylib_handle, "a_function");
+  if (a_function == NULL) {
+fprintf(stderr, "%s\n", dylib_last_error());
+exit(6);
+  }
+  printf("Second time around, got: %d\n", a_function());
+  dylib_close(a_dylib_handle);
+
+  int LLDB_DYLIB_IMPORT d_function(void);
+  printf("d_function returns: %d\n", d_function());
+
+  return 0;
 }
Index: lldb/test/API/functionalities/load_unload/d.cpp
===
--- lldb/test/API/functionalities/load_unload/d.cpp
+++ lldb/test/API/functionalities/load_unload/d.cpp
@@ -5,8 +5,6 @@
 
 int d_global = d_init();
 
-int
-d_function ()
-{ // Find this line number within d_dunction().
-return 700;
+int LLDB_DYLIB_EXPORT d_function() {
+  return 

[Lldb-commits] [PATCH] D77662: [WIP][lldb/test] Make TestLoadUnload compatible with windows

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: lldb/packages/Python/lldbsuite/test/make/dylib.h:50
+  FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM,
+  NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (char *), 0, 
NULL);
+  return msg;

amccarth wrote:
> This leaks the buffer allocated for the message.  I guess we don't expect 
> this to happen often, so maybe that's not a big deal, and it's a terrible API 
> to have to deal with.
yeah, that's just the test code and it's only purpose is to give some 
indication for when things go wrong. I've contemplated returning std::string 
here, but then I figured we may want to use this from some C code too and it 
may be better to just stick to C.



Comment at: lldb/test/API/functionalities/load_unload/TestLoadUnload.py:209
+env_cmd_string = "settings set target.env-vars " + self.dylibPath + 
"=" + self.getBuildArtifact(".")
+self.runCmd(env_cmd_string)
+

amccarth wrote:
> The env_cmd_string is going to eliminate ALL of the environment variables for 
> the target except the one(s) that it explicitly sets.  Is that what you 
> intended?
> 
> The lldb help says:
> > Warning:  The 'set' command re-sets the entire array or dictionary.  If you 
> > just want to add, remove or update individual values (or add something to 
> > the end), use one of the other settings sub-commands: append, replace, 
> > insert-before or insert-after.
> 
That's not as bad as it sounds because target.env-vars (as of last week, 
anyway) will only contain the variables set by the user, and the host 
environment will be applied separately.

Nonetheless, using append does seem like a better choice here.



Comment at: lldb/test/API/functionalities/load_unload/TestLoadUnload.py:419
 self.copy_shlibs_to_remote()
+env_cmd_string = "settings set target.env-vars " + self.dylibPath + 
"=" + self.getBuildArtifact(".")
+self.runCmd(env_cmd_string)

jingham wrote:
> You could also do this by making an SBLaunchInfo, adding the environment 
> variable to it, and then you could use lldbutil.run_to_name_breakpoint.  That 
> would remove some of the boilerplate below.
run_to_name_breakpoint does not seem like a completely good fit here as the 
test has assertions about numbers of breakpoint locations being found before 
running the process.

However, I think I've found an even better solution -- the test actually 
already contains logic for setting these paths, but it was hidden behind an 
`if(!darwin)` as it was not needed due to the use of `@executable_path` in 
dlopen calls. Now I've just removed the `if` and made that code slightly more 
generic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77662



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

[Lldb-commits] [PATCH] D77588: [lldb/Reproducers] Make it possible to capture reproducers for the Python test suite.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think the code looks ok now, though I have to say I am still somewhat fuzzy 
on what exactly will this patch enable. I think I understand the capture part, 
but what happens during replay? Will that already be smart enough to match the 
SB calls made by the test harness during replay to the captured calls that were 
made during recording, or is there some more work needed there? Could you add a 
brief documentation on this somewhere?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D77588



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

[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:190-195
+const_iterator left = llvm::lower_bound(m_map, unique_cstr, Compare());
+if (left != m_map.end() && left->cstring != unique_cstr)
+  left = m_map.end();
+const_iterator right =
+std::upper_bound(left, m_map.end(), unique_cstr, Compare());
+return llvm::make_range(left, right);

Looks better, but I have a feeling it could be simplified even further. 
Wouldn't a plain `return llvm::make_range(std::equal_range(m_map.begin(), 
m_map.end(), Compare());` work just as well? (Sorry for taking you down the 
wrong path with the lower/upper_bound comment -- equal_range is basically a 
combination of lower_bound and upper_bound calls.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327



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

[Lldb-commits] [PATCH] D77661: [lldb/Python] Add lldbconfig Python module to make the lldb module configurable.

[Pavel Labath via Phabricator via lldb-commits]

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

Let's see how this goes.


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

https://reviews.llvm.org/D77661



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

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: aprantl.
labath added a comment.

> I also need to craft a testcase. Ideas on how to do this are appreciated

You could check in the assembly generated by clang for this test case and then 
run `llvm-mc && lldb-test symbols` over it. Currently I get something like 
`Variable{0x7fff007f}, name = "b", type = {7fff0044} 
0x564FADF7F9F0 (int), scope = ??? (2)` when running that. After this patch 
that should presumably say `scope = local`. That won't actually check that the 
"frame variable" command succeeds, but maybe that's enough for this patch?

Using llvm ir instead of assembly might work too .. the relevant ir (`call void 
@llvm.dbg.value(metadata i32 3, metadata !16, metadata !DIExpression()), !dbg 
!18`) generates `DW_AT_const_value` even without any further optimizations, and 
it looks like it could be reasonably expected to keep doing that in the future. 
Adding @aprantl for IR insights.




Comment at: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1028-1032
+  // Even if this is a function level static, we don't add it. We
+  // could theoretically add these if we wanted to by
+  // introspecting into the DW_AT_location and seeing if the
+  // location describes a hard coded address, but we don't want
+  // the performance penalty of that right now.

This comment does not make sense in the new setting.


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

https://reviews.llvm.org/D77698



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

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[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 now. Sorry about the delay, I have a lot of things in my queue now 
and this got lost at the bottom of the stack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968



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