date:20200407

[Lldb-commits] [PATCH] D77568: Return correct entry in RangeDataVector::FindEntryThatContains

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

This is definitely a bug, but I fear that fixing this via a linear search will 
cause a big performance regression. Fortunately, since D74759 
 we now have a way to implement this more 
efficiently. Could you reimplement this function to use the binary search tree 
introduced in that patch?

If not, I can get around to it soon-ish, but I currently have a bunch of other 
things waiting in my queue...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77568



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

[Lldb-commits] [PATCH] D77480: Fix illegal early call to PyBuffer_Release in swig typemaps

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

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

I'm not exactly happy about the lack of a test case, but I guess we can let 
this slide on the account of it requiring implementing a non-standard PyBuffer 
in c++ and then loading that from a python test suite and passing it to lldb, 
which would require a completely new kind of a test case.




Comment at: lldb/bindings/python/python-typemaps.swig:495
+  Py_buffer buffer;
+  Py_buffer_RAII() { buffer = {}; };
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;

Move this into the initializer list, or maybe even into the default initializer 
(`Py_buffer buffer = {};`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77480



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

[Lldb-commits] [PATCH] D77214: [lldb] Add option to retry Fix-Its multiple times to failed expressions

2020-04-07 Thread Muhammad Omair Javaid via Phabricator via lldb-commits

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

This patch fails on lldb-aarch64-linux with following backtrace, apparently 
failing experession eval. I am marking it xfail for now.

Traceback (most recent call last):

  File 
"/home/omair.javaid/work/lldb/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
 line 1751, in test_method
return attrvalue(self)
  File 
"/home/omair.javaid/work/lldb/llvm-project/lldb/test/API/commands/expression/fixits/TestFixIts.py",
 line 151, in test_with_target
self.expect_expr("test_X(1)", result_type="int", result_value="123")
  File 
"/home/omair.javaid/work/lldb/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
 line 2426, in expect_expr
"Unexpected failure with msg: " + eval_result.GetError().GetCString())

AssertionError: False is not True : Unexpected failure with msg: error: 
Execution was interrupted, reason: signal SIGILL: illegal instruction.
The process has been returned to the state before expression evaluation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77214



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

[Lldb-commits] [PATCH] D77045: Add invalidate list to primary regs in arm64 register infos

2020-04-07 Thread Muhammad Omair Javaid via Phabricator via lldb-commits

omjavaid added a comment.

In D77045#1963896 , @labath wrote:

> In D77045#1956879 , @omjavaid wrote:
>
> > Adding a testcase would be tricky as these register overlap in memory and 
> > we store them with overlapping offsets with their children we should not 
> > need to invalidate the children when we write the parent but for some 
> > strange unexplainable reason QEMU was behaving strangely and not updating 
> > the first half in certain random cases. I just thought invalidation of 
> > children will force a read after write for that case.
>
>
> Thanks for the explanation, but I'm afraid I still don't get what is going on 
> here. Can you walk me through the individual steps here? Something like:
>
> 1. user does "register write x0 xx"
> 2. lldb translates that to the appropriate `p` packet
> 3. ???
> 4. user does "register read w0"
> 5. bad value comes out because...


Let me debug it separately from SVE and I will get back to you with an update.


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

https://reviews.llvm.org/D77045



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

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and ptrace support

2020-04-07 Thread Muhammad Omair Javaid via Phabricator via lldb-commits

omjavaid added a comment.

In D77047#1956774 , @omjavaid wrote:

> In D77047#1954696 , @labath wrote:
>
> > > There is no physical hardware currently available to test SVE and we make 
> > > use of QEMU for the purpose of testing this feature.
> >
> > Are these registers presented in core files in any way? Would it be 
> > possible to at least test the `RegisterInfoPOSIX_arm64.h` changes that way?
>
>
> Yes. Core file section is added for SVE registers but i have not tried to get 
> that working with LLDB. I will get back to you on this.


@labath Core file FP register access is not supported on AArch64. I am working 
on a follow up patch to fix it. This patch only has linux changes so can we get 
this through?


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

https://reviews.llvm.org/D77047



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

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

2020-04-07 Thread Muhammad Omair Javaid via Phabricator via lldb-commits

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

This patch breaks on TestMemoryCache.py lldb AArch64/Linux. we ll have to 
revert it untill fixed




FAIL: test_memory_cache_dwarf (TestMemoryCache.MemoryCacheTestCase)

  Test the MemoryCache class with a sequence of 'memory read' and 'memory 
write' operations.

--

Traceback (most recent call last):

  File 
"/home/omair.javaid/work/lldb/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
 line 1751, in test_method
return attrvalue(self)
  File 
"/home/omair.javaid/work/lldb/llvm-project/lldb/test/API/functionalities/memory/cache/TestMemoryCache.py",
 line 62, in test_memory_cache
self.assertEquals(0x00AA, int(line.split(':')[1], 0))

AssertionError: 170 != 66

Config=aarch64-/home/omair.javaid/work/lldb/build/arm64/bin/clang-11


FAIL: test_memory_cache_dwo (TestMemoryCache.MemoryCacheTestCase)

  Test the MemoryCache class with a sequence of 'memory read' and 'memory 
write' operations.

--

Traceback (most recent call last):

  File 
"/home/omair.javaid/work/lldb/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
 line 1751, in test_method
return attrvalue(self)
  File 
"/home/omair.javaid/work/lldb/llvm-project/lldb/test/API/functionalities/memory/cache/TestMemoryCache.py",
 line 62, in test_memory_cache
self.assertEquals(0x00AA, int(line.split(':')[1], 0))

AssertionError: 170 != 66

Config=aarch64-/home/omair.javaid/work/lldb/build/arm64/bin/clang-11



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398



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

[Lldb-commits] [lldb] e609fe6 - Revert "[lldb-server] jThreadsInfo returns stack memory"

2020-04-07 Thread Muhammad Omair Javaid via lldb-commits


Author: Muhammad Omair Javaid
Date: 2020-04-07T17:11:22+05:00
New Revision: e609fe68b2c59b145c0a5c66bedbee2bff545200

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

LOG: Revert "[lldb-server] jThreadsInfo returns stack memory"

This reverts commit a53bf9b7c8f1ca950226a55c0e99fd706a7b6ad2.

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py

Removed: 
lldb/test/API/tools/lldb-server/threads-info/Makefile

lldb/test/API/tools/lldb-server/threads-info/TestGdbRemoteThreadsInfoMemory.py
lldb/test/API/tools/lldb-server/threads-info/main.cpp



diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index f8b09d5207ab..7d6cb2a3484b 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -31,7 +31,6 @@
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBuffer.h"
-#include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
@@ -563,119 +562,6 @@ GetRegistersAsJSON(NativeThreadProtocol &thread) {
   return register_object;
 }
 
-static llvm::Optional
-GetRegisterValue(NativeRegisterContext ®_ctx, uint32_t generic_regnum) {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
-  uint32_t reg_num = reg_ctx.ConvertRegisterKindToRegisterNumber(
-  eRegisterKindGeneric, generic_regnum);
-  const RegisterInfo *const reg_info_p =
-  reg_ctx.GetRegisterInfoAtIndex(reg_num);
-
-  if (reg_info_p == nullptr || reg_info_p->value_regs != nullptr) {
-LLDB_LOGF(log, "%s failed to get register info for register index %" 
PRIu32,
-  __FUNCTION__, reg_num);
-return {};
-  }
-
-  RegisterValue reg_value;
-  Status error = reg_ctx.ReadRegister(reg_info_p, reg_value);
-  if (error.Fail()) {
-LLDB_LOGF(log, "%s failed to read register '%s' index %" PRIu32 ": %s",
-  __FUNCTION__,
-  reg_info_p->name ? reg_info_p->name : "",
-  reg_num, error.AsCString());
-return {};
-  }
-  return reg_value;
-}
-
-static json::Object CreateMemoryChunk(json::Array &stack_memory_chunks,
-  addr_t address,
-  std::vector &bytes) {
-  json::Object chunk;
-  chunk.try_emplace("address", static_cast(address));
-  StreamString stream;
-  for (uint8_t b : bytes)
-stream.PutHex8(b);
-  chunk.try_emplace("bytes", stream.GetString().str());
-  return chunk;
-}
-
-static json::Array GetStackMemoryAsJSON(NativeProcessProtocol &process,
-NativeThreadProtocol &thread) {
-  uint32_t address_size = process.GetArchitecture().GetAddressByteSize();
-  const size_t kStackTopMemoryInfoWordSize = 12;
-  size_t stack_top_memory_info_byte_size =
-  kStackTopMemoryInfoWordSize * address_size;
-  const size_t kMaxStackSize = 128 * 1024;
-  const size_t kMaxFrameSize = 4 * 1024;
-  size_t fp_and_ra_size = 2 * address_size;
-  const size_t kMaxFrameCount = 128;
-
-  NativeRegisterContext ®_ctx = thread.GetRegisterContext();
-
-  json::Array stack_memory_chunks;
-
-  lldb::addr_t sp_value;
-  if (llvm::Optional optional_sp_value =
-  GetRegisterValue(reg_ctx, LLDB_REGNUM_GENERIC_SP)) {
-sp_value = optional_sp_value->GetAsUInt64();
-  } else {
-return stack_memory_chunks;
-  }
-  lldb::addr_t fp_value;
-  if (llvm::Optional optional_fp_value =
-  GetRegisterValue(reg_ctx, LLDB_REGNUM_GENERIC_FP)) {
-fp_value = optional_fp_value->GetAsUInt64();
-  } else {
-return stack_memory_chunks;
-  }
-
-  // First, make sure we copy the top stack_top_memory_info_byte_size bytes
-  // from the stack.
-  size_t byte_count = std::min(stack_top_memory_info_byte_size,
-   static_cast(fp_value - sp_value));
-  std::vector buf(byte_count, 0);
-
-  size_t bytes_read = 0;
-  Status error = process.ReadMemoryWithoutTrap(sp_value, buf.data(), 
byte_count,
-   bytes_read);
-  if (error.Success() && bytes_read > 0) {
-buf.resize(bytes_read);
-stack_memory_chunks.push_back(
-CreateMemoryChunk(stack_memory_chunks, sp_value, buf));
-  }
-
-  // Additionally, try to walk the frame pointer link chain. If the frame
-  // is too big or if the frame pointer points too far, stop the walk.
-  addr_t max_frame_pointer = sp_value + kMaxStackSize;
-  for (size_t i = 0; i < kMaxFrameCount; i++) {

[Lldb-commits] [lldb] 2a436a0 - Mark TestFixIts.py xfail for LLDB AArch64/Linux

2020-04-07 Thread Muhammad Omair Javaid via lldb-commits


Author: Muhammad Omair Javaid
Date: 2020-04-07T17:11:22+05:00
New Revision: 2a436a07ae9749ed4d3f42c0d1e660eb4f7ca6e8

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

LOG: Mark TestFixIts.py xfail for LLDB AArch64/Linux

Added: 


Modified: 
lldb/test/API/commands/expression/fixits/TestFixIts.py

Removed: 




diff  --git a/lldb/test/API/commands/expression/fixits/TestFixIts.py 
b/lldb/test/API/commands/expression/fixits/TestFixIts.py
index 44d3ddd34068..b348e2dca2cc 100644
--- a/lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ b/lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -23,6 +23,7 @@ def test_with_dummy_target(self):
 self.assertEqual(result, lldb.eReturnStatusSuccessFinishResult, "The 
expression was successful.")
 self.assertTrue("Fix-it applied" in ret_val.GetError(), "Found the 
applied FixIt.")
 
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"])
 def test_with_target(self):
 """Test calling expressions with errors that can be fixed by the 
FixIts."""
 self.build()



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

[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Follow DW_AT_decl_file when setting breakpoint

2020-04-07 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

kwk updated this revision to Diff 255648.
kwk added a comment.

- Simplified test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.cpp
  lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
  lldb/test/Shell/Breakpoint/Inputs/search-support-files.h
  lldb/test/Shell/Breakpoint/search-support-files.test

Index: lldb/test/Shell/Breakpoint/search-support-files.test
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/search-support-files.test
@@ -0,0 +1,46 @@
+# In these tests we will set breakpoints on a function by name. That function
+# is defined in a header file (search-support-files.h) and will therefore be
+# inlined into the file that includes it (search-support-files.cpp).
+#
+# TODO(kwk): Check that we can also do the same with C++ methods in files?
+#(See https://lldb.llvm.org/use/tutorial.html and look for --method.)
+
+# RUN: %build %p/Inputs/search-support-files.cpp -o dummy.out
+# RUN: %lldb -b -s %s dummy.out | FileCheck --color --dump-input=fail %s 
+
+#Set breakpoint by function name.
+
+breakpoint set -n inlined_42
+# CHECK: (lldb) breakpoint set -n inlined_42
+# CHECK-NEXT: Breakpoint 1: where = dummy.out`inlined_42() + 4 at search-support-files.h:1:20, address = 0x0{{.*}}
+
+breakpoint set -n main
+# CHECK: (lldb) breakpoint set -n main
+# CHECK-NEXT: Breakpoint 2: where = dummy.out`main + 22 at search-support-files.cpp:4:11, address = 0x0{{.*}}
+
+#   Set breakpoint by function name and filename (the one in which the function
+#   is inlined, aka the compilation unit).
+
+breakpoint set -n inlined_42 -f search-support-files.cpp
+# CHECK: (lldb) breakpoint set -n inlined_42 -f search-support-files.cpp
+# CHECK-NEXT: Breakpoint 3: no locations (pending).
+
+#   Set breakpoint by function name and source filename (the file in which the
+#   function is defined).
+#
+#   NOTE: This test is the really interesting one as it shows that we can
+# search by source files that are themselves no compulation units.
+
+breakpoint set -n inlined_42 -f search-support-files.h
+# CHECK: (lldb) breakpoint set -n inlined_42 -f search-support-files.h
+# CHECK-NEXT: Breakpoint 4: where = dummy.out`inlined_42() + 4 at search-support-files.h:1:20, address = 0x0{{.*}}
+
+#   Set breakpoint by function name and source filename. This time the file
+#   doesn't exist to prove that we haven't widen the search space too much. When
+#   we search for a function in a file that doesn't exist, we should get no
+#   results.
+
+breakpoint set -n inlined_42 -f file-not-existing.h
+# CHECK: (lldb) breakpoint set -n inlined_42 -f file-not-existing.h
+# CHECK-NEXT: Breakpoint 5: no locations (pending).
+# CHECK-NEXT: WARNING: Unable to resolve breakpoint to any actual locations.
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.h
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.h
@@ -0,0 +1 @@
+int inlined_42() { return 42; }
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
@@ -0,0 +1,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = inlined_42();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -715,9 +715,13 @@
   FileSpec cu_spec;
   if (sym_ctx.comp_unit)
 cu_spec = sym_ctx.comp_unit->GetPrimaryFile();
-  if (m_cu_spec_list.FindFileIndex(0, cu_spec, false) == UINT32_MAX)
-return false; // Fails the file check
-  return SearchFilterByModuleList::ModulePasses(sym_ctx.module_sp); 
+  if (m_cu_spec_list.FindFileIndex(0, cu_spec, false) != UINT32_MAX)
+return true;
+  // ^ If the primary source file associated with the symbol's compile unit is in
+  // the list of CU's or support files, that's enough.
+  // TODO(kwk): Is that enough?
+
+  return SearchFilterByModuleList::ModulePasses(sym_ctx.module_sp);
 }
 
 bool SearchFilterByModuleListAndCU::CompUnitPasses(FileSpec &fileSpec) {
@@ -811,7 +815,7 @@
 }
 
 uint32_t SearchFilterByModuleListAndCU::GetFilterRequiredItems() {
-  return eSymbolContextModule | eSymbolContextCompUnit;
+  return eSymbolContextModule | eSymbolContextCompUnit | eSymbolContextFunction;
 }
 
 void SearchFilterByModuleListAndCU::Dump(Stream *s) const {}
Index: lldb/source/Breakpoint/BreakpointResolverName.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ lldb/source/Breakpoint/BreakpointResolverNam

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

2020-04-07 Thread Jaroslav Sevcik via Phabricator via lldb-commits

jarin added a comment.

Looking at the code for flushing L1 cache 
,
 it appears broken. I am guessing that is the reason for the failure of my 
patch on the bots.

Here is the L1  flushing code (after checking L1 
 is not empty).

  AddrRange flush_range(addr, size);
  BlockMap::iterator pos = m_L1_cache.upper_bound(addr);
  if (pos != m_L1_cache.begin()) {
--pos;
  }
  while (pos != m_L1_cache.end()) {
AddrRange chunk_range(pos->first, pos->second->GetByteSize());
if (!chunk_range.DoesIntersect(flush_range))
  break;
pos = m_L1_cache.erase(pos);
  }

For instance, let the cache contains two chunks (10, 10) and (30, 10). Let us 
flush (25, 10). Then the while loop chunk_range will be (10, 10), which is not 
intersecting with (25, 10), so we do not flush anything. This is clearly wrong 
because (30, 10) should be flushed.

I am wondering whether something like this would be correct (I am still a bit 
worried about the case of overlapping things in L1 
).

  AddrRange flush_range(addr, size);
  BlockMap::iterator pos = m_L1_cache.upper_bound(addr);
  if (pos != m_L1_cache.begin()) {
// If we are not in the beginning, the previous range might be also 
intersecting.
BlockMap::iterator previous = pos;
previous--;
AddrRange previous_range(previous->first, previous->second->GetByteSize()) 
if (!previous_range.DoesIntersect(flush_range))
  pos = m_L1_cache.erase(previous);
  }
  while (pos != m_L1_cache.end()) {
AddrRange chunk_range(pos->first, pos->second->GetByteSize());
if (!chunk_range.DoesIntersect(flush_range))
  break;
pos = m_L1_cache.erase(pos);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398



___
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

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Thank you for trying out gtest. I'm very happy that this is working out for 
you. The test looks fine -- just please move it to `unittests/API` (new folder) 
 -- that's where it logically belongs as it's testing the API code (and it also 
avoids dangerous ODR violations resulting from linking in both liblldb and its 
constituent libraries).

It sounds like you and Jim have mostly agreed on the way this should be 
implemented. I'll let you two work out any kinks there.




Comment at: lldb/unittests/Interpreter/TestAutoRepeat.cpp:25
+SBDebugger::Initialize();
+m_dbg = SBDebugger::Create(false /*source_init_files*/);
+m_interp = m_dbg.GetCommandInterpreter();

The [[ http://llvm.org/docs/CodingStandards.html#comment-formatting | 
recommended llvm style ]] for this is `/*arg_name=*/value`.



Comment at: lldb/unittests/Interpreter/TestAutoRepeat.cpp:61
+m_interp.HandleCommand("", result);
+assert(!result.Succeeded() &&
+   "The command should fail as a repeated command");

make a gtest assert out of this?


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] 95054ae - [lldb][NFC] Fix typo in 'watchpoint delete' error message

2020-04-07 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2020-04-07T16:11:32+02:00
New Revision: 95054aeb07061dbfd6575c10673b9563430de64a

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

LOG: [lldb][NFC] Fix typo in 'watchpoint delete' error message

Added: 


Modified: 
lldb/source/Commands/CommandObjectWatchpoint.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp 
b/lldb/source/Commands/CommandObjectWatchpoint.cpp
index 63baae8e4760..ce4662930a7c 100644
--- a/lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -41,7 +41,7 @@ static bool CheckTargetForWatchpointOperations(Target *target,
   bool process_is_valid =
   target->GetProcessSP() && target->GetProcessSP()->IsAlive();
   if (!process_is_valid) {
-result.AppendError("Thre's no process or it is not alive.");
+result.AppendError("There's no process or it is not alive.");
 result.SetStatus(eReturnStatusFailed);
 return false;
   }



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

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

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

I don't have a hard objection to this, but if we're don't have a use for it, I 
don't see a need for adding it right now.
Particularly as it sounds weird to me that "user commands" are not a subset of 
"commands". I know that is how the underlying api works, but the underlying api 
looks weird to me too. However, that one (unlike the SB version) can be changed 
whenever we want.

As for the test, the same remarks I made on the other review apply here too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582



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

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added a comment.

I agree, if we don't have an immediate use then I don't think we should add it 
now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582



___
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

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

BTW, in gtest tests, the prevailing convention is to name test .cpp files 
according to the name of the .cpp file under test. So, in this case, that would 
be something like `unittests/API/SBCommandInterpreterTest.cpp`. I wouldn't want 
to take that convention to the extreme and have 10kloc test files, but until we 
start having more of these tests, I think we can stick to that.


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] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

I think the part of the patch that adds documentation to the existing method 
should go in (In general all patches that just add documentation have my 
blanket approval, so please just commit those).

For the additional function: If you just change that CommandExists also checks 
user commands, I think that seems reasonable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582



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

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/bindings/python.swig:132-136
+if 'LLDB_REPRODUCER_CAPTURE_PATH' in os.environ:
+   SBReproducer.Capture(os.environ['LLDB_REPRODUCER_CAPTURE_PATH'])
+   SBReproducer.SetAutoGenerate(True)
+elif 'LLDB_REPRODUCER_REPLAY_PATH' in os.environ:
+   SBReproducer.APIReplay(os.environ['LLDB_REPRODUCER_REPLAY_PATH'])

I am doubly unhappy about this. Not only does it leak testing logic into 
production code, it actually does that using environment variables... :/

I take it the problem here is that reproducers need to be set up before the 
Initialize call, but the lldb module does not let us do that as it calls 
Initialize automatically? I am not sure that was such a wise idea, but that 
ship has sailed a long time ago.

So, can we at least do something like [[ 
https://stackoverflow.com/questions/3720740/pass-variable-on-import/39360070#39360070
 | this ]] instead? I.e. have a separate configuration module which would be 
imported here and would drive this logic.

I believe it should be possible to arrange it so that the configuration module 
does not even have to get shipped -- this code could just attempt the import 
and ignore any errors.

And the configuration module itself would not need to do anything 
reproducer-related either. It could just contain a single flag to disable the 
automatic initialization. That sounds like a thing someone might conceivably 
want anyway, and it could be later extended to handle other kinds of global 
lldb configuration (ConstString pool size maybe ?).

Once the Initialize call is disabled, the normal test harness can handle the 
rest...


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] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D77582#1966962 , @teemperor wrote:

> I think the part of the patch that adds documentation to the existing method 
> should go in (In general all patches that just add documentation have my 
> blanket approval, so please just commit those).

About that... the existing practice has been to attach documentation to the 
python versions of these interfaces (`bindings/interface/*.i`). Now we started 
to add it to the c++ headers too. More documentation is always better, but OTOH 
repetition creates maintenance problems and increases the risk of it going 
stale. Should choose one of the two interfaces and put all documentation there 
(and maybe have a way to auto-generate the other one) ?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582

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

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

2020-04-07 Thread Frank Ch. Eigler via Phabricator via lldb-commits

fche2 added a comment.

>> So it might be good to have the SymbolVendors use one or more SymbolServer 
>> plug-ins.
> 
> I don't believe we have anything that would require all modules in a given 
> target (or whatever) to use the same symbol vendor type.  [...]

Just for clarity, is someone proposing to undertake such a rework of that 
infrastructure?  It sounds like this is becoming a prerequisite for Konrad's 
patch, but if no one's actually doing it, that means Konrad's work is on hold 
indefinitely.  Is that the intent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



___
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

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I can't say I have tried this, but ummm.. why is there a need for a separate 
iterator class for this equal_range stuff? Couldn't this be implemented via an 
iterator_pair of the iterators of the underlying container (found via 
lower/upper_bound) ?


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] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:130-163
+  bool GetValues(ConstString unique_cstr,
+ std::function callback) const {
+for (const Entry &entry : llvm::make_range(std::equal_range(
+ m_map.begin(), m_map.end(), unique_cstr, Compare(
+  if (callback(entry.value))
+return true;
+

jankratochvil wrote:
> jankratochvil wrote:
> > labath wrote:
> > > I'm not sure these functions are really needed. They have just one 
> > > caller, and they'd be trivial if this class provided appropriate 
> > > abstractions. Maybe just add begin/end/equal_range methods, so that one 
> > > can write:
> > > ```
> > > for (value: map / map.equal_range(str))
> > >   stuff
> > > ```
> > > ?
> > > 
> > > (ideally, I'd like to have iterators instead of callbacks for the index 
> > > classes too, but iterators and class hierarchies don't mix very well)
> > Do you mean the standard iteration:
> > ```
> > for (std::pair pair : map)
> >   stuff;
> > ```
> > or really
> > ```
> > for (DIERef ref : map)
> >   stuff;
> > ```
> > ?
> > 
> Implemented. IIUC for regexes it should be matched in a caller - or to make 
> also an iterator for regexes?
> ```
>   for (const auto &entry : m_map.equal_range(name))
> if (callback(entry.value))
> ```
> ```
>   for (const auto &entry : m_map)
> if (regex.Execute(entry.cstring.GetCString())) {
>   if (callback(entry.value))
> ```
> 
Yes, that is correct.


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] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-07 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

In D77327#1967060 , @labath wrote:

> I can't say I have tried this, but ummm.. why is there a need for a separate 
> iterator class for this equal_range stuff? Couldn't this be implemented via 
> an iterator_pair of the iterators of the underlying container (found via 
> lower/upper_bound) ?


OK, true, I will rework it.


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] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-07 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

ValueObjects know their execution context, and so they can get to their frame, 
if they have one.  The language of the frame seems like the best thing to clue 
off here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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

2020-04-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added a reviewer: labath.
Herald added a subscriber: mgorny.
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:957
 
+import lldbconfig
 import lldb

This is a NO-OP for now, but just shows that we can load the lldbconfig module. 


Using the approach described in [1] and suggested by Pavel 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. More specifically it makes it possible to delay 
initializing the debugger, which is needed for testing the reproducer.

I chose to name the module lldbconfig rather than `lldb.config` because I 
thought it would be weird to import a "submodule" before the main module, but 
if anyone feels the other approach is more desirable I'd be happy to change it.

[1] 
https://stackoverflow.com/questions/3720740/pass-variable-on-import/39360070#39360070


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D77661

Files:
  lldb/CMakeLists.txt
  lldb/bindings/python.swig
  lldb/bindings/python/lldbconfig.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/bindings/python/lldbconfig.py
===
--- /dev/null
+++ lldb/bindings/python/lldbconfig.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/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -99,11 +99,13 @@
   get_target_property(lldb_bindings_dir swig_wrapper BINARY_DIR)
 
   if(LLDB_BUILD_FRAMEWORK)
-set(lldb_python_build_path 
"${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework/Resources/Python/lldb")
+set(python_build_path 
"${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework/Resources/Python")
   else()
-set(lldb_python_build_path 
"${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_PYTHON_RELATIVE_PATH}/lldb")
+set(python_build_path 
"${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_PYTHON_RELATIVE_PATH}")
   endif()
 
+  set(lldb_python_build_path "${python_build_path}/lldb")
+
   # Add a Post-Build Event to copy over Python files and create the symlink
   # to liblldb.so for the Python API(hardlink on Windows).
   add_custom_target(finish_swig ALL VERBATIM
@@ -115,14 +117,24 @@
 add_custom_command(TARGET finish_swig POST_BUILD VERBATIM
   COMMAND ${CMAKE_COMMAND} -E copy
 "${LLDB_SOURCE_DIR}/third_party/Python/module/six/six.py"
-"${lldb_python_build_path}/../six.py")
+"${python_build_path}/six.py")
   endif()
 
+  add_custom_command(TARGET finish_swig POST_BUILD VERBATIM
+COMMAND ${CMAKE_COMMAND} -E make_directory
+  "${python_build_path}/lldbconfig")
+
+  add_custom_command(TARGET finish_swig POST_BUILD VERBATIM
+COMMAND ${CMAKE_COMMAND} -E copy
+  "${LLDB_SOURCE_DIR}/bindings/python/lldbconfig.py"
+  "${python_build_path}/lldbconfig/__init__.py")
+
   add_custom_command(TARGET finish_swig POST_BUILD VERBATIM
 COMMAND ${CMAKE_COMMAND} -E copy
   "${lldb_bindings_dir}/lldb.py"
   "${lldb_python_build_path}/__init__.py")
 
+
   function(create_python_package pkg_dir)
 cmake_parse_arguments(ARG "NOINIT" "" "FILES" ${ARGN})
 if(ARG_FILES)


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/bindings/python/lldbconfig.py
===
--- /dev/null
+++ lldb/bindings/python/lldbconfig.py
@@ -0,0 +1 @@
+INITIALIZE = True
Index: lldb/bindings/python.swig
===

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

2020-04-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

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



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:957
 
+import lldbconfig
 import lldb

This is a NO-OP for now, but just shows that we can load the lldbconfig module. 


Repository:
  rLLDB LLDB

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] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

In D77582#1967031 , @labath wrote:

> In D77582#1966962 , @teemperor wrote:
>
> > I think the part of the patch that adds documentation to the existing 
> > method should go in (In general all patches that just add documentation 
> > have my blanket approval, so please just commit those).
>
>
> About that... the existing practice has been to attach documentation to the 
> python versions of these interfaces (`bindings/interface/*.i`). Now we 
> started to add it to the c++ headers too. More documentation is always 
> better, but OTOH repetition creates maintenance problems and increases the 
> risk of it going stale. Should choose one of the two interfaces and put all 
> documentation there (and maybe have a way to auto-generate the other one) ?

We need to keep the documentation in the interface files, that's what produces 
the python and online doc help.  That's where the vast majority of users of the 
API will go to find help for it.  But the C++ docs do go into the C++ API docs 
on the website so they also have some (if not as much) value.  Having a way to 
salt one from the other would be ideal, though sometimes the Python API needs a 
little extra color so this isn't entirely trivial.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582

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

2020-04-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Use `lldbconfig` to initialize reproducers.


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

https://reviews.llvm.org/D77588

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBReproducer.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/SBReproducer.h
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/source/API/SBReproducer.cpp
  lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
  lldb/test/API/lit.cfg.py
  lldb/test/API/lldbtest.py

Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -1,6 +1,6 @@
 from __future__ import absolute_import
 import os
-
+import tempfile
 import subprocess
 import sys
 
@@ -86,6 +86,14 @@
 shutil.copy(python, copied_python)
 cmd[0] = copied_python
 
+reproducer_path = os.path.join(tempfile.gettempdir(), testFile)
+if 'lldb-repro-capture' in test.config.available_features:
+test.config.environment[
+'LLDB_REPRODUCER_CAPTURE_PATH'] = reproducer_path
+elif 'lldb-repro-replay' in test.config.available_features:
+test.config.environment[
+'LLDB_REPRODUCER_REPLAY_PATH'] = reproducer_path
+
 timeoutInfo = None
 try:
 out, err, exitCode = lit.util.executeCommand(
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -60,6 +60,17 @@
   config.environment['LLDB_CAPTURE_REPRODUCER'] = os.environ[
   'LLDB_CAPTURE_REPRODUCER']
 
+# Support running the test suite under the lldb-repro wrapper. This makes it
+# possible to capture a test suite run and then rerun all the test from the
+# just captured reproducer.
+lldb_repro_mode = lit_config.params.get('lldb-run-with-repro', None)
+if lldb_repro_mode:
+  lit_config.note("Running Shell test with lldb-repo in {} mode.".format(lldb_repro_mode))
+  if lldb_repro_mode == 'capture':
+config.available_features.add('lldb-repro-capture')
+  elif lldb_repro_mode == 'replay':
+config.available_features.add('lldb-repro-replay')
+
 # Clean the module caches in the test build directory. This is necessary in an
 # incremental build whenever clang changes underneath, so doing it once per
 # lit.py invocation is close enough.
Index: lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
===
--- lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
+++ lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
@@ -20,6 +20,7 @@
 @skipIfWindows
 @skipIfRemote
 @skipIfiOSSimulator
+@skipIfReproducer
 def test_reproducer_attach(self):
 """Test thread creation after process attach."""
 exe = '%s_%d' % (self.testMethodName, os.getpid())
Index: lldb/source/API/SBReproducer.cpp
===
--- lldb/source/API/SBReproducer.cpp
+++ lldb/source/API/SBReproducer.cpp
@@ -124,6 +124,15 @@
   return nullptr;
 }
 
+const char *SBReproducer::APIReplay(const char *path) {
+  static std::string error;
+  if (auto e = Reproducer::Initialize(ReproducerMode::Replay, FileSpec(path))) {
+error = llvm::toString(std::move(e));
+return error.c_str();
+  }
+  return nullptr;
+}
+
 const char *SBReproducer::Replay(const char *path) {
   return SBReproducer::Replay(path, false);
 }
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -196,6 +196,17 @@
 metavar='platform-working-dir',
 help='The directory to use on the remote platform.')
 
+# Reproducer options
+group = parser.add_argument_group('Reproducer options')
+group.add_argument(
+'--capture-path',
+metavar='reproducer path',
+help='The reproducer capture path')
+group.add_argument(
+'--replay-path',
+metavar='reproducer path',
+help='The reproducer replay path')
+
 # Test-suite behaviour
 group = parser.add_argument_group('Runtime behaviour options')
 X('-d', 'Suspend the process after launch to wait indefinitely for a debugger to attach')
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -429,6 +429,17 @@

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

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: amccarth, jingham, JDevlieghere, compnerd.
Herald added a project: LLDB.
labath updated this revision to Diff 255727.
labath added a comment.

include some changes I forgot (they're the ones that need cleaning up the most)


This is WIP because it could still use a bit of cleanup (and a better
commit message). However, I believe the general direction is pretty
obvious, so I'd like to hear your thoughts on this.

With this patch some of the TestLoadUnload tests already pass on
windows, and even more will pass once D77287  
lands.


Repository:
  rG LLVM Github Monorepo

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,71 +1,62 @@
-#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);
-}
+  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 ());
-dlclose (a_dylib_handle);
+dylib_close(a_dylib_handle);
 
-c_dylib_handle = dlopen (c_name, RTLD_NOW);
+c_dylib_handle = dylib_open(c_name);
 if (c_dylib_handle == NULL)
 {
-fprintf (stderr, "%s\n", dlerror());
-exit (3);
+  fprintf(stderr, "%s\n", dylib_last_error());
+  exit(3);
 }
-a_function = (int (*) ()) dlsym (c_dylib_handle, "c_function");
+a_function = (int (*)())dylib_get_symbol(c_dylib_handle, "c_function");
 if (a_function == NULL)
 {
-fprintf (stderr, "%s\n", dlerror());
-exit (4);
+  fprintf(stderr, "%s\n", dylib_last_error());
+  exit(4);
 }
 
-a_dylib_handle = dlopen (a_name, RTLD_NOW);
+a_dylib_handle = dylib_open(a_name);
 if (a_dylib_handle == NULL)
 {
-fprintf (stderr, "%s\n", dlerror());
-exit (5);
+  fprintf(stderr, "%s\n", dylib_last_error());
+  exit(5);
 }
 
-a_function = (int (*) ()) dlsym (a_dylib_handle, "a_function");
+a_function = (int (*)())dylib_get_symbol(a_dylib_handle, "a_function");
 if (a_function == NULL)
 {
-fprintf (stderr, "%s\n", dlerror());
-exit (6);
+  fprintf(stderr, "%s\n", dylib_last_error());
+  exit(6);
 }
 printf ("Second time around, got: %d\n", a_function ());
-dlclose (a_dylib_handle);
+dylib_close(a_dylib_handle);
 
-int d_function(void);
+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_DYLI

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

2020-04-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Update `lldbtest.py` too.


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

https://reviews.llvm.org/D77588

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBReproducer.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/SBReproducer.h
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/source/API/SBReproducer.cpp
  lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
  lldb/test/API/lit.cfg.py
  lldb/test/API/lldbtest.py

Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -1,6 +1,6 @@
 from __future__ import absolute_import
 import os
-
+import tempfile
 import subprocess
 import sys
 
@@ -86,6 +86,12 @@
 shutil.copy(python, copied_python)
 cmd[0] = copied_python
 
+reproducer_path = os.path.join(tempfile.gettempdir(), testFile)
+if 'lldb-repro-capture' in test.config.available_features:
+cmd.extend(['--capture-path', reproducer_path])
+elif 'lldb-repro-replay' in test.config.available_features:
+cmd.extend(['--replay-path', reproducer_path])
+
 timeoutInfo = None
 try:
 out, err, exitCode = lit.util.executeCommand(
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -60,6 +60,17 @@
   config.environment['LLDB_CAPTURE_REPRODUCER'] = os.environ[
   'LLDB_CAPTURE_REPRODUCER']
 
+# Support running the test suite under the lldb-repro wrapper. This makes it
+# possible to capture a test suite run and then rerun all the test from the
+# just captured reproducer.
+lldb_repro_mode = lit_config.params.get('lldb-run-with-repro', None)
+if lldb_repro_mode:
+  lit_config.note("Running Shell test with lldb-repo in {} mode.".format(lldb_repro_mode))
+  if lldb_repro_mode == 'capture':
+config.available_features.add('lldb-repro-capture')
+  elif lldb_repro_mode == 'replay':
+config.available_features.add('lldb-repro-replay')
+
 # Clean the module caches in the test build directory. This is necessary in an
 # incremental build whenever clang changes underneath, so doing it once per
 # lit.py invocation is close enough.
Index: lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
===
--- lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
+++ lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
@@ -20,6 +20,7 @@
 @skipIfWindows
 @skipIfRemote
 @skipIfiOSSimulator
+@skipIfReproducer
 def test_reproducer_attach(self):
 """Test thread creation after process attach."""
 exe = '%s_%d' % (self.testMethodName, os.getpid())
Index: lldb/source/API/SBReproducer.cpp
===
--- lldb/source/API/SBReproducer.cpp
+++ lldb/source/API/SBReproducer.cpp
@@ -124,6 +124,15 @@
   return nullptr;
 }
 
+const char *SBReproducer::APIReplay(const char *path) {
+  static std::string error;
+  if (auto e = Reproducer::Initialize(ReproducerMode::Replay, FileSpec(path))) {
+error = llvm::toString(std::move(e));
+return error.c_str();
+  }
+  return nullptr;
+}
+
 const char *SBReproducer::Replay(const char *path) {
   return SBReproducer::Replay(path, false);
 }
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -196,6 +196,17 @@
 metavar='platform-working-dir',
 help='The directory to use on the remote platform.')
 
+# Reproducer options
+group = parser.add_argument_group('Reproducer options')
+group.add_argument(
+'--capture-path',
+metavar='reproducer path',
+help='The reproducer capture path')
+group.add_argument(
+'--replay-path',
+metavar='reproducer path',
+help='The reproducer replay path')
+
 # Test-suite behaviour
 group = parser.add_argument_group('Runtime behaviour options')
 X('-d', 'Suspend the process after launch to wait indefinitely for a debugger to attach')
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -429,6 +429,17 @@
 configuration.results_formatter_name = (
 "lldbsuite.test_event.formatter.results_formatte

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

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

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

include some changes I forgot (they're the ones that need cleaning up the most)


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,71 +1,62 @@
-#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);
-}
+  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 ());
-dlclose (a_dylib_handle);
+dylib_close(a_dylib_handle);
 
-c_dylib_handle = dlopen (c_name, RTLD_NOW);
+c_dylib_handle = dylib_open(c_name);
 if (c_dylib_handle == NULL)
 {
-fprintf (stderr, "%s\n", dlerror());
-exit (3);
+  fprintf(stderr, "%s\n", dylib_last_error());
+  exit(3);
 }
-a_function = (int (*) ()) dlsym (c_dylib_handle, "c_function");
+a_function = (int (*)())dylib_get_symbol(c_dylib_handle, "c_function");
 if (a_function == NULL)
 {
-fprintf (stderr, "%s\n", dlerror());
-exit (4);
+  fprintf(stderr, "%s\n", dylib_last_error());
+  exit(4);
 }
 
-a_dylib_handle = dlopen (a_name, RTLD_NOW);
+a_dylib_handle = dylib_open(a_name);
 if (a_dylib_handle == NULL)
 {
-fprintf (stderr, "%s\n", dlerror());
-exit (5);
+  fprintf(stderr, "%s\n", dylib_last_error());
+  exit(5);
 }
 
-a_function = (int (*) ()) dlsym (a_dylib_handle, "a_function");
+a_function = (int (*)())dylib_get_symbol(a_dylib_handle, "a_function");
 if (a_function == NULL)
 {
-fprintf (stderr, "%s\n", dlerror());
-exit (6);
+  fprintf(stderr, "%s\n", dylib_last_error());
+  exit(6);
 }
 printf ("Second time around, got: %d\n", a_function ());
-dlclose (a_dylib_handle);
+dylib_close(a_dylib_handle);
 
-int d_function(void);
+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 700; // Find this line number within d_dunction().
 }
Index: lldb/test/API/functionalities/load_unload/b.cpp
===
--- lldb/test/API/functionalities/load_unload/b.cpp
+++ lldb/test/API/functionalities/load_unload/b.cpp
@@ -5,8 +5,4 @@
 
 int b_global = b_init();
 
-int
-b_function ()
-{
-return 500;
-}
+i

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

2020-04-07 Thread Jim Ingham via Phabricator via lldb-commits

jingham added inline comments.



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)

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.


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] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-07 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil updated this revision to Diff 255761.

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 &method_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 &method_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 &SymbolFileDWARFDwo::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 &dwarf_cu);
 
-  virtual size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray &method_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 &method_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 &die, SymbolContext &sc) {
@@ -2048,24 +2046,20 @@
   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 &die_ref = die_offsets[i];
+  SymbolCo

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread walter erquinigo via Phabricator via lldb-commits

wallace abandoned this revision.
wallace added a comment.

Will revisit it whenever I need it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582



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

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Thanks. This looks fine, just a question about the placement of the lldbconfig 
module.




Comment at: lldb/CMakeLists.txt:130
+  "${LLDB_SOURCE_DIR}/bindings/python/lldbconfig.py"
+  "${python_build_path}/lldbconfig/__init__.py")
+

If we're not going to ship this file, should we place it under 
`packages/Python/...` (I'm sure there is some place we could put it where it 
would already be in `sys.path` ? That way it would not be accessible from 
regular lldb runs, only for those from dotest, which would decrease the 
likelyhood of lldb suddenly stopping to work after being installed.



Comment at: lldb/bindings/python.swig:131
 %pythoncode%{
+INITIALIZE = True
+try:

Maybe `_initialize` (the convention for "private" variables in python)?


Repository:
  rLLDB LLDB

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] D77588: [lldb/Reproducers] Make it possible to capture reproducers for the Python test suite.

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/decorators.py:861
+def is_reproducer():
+if ('LLDB_REPRODUCER_CAPTURE_PATH' in os.environ or 
'LLDB_REPRODUCER_REPLAY_PATH' in os.environ):
+return "reproducers unsupported"

Should this use variables from configuration.py now?



Comment at: lldb/test/API/lit.cfg.py:68
+if lldb_repro_mode:
+  lit_config.note("Running Shell test with lldb-repo in {} 
mode.".format(lldb_repro_mode))
+  if lldb_repro_mode == 'capture':

lldb-repro



Comment at: lldb/test/API/lldbtest.py:89
 
+reproducer_path = os.path.join(tempfile.gettempdir(), testFile)
+if 'lldb-repro-capture' in test.config.available_features:

I remember some objections to shoving everything in $TMP. Are we not able to 
put this in the build dir somewhere?


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

2020-04-07 Thread Jan Kratochvil via Phabricator via lldb-commits

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



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:102
 
+  // Helper iterator for the 'equal_range' method.
+  class CString_iterator {

aprantl wrote:
> ///
I am sorry but all comments in this file are // . I can make a conversion of 
the file to Doxygen in a separate file if you wish but I am not sure how much 
is Doxygen widespread in LLDB etc.



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:103
+  // Helper iterator for the 'equal_range' method.
+  class CString_iterator {
+  public:

shafik wrote:
> `CStringIterator`? 
This custom iterator has been now dropped upon advice by @labath the standard 
iterator is enough.



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:134
+  private:
+const UniqueCStringMap &m_map;
+const Entry *m_entry_ptr;

shafik wrote:
> It feels like this should be a pointer, `m_entry_ptr` is a pointer already 
> and most uses in the class use the pointer value already.
This custom iterator has been now dropped upon advice by @labath the standard 
iterator is enough.
BTW I have seen [[ 
https://github.com/llvm/llvm-project/commit/6a2eb3671018993c316451884c530d6c208f0b80
 | patches ]] all pointers which cannot be `nullptr` to be converted to 
references which is why I used the reference here. But it is gone now anyway.



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] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-07 Thread Jan Kratochvil via Phabricator via lldb-commits

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

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 &method_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 &method_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 &SymbolFileDWARFDwo::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 &dwarf_cu);
 
-  virtual size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray &method_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 &method_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 &die, SymbolContext &sc) {
@@ -2048,24 +2046,20 @@
   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) {
-con

[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits

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

Adding @JDevlieghere as he is in the same time zone and knows about all this 
stuff. This is on the right track, but changing lldb-dotest is not enough -- 
you'll also need to change `test/API/lit.cfg.py` as that's what's used for 
regular "check-lldb" runs. Also, instead of environment variables, please add a 
command line switch to dotest. Environment variables make it hard to reproduce 
steps when copy-pasting commands.




Comment at: lldb/utils/lldb-dotest/lldb-dotest.in:32
+
+if lldb_build_intel_pt not in ["", "false", "off", "n", "no", "ignore"] \
+and not lldb_build_intel_pt.endswith("-NOTFOUND"):

The usual solution to this is to run `llvm_canonicalize_cmake_booleans` in 
cmake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77452



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

[Lldb-commits] [PATCH] D77480: Fix illegal early call to PyBuffer_Release in swig typemaps

2020-04-07 Thread Lawrence D'Anna via Phabricator via lldb-commits

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

fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77480



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

[Lldb-commits] [PATCH] D77480: Fix illegal early call to PyBuffer_Release in swig typemaps

2020-04-07 Thread Lawrence D'Anna via Phabricator via lldb-commits

lawrence_danna updated this revision to Diff 255772.
lawrence_danna added a comment.

fix initializer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77480

Files:
  lldb/bindings/python/python-typemaps.swig


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -488,39 +488,53 @@
 }
 }
 
+%inline %{
+
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII() {};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
+
+%}
+
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
+//
+// I've also moved the call to PyBuffer_Release to the end of the SWIG wrapper,
+// doing it right away is not legal according to the python buffer protocol.
 
 %define %pybuffer_mutable_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_WRITABLE);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_WRITABLE);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size/sizeof($*1_type));
 }
 %enddef
 
 %define %pybuffer_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; const void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_CONTIG_RO);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_CONTIG_RO);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size / sizeof($*1_type));
 }


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -488,39 +488,53 @@
 }
 }
 
+%inline %{
+
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII() {};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
+
+%}
+
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
+//
+// I've also moved the call to PyBuffer_Release to the end of the SWIG wrapper,
+// doing it right away is not legal according to the python buffer protocol.
 
 %define %pybuffer_mutable_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_WRITABLE);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_WRITABLE);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size/sizeof($*1_type));
 }
 %enddef
 
 %define %pybuffer_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; const void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_CONTIG_RO);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_CONTIG_RO);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size / sizeof($*1_type));
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 30a292c - [ScriptInterpreterPython] Remove buggy code to save/restore stdin.

2020-04-07 Thread Davide Italiano via lldb-commits


Author: Davide Italiano
Date: 2020-04-07T12:43:25-07:00
New Revision: 30a292c25df327eb35b341b919c4e9b5e80323be

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

LOG: [ScriptInterpreterPython] Remove buggy code to save/restore stdin.

Discussed on lldb-dev with Pavel Labath. This doesn't work for
background processes [causes Python to be stuck forever], and it's
unclear whether it's needed. There's no test, also. If this turns
out to be useful, it can be recommitted with a functional implementation
and a test.

Added: 


Modified: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index ee94a183e0dc..c53b3bd0fb65 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -224,10 +224,6 @@ struct InitializePythonRAII {
 public:
   InitializePythonRAII()
   : m_gil_state(PyGILState_UNLOCKED), m_was_already_initialized(false) {
-// Python will muck with STDIN terminal state, so save off any current TTY
-// settings so we can restore them.
-m_stdin_tty_state.Save(STDIN_FILENO, false);
-
 InitializePythonHome();
 
 #ifdef LLDB_USE_LIBEDIT_READLINE_COMPAT_MODULE
@@ -271,8 +267,6 @@ struct InitializePythonRAII {
   // We initialized the threads in this function, just unlock the GIL.
   PyEval_SaveThread();
 }
-
-m_stdin_tty_state.Restore();
   }
 
 private:



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

[Lldb-commits] [lldb] c8de17b - Fix illegal early call to PyBuffer_Release in swig typemaps

2020-04-07 Thread Lawrence D'Anna via lldb-commits


Author: Lawrence D'Anna
Date: 2020-04-07T13:31:29-07:00
New Revision: c8de17bca658e62bbf8c33eae839e457332e885e

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

LOG: Fix illegal early call to PyBuffer_Release in swig typemaps

Summary:
The buffer protocol does not allow us to just call PyBuffer_Release
and assume the buffer will still be there.   Most things that implement the
buffer protocol will let us get away with that, but not all.   We need
to release it at the end of the SWIG wrapper.

Reviewers: labath, jasonmolenda, JDevlieghere, vadimcn

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/bindings/python/python-typemaps.swig

Removed: 




diff  --git a/lldb/bindings/python/python-typemaps.swig 
b/lldb/bindings/python/python-typemaps.swig
index bfd7ef9007d1..46dcaf611a4f 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -488,39 +488,53 @@ bool SetNumberFromPyObject(double &number, 
PyObject *obj) {
 }
 }
 
+%inline %{
+
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII() {};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
+
+%}
+
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
+//
+// I've also moved the call to PyBuffer_Release to the end of the SWIG wrapper,
+// doing it right away is not legal according to the python buffer protocol.
 
 %define %pybuffer_mutable_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_WRITABLE);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_WRITABLE);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size/sizeof($*1_type));
 }
 %enddef
 
 %define %pybuffer_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; const void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_CONTIG_RO);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_CONTIG_RO);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size / sizeof($*1_type));
 }



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

2020-04-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Address code review feedback


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D77588

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBReproducer.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/SBReproducer.h
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/source/API/SBReproducer.cpp
  lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
  lldb/test/API/lit.cfg.py
  lldb/test/API/lldbtest.py

Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -1,6 +1,6 @@
 from __future__ import absolute_import
 import os
-
+import tempfile
 import subprocess
 import sys
 
@@ -67,14 +67,15 @@
 # python exe as the first parameter of the command.
 cmd = [sys.executable] + self.dotest_cmd + [testPath, '-p', testFile]
 
+builddir = getBuildDir(cmd)
+mkdir_p(builddir)
+
 # The macOS system integrity protection (SIP) doesn't allow injecting
 # libraries into system binaries, but this can be worked around by
 # copying the binary into a different location.
 if 'DYLD_INSERT_LIBRARIES' in test.config.environment and \
 (sys.executable.startswith('/System/') or \
 sys.executable.startswith('/usr/bin/')):
-builddir = getBuildDir(cmd)
-mkdir_p(builddir)
 copied_python = os.path.join(builddir, 'copied-system-python')
 if not os.path.isfile(copied_python):
 import shutil, subprocess
@@ -86,6 +87,16 @@
 shutil.copy(python, copied_python)
 cmd[0] = copied_python
 
+if 'lldb-repro-capture' in test.config.available_features or \
+   'lldb-repro-replay' in test.config.available_features:
+reproducer_root = os.path.join(builddir, 'reproducers')
+mkdir_p(reproducer_root)
+reproducer_path = os.path.join(reproducer_root, testFile)
+if 'lldb-repro-capture' in test.config.available_features:
+cmd.extend(['--capture-path', reproducer_path])
+else:
+cmd.extend(['--replay-path', reproducer_path])
+
 timeoutInfo = None
 try:
 out, err, exitCode = lit.util.executeCommand(
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -60,6 +60,17 @@
   config.environment['LLDB_CAPTURE_REPRODUCER'] = os.environ[
   'LLDB_CAPTURE_REPRODUCER']
 
+# Support running the test suite under the lldb-repro wrapper. This makes it
+# possible to capture a test suite run and then rerun all the test from the
+# just captured reproducer.
+lldb_repro_mode = lit_config.params.get('lldb-run-with-repro', None)
+if lldb_repro_mode:
+  lit_config.note("Running API tests in {} mode.".format(lldb_repro_mode))
+  if lldb_repro_mode == 'capture':
+config.available_features.add('lldb-repro-capture')
+  elif lldb_repro_mode == 'replay':
+config.available_features.add('lldb-repro-replay')
+
 # Clean the module caches in the test build directory. This is necessary in an
 # incremental build whenever clang changes underneath, so doing it once per
 # lit.py invocation is close enough.
Index: lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
===
--- lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
+++ lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
@@ -20,6 +20,7 @@
 @skipIfWindows
 @skipIfRemote
 @skipIfiOSSimulator
+@skipIfReproducer
 def test_reproducer_attach(self):
 """Test thread creation after process attach."""
 exe = '%s_%d' % (self.testMethodName, os.getpid())
Index: lldb/source/API/SBReproducer.cpp
===
--- lldb/source/API/SBReproducer.cpp
+++ lldb/source/API/SBReproducer.cpp
@@ -124,6 +124,15 @@
   return nullptr;
 }
 
+const char *SBReproducer::APIReplay(const char *path) {
+  static std::string error;
+  if (auto e = Reproducer::Initialize(ReproducerMode::Replay, FileSpec(path))) {
+error = llvm::toString(std::move(e));
+return error.c_str();
+  }
+  return nullptr;
+}
+
 const char *SBReproducer::Replay(const char *path) {
   return SBReproducer::Replay(path, false);
 }
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dote

[Lldb-commits] [PATCH] D77480: Fix illegal early call to PyBuffer_Release in swig typemaps

2020-04-07 Thread Lawrence D'Anna via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc8de17bca658: Fix illegal early call to PyBuffer_Release in 
swig typemaps (authored by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77480

Files:
  lldb/bindings/python/python-typemaps.swig


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -488,39 +488,53 @@
 }
 }
 
+%inline %{
+
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII() {};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
+
+%}
+
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
+//
+// I've also moved the call to PyBuffer_Release to the end of the SWIG wrapper,
+// doing it right away is not legal according to the python buffer protocol.
 
 %define %pybuffer_mutable_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_WRITABLE);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_WRITABLE);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size/sizeof($*1_type));
 }
 %enddef
 
 %define %pybuffer_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; const void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_CONTIG_RO);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_CONTIG_RO);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size / sizeof($*1_type));
 }


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -488,39 +488,53 @@
 }
 }
 
+%inline %{
+
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII() {};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
+
+%}
+
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
+//
+// I've also moved the call to PyBuffer_Release to the end of the SWIG wrapper,
+// doing it right away is not legal according to the python buffer protocol.
 
 %define %pybuffer_mutable_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_WRITABLE);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_WRITABLE);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size/sizeof($*1_type));
 }
 %enddef
 
 %define %pybuffer_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; const void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_CONTIG_RO);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_CONTIG_RO);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size / sizeof($*1_type));
 }
___
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.

2020-04-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 255792.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.

Address code review feedback


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] 873b79b - Don't access reference to a vector after pop_back

2020-04-07 Thread Benjamin Kramer via lldb-commits


Author: Benjamin Kramer
Date: 2020-04-07T23:10:58+02:00
New Revision: 873b79b8675d0c8eca6055ae8a874fe52b033c28

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

LOG: Don't access reference to a vector after pop_back

This is undefined behavior. Found by asan's detect_container_overflow.

Added: 


Modified: 
lldb/source/Target/ThreadPlanStack.cpp

Removed: 




diff  --git a/lldb/source/Target/ThreadPlanStack.cpp 
b/lldb/source/Target/ThreadPlanStack.cpp
index 44e47f385a82..c51946aae71c 100644
--- a/lldb/source/Target/ThreadPlanStack.cpp
+++ b/lldb/source/Target/ThreadPlanStack.cpp
@@ -156,7 +156,7 @@ void ThreadPlanStack::PushPlan(lldb::ThreadPlanSP 
new_plan_sp) {
 lldb::ThreadPlanSP ThreadPlanStack::PopPlan() {
   assert(m_plans.size() > 1 && "Can't pop the base thread plan");
 
-  lldb::ThreadPlanSP &plan_sp = m_plans.back();
+  lldb::ThreadPlanSP plan_sp = std::move(m_plans.back());
   m_completed_plans.push_back(plan_sp);
   plan_sp->WillPop();
   m_plans.pop_back();
@@ -166,7 +166,7 @@ lldb::ThreadPlanSP ThreadPlanStack::PopPlan() {
 lldb::ThreadPlanSP ThreadPlanStack::DiscardPlan() {
   assert(m_plans.size() > 1 && "Can't discard the base thread plan");
 
-  lldb::ThreadPlanSP &plan_sp = m_plans.back();
+  lldb::ThreadPlanSP plan_sp = std::move(m_plans.back());
   m_discarded_plans.push_back(plan_sp);
   plan_sp->WillPop();
   m_plans.pop_back();



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

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

2020-04-07 Thread Jan Kratochvil via Phabricator via lldb-commits

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2016
+for (size_t i = 0; i < num_matches; ++i) {
+  const DIERef &die_ref = method_die_offsets[i];
   DWARFDebugInfo &debug_info = dwarf->DebugInfo();

aprantl wrote:
> Can this be made into a range-based for loop in a separate commit?
In D77327 it gets changed into a callback anyway so `method_die_offsets` no 
longer exists there.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp:70
 const dw_tag_t die_tag = die_info_array[i].tag;
-if (die_tag != 0 && die_tag != DW_TAG_class_type &&
-die_tag != DW_TAG_structure_type)
+if (!(die_tag == 0 || die_tag == DW_TAG_class_type ||
+  die_tag == DW_TAG_structure_type))

shafik wrote:
> I also feel like the previous version was more readable. Is there another 
> gain that I am missing?
I find both versions OK but this patch is going to revert my change in 
rG80237523193d so everyone should be happy.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2057
 
-  if (die) {
-switch (die.Tag()) {
-default:
-case DW_TAG_subprogram:
-case DW_TAG_inlined_subroutine:
-case DW_TAG_try_block:
-case DW_TAG_catch_block:
-  break;
+  for (size_t i = 0; i < num_die_matches; ++i) {
+const DIERef &die_ref = die_offsets[i];

aprantl wrote:
> same here (later)
In D77327 it gets changed into a callback anyway so `die_offsets` no longer 
exists there.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2915
+  if (!log)
+continue;
+  std::string qualified_name;

aprantl wrote:
> These two continues IMO are a bit confusing to read this way. Perhaps in this 
> case an if (log) block with just one continue at the end is easier to read. 
> Up to you.
OK, changed back to `if (log) { ... }`. I even was not sure with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



___
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

2020-04-07 Thread Adrian McCarthy via Phabricator via lldb-commits

amccarth added a comment.

I like where this is going.




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 *)&msg, 0, 
NULL);
+  return msg;

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.



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

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.



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] D77326: 1/2: [nfc] [lldb] Unindent code

2020-04-07 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil updated this revision to Diff 255801.
jankratochvil marked 4 inline comments as done.
jankratochvil added a comment.

Implemented all the review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2051,64 +2051,53 @@
   DIEArray die_offsets;
   m_index->GetGlobalVariables(ConstString(basename), die_offsets);
   const size_t num_die_matches = die_offsets.size();
-  if (num_die_matches) {
-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;
+  SymbolContext sc;
+  sc.module_sp = m_objfile_sp->GetModule();
+  assert(sc.module_sp);
 
-bool done = false;
-for (size_t i = 0; i < num_die_matches && !done; ++i) {
-  const DIERef &die_ref = die_offsets[i];
-  DWARFDIE die = GetDIE(die_ref);
+  // Loop invariant: Variables up to this index have been checked for context
+  // matches.
+  uint32_t pruned_idx = original_size;
 
-  if (die) {
-switch (die.Tag()) {
-default:
-case DW_TAG_subprogram:
-case DW_TAG_inlined_subroutine:
-case DW_TAG_try_block:
-case DW_TAG_catch_block:
-  break;
+  for (size_t i = 0; i < num_die_matches; ++i) {
+const DIERef &die_ref = die_offsets[i];
+DWARFDIE die = GetDIE(die_ref);
+if (!die) {
+  m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+  continue;
+}
 
-case DW_TAG_variable: {
-  auto *dwarf_cu = llvm::dyn_cast(die.GetCU());
-  if (!dwarf_cu)
-continue;
-  sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
-
-  if (parent_decl_ctx) {
-if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
-  CompilerDeclContext actual_parent_decl_ctx =
-  dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
-  if (!actual_parent_decl_ctx ||
-  actual_parent_decl_ctx != parent_decl_ctx)
-continue;
-}
-  }
+if (die.Tag() != DW_TAG_variable)
+  continue;
 
-  ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false,
- &variables);
-  while (pruned_idx < variables.GetSize()) {
-VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
-if (name_is_mangled ||
-var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
-  ++pruned_idx;
-else
-  variables.RemoveVariableAtIndex(pruned_idx);
-  }
+auto *dwarf_cu = llvm::dyn_cast(die.GetCU());
+if (!dwarf_cu)
+  continue;
+sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
 
-  if (variables.GetSize() - original_size >= max_matches)
-done = true;
-} break;
-}
-  } else {
-m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+if (parent_decl_ctx) {
+  if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
+CompilerDeclContext actual_parent_decl_ctx =
+dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
+if (!actual_parent_decl_ctx ||
+actual_parent_decl_ctx != parent_decl_ctx)
+  continue;
   }
 }
+
+ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+while (pruned_idx < variables.GetSize()) {
+  VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
+  if (name_is_mangled ||
+  var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
+++pruned_idx;
+  else
+variables.RemoveVariableAtIndex(pruned_idx);
+}
+
+if (variables.GetSize() - original_size >= max_matches)
+  break;
   }
 
   // Return the number of variable that were appended to the list
@@ -2693,54 +2682,53 @@
 
   const size_t num_matches = die_offsets.size();
 
-  if (num_matches) {
-for (size_t i = 0; i < num_matches; ++i) {
-  const DIERef &die_ref = die_offsets[i];
-  DWARFDIE type_die = GetDIE(die_ref);
-
-  if (type_die) {
-bool try_resolving_type = false;
+  for (size_t i = 0; i < num_matches; ++i) {
+const DIERef &die_ref = die_offsets[i];
+DWARFDIE type_die = GetDIE(die_ref);
+if (!type_die) {
+  m_index->Rep

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

2020-04-07 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil updated this revision to Diff 255813.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2051,64 +2051,53 @@
   DIEArray die_offsets;
   m_index->GetGlobalVariables(ConstString(basename), die_offsets);
   const size_t num_die_matches = die_offsets.size();
-  if (num_die_matches) {
-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;
+  SymbolContext sc;
+  sc.module_sp = m_objfile_sp->GetModule();
+  assert(sc.module_sp);
 
-bool done = false;
-for (size_t i = 0; i < num_die_matches && !done; ++i) {
-  const DIERef &die_ref = die_offsets[i];
-  DWARFDIE die = GetDIE(die_ref);
+  // Loop invariant: Variables up to this index have been checked for context
+  // matches.
+  uint32_t pruned_idx = original_size;
 
-  if (die) {
-switch (die.Tag()) {
-default:
-case DW_TAG_subprogram:
-case DW_TAG_inlined_subroutine:
-case DW_TAG_try_block:
-case DW_TAG_catch_block:
-  break;
+  for (size_t i = 0; i < num_die_matches; ++i) {
+const DIERef &die_ref = die_offsets[i];
+DWARFDIE die = GetDIE(die_ref);
+if (!die) {
+  m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+  continue;
+}
 
-case DW_TAG_variable: {
-  auto *dwarf_cu = llvm::dyn_cast(die.GetCU());
-  if (!dwarf_cu)
-continue;
-  sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
-
-  if (parent_decl_ctx) {
-if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
-  CompilerDeclContext actual_parent_decl_ctx =
-  dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
-  if (!actual_parent_decl_ctx ||
-  actual_parent_decl_ctx != parent_decl_ctx)
-continue;
-}
-  }
+if (die.Tag() != DW_TAG_variable)
+  continue;
 
-  ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false,
- &variables);
-  while (pruned_idx < variables.GetSize()) {
-VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
-if (name_is_mangled ||
-var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
-  ++pruned_idx;
-else
-  variables.RemoveVariableAtIndex(pruned_idx);
-  }
+auto *dwarf_cu = llvm::dyn_cast(die.GetCU());
+if (!dwarf_cu)
+  continue;
+sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
 
-  if (variables.GetSize() - original_size >= max_matches)
-done = true;
-} break;
-}
-  } else {
-m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+if (parent_decl_ctx) {
+  if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
+CompilerDeclContext actual_parent_decl_ctx =
+dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
+if (!actual_parent_decl_ctx ||
+actual_parent_decl_ctx != parent_decl_ctx)
+  continue;
   }
 }
+
+ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+while (pruned_idx < variables.GetSize()) {
+  VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
+  if (name_is_mangled ||
+  var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
+++pruned_idx;
+  else
+variables.RemoveVariableAtIndex(pruned_idx);
+}
+
+if (variables.GetSize() - original_size >= max_matches)
+  break;
   }
 
   // Return the number of variable that were appended to the list
@@ -2693,54 +2682,53 @@
 
   const size_t num_matches = die_offsets.size();
 
-  if (num_matches) {
-for (size_t i = 0; i < num_matches; ++i) {
-  const DIERef &die_ref = die_offsets[i];
-  DWARFDIE type_die = GetDIE(die_ref);
-
-  if (type_die) {
-bool try_resolving_type = false;
+  for (size_t i = 0; i < num_matches; ++i) {
+const DIERef &die_ref = die_offsets[i];
+DWARFDIE type_die = GetDIE(die_ref);
+if (!type_die) {
+  m_index->ReportInvalidDIERef(die_ref, type_name.GetStringRef());
+  continue;
+}
 
-// Don't try and resolve the D

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

2020-04-07 Thread Jan Kratochvil via Phabricator via lldb-commits

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2951
+type_sp = resolved_type->shared_from_this();
+break;
   }

aprantl wrote:
> I think I'd prefer a return over a break, (iff they are equivalent!).
Wrote above - This `break` cannot be changed to `return` as in D77327 it will 
become a callback lambda.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2949
+type_sp = resolved_type->shared_from_this();
+break;
   }

This `break` cannot be changed to `return` as in D77327 it will become a 
callback lambda.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



___
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

2020-04-07 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil updated this revision to Diff 255815.

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 &method_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 &method_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 &SymbolFileDWARFDwo::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 &dwarf_cu);
 
-  virtual size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray &method_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 &method_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 &die, SymbolContext &sc) {
@@ -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 &die_ref = die_offsets[i];
+  SymbolCo

[Lldb-commits] [PATCH] D73206: Pass `CompileUnit *` along `DWARFDIE` for DWZ

2020-04-07 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

In D73206#1954668 , @labath wrote:

> Having a iterator/callback based api would allow us to minimize the impact of 
> that, as it would only need to happen for the entries that are really used. 
> And /I think/ we could make it interface returns DWARFDies directly, and each 
> index converts to that using the most direct approach available.


When I tried to rebase this patch on top of D77327 
 it looks like:

  <<< HEAD = D77327
bool GetObjCMethods(lldb_private::ConstString class_name,
llvm::function_ref callback) override;
  ||| aed2fdb1671
size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
   DIEArray &method_die_offsets) override;
  ===
size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
   std::vector 
&method_die_offsets) override;
  >>> user_id_t

So IIUC I should convert it first to: `llvm::function_ref 
callback`
Going to try that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73206



___
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

2020-04-07 Thread walter erquinigo via Phabricator via lldb-commits

wallace added a comment.

Ping ping. This will fix some existing issues reported by users :)


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

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

2020-04-07 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

This looks good to me, except that instead of leaving all the other variants of 
AddCommand, they should funnel through the one that takes the most arguments.  
It was poor form to leave two around and more so now that there's three.

I'm beginning to think we need an SBAddCommandOptions or something like that so 
we don't need to add yet another variant next time we want to add some flags to 
a command.  That is a nice-to-have, however.  If you are done with this change, 
it's okay as it stands.

In retrospect, I think it would be better if the CommandInterpreter were to 
prepend the container commands to the repeat command string.  It is bogus that 
multi-word commands have to know their hierarchy to emit a useful repeat 
command string.

Prepending the containing commands would mean that command A couldn't invoke a 
totally different command as its repeat command.  But that seems like an odd 
thing to do anyway.  And that way a command wouldn't have to know where it is 
situated in the command hierarchy for the repeat command to work.  That makes 
your test ugly, but that's not your fault (it's probably some earlier version 
of me's decision).




Comment at: lldb/source/API/SBCommandInterpreter.cpp:857
 
+lldb::SBCommand SBCommand::AddCommand(const char *name,
+  lldb::SBCommandPluginInterface *impl,

Instead of keeping these copies of AddCommand around, can you funnel all of 
them through the one that takes the auto_repeat command?  That would reduce 
boilerplate and make adding the next thing easier...


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] f30ebf4 - [ManualDWARFIndex] Remove dead code, in preparation for moving this function.

2020-04-07 Thread Davide Italiano via lldb-commits


Author: Davide Italiano
Date: 2020-04-07T16:28:13-07:00
New Revision: f30ebf437851d3c68fd0eee82afbc0cef7373c00

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

LOG: [ManualDWARFIndex] Remove dead code, in preparation for moving this 
function.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index cb783314d320..3951a2a32a1b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -185,12 +185,6 @@ void ManualDWARFIndex::IndexUnitImpl(DWARFUnit &unit,
 is_declaration = form_value.Unsigned() != 0;
   break;
 
-//case DW_AT_artificial:
-//if (attributes.ExtractFormValueAtIndex(i,
-//form_value))
-//is_artificial = form_value.Unsigned() != 0;
-//break;
-
 case DW_AT_MIPS_linkage_name:
 case DW_AT_linkage_name:
   if (attributes.ExtractFormValueAtIndex(i, form_value))
@@ -223,20 +217,6 @@ void ManualDWARFIndex::IndexUnitImpl(DWARFUnit &unit,
 // location describes a hard coded address, but we don't want
 // the performance penalty of that right now.
 is_global_or_static_variable = false;
-// if (attributes.ExtractFormValueAtIndex(dwarf, i,
-//form_value)) {
-//   // If we have valid block data, then we have location
-//   // expression bytesthat are fixed (not a location list).
-//   const uint8_t *block_data = form_value.BlockData();
-//   if (block_data) {
-// uint32_t block_length = form_value.Unsigned();
-// if (block_length == 1 +
-// attributes.UnitAtIndex(i)->GetAddressByteSize()) {
-//   if (block_data[0] == DW_OP_addr)
-// add_die = true;
-// }
-//   }
-// }
 parent_die = nullptr; // Terminate the while loop.
 break;
 



___
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

2020-04-07 Thread Davide Italiano via Phabricator via lldb-commits

davide added a comment.

See also https://bugs.llvm.org/show_bug.cgi?id=45471


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

2020-04-07 Thread Davide Italiano via Phabricator via lldb-commits

davide created this revision.
davide added reviewers: jingham, labath.
Herald added subscribers: arphaman, aprantl.
davide added a comment.
davide added a reviewer: friss.

See also https://bugs.llvm.org/show_bug.cgi?id=45471


SymbolFileDWARF::ParseVariableDIE consider all constant variables as "static".
This is incorrect, and causes `frame var` to fail when printing them.

e.g.

  volatile int a;   
 main() {
  {
int b = 3;
a;
  }
}

const_value variables are more common at `-O1/-O2/..`, so this wasn't noticed 
by many.

Fixes rdar://problem/61402307

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


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

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3442,7 +3442,7 @@
 }
   }
 } else {
-  if (location_is_const_value_data)
+  if (location_is_const_value_data && die.GetDIE()->IsGlobalOrStaticVariable())
 scope = eValueTypeVariableStatic;
   else {
 scope = eValueTypeVariableLocal;
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -204,35 +204,8 @@
 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:
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -167,6 +167,8 @@
   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);
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -1016,6 +1016,34 @@
   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:
+  // 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.
+  return false;
+
+case DW_TAG_compile_unit:
+case DW_TAG_partial_unit:
+  return true;
+
+default:
+

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

2020-04-07 Thread 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

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

2020-04-07 Thread 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] D77661: [lldb/Python] Add lldbconfig Python module to make the lldb module configurable.

2020-04-07 Thread 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] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-07 Thread 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] D77588: [lldb/Reproducers] Make it possible to capture reproducers for the Python test suite.

2020-04-07 Thread 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

63 matches

Mail list logo