[Lldb-commits] [PATCH] D129257: [trace][intel pt] Add a cgroup filter

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: jj10306, persona0220.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

It turns out that cgroup filtering is relatively trivial and works
really nicely. Thid diffs adds automatic cgroup filtering when in
per-cpu mode, unless a new --disable-cgroup-filtering flag is passed in
the start command. At least on Meta machines, all processes are spawned
inside a cgroup by default, which comes super handy, because per cpu
tracing is now much more precise.

A manual test gave me this result

- Without filtering: Total number of trace items: 36083 Total number of 
continuous executions found: 229 Number of continuous executions for this 
thread: 2 Total number of PSB blocks found: 98 Number of PSB blocks for this 
thread 2 Total number of unattributed PSB blocks found: 38

- With filtering: Total number of trace items: 87756 Total number of continuous 
executions found: 123 Number of continuous executions for this thread: 2 Total 
number of PSB blocks found: 10 Number of PSB blocks for this thread 3 Total 
number of unattributed PSB blocks found: 2

Filtering gives us great results. The number of instructions collected
more than double (probalby because we have less noise in the trace), and
we have much less unattributed PSBs blocks and unrelated PSBs in
general. The ones that are unrelated probably belong to other processes
in the same cgroup.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129257

Files:
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp

Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -53,7 +53,8 @@
 
   if (packet.IsProcessTracing()) {
 if (!o.map("processBufferSizeLimit", packet.process_buffer_size_limit) ||
-!o.map("perCpuTracing", packet.per_cpu_tracing))
+!o.map("perCpuTracing", packet.per_cpu_tracing) ||
+!o.map("disableCgroupTracing", packet.disable_cgroup_filtering))
   return false;
   }
   return true;
@@ -67,6 +68,7 @@
   obj.try_emplace("psbPeriod", packet.psb_period);
   obj.try_emplace("enableTsc", packet.enable_tsc);
   obj.try_emplace("perCpuTracing", packet.per_cpu_tracing);
+  obj.try_emplace("disableCgroupTracing", packet.disable_cgroup_filtering);
   return base;
 }
 
@@ -108,13 +110,15 @@
   json::Path path) {
   ObjectMapper o(value, path);
   return o && fromJSON(value, (TraceGetStateResponse &)packet, path) &&
- o.map("tscPerfZeroConversion", packet.tsc_perf_zero_conversion);
+ o.map("tscPerfZeroConversion", packet.tsc_perf_zero_conversion) &&
+ o.map("usingCgroupFiltering", packet.using_cgroup_filtering);
 }
 
 json::Value toJSON(const TraceIntelPTGetStateResponse &packet) {
   json::Value base = toJSON((const TraceGetStateResponse &)packet);
-  base.getAsObject()->insert(
-  {"tscPerfZeroConversion", packet.tsc_perf_zero_conversion});
+  json::Object &obj = *base.getAsObject();
+  obj.insert({"tscPerfZeroConversion", packet.tsc_perf_zero_conversion});
+  obj.insert({"usingCgroupFiltering", packet.using_cgroup_filtering});
   return base;
 }
 
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
@@ -83,6 +83,11 @@
  "converted to the approximate number of raw trace bytes between PSB "
  "packets as: 2 ^ (value + 11), e.g. value 3 means 16KiB between PSB "
  "packets. Defaults to 0 if supported.">;
+  def process_trace_start_intel_pt_disable_cgroup_filtering:
+Option<"disable-cgroup-filtering", "d">,
+Desc<"Disable the automatic cgroup filtering that is applied if --per-cpu "
+ "is provided. Cgroup filtering allows collecting intel pt data "
+   

[Lldb-commits] [PATCH] D129261: [lldb/test] Add Shell/Expr/TestStringLiteralExpr.test

[Jesus Checa Hidalgo via Phabricator via lldb-commits]

jchecahi created this revision.
Herald added a project: All.
jchecahi requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This test should exercise the usage of expressions containing
string literals and ensure that lldb doesn't crash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129261

Files:
  lldb/test/Shell/Expr/TestStringLiteralExpr.test


Index: lldb/test/Shell/Expr/TestStringLiteralExpr.test
===
--- /dev/null
+++ lldb/test/Shell/Expr/TestStringLiteralExpr.test
@@ -0,0 +1,13 @@
+# RUN: echo %t
+# RUN: echo %s
+# RUN: echo "int main() { return 0; }" | %clang_host -x c -o %t -
+# RUN: %lldb -s %s %t | FileCheck %s
+
+# Make sure that lldb doesn't crash when evaluating expressions with string 
literals
+b main
+run
+expr "hello there"
+expr printf("hello there")
+
+# CHECK: (const char[12]) $0 = "hello there"
+# CHECK: (int) $1 = 11


Index: lldb/test/Shell/Expr/TestStringLiteralExpr.test
===
--- /dev/null
+++ lldb/test/Shell/Expr/TestStringLiteralExpr.test
@@ -0,0 +1,13 @@
+# RUN: echo %t
+# RUN: echo %s
+# RUN: echo "int main() { return 0; }" | %clang_host -x c -o %t -
+# RUN: %lldb -s %s %t | FileCheck %s
+
+# Make sure that lldb doesn't crash when evaluating expressions with string literals
+b main
+run
+expr "hello there"
+expr printf("hello there")
+
+# CHECK: (const char[12]) $0 = "hello there"
+# CHECK: (int) $1 = 11
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D129261: [lldb/test] Add Shell/Expr/TestStringLiteralExpr.test

[serge via Phabricator via lldb-commits]

serge-sans-paille added a reviewer: JDevlieghere.
serge-sans-paille added inline comments.



Comment at: lldb/test/Shell/Expr/TestStringLiteralExpr.test:1
+# RUN: echo %t
+# RUN: echo %s

This line and the one below looks like debug code and should be removed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129261

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

[Lldb-commits] [PATCH] D129261: [lldb/test] Add Shell/Expr/TestStringLiteralExpr.test

[Jesus Checa Hidalgo via Phabricator via lldb-commits]

jchecahi updated this revision to Diff 442819.
jchecahi added a comment.

[lldb/test] Add Shell/Expr/TestStringLiteralExpr.test

  

This test should exercise the usage of expressions containing
string literals and ensure that lldb doesn't crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129261

Files:
  lldb/test/Shell/Expr/TestStringLiteralExpr.test


Index: lldb/test/Shell/Expr/TestStringLiteralExpr.test
===
--- /dev/null
+++ lldb/test/Shell/Expr/TestStringLiteralExpr.test
@@ -0,0 +1,11 @@
+# RUN: echo "int main() { return 0; }" | %clang_host -x c -o %t -
+# RUN: %lldb -s %s %t | FileCheck %s
+
+# Make sure that lldb doesn't crash when evaluating expressions with string 
literals
+b main
+run
+expr "hello there"
+expr printf("hello there")
+
+# CHECK: (const char[12]) $0 = "hello there"
+# CHECK: (int) $1 = 11


Index: lldb/test/Shell/Expr/TestStringLiteralExpr.test
===
--- /dev/null
+++ lldb/test/Shell/Expr/TestStringLiteralExpr.test
@@ -0,0 +1,11 @@
+# RUN: echo "int main() { return 0; }" | %clang_host -x c -o %t -
+# RUN: %lldb -s %s %t | FileCheck %s
+
+# Make sure that lldb doesn't crash when evaluating expressions with string literals
+b main
+run
+expr "hello there"
+expr printf("hello there")
+
+# CHECK: (const char[12]) $0 = "hello there"
+# CHECK: (int) $1 = 11
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

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

mgorny updated this revision to Diff 442823.
mgorny edited the summary of this revision.
mgorny added a comment.

Ok, so the previous version was fundamentally botched wrt synchronization.

Here's a new that:

1. Uses a barrier to ensure that all threads actually start before we 
`SIGSTOP`. Otherwise, the debugger would sometimes not get all threads on 
FreeBSD.
2. Uses a `std::atomic` to ensure that threads don't start printing until 
we're past `SIGSTOP`. Originally, I wanted to abuse `print_mutex` for that but 
this caused deadlocks on FreeBSD (I guess a suspended thread got in line for 
the mutex and blocked others from getting it).
3. Marks `can_exit_now` as a `thread_local` variable.


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

https://reviews.llvm.org/D129012

Files:
  lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
  lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
  lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
  lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

Index: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
===
--- lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
+++ lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
@@ -1,31 +1,54 @@
 #include "pseudo_barrier.h"
 #include "thread.h"
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 
 pseudo_barrier_t barrier;
+std::mutex print_mutex;
+std::atomic can_work = ATOMIC_VAR_INIT(false);
+thread_local bool can_exit_now = false;
 
 static void sigusr1_handler(int signo) {
-  char buf[100];
-  std::snprintf(buf, sizeof(buf),
-"received SIGUSR1 on thread id: %" PRIx64 "\n",
-get_thread_id());
-  write(STDOUT_FILENO, buf, strlen(buf));
+  std::lock_guard lock{print_mutex};
+  std::printf("received SIGUSR1 on thread id: %" PRIx64 "\n", get_thread_id());
+  can_exit_now = true;
 }
 
 static void thread_func() {
+  // this ensures that all threads start before we SIGSTOP
   pseudo_barrier_wait(barrier);
-  for (int i = 0; i < 300; ++i) {
+
+  // wait till the main thread indicates that we can go
+  // (note: using a mutex here causes hang on FreeBSD when another thread
+  // is suspended)
+  while (!can_work.load())
+std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+  // the mutex guarantees that two writes don't get interspersed
+  {
+std::lock_guard lock{print_mutex};
 std::printf("thread %" PRIx64 " running\n", get_thread_id());
+  }
+
+  // give other threads a fair chance to run
+  for (int i = 0; i < 5; ++i) {
+std::this_thread::yield();
 std::this_thread::sleep_for(std::chrono::milliseconds(200));
+if (can_exit_now)
+  return;
   }
+
+  // if we didn't get signaled, terminate the program explicitly.
+  _exit(0);
 }
 
 int main(int argc, char **argv) {
@@ -36,12 +59,15 @@
   signal(SIGUSR1, sigusr1_handler);
 
   std::vector threads;
-  for(int i = 0; i < num; ++i)
+  for (int i = 0; i < num; ++i)
 threads.emplace_back(thread_func);
 
+  // use the barrier to make sure all threads start before we SIGSTOP
   pseudo_barrier_wait(barrier);
+  std::raise(SIGSTOP);
 
-  std::puts("@started");
+  // allow the threads to work
+  can_work.store(true);
 
   for (std::thread &thread : threads)
 thread.join();
Index: lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
===
--- lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
+++ lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
@@ -1,23 +1,18 @@
-import json
 import re
-import time
 
 import gdbremote_testcase
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-class TestGdbRemote_vContThreads(gdbremote_testcase.GdbRemoteTestCaseBase):
 
+class TestSignal(gdbremote_testcase.GdbRemoteTestCaseBase):
 def start_threads(self, num):
 procs = self.prep_debug_monitor_and_inferior(inferior_args=[str(num)])
-# start the process and wait for output
 self.test_sequence.add_log_lines([
 "read packet: $c#63",
-{"type": "output_match", "regex": r".*@started\r\n.*"},
+{"direction": "send", "regex": "[$]T.*;reason:signal.*"},
 ], True)
-# then interrupt it
-self.add_interrupt_packets()
 self.add_threadinfo_collection_packets()
 
 context = self.expect_gdbremote_sequence()
@@ -29,22 +24,21 @@
 self.reset_test_sequence()
 return threads
 
+SIGNAL_MATCH_RE = re.compile(r"received SIGUSR1 on thread id: ([0-9a-f]+)")
+
 def send_and_check_signal(self, vCont_data, threads):
 self.test_sequence.add_log_lines([
 "read packet: $vCont;{0}#00".format(vCont_data),
-{"type": "output_match",
-   

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.



Comment at: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp:18
+std::atomic can_work = ATOMIC_VAR_INIT(false);
+thread_local bool can_exit_now = false;
 

I guess this should technically be a `volatile sig_atomic_t`


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

https://reviews.llvm.org/D129012

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

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The overall logic and the include and library paths make sense to me. The rpath 
thingy will not work on windows as it (afaik) has no equivalent feature (it has 
the equivalent of (DY)LD_LIBRARY_PATH though). Do you just want to make that 
unsupported. I don't think we're running libc++ tests on windows right now, so 
I guess nothing will be lost there, though we will need to implement this 
differently at some point.

And I suppose that the plan is to not use this path at all for remote runs (?) 
That makes sense to me, since the build tree is unlikely to contain a usable 
libc++ (unless the remote machine has the same architecture).




Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:284
 
+configuration.hermetic_libxxx = args.hermetic_libcxx
+

libcxx



Comment at: lldb/packages/Python/lldbsuite/test/make/Makefile.rules:394
else
-   CXXFLAGS += -stdlib=libc++
-   LDFLAGS += -stdlib=libc++
-   endif
-   ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
-   ifneq (,$(LLVM_LIBS_DIR))
-   LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+   CXXFLAGS += -DLLDB_USING_LIBCPP
+   ifeq "$(OS)" "Android"

I think this should be set in the "hermetic" case as well (or, given that it is 
unused, we might delete it completely).


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

https://reviews.llvm.org/D129166

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

[Lldb-commits] [PATCH] D129272: [lldb][Windows] Fix memory region base addresses when a range is split

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Previously we recorded AllocationBase as the base address of the region
we get from VirtualQueryEx. However, this is the base of the allocation,
which can later be split into more regions.

So you got stuff like:
[0x7fff377c-0x7fff377c1000) r-- PECOFF header
[0x7fff377c-0x7fff3784) r-x .text
[0x7fff377c-0x7fff3787) r-- .rdata

Where all the base addresses were the same.

Instead, use BaseAddress as the base of the region. So we get:
[0x7fff377c-0x7fff377c1000) r-- PECOFF header
[0x7fff377c1000-0x7fff3784) r-x .text
[0x7fff3784-0x7fff3787) r-- .rdata

https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-memory_basic_information

The added test is looking for the Windows situation
but we should not see repeated base addresses anywhere
else either. So I've not restricted it to Windows.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129272

Files:
  lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -27,7 +27,7 @@
 substrs=["memory region ",
  "memory region -a"])
 
-def test(self):
+def setup_program(self):
 self.build()
 
 # Set breakpoint in main and run
@@ -37,6 +37,9 @@
 
 self.runCmd("run", RUN_SUCCEEDED)
 
+def test_command(self):
+self.setup_program()
+
 interp = self.dbg.GetCommandInterpreter()
 result = lldb.SBCommandReturnObject()
 
@@ -84,3 +87,23 @@
 interp.HandleCommand("memory region --all", result)
 self.assertTrue(result.Succeeded())
 self.assertEqual(result.GetOutput(), all_regions)
+
+def test_unique_base_addresses(self):
+# In the past on Windows we were recording AllocationBase as the base 
address
+# of the current region, not BaseAddress. So if a range of pages was 
split
+# into regions you would see several regions with the same base 
address.
+# This checks that this no longer happens (and it shouldn't happen on 
any
+# other OS either).
+self.setup_program()
+
+process = self.dbg.GetTargetAtIndex(0).GetProcess()
+regions = process.GetMemoryRegions()
+base_addresses = set()
+
+for idx in range(regions.GetSize()):
+region = lldb.SBMemoryRegionInfo()
+regions.GetMemoryRegionAtIndex(idx, region)
+base_address = region.GetRegionBase()
+self.assertFalse(base_address in base_addresses,
+"Did not expect to see a repeated base region address.")
+base_addresses.add(base_address)
\ No newline at end of file
Index: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -441,7 +441,7 @@
   // AllocationBase is defined for MEM_COMMIT and MEM_RESERVE but not MEM_FREE.
   if (mem_info.State != MEM_FREE) {
 info.GetRange().SetRangeBase(
-reinterpret_cast(mem_info.AllocationBase));
+reinterpret_cast(mem_info.BaseAddress));
 info.GetRange().SetRangeEnd(reinterpret_cast(mem_info.BaseAddress) 
+
 mem_info.RegionSize);
 info.SetMapped(MemoryRegionInfo::eYes);


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -27,7 +27,7 @@
 substrs=["memory region ",
  "memory region -a"])
 
-def test(self):
+def setup_program(self):
 self.build()
 
 # Set breakpoint in main and run
@@ -37,6 +37,9 @@
 
 self.runCmd("run", RUN_SUCCEEDED)
 
+def test_command(self):
+self.setup_program()
+
 interp = self.dbg.GetCommandInterpreter()
 result = lldb.SBCommandReturnObject()
 
@@ -84,3 +87,23 @@
 interp.HandleCommand("memory region --all", result)
 self.assertTrue(result.Succeeded())
 self.assertEqual(result.GetOutput(), all_regions)
+
+def test_unique_base_addresses(self):
+# In the past on Windows we were recording AllocationBase as the base address
+# of the current region, not Ba

[Lldb-commits] [PATCH] D129272: [lldb][Windows] Fix memory region base addresses when a range is split

[David Spickett via Phabricator via lldb-commits]

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

Follow up to 
https://discourse.llvm.org/t/unexplained-memory-regions-on-windows/62662.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129272

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

[Lldb-commits] [PATCH] D128932: [lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

[Pavel Labath via Phabricator via lldb-commits]

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

Ok, let's give this a shot.




Comment at: lldb/test/API/tools/lldb-server/main.cpp:351
+  break;
+std::this_thread::sleep_for(std::chrono::milliseconds(125 * i));
+  }

I'm not convinced this will be enough. How about `125 * (1

[Lldb-commits] [PATCH] D129272: [lldb][Windows] Fix memory region base addresses when a range is split

[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, thanks. I'd consider strengthening the test to look for overlapping 
address ranges instead of just identical base addresses.




Comment at: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py:99
+
+process = self.dbg.GetTargetAtIndex(0).GetProcess()
+regions = process.GetMemoryRegions()

use self.process() instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129272

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

[Lldb-commits] [lldb] 82ba3f4 - Recommit "[lldb/test] Don't use preexec_fn for launching inferiors"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-07-07T14:38:33+02:00
New Revision: 82ba3f44657ac91b4bef95dbf647dfbab482e502

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

LOG: Recommit "[lldb/test] Don't use preexec_fn for launching inferiors"

This recommits b15b1421, which reverted in was reverted in f51c47d98 due to
failures on apple systems. The problem was that the patch introduced a race
where the debug server could start the attach process before the first process
(which isn't supposed to be attached to) was set up. This caused us to attach
to the wrong process.

The new version introduces additional synchronization to ensure that does not
happen.

Original commit message was:
As the documentation states, using this is not safe in multithreaded
programs, and I have traced it to a rare deadlock in some of the tests.

The reason this was introduced was to be able to attach to a program
from the very first instruction, where our usual mechanism of
synchronization -- waiting for a file to appear -- does not work.

However, this is only needed for a single test
(TestGdbRemoteAttachWait) so instead of doing this everywhere, I create
a bespoke solution for that single test. The solution basically
consists of outsourcing the preexec_fn code to a separate (and
single-threaded) shim process, which enables attaching and then executes
the real program.

This pattern could be generalized in case we needed to use it for other
tests, but I suspect that we will not be having many tests like this.

This effectively reverts commit
a997a1d7fbe229433fb458bb0035b32424ecf3bd.

Added: 
lldb/test/API/tools/lldb-server/attach-wait/Makefile
lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py
lldb/test/API/tools/lldb-server/attach-wait/main.cpp
lldb/test/API/tools/lldb-server/attach-wait/shim.cpp

Modified: 
lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
lldb/packages/Python/lldbsuite/test/lldbtest.py

Removed: 
lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py



diff  --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py 
b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
index e14c4f8c0d383..719131c9248e8 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -4,12 +4,11 @@
 from __future__ import absolute_import
 
 # System modules
-import ctypes
 import itertools
-import os
 import re
 import subprocess
 import sys
+import os
 
 # Third-party modules
 import six
@@ -192,14 +191,3 @@ def hasChattyStderr(test_case):
 if match_android_device(test_case.getArchitecture(), ['aarch64'], 
range(22, 25+1)):
 return True  # The dynamic linker on the device will complain about 
unknown DT entries
 return False
-
-if getHostPlatform() == "linux":
-def enable_attach():
-"""Enable attaching to _this_ process, if host requires such an action.
-Suitable for use as a preexec_fn in subprocess.Popen and similar."""
-c = ctypes.CDLL(None)
-PR_SET_PTRACER = ctypes.c_int(0x59616d61)
-PR_SET_PTRACER_ANY = ctypes.c_ulong(-1)
-c.prctl(PR_SET_PTRACER, PR_SET_PTRACER_ANY)
-else:
-enable_attach = None

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 22851730153e0..d46e54f30bd55 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -393,7 +393,6 @@ def launch(self, executable, args, extra_env):
 stdout=open(
 os.devnull) if not self._trace_on else None,
 stdin=PIPE,
-preexec_fn=lldbplatformutil.enable_attach,
 env=env)
 
 def terminate(self):

diff  --git a/lldb/test/API/tools/lldb-server/attach-wait/Makefile 
b/lldb/test/API/tools/lldb-server/attach-wait/Makefile
new file mode 100644
index 0..22f1051530f87
--- /dev/null
+++ b/lldb/test/API/tools/lldb-server/attach-wait/Makefile
@@ -0,0 +1 @@
+include Makefile.rules

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py 
b/lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py
similarity index 75%
rename from lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
rename to lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py
index ed31b5645c336..b1ec443788bfc 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
+++ b/lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py
@@ -14,10 +14,13 @@ class 
TestGdbRemoteAttachWait(gdbremote_testcase.GdbRemoteTestCaseBase):
 @skipIfWindows # This test is flaky on Windows
 def test_attach_with_vAttachWait(self):
   

[Lldb-commits] [lldb] 11a0969 - [lldb] Fixup TestLoadAfterAttach for 82ba3f4

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-07-07T15:06:40+02:00
New Revision: 11a09692ad960a8e547f05f070fe8b24664a4e24

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

LOG: [lldb] Fixup TestLoadAfterAttach for 82ba3f4

After 82ba3f4, we (again) need to call lldb_enable_attach to be able to
attach to processes on linux. This was a new test, so it does not have
the necessary boilerplate.

Added: 


Modified: 
lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
lldb/test/API/functionalities/load_after_attach/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py 
b/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
index 6ab82205a7863..39fc096a1a422 100644
--- a/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
+++ b/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
@@ -10,6 +10,8 @@ class TestCase(TestBase):
 def test_load_after_attach(self):
 self.build()
 
+sync_file_path = lldbutil.append_to_process_working_directory(self, 
"process_ready")
+
 ctx = self.platformContext
 lib_name = ctx.shlib_prefix + 'lib_b.' + ctx.shlib_extension
 
@@ -22,7 +24,9 @@ def test_load_after_attach(self):
 # Spawn a new process.
 # use realpath to workaround llvm.org/pr48376
 # Pass path to solib for dlopen to properly locate the library.
-popen = self.spawnSubprocess(os.path.realpath(exe), 
extra_env=environment)
+popen = self.spawnSubprocess(os.path.realpath(exe), [sync_file_path],
+extra_env=environment)
+lldbutil.wait_for_file_on_target(self, sync_file_path)
 
 # Attach to the spawned process.
 error = lldb.SBError()

diff  --git a/lldb/test/API/functionalities/load_after_attach/main.cpp 
b/lldb/test/API/functionalities/load_after_attach/main.cpp
index 97eea20a581d8..96012a126001a 100644
--- a/lldb/test/API/functionalities/load_after_attach/main.cpp
+++ b/lldb/test/API/functionalities/load_after_attach/main.cpp
@@ -3,8 +3,12 @@
 #include 
 #include 
 #include 
+#include 
 
 int main(int argc, char* argv[]) {
+  lldb_enable_attach();
+  std::ofstream(argv[1]).close();
+
   // Wait until debugger is attached.
   int main_thread_continue = 0;
   int i = 0;



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

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

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

mgorny added inline comments.



Comment at: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp:18
+std::atomic can_work = ATOMIC_VAR_INIT(false);
+thread_local bool can_exit_now = false;
 

labath wrote:
> I guess this should technically be a `volatile sig_atomic_t`
But with `thread_local`, correct?


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

https://reviews.llvm.org/D129012

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

[Lldb-commits] [PATCH] D128956: make debugserver able to inspect mach-o binaries present in memory, but not yet registered with dyld

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py:46
+self.expect (both_gdb_packet, substrs=['response: 
{"images":[{"load_address":%d,' % macho_addr])
+self.expect (both_gdb_packet, substrs=['response: {"load_address":%d,' 
% invalid_macho_addr], matching=False)
+

For these would it be more reliable to just look for the `load_address:` bits?

If the packet contains multiple `response`, then fine but if it's one response 
then the second one wouldn't ever match even if debugserver did somehow find 
the one at the invalid address. Also if it did would it put it in a `images` 
section and therefore not match because of that too?



Comment at: lldb/test/API/macosx/unregistered-macho/main.c:48
+  memcpy(p, &uuid, sizeof(uuid));
+  p += sizeof(uuid);
+

This is redundant (the test uses `macho_buf`).



Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:756
 
 bool MachProcess::GetMachOInformationFromMemory(
 uint32_t dyld_platform, nub_addr_t mach_o_header_addr, int wordsize,

Could this set `inf.valid` instead of having to loop later to set it?



Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:768
+return true;
+  };
+

Personally I'd put this as a static function just before this one as it doesn't 
capture anything, but up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128956

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

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp:18
+std::atomic can_work = ATOMIC_VAR_INIT(false);
+thread_local bool can_exit_now = false;
 

mgorny wrote:
> labath wrote:
> > I guess this should technically be a `volatile sig_atomic_t`
> But with `thread_local`, correct?
Yes. If that's not enough qualifiers, you can even throw in `static`, for good 
measure. :)


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

https://reviews.llvm.org/D129012

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

[Lldb-commits] [PATCH] D129272: [lldb][Windows] Fix memory region base addresses when a range is split

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 442896.
DavidSpickett added a comment.

- Use self.process()
- Check for overlapping regions instead of just the base addresses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129272

Files:
  lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -27,7 +27,7 @@
 substrs=["memory region ",
  "memory region -a"])
 
-def test(self):
+def setup_program(self):
 self.build()
 
 # Set breakpoint in main and run
@@ -37,6 +37,9 @@
 
 self.runCmd("run", RUN_SUCCEEDED)
 
+def test_command(self):
+self.setup_program()
+
 interp = self.dbg.GetCommandInterpreter()
 result = lldb.SBCommandReturnObject()
 
@@ -84,3 +87,37 @@
 interp.HandleCommand("memory region --all", result)
 self.assertTrue(result.Succeeded())
 self.assertEqual(result.GetOutput(), all_regions)
+
+def test_unique_base_addresses(self):
+# In the past on Windows we were recording AllocationBase as the base 
address
+# of the current region, not BaseAddress. So if a range of pages was 
split
+# into regions you would see several regions with the same base 
address.
+# This checks that this no longer happens (and it shouldn't happen on 
any
+# other OS either).
+self.setup_program()
+
+regions = self.process().GetMemoryRegions()
+num_regions = regions.GetSize()
+
+if num_regions:
+region = lldb.SBMemoryRegionInfo()
+regions.GetMemoryRegionAtIndex(0, region)
+previous_base = region.GetRegionBase()
+previous_end = region.GetRegionEnd()
+
+for idx in range(1, regions.GetSize()):
+regions.GetMemoryRegionAtIndex(idx, region)
+
+# Check that it does not overlap the previous region.
+# This could happen if we got the base addresses or size wrong.
+# Also catches the base addresses being the same.
+region_base = region.GetRegionBase()
+region_end = region.GetRegionEnd()
+
+if (region_base >= previous_base and region_base < 
previous_end) \
+or (region_end > previous_base and region_end <= 
previous_end):
+self.assertFalse(base_address in base_addresses,
+"Unexpected overlapping memory region found.")
+
+previous_base = region_base
+previous_end = region_end
\ No newline at end of file
Index: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -441,7 +441,7 @@
   // AllocationBase is defined for MEM_COMMIT and MEM_RESERVE but not MEM_FREE.
   if (mem_info.State != MEM_FREE) {
 info.GetRange().SetRangeBase(
-reinterpret_cast(mem_info.AllocationBase));
+reinterpret_cast(mem_info.BaseAddress));
 info.GetRange().SetRangeEnd(reinterpret_cast(mem_info.BaseAddress) 
+
 mem_info.RegionSize);
 info.SetMapped(MemoryRegionInfo::eYes);


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -27,7 +27,7 @@
 substrs=["memory region ",
  "memory region -a"])
 
-def test(self):
+def setup_program(self):
 self.build()
 
 # Set breakpoint in main and run
@@ -37,6 +37,9 @@
 
 self.runCmd("run", RUN_SUCCEEDED)
 
+def test_command(self):
+self.setup_program()
+
 interp = self.dbg.GetCommandInterpreter()
 result = lldb.SBCommandReturnObject()
 
@@ -84,3 +87,37 @@
 interp.HandleCommand("memory region --all", result)
 self.assertTrue(result.Succeeded())
 self.assertEqual(result.GetOutput(), all_regions)
+
+def test_unique_base_addresses(self):
+# In the past on Windows we were recording AllocationBase as the base address
+# of the current region, not BaseAddress. So if a range of pages was split
+# into regions you would see several regions with the same base address.
+# This checks that this no longer happens (and it shou

[Lldb-commits] [PATCH] D129272: [lldb][Windows] Fix memory region base addresses when a range is split

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1ca8a978023f: [lldb][Windows] Fix memory region base 
addresses when a range is split (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129272

Files:
  lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -27,7 +27,7 @@
 substrs=["memory region ",
  "memory region -a"])
 
-def test(self):
+def setup_program(self):
 self.build()
 
 # Set breakpoint in main and run
@@ -37,6 +37,9 @@
 
 self.runCmd("run", RUN_SUCCEEDED)
 
+def test_command(self):
+self.setup_program()
+
 interp = self.dbg.GetCommandInterpreter()
 result = lldb.SBCommandReturnObject()
 
@@ -84,3 +87,37 @@
 interp.HandleCommand("memory region --all", result)
 self.assertTrue(result.Succeeded())
 self.assertEqual(result.GetOutput(), all_regions)
+
+def test_unique_base_addresses(self):
+# In the past on Windows we were recording AllocationBase as the base 
address
+# of the current region, not BaseAddress. So if a range of pages was 
split
+# into regions you would see several regions with the same base 
address.
+# This checks that this no longer happens (and it shouldn't happen on 
any
+# other OS either).
+self.setup_program()
+
+regions = self.process().GetMemoryRegions()
+num_regions = regions.GetSize()
+
+if num_regions:
+region = lldb.SBMemoryRegionInfo()
+regions.GetMemoryRegionAtIndex(0, region)
+previous_base = region.GetRegionBase()
+previous_end = region.GetRegionEnd()
+
+for idx in range(1, regions.GetSize()):
+regions.GetMemoryRegionAtIndex(idx, region)
+
+# Check that it does not overlap the previous region.
+# This could happen if we got the base addresses or size wrong.
+# Also catches the base addresses being the same.
+region_base = region.GetRegionBase()
+region_end = region.GetRegionEnd()
+
+if (region_base >= previous_base and region_base < 
previous_end) \
+or (region_end > previous_base and region_end <= 
previous_end):
+self.assertFalse(base_address in base_addresses,
+"Unexpected overlapping memory region found.")
+
+previous_base = region_base
+previous_end = region_end
\ No newline at end of file
Index: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -441,7 +441,7 @@
   // AllocationBase is defined for MEM_COMMIT and MEM_RESERVE but not MEM_FREE.
   if (mem_info.State != MEM_FREE) {
 info.GetRange().SetRangeBase(
-reinterpret_cast(mem_info.AllocationBase));
+reinterpret_cast(mem_info.BaseAddress));
 info.GetRange().SetRangeEnd(reinterpret_cast(mem_info.BaseAddress) 
+
 mem_info.RegionSize);
 info.SetMapped(MemoryRegionInfo::eYes);


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -27,7 +27,7 @@
 substrs=["memory region ",
  "memory region -a"])
 
-def test(self):
+def setup_program(self):
 self.build()
 
 # Set breakpoint in main and run
@@ -37,6 +37,9 @@
 
 self.runCmd("run", RUN_SUCCEEDED)
 
+def test_command(self):
+self.setup_program()
+
 interp = self.dbg.GetCommandInterpreter()
 result = lldb.SBCommandReturnObject()
 
@@ -84,3 +87,37 @@
 interp.HandleCommand("memory region --all", result)
 self.assertTrue(result.Succeeded())
 self.assertEqual(result.GetOutput(), all_regions)
+
+def test_unique_base_addresses(self):
+# In the past on Windows we were recording AllocationBase as the base address
+# of the current region, not BaseAddress. So if a range of pages was split
+# into regions you would see several regions with the same base address.
+# This checks that t

[Lldb-commits] [lldb] 1ca8a97 - [lldb][Windows] Fix memory region base addresses when a range is split

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-07-07T13:55:48Z
New Revision: 1ca8a978023f1c1fe6018ac87ed06b7ce26f0b77

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

LOG: [lldb][Windows] Fix memory region base addresses when a range is split

Previously we recorded AllocationBase as the base address of the region
we get from VirtualQueryEx. However, this is the base of the allocation,
which can later be split into more regions.

So you got stuff like:
[0x7fff377c-0x7fff377c1000) r-- PECOFF header
[0x7fff377c-0x7fff3784) r-x .text
[0x7fff377c-0x7fff3787) r-- .rdata

Where all the base addresses were the same.

Instead, use BaseAddress as the base of the region. So we get:
[0x7fff377c-0x7fff377c1000) r-- PECOFF header
[0x7fff377c1000-0x7fff3784) r-x .text
[0x7fff3784-0x7fff3787) r-- .rdata

https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-memory_basic_information

The added test checks for any overlapping regions which means
if we get the base or size wrong it'll fail. This logic
applies to any OS so the test isn't restricted to Windows.

Reviewed By: labath

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

Added: 


Modified: 
lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
lldb/test/API/functionalities/memory-region/TestMemoryRegion.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp 
b/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
index 59f5f6dfe7716..2aa7de6c853a4 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -441,7 +441,7 @@ Status ProcessDebugger::GetMemoryRegionInfo(lldb::addr_t 
vm_addr,
   // AllocationBase is defined for MEM_COMMIT and MEM_RESERVE but not MEM_FREE.
   if (mem_info.State != MEM_FREE) {
 info.GetRange().SetRangeBase(
-reinterpret_cast(mem_info.AllocationBase));
+reinterpret_cast(mem_info.BaseAddress));
 info.GetRange().SetRangeEnd(reinterpret_cast(mem_info.BaseAddress) 
+
 mem_info.RegionSize);
 info.SetMapped(MemoryRegionInfo::eYes);

diff  --git a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py 
b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
index d84e0bc814436..18118377a8b8b 100644
--- a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -27,7 +27,7 @@ def test_help(self):
 substrs=["memory region ",
  "memory region -a"])
 
-def test(self):
+def setup_program(self):
 self.build()
 
 # Set breakpoint in main and run
@@ -37,6 +37,9 @@ def test(self):
 
 self.runCmd("run", RUN_SUCCEEDED)
 
+def test_command(self):
+self.setup_program()
+
 interp = self.dbg.GetCommandInterpreter()
 result = lldb.SBCommandReturnObject()
 
@@ -84,3 +87,37 @@ def test(self):
 interp.HandleCommand("memory region --all", result)
 self.assertTrue(result.Succeeded())
 self.assertEqual(result.GetOutput(), all_regions)
+
+def test_unique_base_addresses(self):
+# In the past on Windows we were recording AllocationBase as the base 
address
+# of the current region, not BaseAddress. So if a range of pages was 
split
+# into regions you would see several regions with the same base 
address.
+# This checks that this no longer happens (and it shouldn't happen on 
any
+# other OS either).
+self.setup_program()
+
+regions = self.process().GetMemoryRegions()
+num_regions = regions.GetSize()
+
+if num_regions:
+region = lldb.SBMemoryRegionInfo()
+regions.GetMemoryRegionAtIndex(0, region)
+previous_base = region.GetRegionBase()
+previous_end = region.GetRegionEnd()
+
+for idx in range(1, regions.GetSize()):
+regions.GetMemoryRegionAtIndex(idx, region)
+
+# Check that it does not overlap the previous region.
+# This could happen if we got the base addresses or size wrong.
+# Also catches the base addresses being the same.
+region_base = region.GetRegionBase()
+region_end = region.GetRegionEnd()
+
+if (region_base >= previous_base and region_base < 
previous_end) \
+or (region_end > previous_base and region_end <= 
previous_end):
+self.assertFalse(base_address in base_addresses,
+"Unexpected overlapping memory region found

[Lldb-commits] [PATCH] D129272: [lldb][Windows] Fix memory region base addresses when a range is split

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py:91
+
+def test_unique_base_addresses(self):
+# In the past on Windows we were recording AllocationBase as the base 
address

rename to test_region_overlap or something along those lines?



Comment at: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py:119
+or (region_end > previous_base and region_end <= 
previous_end):
+self.assertFalse(base_address in base_addresses,
+"Unexpected overlapping memory region found.")

I don't see `base_addresses` defined anywhere. I guess this should just be 
`assertFalse`, and the overlap condition right?

Btw, can [[ 
https://stackoverflow.com/questions/3269434/whats-the-most-efficient-way-to-test-if-two-ranges-overlap
 | check for overlap ]] with `region_base < previous_end && previous_base < 
region_end`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129272

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

[Lldb-commits] [lldb] 8262ff4 - [lldb/test] Add a couple of libc++ std::string layouts

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-07-07T16:20:20+02:00
New Revision: 8262ff44c53568564dc919a43fae274c8b748176

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

LOG: [lldb/test] Add a couple of libc++ std::string layouts

.. to the "string simulator" test. These layouts were used at some point in
the past few months, and are already supported by the code.

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/TestDataFormatterLibcxxStringSimulator.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/TestDataFormatterLibcxxStringSimulator.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/TestDataFormatterLibcxxStringSimulator.py
index 76cd64660aab0..e156447121cca 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/TestDataFormatterLibcxxStringSimulator.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/TestDataFormatterLibcxxStringSimulator.py
@@ -8,6 +8,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+import functools
 
 
 class LibcxxStringDataFormatterSimulatorTestCase(TestBase):
@@ -23,23 +24,13 @@ def _run_test(self, defines):
 self.expect_var_path("shortstring", summary='"short"')
 self.expect_var_path("longstring", summary='"I am a very long string"')
 
-def test_v1_layout(self):
-""" Current v1 layout. """
-self._run_test([])
-
-def test_v2_layout(self):
-""" Current v2 layout. """
-self._run_test(["ALTERNATE_LAYOUT"])
-
-def test_v1_layout_bitmasks(self):
-""" Pre-D123580 v1 layout. """
-self._run_test(["BITMASKS"])
-
-def test_v2_layout_bitmasks(self):
-""" Pre-D123580 v2 layout. """
-self._run_test(["ALTERNATE_LAYOUT", "BITMASKS"])
-
-def test_v2_layout_subclass_padding(self):
-""" Pre-c3d0205ee771 v2 layout. """
-self._run_test(["ALTERNATE_LAYOUT", "BITMASKS", "SUBCLASS_PADDING"])
-
+for v in [None, "ALTERNATE_LAYOUT"]:
+for r in range(5):
+name = "test_r%d"%r
+defines = ["REVISION=%d"%r]
+if v:
+name += "_" + v
+defines += [v]
+f = functools.partialmethod(
+LibcxxStringDataFormatterSimulatorTestCase._run_test, defines)
+setattr(LibcxxStringDataFormatterSimulatorTestCase, name, f)

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp
index 4852dfd456687..33e71044482a7 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp
@@ -2,6 +2,33 @@
 #include 
 #include 
 
+#if REVISION == 0
+// Pre-c3d0205ee771 layout.
+#define SUBCLASS_PADDING
+#endif
+#if REVISION <= 1
+// Pre-D123580 layout.
+#define BITMASKS
+#endif
+#if REVISION <= 2
+// Pre-D125496 layout.
+#define SHORT_UNION
+#endif
+#if REVISION == 3
+// Pre-D128285 layout.
+#define PACKED_ANON_STRUCT
+#endif
+// REVISION == 4: current layout
+
+#ifdef PACKED_ANON_STRUCT
+#define BEGIN_PACKED_ANON_STRUCT struct __attribute__((packed)) {
+#define END_PACKED_ANON_STRUCT };
+#else
+#define BEGIN_PACKED_ANON_STRUCT
+#define END_PACKED_ANON_STRUCT
+#endif
+
+
 namespace std {
 namespace __lldb {
 
@@ -110,8 +137,10 @@ template  
class basic_string {
 #ifdef BITMASKS
 size_type __cap_;
 #else
+BEGIN_PACKED_ANON_STRUCT
 size_type __is_long_ : 1;
 size_type __cap_ : sizeof(size_type) * CHAR_BIT - 1;
+END_PACKED_ANON_STRUCT
 #endif
 size_type __size_;
 pointer __data_;
@@ -124,6 +153,7 @@ template  
class basic_string {
   };
 
   struct __short {
+#ifdef SHORT_UNION
 union {
 #ifdef BITMASKS
   unsigned char __size_;
@@ -135,6 +165,13 @@ template  
class basic_string {
 #endif
   value_type __lx;
 };
+#else
+BEGIN_PACKED_ANON_STRUCT
+unsigned char __is_long_ : 1;
+unsigned char __size_ : 7;
+END_PACKED_ANON_STRUCT
+char __padding_[sizeof(value_type) - 1];
+#endif
 value_type __data_[__min_cap];
   };
 



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

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

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

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



Comment at: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp:18
+std::atomic can_work = ATOMIC_VAR_INIT(false);
+thread_local bool can_exit_now = false;
 

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > I guess this should technically be a `volatile sig_atomic_t`
> > But with `thread_local`, correct?
> Yes. If that's not enough qualifiers, you can even throw in `static`, for 
> good measure. :)
Better overqualified than underqualified!


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

https://reviews.llvm.org/D129012

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

[Lldb-commits] [lldb] 86e4723 - [lldb] [test] Improve stability of llgs vCont-threads tests

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-07-07T16:33:55+02:00
New Revision: 86e472317c8fd9309b76c32ca55fcdeaf63f853b

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

LOG: [lldb] [test] Improve stability of llgs vCont-threads tests

Perform a major refactoring of vCont-threads tests in order to attempt
to improve their stability and performance.

Split test_vCont_run_subset_of_threads() into smaller test cases,
and split the whole suite into two files: one for signal-related tests,
the running-subset-of tests.

Eliminate output_match checks entirely, as they are fragile to
fragmentation of output.  Instead, for the initial thread list capture
raise an explicit SIGSTOP from inside the test program, and for
the remaining output let the test program run until exit, and check all
the captured output afterwards.

For resume tests, capture the LLDB's thread view before and after
starting new threads in order to determine the IDs corresponding
to subthreads rather than relying on program output for that.

Add a mutex for output to guarantee serialization.  A barrier is used
to guarantee that all threads start before SIGSTOP, and an atomic bool
is used to delay prints from happening until after SIGSTOP.

Call std::this_thread::yield() to reduce the risk of one of the threads
not being run.

This fixes the test hangs on FreeBSD.  Hopefully, it will also fix all
the flakiness on buildbots.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D129012

Added: 
lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py

Modified: 
lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

Removed: 
lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py



diff  --git 
a/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py 
b/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
new file mode 100644
index 0..2cc60d3d0a5c0
--- /dev/null
+++ b/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
@@ -0,0 +1,128 @@
+import re
+
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestPartialResume(gdbremote_testcase.GdbRemoteTestCaseBase):
+THREAD_MATCH_RE = re.compile(r"thread ([0-9a-f]+) running")
+
+def start_vCont_run_subset_of_threads_test(self):
+self.build()
+self.set_inferior_startup_launch()
+
+procs = self.prep_debug_monitor_and_inferior(inferior_args=["3"])
+# grab the main thread id
+self.add_threadinfo_collection_packets()
+main_thread = self.parse_threadinfo_packets(
+self.expect_gdbremote_sequence())
+self.assertEqual(len(main_thread), 1)
+self.reset_test_sequence()
+
+# run until threads start, then grab full thread list
+self.test_sequence.add_log_lines([
+"read packet: $c#63",
+{"direction": "send", "regex": "[$]T.*;reason:signal.*"},
+], True)
+self.add_threadinfo_collection_packets()
+
+all_threads = self.parse_threadinfo_packets(
+self.expect_gdbremote_sequence())
+self.assertEqual(len(all_threads), 4)
+self.assertIn(main_thread[0], all_threads)
+self.reset_test_sequence()
+
+all_subthreads = set(all_threads) - set(main_thread)
+self.assertEqual(len(all_subthreads), 3)
+
+return (main_thread[0], list(all_subthreads))
+
+def continue_and_get_threads_running(self, main_thread, vCont_req):
+self.test_sequence.add_log_lines(
+["read packet: $vCont;c:{:x};{}#00".format(main_thread, vCont_req),
+ "send packet: $W00#00",
+ ], True)
+exp = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+found = set()
+for line in exp["O_content"].decode().splitlines():
+m = self.THREAD_MATCH_RE.match(line)
+if m is not None:
+found.add(int(m.group(1), 16))
+return found
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_vCont_cxcx(self):
+main_thread, all_subthreads_list = (
+self.start_vCont_run_subset_of_threads_test())
+# resume two threads explicitly, stop the third one implicitly
+self.assertEqual(
+self.continue_and_get_threads_running(
+main_thread,
+"c:{:x};c:{:x}".format(*all_subthreads_list[:2])),
+set(all_subthreads_list[:2]))
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_vCont_cxcxt(self):
+main_thread, all_subthreads_list = (
+self.start_vCont_run_subset_of_th

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

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

This revision was automatically updated to reflect the committed changes.
mgorny marked an inline comment as done.
Closed by commit rG86e472317c8f: [lldb] [test] Improve stability of llgs 
vCont-threads tests (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D129012?vs=442823&id=442916#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129012

Files:
  lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
  lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
  lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
  lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

Index: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
===
--- lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
+++ lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
@@ -1,31 +1,54 @@
 #include "pseudo_barrier.h"
 #include "thread.h"
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 
 pseudo_barrier_t barrier;
+std::mutex print_mutex;
+std::atomic can_work = ATOMIC_VAR_INIT(false);
+thread_local volatile sig_atomic_t can_exit_now = false;
 
 static void sigusr1_handler(int signo) {
-  char buf[100];
-  std::snprintf(buf, sizeof(buf),
-"received SIGUSR1 on thread id: %" PRIx64 "\n",
-get_thread_id());
-  write(STDOUT_FILENO, buf, strlen(buf));
+  std::lock_guard lock{print_mutex};
+  std::printf("received SIGUSR1 on thread id: %" PRIx64 "\n", get_thread_id());
+  can_exit_now = true;
 }
 
 static void thread_func() {
+  // this ensures that all threads start before we SIGSTOP
   pseudo_barrier_wait(barrier);
-  for (int i = 0; i < 300; ++i) {
+
+  // wait till the main thread indicates that we can go
+  // (note: using a mutex here causes hang on FreeBSD when another thread
+  // is suspended)
+  while (!can_work.load())
+std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+  // the mutex guarantees that two writes don't get interspersed
+  {
+std::lock_guard lock{print_mutex};
 std::printf("thread %" PRIx64 " running\n", get_thread_id());
+  }
+
+  // give other threads a fair chance to run
+  for (int i = 0; i < 5; ++i) {
+std::this_thread::yield();
 std::this_thread::sleep_for(std::chrono::milliseconds(200));
+if (can_exit_now)
+  return;
   }
+
+  // if we didn't get signaled, terminate the program explicitly.
+  _exit(0);
 }
 
 int main(int argc, char **argv) {
@@ -36,12 +59,15 @@
   signal(SIGUSR1, sigusr1_handler);
 
   std::vector threads;
-  for(int i = 0; i < num; ++i)
+  for (int i = 0; i < num; ++i)
 threads.emplace_back(thread_func);
 
+  // use the barrier to make sure all threads start before we SIGSTOP
   pseudo_barrier_wait(barrier);
+  std::raise(SIGSTOP);
 
-  std::puts("@started");
+  // allow the threads to work
+  can_work.store(true);
 
   for (std::thread &thread : threads)
 thread.join();
Index: lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
===
--- lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
+++ lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
@@ -1,23 +1,18 @@
-import json
 import re
-import time
 
 import gdbremote_testcase
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-class TestGdbRemote_vContThreads(gdbremote_testcase.GdbRemoteTestCaseBase):
 
+class TestSignal(gdbremote_testcase.GdbRemoteTestCaseBase):
 def start_threads(self, num):
 procs = self.prep_debug_monitor_and_inferior(inferior_args=[str(num)])
-# start the process and wait for output
 self.test_sequence.add_log_lines([
 "read packet: $c#63",
-{"type": "output_match", "regex": r".*@started\r\n.*"},
+{"direction": "send", "regex": "[$]T.*;reason:signal.*"},
 ], True)
-# then interrupt it
-self.add_interrupt_packets()
 self.add_threadinfo_collection_packets()
 
 context = self.expect_gdbremote_sequence()
@@ -29,22 +24,21 @@
 self.reset_test_sequence()
 return threads
 
+SIGNAL_MATCH_RE = re.compile(r"received SIGUSR1 on thread id: ([0-9a-f]+)")
+
 def send_and_check_signal(self, vCont_data, threads):
 self.test_sequence.add_log_lines([
 "read packet: $vCont;{0}#00".format(vCont_data),
-{"type": "output_match",
- "regex": len(threads) *
-  r".*received SIGUSR1 on thread id: ([0-9a-f]+)\r\n.*",
- "capture": dict((i, "tid{0}".format(i)) for i
- in range(1, len(threads)+1)),
- },
+"send packet: $W00#00",
 ], True)
-

[Lldb-commits] [PATCH] D129078: [LLDB][ClangExpression] Allow expression evaluation from within C++ Lambdas

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Expression/Materializer.cpp:777
+  lldb::ValueObjectSP
+  GetValueObject(ExecutionContextScope *scope) const override {
+return ValueObjectVariable::Create(scope, m_variable_sp);

jingham wrote:
> Naively it seems like it would be a bad idea to call GetValueObject twice.  
> We don't want independent ValueObjectVariable shared pointers floating around 
> for the same Entity.  Should this maybe do an `if (!m_variable_sp)` first?
`m_variable_sp` is used here to create a new `ValueObjectVariable`. It's a 
precondition that `m_variable_sp != nullptr` (will add that to the function 
documentation). I could add a `m_value_object_var` member that we set if it 
hasn't been set before.



Comment at: lldb/source/Expression/Materializer.cpp:819
+
+  bool LocationExpressionIsValid() const override { return true; }
+

jingham wrote:
> Is this right?  For instance, in the case where the fake "this" Variable in 
> the lambda expression has an invalid expression, all the ValueObjects you try 
> to make (the "real captured this" as well as any other captures that were 
> hanging off the fake "this" would have invalid location expressions.
If the location expression is invalid for the fake "this", would we ever be 
able to get ValueObject's out of it? By the time these Entity objects get 
instantiated we either managed to get ValueObject's from "this" or if we 
didn't, then we wouldn't have added them to the `$__lldb_local_vars` namespace



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1725
+
+  if (ClangExpressionVariable::ParserVars *parser_vars =
+  AddExpressionVariable(context, pt, ut, std::move(valobj))) {

jingham wrote:
> Is there enough advantage to move-ing the incoming valobj as opposed to just 
> copying the shared pointer?  It's a little weird to have API's that consume 
> one of their incoming arguments.  If that really is worth doing, you should 
> note that you've done that.
Not particularly, don't imagine there's a measurable difference. A copy isn't 
necessary so I was trying to avoid it. I guess a clearer way would be to just 
not pass the shared_ptr around, and instead a `ValueObject const&`



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp:227
+
+  if (auto thisValSP = GetLambdaValueObject(frame)) {
+uint32_t numChildren = thisValSP->GetNumChildren();

jingham wrote:
> It's worth noting that you handle lambda's that capture "this" and lambda's 
> that don't capture "this" differently.  In the former case, we promote all 
> the captures to local variables and ignore the fake this.  In the latter case 
> (because GetLambdaValueObject only returns a valid ValueObjectSP if it has a 
> child called "this"), we keep finding the values by implicit lookup in the 
> fake this instead.
> 
> I don't think that a problem, no need to use the more complex method just for 
> consistency, but you should note that somewhere.
Good point, will add a comment



Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h:202
+
+  lldb::addr_t GetCppObjectPointer(StackFrame *frame, ConstString &object_name,
+   Status &err);

jingham wrote:
> Why did you make this take a StackFrame *?  It seems to force some other 
> functions to change from StackFrameSP to StackFrame * but the only place it 
> gets called it has a StackFrameSP, and explicitly calls get to make it a 
> pointer.  That seems awkward.
Was simply trying to avoid a shared_ptr copy where it wasn't necessary. 
Arguably the `GetObjectPointer` API should just take a `StackFrame const&`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129078

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

[Lldb-commits] [lldb] fad93cd - Revert "[lldb] [test] Improve stability of llgs vCont-threads tests"

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-07-07T17:01:43+02:00
New Revision: fad93cd6821992baf0e1b5c45c1606aa5fde2938

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

LOG: Revert "[lldb] [test] Improve stability of llgs vCont-threads tests"

This reverts commit 86e472317c8fd9309b76c32ca55fcdeaf63f853b.
It breaks Debian buildbot, for some reason.

Added: 
lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py

Modified: 
lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

Removed: 
lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py



diff  --git a/lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py 
b/lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
similarity index 71%
rename from lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
rename to 
lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
index 6dde4f9555cf3..606875ed56213 100644
--- a/lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
+++ 
b/lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
@@ -1,18 +1,23 @@
+import json
 import re
+import time
 
 import gdbremote_testcase
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+class TestGdbRemote_vContThreads(gdbremote_testcase.GdbRemoteTestCaseBase):
 
-class TestSignal(gdbremote_testcase.GdbRemoteTestCaseBase):
 def start_threads(self, num):
 procs = self.prep_debug_monitor_and_inferior(inferior_args=[str(num)])
+# start the process and wait for output
 self.test_sequence.add_log_lines([
 "read packet: $c#63",
-{"direction": "send", "regex": "[$]T.*;reason:signal.*"},
+{"type": "output_match", "regex": r".*@started\r\n.*"},
 ], True)
+# then interrupt it
+self.add_interrupt_packets()
 self.add_threadinfo_collection_packets()
 
 context = self.expect_gdbremote_sequence()
@@ -24,21 +29,22 @@ def start_threads(self, num):
 self.reset_test_sequence()
 return threads
 
-SIGNAL_MATCH_RE = re.compile(r"received SIGUSR1 on thread id: ([0-9a-f]+)")
-
 def send_and_check_signal(self, vCont_data, threads):
 self.test_sequence.add_log_lines([
 "read packet: $vCont;{0}#00".format(vCont_data),
-"send packet: $W00#00",
+{"type": "output_match",
+ "regex": len(threads) *
+  r".*received SIGUSR1 on thread id: ([0-9a-f]+)\r\n.*",
+ "capture": dict((i, "tid{0}".format(i)) for i
+ in range(1, len(threads)+1)),
+ },
 ], True)
-exp = self.expect_gdbremote_sequence()
-self.reset_test_sequence()
-tids = []
-for line in exp["O_content"].decode().splitlines():
-m = self.SIGNAL_MATCH_RE.match(line)
-if m is not None:
-tids.append(int(m.group(1), 16))
-self.assertEqual(sorted(tids), sorted(threads))
+
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+tids = sorted(int(context["tid{0}".format(x)], 16)
+  for x in range(1, len(threads)+1))
+self.assertEqual(tids, sorted(threads))
 
 def get_pid(self):
 self.add_process_info_collection_packets()
@@ -236,3 +242,72 @@ def test_signal_two_signals(self):
 
 context = self.expect_gdbremote_sequence()
 self.assertIsNotNone(context)
+
+THREAD_MATCH_RE = re.compile(r"thread ([0-9a-f]+) running")
+
+def continue_and_get_threads_running(self, continue_packet):
+self.test_sequence.add_log_lines(
+["read packet: ${}#00".format(continue_packet),
+ ], True)
+self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+time.sleep(1)
+self.add_interrupt_packets()
+exp = self.expect_gdbremote_sequence()
+found = set()
+for line in exp["O_content"].decode().splitlines():
+m = self.THREAD_MATCH_RE.match(line)
+if m is not None:
+found.add(int(m.group(1), 16))
+return found
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_vCont_run_subset_of_threads(self):
+self.build()
+self.set_inferior_startup_launch()
+
+threads = set(self.start_threads(3))
+all_subthreads = self.continue_and_get_threads_running("c")
+all_subthreads_list = list(all_subthreads)
+self.assertEqual(len(all_subthreads), 3)
+self.assertEqual(threads & all_subthreads, all_subthreads)
+

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

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

mgorny added a comment.
Herald added a subscriber: JDevlieghere.

@labath, any clue why it'd be broken on Debian?

https://lab.llvm.org/buildbot/#/builders/68/builds/35396

  AssertionError: 
'$T0athread:1ddf55;name:a.out;00:;01:;02:e1bf0b8
 [truncated]... != '$W00'
  - 
$T0athread:1ddf55;name:a.out;00:;01:;02:e1bf0b8d067f;03:;04:0200;05:e0272245fc7f;06:90292245fc7f;07:e0272245fc7f;08:;09:e0272245fc7f;0a:0800;0b:4602;0c:c091997f0756;0d:;0e:;0f:;10:e1bf0b8d067f;11:4602;12:3300;13:;14:;15:2b00;16:;17:;reason:signal;+
 $W00

I mean, this makes no sense to me. Why is it stopping due to a signal LLDB was 
supposed to send to it? And why can't I reproduce it on Gentoo or FreeBSD?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129012

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

[Lldb-commits] [PATCH] D129272: [lldb][Windows] Fix memory region base addresses when a range is split

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py:119
+or (region_end > previous_base and region_end <= 
previous_end):
+self.assertFalse(base_address in base_addresses,
+"Unexpected overlapping memory region found.")

labath wrote:
> I don't see `base_addresses` defined anywhere. I guess this should just be 
> `assertFalse`, and the overlap condition right?
> 
> Btw, can [[ 
> https://stackoverflow.com/questions/3269434/whats-the-most-efficient-way-to-test-if-two-ranges-overlap
>  | check for overlap ]] with `region_base < previous_end && previous_base < 
> region_end`.
Completely missed that yes, I'll fix it up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129272

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

[Lldb-commits] [lldb] d955185 - [lldb/test] Use the shim executable for TestGdbRemoteAttach*Or*Wait as well

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-07-07T17:35:15+02:00
New Revision: d955185b9413c4bbe90d4b1f91790ddcc78baf99

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

LOG: [lldb/test] Use the shim executable for TestGdbRemoteAttach*Or*Wait as well

Without it, the test may nondeterminstically fail due to YAMA
restrictions.

Also, merge the two tests into one to reduce duplication.

Added: 


Modified: 
lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py

Removed: 
lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py



diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
deleted file mode 100644
index 90a7eee7536f..
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
+++ /dev/null
@@ -1,111 +0,0 @@
-
-import os
-from time import sleep
-
-import gdbremote_testcase
-import lldbgdbserverutils
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-
-class TestGdbRemoteAttachOrWait(gdbremote_testcase.GdbRemoteTestCaseBase):
-
-@skipIfWindows # This test is flaky on Windows
-def test_launch_before_attach_with_vAttachOrWait(self):
-exe = '%s_%d' % (self.testMethodName, os.getpid())
-self.build(dictionary={'EXE': exe})
-self.set_inferior_startup_attach_manually()
-
-# Start the inferior, start the debug monitor, nothing is attached yet.
-procs = self.prep_debug_monitor_and_inferior(
-inferior_args=["sleep:60"],
-inferior_exe_path=self.getBuildArtifact(exe))
-self.assertIsNotNone(procs)
-
-# Make sure the target process has been launched.
-inferior = procs.get("inferior")
-self.assertIsNotNone(inferior)
-self.assertTrue(inferior.pid > 0)
-self.assertTrue(
-lldbgdbserverutils.process_is_running(
-inferior.pid, True))
-
-# Add attach packets.
-self.test_sequence.add_log_lines([
-# Do the attach.
-"read packet: 
$vAttachOrWait;{}#00".format(lldbgdbserverutils.gdbremote_hex_encode_string(exe)),
-# Expect a stop notification from the attach.
-{"direction": "send",
- "regex": r"^\$T([0-9a-fA-F]{2})[^#]*#[0-9a-fA-F]{2}$",
- "capture": {1: "stop_signal_hex"}},
-], True)
-self.add_process_info_collection_packets()
-
-# Run the stream
-context = self.expect_gdbremote_sequence()
-self.assertIsNotNone(context)
-
-# Gather process info response
-process_info = self.parse_process_info_response(context)
-self.assertIsNotNone(process_info)
-
-# Ensure the process id matches what we expected.
-pid_text = process_info.get('pid', None)
-self.assertIsNotNone(pid_text)
-reported_pid = int(pid_text, base=16)
-self.assertEqual(reported_pid, inferior.pid)
-
-@skipIfWindows # This test is flaky on Windows
-def test_launch_after_attach_with_vAttachOrWait(self):
-exe = '%s_%d' % (self.testMethodName, os.getpid())
-self.build(dictionary={'EXE': exe})
-self.set_inferior_startup_attach_manually()
-
-server = self.connect_to_debug_monitor()
-self.assertIsNotNone(server)
-
-self.do_handshake()
-self.test_sequence.add_log_lines([
-# Do the attach.
-"read packet: 
$vAttachOrWait;{}#00".format(lldbgdbserverutils.gdbremote_hex_encode_string(exe)),
-], True)
-# Run the stream until attachWait.
-context = self.expect_gdbremote_sequence()
-self.assertIsNotNone(context)
-
-# Sleep so we're sure that the inferior is launched after we ask for 
the attach.
-sleep(1)
-
-# Launch the inferior.
-inferior = self.launch_process_for_attach(
-inferior_args=["sleep:60"],
-exe_path=self.getBuildArtifact(exe))
-self.assertIsNotNone(inferior)
-self.assertTrue(inferior.pid > 0)
-self.assertTrue(
-lldbgdbserverutils.process_is_running(
-inferior.pid, True))
-
-# Make sure the attach succeeded.
-self.test_sequence.add_log_lines([
-{"direction": "send",
- "regex": r"^\$T([0-9a-fA-F]{2})[^#]*#[0-9a-fA-F]{2}$",
- "capture": {1: "stop_signal_hex"}},
-], True)
-self.add_process_info_collection_packets()
-
-
-# Run the stream sending the response..
-context = self.expect_gdbremote_sequence()
-self.assertIsNotNone(context)
-
-# Gather process info response.
-process_info = self.parse_process_in

[Lldb-commits] [lldb] f3d43ec - [lldb][Windows] Fixup overlapping memory regions tests

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-07-07T15:36:14Z
New Revision: f3d43eca34d4e3ddff2ba8a020ca21a9e963dd4f

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

LOG: [lldb][Windows] Fixup overlapping memory regions tests

As suggested in post-commit review on https://reviews.llvm.org/D129272.

* Rename the test case.
* Simplify the overlap check.
* Correct assertion.

Added: 


Modified: 
lldb/test/API/functionalities/memory-region/TestMemoryRegion.py

Removed: 




diff  --git a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py 
b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
index 18118377a8b8..3ea4a75ec9b9 100644
--- a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -88,7 +88,7 @@ def test_command(self):
 self.assertTrue(result.Succeeded())
 self.assertEqual(result.GetOutput(), all_regions)
 
-def test_unique_base_addresses(self):
+def test_no_overlapping_regions(self):
 # In the past on Windows we were recording AllocationBase as the base 
address
 # of the current region, not BaseAddress. So if a range of pages was 
split
 # into regions you would see several regions with the same base 
address.
@@ -114,10 +114,9 @@ def test_unique_base_addresses(self):
 region_base = region.GetRegionBase()
 region_end = region.GetRegionEnd()
 
-if (region_base >= previous_base and region_base < 
previous_end) \
-or (region_end > previous_base and region_end <= 
previous_end):
-self.assertFalse(base_address in base_addresses,
-"Unexpected overlapping memory region found.")
+self.assertFalse(
+(region_base < previous_end) and (previous_base < 
region_end),
+"Unexpected overlapping memory region found.")
 
 previous_base = region_base
 previous_end = region_end
\ No newline at end of file



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

[Lldb-commits] [PATCH] D129272: [lldb][Windows] Fix memory region base addresses when a range is split

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked 2 inline comments as done.
DavidSpickett added a comment.

Fixed up in https://reviews.llvm.org/rGf3d43eca34d4. Next time I will remember 
to check what a test failure looks like.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129272

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

[Lldb-commits] [PATCH] D128932: [lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

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

mgorny added inline comments.



Comment at: lldb/test/API/tools/lldb-server/main.cpp:351
+  break;
+std::this_thread::sleep_for(std::chrono::milliseconds(125 * i));
+  }

labath wrote:
> I'm not convinced this will be enough. How about `125 * (1

[Lldb-commits] [PATCH] D129078: [LLDB][ClangExpression] Allow expression evaluation from within C++ Lambdas

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 442954.
Michael137 added a comment.
Herald added a subscriber: mgorny.

- Add more documentation
- Moved `GetLambdaValueObject` into common utility header
- Added defensive check to `EntityVariable::GetValueObject`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129078

Files:
  lldb/include/lldb/Expression/Materializer.h
  lldb/include/lldb/Expression/UserExpression.h
  lldb/source/Expression/Materializer.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
  lldb/test/API/commands/expression/expr_inside_lambda/Makefile
  lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
  lldb/test/API/commands/expression/expr_inside_lambda/main.cpp

Index: lldb/test/API/commands/expression/expr_inside_lambda/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/expr_inside_lambda/main.cpp
@@ -0,0 +1,99 @@
+#include 
+#include 
+
+namespace {
+int global_var = -5;
+} // namespace
+
+struct Baz {
+  virtual ~Baz() = default;
+
+  virtual int baz_virt() = 0;
+
+  int base_base_var = 12;
+};
+
+struct Bar : public Baz {
+  virtual ~Bar() = default;
+
+  virtual int baz_virt() override {
+base_var = 10;
+return 1;
+  }
+
+  int base_var = 15;
+};
+
+struct Foo : public Bar {
+  int class_var = 9;
+  int shadowed = -137;
+  int *class_ptr;
+
+  virtual ~Foo() = default;
+
+  virtual int baz_virt() override {
+shadowed = -1;
+return 2;
+  }
+
+  void method() {
+int local_var = 137;
+int shadowed;
+class_ptr = &local_var;
+auto lambda = [&shadowed, this, &local_var,
+   local_var_copy = local_var]() mutable {
+  int lambda_local_var = 5;
+  shadowed = 5;
+  class_var = 109;
+  --base_var;
+  --base_base_var;
+  std::puts("break here");
+
+  auto nested_lambda = [this, &lambda_local_var] {
+std::puts("break here");
+lambda_local_var = 0;
+  };
+
+  nested_lambda();
+  --local_var_copy;
+  std::puts("break here");
+
+  struct LocalLambdaClass {
+int lambda_class_local = -12345;
+Foo *outer_ptr;
+
+void inner_method() {
+  auto lambda = [this] {
+std::puts("break here");
+lambda_class_local = -2;
+outer_ptr->class_var *= 2;
+  };
+
+  lambda();
+}
+  };
+
+  LocalLambdaClass l;
+  l.outer_ptr = this;
+  l.inner_method();
+};
+lambda();
+  }
+
+  void non_capturing_method() {
+int local = 5;
+int local2 = 10;
+
+class_var += [=] {
+  std::puts("break here");
+  return local + local2;
+}();
+  }
+};
+
+int main() {
+  Foo f;
+  f.method();
+  f.non_capturing_method();
+  return global_var;
+}
Index: lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
@@ -0,0 +1,123 @@
+""" Test that evaluating expressions from within C++ lambdas works
+Particularly, we test the case of capturing "this" and
+using members of the captured object in expression evaluation
+while we're on a breakpoint inside a lambda.
+"""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+
+class ExprInsideLambdaTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def expectExprError(self, expr : str, expected : str):
+frame = self.thread.GetFrameAtIndex(0)
+value = frame.EvaluateExpression(expr)
+errmsg = value.GetError().GetCString()
+self.assertIn(expected, errmsg)
+
+def test_expr_inside_lambda(self):
+"""Test that lldb evaluating expressions inside lambda expressions works correctly."""
+self.build()
+(target, process, self.thread, bkpt) = \
+lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.cpp"))
+
+# Inside 'Foo::method'
+
+# Check access to captured 'this'
+self.expect_expr("class_var", result_type="int", result_value="109")
+self.expect_expr("this->clas

[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan created this revision.
yinghuitan added reviewers: clayborg, labath, jingham, jdoerfert, JDevlieghere, 
aadsm, kusmour.
Herald added a project: All.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Some build system (like Buck) would normalize file paths into relative paths 
in debug info to support hermetic/stable build caching.
This requires IDE/debugger users to configure correct source mapping if they
are using full path for file line breakpoint.

We are seeing many users fail to bind/resolve breakpoints due to 
incorrect/missing source map.

This patch adds a new partial match setting (target.breakpoint-partial-match) 
 which enables matching breakpoint request by partial suffix.
The number of suffix directories required to match is controlled by another
setting (target.breakpoint-partial-match-dir-count). The default value is zero
which means matching base file name only.

This mimic what most command line lldb users/testcases are doing -- they use 
base file name for setting file line breakpoint.

This setting will greatly improve breakpoint reliability in lldb-vscode useage
and can help post-mortem/off-host debugging which the file path in debug info
may not match the source location in the debug machine.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129307

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  lldb/test/API/breakpoint/Makefile
  lldb/test/API/breakpoint/TestBreakpointPartialMatch.py
  lldb/test/API/breakpoint/main.cpp

Index: lldb/test/API/breakpoint/main.cpp
===
--- /dev/null
+++ lldb/test/API/breakpoint/main.cpp
@@ -0,0 +1,10 @@
+#include 
+
+void foo() {
+  printf("hello world from foo"); // Set break point at this line.
+}
+
+int main() {
+  foo();
+  return 0;
+}
Index: lldb/test/API/breakpoint/TestBreakpointPartialMatch.py
===
--- /dev/null
+++ lldb/test/API/breakpoint/TestBreakpointPartialMatch.py
@@ -0,0 +1,193 @@
+"""
+Test breakpoint partial match feature.
+"""
+
+
+import os
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+def split_all(path):
+"""Split path into parts"""
+allparts = []
+while True:
+parts = os.path.split(path)
+if parts[0] == path:  # sentinel for absolute paths
+allparts.insert(0, parts[0])
+break
+elif parts[1] == path:  # sentinel for relative paths
+allparts.insert(0, parts[1])
+break
+else:
+path = parts[0]
+allparts.insert(0, parts[1])
+return allparts
+
+
+class TestBreakpointPartialMatch(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+
+self.source_file_name = "main.cpp"
+self.line = line_number(
+self.source_file_name, "// Set break point at this line."
+)
+
+dir_path = os.path.dirname(os.path.realpath(__file__))
+dirs = split_all(dir_path)
+
+self.partial_match_dir_count = min(2, len(dirs))
+
+# Take last self.partial_match_dir_count directories out of dirs.
+self.matched_dirs = dirs[-self.partial_match_dir_count :]
+self.unmatched_dirs = ["non", "exist", "root"]
+
+# partial match debug info path with base name.
+self.base_name_match_breakpoint_file = os.path.join(
+*self.unmatched_dirs, self.source_file_name
+)
+
+# partial match debug info path with
+# last self.partial_match_dir_count directories.
+self.partial_match_breakpoint_file = os.path.join(
+*self.unmatched_dirs, *self.matched_dirs, self.source_file_name
+)
+
+def test_default(self):
+'''
+Test default settings of partial match.
+'''
+self.build()
+
+# Disable partial match.
+self.runCmd("settings set target.breakpoint-partial-match false")
+
+# Create a target by the debugger.
+exe = self.getBuildArtifact("a.out")
+error = lldb.SBError()
+# Don't read in dependencies so we don't come across false matches that
+# add unwanted breakpoint hits.
+self.target = self.dbg.CreateTarget(exe, None, None, False, error)
+self.assertTrue(self.target, VALID_TARGET)
+
+default_bp = self.target.BreakpointCreateByLocation(
+self.partial_match_breakpoint_file, self.line
+)
+# Breakpoint shouldn't bind due to mismatch path.
+self.assertEqual(default_bp.GetNumLocations(), 0)
+
+# Enable partial match.
+self.runCmd("settings set target.breakpoint-par

[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan updated this revision to Diff 442973.
yinghuitan added a comment.

Remove unnecessary format changes caused by IDE.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129307

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  lldb/test/API/breakpoint/Makefile
  lldb/test/API/breakpoint/TestBreakpointPartialMatch.py
  lldb/test/API/breakpoint/main.cpp

Index: lldb/test/API/breakpoint/main.cpp
===
--- /dev/null
+++ lldb/test/API/breakpoint/main.cpp
@@ -0,0 +1,10 @@
+#include 
+
+void foo() {
+  printf("hello world from foo"); // Set break point at this line.
+}
+
+int main() {
+  foo();
+  return 0;
+}
Index: lldb/test/API/breakpoint/TestBreakpointPartialMatch.py
===
--- /dev/null
+++ lldb/test/API/breakpoint/TestBreakpointPartialMatch.py
@@ -0,0 +1,193 @@
+"""
+Test breakpoint partial match feature.
+"""
+
+
+import os
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+def split_all(path):
+"""Split path into parts"""
+allparts = []
+while True:
+parts = os.path.split(path)
+if parts[0] == path:  # sentinel for absolute paths
+allparts.insert(0, parts[0])
+break
+elif parts[1] == path:  # sentinel for relative paths
+allparts.insert(0, parts[1])
+break
+else:
+path = parts[0]
+allparts.insert(0, parts[1])
+return allparts
+
+
+class TestBreakpointPartialMatch(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+
+self.source_file_name = "main.cpp"
+self.line = line_number(
+self.source_file_name, "// Set break point at this line."
+)
+
+dir_path = os.path.dirname(os.path.realpath(__file__))
+dirs = split_all(dir_path)
+
+self.partial_match_dir_count = min(2, len(dirs))
+
+# Take last self.partial_match_dir_count directories out of dirs.
+self.matched_dirs = dirs[-self.partial_match_dir_count :]
+self.unmatched_dirs = ["non", "exist", "root"]
+
+# partial match debug info path with base name.
+self.base_name_match_breakpoint_file = os.path.join(
+*self.unmatched_dirs, self.source_file_name
+)
+
+# partial match debug info path with
+# last self.partial_match_dir_count directories.
+self.partial_match_breakpoint_file = os.path.join(
+*self.unmatched_dirs, *self.matched_dirs, self.source_file_name
+)
+
+def test_default(self):
+'''
+Test default settings of partial match.
+'''
+self.build()
+
+# Disable partial match.
+self.runCmd("settings set target.breakpoint-partial-match false")
+
+# Create a target by the debugger.
+exe = self.getBuildArtifact("a.out")
+error = lldb.SBError()
+# Don't read in dependencies so we don't come across false matches that
+# add unwanted breakpoint hits.
+self.target = self.dbg.CreateTarget(exe, None, None, False, error)
+self.assertTrue(self.target, VALID_TARGET)
+
+default_bp = self.target.BreakpointCreateByLocation(
+self.partial_match_breakpoint_file, self.line
+)
+# Breakpoint shouldn't bind due to mismatch path.
+self.assertEqual(default_bp.GetNumLocations(), 0)
+
+# Enable partial match.
+self.runCmd("settings set target.breakpoint-partial-match true")
+
+partial_match_bp = self.target.BreakpointCreateByLocation(
+self.partial_match_breakpoint_file, self.line
+)
+# Breakpoint should bind due to partial match.
+self.assertNotEqual(partial_match_bp.GetNumLocations(), 0)
+
+basename_match_bp = self.target.BreakpointCreateByLocation(
+self.base_name_match_breakpoint_file, self.line
+)
+# Breakpoint should bind due to base name match
+self.assertNotEqual(basename_match_bp.GetNumLocations(), 0)
+
+def test_dir_count(self):
+'''
+Test partial match directory count.
+'''
+self.build()
+
+# Enable partial match.
+self.runCmd("settings set target.breakpoint-partial-match true")
+
+# Create a target by the debugger.
+exe = self.getBuildArtifact("a.out")
+error = lldb.SBError()
+# Don't read in dependencies so we don't come across false matches that
+# add unwanted breakpoint hits.
+self.target = self.dbg.CreateTarget(exe, None, None, False, error)
+  

[Lldb-commits] [lldb] ec48a0d - [lldb] Improve the error message in run_to_breakpoint_do_run

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-07-07T10:22:45-07:00
New Revision: ec48a0df9151a5192381e44bee6a48a08ed8932b

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

LOG: [lldb] Improve the error message in run_to_breakpoint_do_run

Improve the error message when we fail to hit the initial breakpoint in
run_to_breakpoint_do_run. In addition to the process state, we now also
report the exit code and reason (if the process exited) as well as the
inferior's output.

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

Added: 
lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/Makefile

lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/TestLLDBUtilFailedToHitBreakpoint.py
lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/main.cpp

Modified: 
lldb/packages/Python/lldbsuite/test/lldbutil.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py 
b/lldb/packages/Python/lldbsuite/test/lldbutil.py
index 74ca7dd060e3b..7863e64e527f1 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -235,7 +235,7 @@ def state_type_to_str(enum):
 elif enum == lldb.eStateSuspended:
 return "suspended"
 else:
-raise Exception("Unknown StateType enum")
+raise Exception("Unknown StateType enum: " + str(enum))
 
 
 def stop_reason_to_str(enum):
@@ -945,7 +945,22 @@ def run_to_breakpoint_do_run(test, target, bkpt, 
launch_info = None,
 test.assertFalse(error.Fail(),
  "Process launch failed: %s" % (error.GetCString()))
 
-test.assertState(process.GetState(), lldb.eStateStopped)
+def processStateInfo(process):
+info = "state: {}".format(state_type_to_str(process.state))
+if process.state == lldb.eStateExited:
+info += ", exit code: {}".format(process.GetExitStatus())
+if process.exit_description:
+info += ", exit description: 
'{}'".format(process.exit_description)
+stdout = process.GetSTDOUT(999)
+if stdout:
+info += ", stdout: '{}'".format(stdout)
+stderr = process.GetSTDERR(999)
+if stderr:
+info += ", stderr: '{}'".format(stderr)
+return info
+
+if process.state != lldb.eStateStopped:
+test.fail("Test process is not stopped at breakpoint: 
{}".format(processStateInfo(process)))
 
 # Frame #0 should be at our breakpoint.
 threads = get_threads_stopped_at_breakpoint(

diff  --git a/lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/Makefile 
b/lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/Makefile
new file mode 100644
index 0..8b20bcb05
--- /dev/null
+++ b/lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/TestLLDBUtilFailedToHitBreakpoint.py
 
b/lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/TestLLDBUtilFailedToHitBreakpoint.py
new file mode 100644
index 0..0ce554a5a53a5
--- /dev/null
+++ 
b/lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/TestLLDBUtilFailedToHitBreakpoint.py
@@ -0,0 +1,27 @@
+"""
+Tests lldbutil's behavior when running to a source breakpoint fails.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class LLDBUtilFailedToHitBreakpointTest(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@expectedFailureAll(oslist=["windows"])
+def test_error_message(self):
+"""
+Tests that run_to_source_breakpoint prints the right error message
+when failing to hit the wanted breakpoint.
+"""
+self.build()
+with self.assertRaisesRegex(
+AssertionError,
+"Test process is not stopped at breakpoint: state: exited, 
exit code: 0, stdout: 'stdout_needlestderr_needle'"
+):
+lldbutil.run_to_source_breakpoint(self, "// break here",
+  lldb.SBFileSpec("main.cpp"))

diff  --git a/lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/main.cpp 
b/lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/main.cpp
new file mode 100644
index 0..267f247b62285
--- /dev/null
+++ b/lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/main.cpp
@@ -0,0 +1,21 @@
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  // Print the string that the test looks for to make sure stdout and stderr
+  // got recorded.
+  std::cout << "stdout_needle" << std::flush;
+  std::cerr << "stderr_needle" << std::flush;
+
+  // Work around a timing issue that sometimes prevents stder

[Lldb-commits] [PATCH] D111978: [lldb] Improve the error message in run_to_breakpoint_do_run

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.
Closed by commit rGec48a0df9151: [lldb] Improve the error message in 
run_to_breakpoint_do_run (authored by JDevlieghere).
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D111978?vs=442603&id=442979#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111978

Files:
  lldb/packages/Python/lldbsuite/test/lldbutil.py
  lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/Makefile
  
lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/TestLLDBUtilFailedToHitBreakpoint.py
  lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/main.cpp

Index: lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/main.cpp
===
--- /dev/null
+++ lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/main.cpp
@@ -0,0 +1,21 @@
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  // Print the string that the test looks for to make sure stdout and stderr
+  // got recorded.
+  std::cout << "stdout_needle" << std::flush;
+  std::cerr << "stderr_needle" << std::flush;
+
+  // Work around a timing issue that sometimes prevents stderr from being
+  // captured.
+  std::this_thread::sleep_for(std::chrono::seconds(1));
+
+  // This is unreachable during normal test execution as we don't pass any
+  // (or +100) arguments. This still needs to be theoretically reachable code
+  // so that the compiler will generate code for this (that we can set a
+  // breakpoint on).
+  if (argc > 100)
+return 1; // break here
+  return 0;
+}
Index: lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/TestLLDBUtilFailedToHitBreakpoint.py
===
--- /dev/null
+++ lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/TestLLDBUtilFailedToHitBreakpoint.py
@@ -0,0 +1,27 @@
+"""
+Tests lldbutil's behavior when running to a source breakpoint fails.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class LLDBUtilFailedToHitBreakpointTest(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@expectedFailureAll(oslist=["windows"])
+def test_error_message(self):
+"""
+Tests that run_to_source_breakpoint prints the right error message
+when failing to hit the wanted breakpoint.
+"""
+self.build()
+with self.assertRaisesRegex(
+AssertionError,
+"Test process is not stopped at breakpoint: state: exited, exit code: 0, stdout: 'stdout_needlestderr_needle'"
+):
+lldbutil.run_to_source_breakpoint(self, "// break here",
+  lldb.SBFileSpec("main.cpp"))
Index: lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/Makefile
===
--- /dev/null
+++ lldb/test/API/lldbutil-tests/failed-to-hit-breakpoint/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/packages/Python/lldbsuite/test/lldbutil.py
===
--- lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -235,7 +235,7 @@
 elif enum == lldb.eStateSuspended:
 return "suspended"
 else:
-raise Exception("Unknown StateType enum")
+raise Exception("Unknown StateType enum: " + str(enum))
 
 
 def stop_reason_to_str(enum):
@@ -945,7 +945,22 @@
 test.assertFalse(error.Fail(),
  "Process launch failed: %s" % (error.GetCString()))
 
-test.assertState(process.GetState(), lldb.eStateStopped)
+def processStateInfo(process):
+info = "state: {}".format(state_type_to_str(process.state))
+if process.state == lldb.eStateExited:
+info += ", exit code: {}".format(process.GetExitStatus())
+if process.exit_description:
+info += ", exit description: '{}'".format(process.exit_description)
+stdout = process.GetSTDOUT(999)
+if stdout:
+info += ", stdout: '{}'".format(stdout)
+stderr = process.GetSTDERR(999)
+if stderr:
+info += ", stderr: '{}'".format(stderr)
+return info
+
+if process.state != lldb.eStateStopped:
+test.fail("Test process is not stopped at breakpoint: {}".format(processStateInfo(process)))
 
 # Frame #0 should be at our breakpoint.
 threads = get_threads_stopped_at_breakpoint(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 227dffd - [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

[Zequan Wu via lldb-commits]

Author: Zequan Wu
Date: 2022-07-07T10:26:58-07:00
New Revision: 227dffd0b6d78154516ace45f6ed28259c7baa48

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

LOG: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

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

Added: 
lldb/include/lldb/Expression/DWARFExpressionList.h
lldb/source/Expression/DWARFExpressionList.cpp

Modified: 
lldb/include/lldb/Expression/DWARFExpression.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/Variable.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Utility/RangeMap.h
lldb/include/lldb/lldb-forward.h
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/Expression/CMakeLists.txt
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Expression/Materializer.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Symbol/Variable.cpp
lldb/source/Target/RegisterContextUnwind.cpp
lldb/source/Target/StackFrame.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn

Removed: 




diff  --git a/lldb/include/lldb/Expression/DWARFExpression.h 
b/lldb/include/lldb/Expression/DWARFExpression.h
index 96a0e8e02da13..49e51d51f211c 100644
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -42,49 +42,14 @@ class DWARFExpression {
   /// \param[in] data
   /// A data extractor configured to read the DWARF location expression's
   /// bytecode.
-  DWARFExpression(lldb::ModuleSP module, const DataExtractor &data,
-  const DWARFUnit *dwarf_cu);
+  DWARFExpression(const DataExtractor &data);
 
   /// Destructor
   virtual ~DWARFExpression();
 
-  /// Print the description of the expression to a stream
-  ///
-  /// \param[in] s
-  /// The stream to print to.
-  ///
-  /// \param[in] level
-  /// The level of verbosity to use.
-  ///
-  /// \param[in] abi
-  /// An optional ABI plug-in that can be used to resolve register
-  /// names.
-  void GetDescription(Stream *s, lldb::DescriptionLevel level, ABI *abi) const;
-
   /// Return true if the location expression contains data
   bool IsValid() const;
 
-  /// Return true if a location list was provided
-  bool IsLocationList() const;
-
-  /// Search for a load address in the location list
-  ///
-  /// \param[in] func_load_addr
-  /// The actual address of the function containing this location list.
-  ///
-  /// \param[in] addr
-  /// The address to resolve
-  ///
-  /// \return
-  /// True if IsLocationList() is true and the address was found;
-  /// false otherwise.
-  //bool
-  //LocationListContainsLoadAddress (Process* process, const Address &addr)
-  //const;
-  //
-  bool LocationListContainsAddress(lldb::addr_t func_load_addr,
-   lldb::addr_t addr) const;
-
   /// If a location is not a location list, return true if the location
   /// contains a DW_OP_addr () opcode in the stream that matches \a file_addr.
   /// If file_addr is LLDB_INVALID_ADDRESS, the this function will return true
@@ -93,6 +58,9 @@ class DWARFExpression {
   /// static variable since there is no other indication from DWARF debug
   /// info.
   ///
+  /// \param[in] dwarf_cu
+  /// The dwarf unit this expression belongs to.
+  ///
   /// \param[in] op_addr_idx
   /// The DW_OP_addr index to retrieve in case there is more than
   /// one DW_OP_addr opcode in the location byte stream.
@@ -104,36 +72,22 @@ class DWARFExpression {
   /// \return
   /// LLDB_INVALID_ADDRESS if the location doesn't contain a
   /// DW_OP_addr for \a op_addr_idx, otherwise a valid file address
-  lldb::addr_t GetLocation_DW_OP_addr(uint32_t op_addr_idx, bool &error) const;
+  lldb::addr_t Ge

[Lldb-commits] [PATCH] D125509: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

[Zequan Wu via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG227dffd0b6d7: [LLDB][NFC] Decouple dwarf location table from 
DWARFExpression. (authored by zequanwu).

Changed prior to commit:
  https://reviews.llvm.org/D125509?vs=439814&id=442981#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125509

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/include/lldb/Expression/DWARFExpressionList.h
  lldb/include/lldb/Symbol/Function.h
  lldb/include/lldb/Symbol/Variable.h
  lldb/include/lldb/Target/StackFrame.h
  lldb/include/lldb/Utility/RangeMap.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Core/ValueObjectVariable.cpp
  lldb/source/Expression/CMakeLists.txt
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/DWARFExpressionList.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Variable.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
  lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
  llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn

Index: llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
@@ -23,6 +23,7 @@
   include_dirs = [ ".." ]
   sources = [
 "DWARFExpression.cpp",
+"DWARFExpressionList.cpp",
 "DiagnosticManager.cpp",
 "Expression.cpp",
 "ExpressionVariable.cpp",
Index: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
@@ -19,29 +19,29 @@
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x0
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F0
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x-0x0001)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0001, 0x0001): DW_OP_reg0 RAX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x, 0x0001): DW_OP_reg0 RAX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0001}, language = "", file = '1.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x2
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F1
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0001-0x0002)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0003, 0x0001): DW_OP_reg1 RDX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0001, 0x0002): DW_OP_reg1 RDX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0002}, language = "", file = '2.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x4
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F2
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0002-0x0003)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0005, 0x0001): DW_OP_reg2 RCX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0002, 0x0003): DW_OP_reg2 RCX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0003}, language = "", file = '3.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x6
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F3
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0003-0x0004)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 442994.

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

https://reviews.llvm.org/D129166

Files:
  lldb/packages/Python/lldbsuite/test/builders/builder.py
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  lldb/test/API/lit.cfg.py
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/CMakeLists.txt

Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -127,6 +127,7 @@
   # dependency as it's also possible to run the libc++ tests against the libc++
   # installed on the system.
   if (TARGET cxx)
+set(LLDB_HAS_LIBCXX ON)
 add_lldb_test_dependency(cxx)
   endif()
 
@@ -172,6 +173,7 @@
   LLDB_ENABLE_LZMA
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
+  LLDB_HAS_LIBCXX
   LLDB_USE_SYSTEM_DEBUGSERVER
   LLDB_IS_64_BITS)
 
Index: lldb/test/API/lit.site.cfg.py.in
===
--- lldb/test/API/lit.site.cfg.py.in
+++ lldb/test/API/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
 config.llvm_libs_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
+config.llvm_include_dir = lit_config.substitute("@LLVM_INCLUDE_DIR@")
 config.llvm_shlib_dir = lit_config.substitute("@SHLIBDIR@")
 config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
@@ -29,6 +30,7 @@
 config.test_arch = '@LLDB_TEST_ARCH@'
 config.test_compiler = lit_config.substitute('@LLDB_TEST_COMPILER@')
 config.dsymutil = lit_config.substitute('@LLDB_TEST_DSYMUTIL@')
+config.has_libcxx = '@LLDB_HAS_LIBCXX@'
 # The API tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api")
 config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-api")
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -158,14 +158,22 @@
 if is_configured('dotest_args_str'):
   dotest_cmd.extend(config.dotest_args_str.split(';'))
 
-# Library path may be needed to locate just-built clang.
+# Library path may be needed to locate just-built clang and libcxx.
 if is_configured('llvm_libs_dir'):
   dotest_cmd += ['--env', 'LLVM_LIBS_DIR=' + config.llvm_libs_dir]
 
+# Include path may be needed to locate just-built libcxx.
+if is_configured('llvm_include_dir'):
+  dotest_cmd += ['--env', 'LLVM_INCLUDE_DIR=' + config.llvm_include_dir]
+
 # This path may be needed to locate required llvm tools
 if is_configured('llvm_tools_dir'):
   dotest_cmd += ['--env', 'LLVM_TOOLS_DIR=' + config.llvm_tools_dir]
 
+# If we have a just-built libcxx, prefer it over the system one.
+if is_configured('has_libcxx') and platform.system() != 'Windows':
+  dotest_cmd += ['--hermetic-libcxx']
+
 # Forward ASan-specific environment variables to tests, as a test may load an
 # ASan-ified dylib.
 for env_var in ('ASAN_OPTIONS', 'DYLD_INSERT_LIBRARIES'):
Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -381,23 +381,27 @@
 ifeq (1,$(USE_LIBSTDCPP))
 	# Clang requires an extra flag: -stdlib=libstdc++
 	ifneq (,$(findstring clang,$(CC)))
-		CXXFLAGS += -stdlib=libstdc++ -DLLDB_USING_LIBSTDCPP
+		CXXFLAGS += -stdlib=libstdc++
 		LDFLAGS += -stdlib=libstdc++
 	endif
 endif
 
 ifeq (1,$(USE_LIBCPP))
-	CXXFLAGS += -DLLDB_USING_LIBCPP
-	ifeq "$(OS)" "Android"
-		# Nothing to do, this is already handled in
-		# Android.rules.
+	ifeq (1,$(USE_HERMETIC_LIBCPP))
+		CXXFLAGS += -nostdlib++ -nostdinc++ -I$(LLVM_INCLUDE_DIR)/c++/v1
+		LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR) -lc++
 	else
-		CXXFLAGS += -stdlib=libc++
-		LDFLAGS += -stdlib=libc++
-	endif
-	ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
-		ifneq (,$(LLVM_LIBS_DIR))
-			LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+		ifeq "$(OS)" "Android"
+			# Nothing to do, this is already handled in
+			# Android.rules.
+		else
+			CXXFLAGS += -stdlib=libc++
+			LDFLAGS += -stdlib=libc++
+		endif
+		ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+			ifneq (,$(LLVM_LIBS_DIR))
+LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+			endif
 		endif
 	endif
 endif
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
@@ -43,6 +43,8 @@
 if sys.platform == 'darwin':
 group.

[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

[Jim Ingham via Phabricator via lldb-commits]

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

I'm not entirely clear what problem this is solving.  Any actor setting 
breakpoints can already choose the match depth by simply providing that many 
directory components.  I.e. if I know I have subdir1/foo.cpp and 
subdir2/foo.cpp I can set a breakpoint on only one of them by doing:

  (lldb) break set -f subdir1/foo.cpp -l 10

So an IDE could implement this by simply passing either the base name or the 
base name plus one sub directory, etc.  As you say, that's what command line 
users do anyway, they seldom type out full paths.  So this is really about 
IDE's running lldb, and this seems more like a feature the IDE should offer, 
not lldb.

It also seems awkward to do all this work as part of the breakpoint filtering - 
which as you have seen is somewhat delicate code.  After all, you can implement 
"only match the last N components" by only submitting the last N components 
when looking up the files.  So it would be much more straightforward to strip 
the leading path components from the file name when you create the breakpoint 
and you don't have to muck with the filtering at all.  That would also help 
remove some confusion when you see that the path submitted was /foo/bar/baz.cpp 
but I matched /some/other/bar/baz.cpp (but not /some/other/different/baz.cpp) 
because I has set the match suffix count to 1.  It would be nice to have break 
list show me exactly what it was going to match on.

I am also not sure doing this as a general setting for values other than 0 is 
really helpful.  So far as I can tell, you would use a value of 1 because you 
know that though you might have files with duplicate names they are always 
disambiguated by their parent directory name.  Using a value of 2 says the 
names are disambiguated by the two parent directory names.  But there's no 
guarantee that one value is going to work for all the files in your project and 
having to change a global setting from breakpoint to breakpoint is awkward.

I also don't like that this makes breakpoint settings depend on the history of 
the session.  For instance:

a) Set the partial match to on and the depth to 2
b) Set a file & line breakpoint
c) Change the partial match depth to 0
d) a library gets loaded with a file that wouldn't have matched with depth of 2 
but does with 0

Also, you are supposed to be able to save breakpoints & restore them and 
provided your binaries haven't changed you will get the same breakpoints.  Now, 
for that to work, you also have to restore some target setting that wasn't 
saved with the breakpoints.

I wouldn't mind so much if this were passed in to the breakpoint setting 
directly, though again, I don't really see the point since this is mostly for 
IDE's and they can strip however much they want off a path before submitting it 
w/o involving lldb.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129307

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

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

> The overall logic and the include and library paths make sense to me. The 
> rpath thingy will not work on windows as it (afaik) has no equivalent feature 
> (it has the equivalent of (DY)LD_LIBRARY_PATH though).

Any idea what the libc++ tests do on Windows then? (on linux they seem to use 
rpath to ensure the tests load the just-built libc++)


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

https://reviews.llvm.org/D129166

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

[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D129307#3636573 , @jingham wrote:

> I'm not entirely clear what problem this is solving.  Any actor setting 
> breakpoints can already choose the match depth by simply providing that many 
> directory components.  I.e. if I know I have subdir1/foo.cpp and 
> subdir2/foo.cpp I can set a breakpoint on only one of them by doing:
>
>   (lldb) break set -f subdir1/foo.cpp -l 10
>
> So an IDE could implement this by simply passing either the base name or the 
> base name plus one sub directory, etc.  As you say, that's what command line 
> users do anyway, they seldom type out full paths.  So this is really about 
> IDE's running lldb, and this seems more like a feature the IDE should offer, 
> not lldb.

Xcode has this same issue if you download a dSYM file from the Apple build 
infrastructure if that dSYM doesn't contain path re-mappings in the UUID 
plists. Xcode, the IDE, will send down full paths to the source files that it 
knows about if you set breakpoints. Granted Xcode could work around this, but 
we added the ability to remap things at the module level in LLDB to work around 
this issue. So yes, it would be great if all IDEs would do this, but currently 
none do.

> It also seems awkward to do all this work as part of the breakpoint filtering 
> - which as you have seen is somewhat delicate code.  After all, you can 
> implement "only match the last N components" by only submitting the last N 
> components when looking up the files.  So it would be much more 
> straightforward to strip the leading path components from the file name when 
> you create the breakpoint and you don't have to muck with the filtering at 
> all.  That would also help remove some confusion when you see that the path 
> submitted was /foo/bar/baz.cpp but I matched /some/other/bar/baz.cpp (but not 
> /some/other/different/baz.cpp) because I had set the match suffix count to 1. 
>  It would be nice to have break list show me exactly what it was going to 
> match on.

We might need to always store the path that was given in source file and line 
breakpoints and then have the breakpoint update its locations when/if either of 
these two settings are modified. This would also help with your example below 
where you comment on breakpoint settings depend on the history of the session 
(they shouldn't).

> I am also not sure doing this as a general setting for values other than 0 is 
> really helpful.  So far as I can tell, you would use a value of 1 because you 
> know that though you might have files with duplicate names they are always 
> disambiguated by their parent directory name.

Exactly the case I was worried about. Usually zero would be the default IMHO, 
but if you have two different binaries that both have a "main.cpp" it could 
help. We can remove the depth and have this setting only set breakpoints by 
basename if needed if no one wants the extra complexity.

> Using a value of 2 says the names are disambiguated by the two parent 
> directory names.  But there's no guarantee that one value is going to work 
> for all the files in your project and having to change a global setting from 
> breakpoint to breakpoint is awkward.
>
> I also don't like that this makes breakpoint settings depend on the history 
> of the session.  For instance:
>
> a) Set the partial match to on and the depth to 2
> b) Set a file & line breakpoint
> c) Change the partial match depth to 0
> d) a library gets loaded with a file that wouldn't have matched with depth of 
> 2 but does with 0

We would need to store the full path in the source file and line breakpoints 
all the time and then set all of the source breakpoints again if this setting 
changes (enabled/disabled or dir depth changes).

> Also, you are supposed to be able to save breakpoints & restore them and 
> provided your binaries haven't changed you will get the same breakpoints.  
> Now, for that to work, you also have to restore some target setting that 
> wasn't saved with the breakpoints.

We wouldn't actually need to if we always store the path we are given when the 
breakpoint is originally set, and then update the breakpoint locations when/if 
the settings are changed (which might cause more locations to show up, or less).

> I wouldn't mind so much if this were passed in to the breakpoint setting 
> directly, though again, I don't really see the point since this is mostly for 
> IDE's and they can strip however much they want off a path before submitting 
> it w/o involving lldb.

So neither Xcode nor Visual Studio Code make any attempt to remap sources for 
anyone. The IDEs always pass down the full path currently.

Just to let everyone know where we are going with this: we want to implement an 
auto source map feature after this patch where if the user sets a breakpoint 
with "/some/build/path//bar/baz/foo.cpp", and the debug info contains 
"//bar/baz.foo.cpp", we can have LLDB automatically create 
a sourc

[Lldb-commits] [PATCH] D129319: [lldb] Add comments to describe m_memory_addr and IsInMemory

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 created this revision.
augusto2112 added a reviewer: jasonmolenda.
Herald added a project: All.
augusto2112 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129319

Files:
  lldb/include/lldb/Symbol/ObjectFile.h


Index: lldb/include/lldb/Symbol/ObjectFile.h
===
--- lldb/include/lldb/Symbol/ObjectFile.h
+++ lldb/include/lldb/Symbol/ObjectFile.h
@@ -673,6 +673,7 @@
   virtual size_t ReadSectionData(Section *section,
  DataExtractor §ion_data);
 
+  /// Returns true if the object file exists only in memory.
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 
   // Strip linker annotations (such as @@VERSION) from symbol names.
@@ -736,6 +737,7 @@
   DataExtractor
   m_data; ///< The data for this object file so things can be parsed 
lazily.
   lldb::ProcessWP m_process_wp;
+  /// Set if the object file only exists in memory.
   const lldb::addr_t m_memory_addr;
   std::unique_ptr m_sections_up;
   std::unique_ptr m_symtab_up;


Index: lldb/include/lldb/Symbol/ObjectFile.h
===
--- lldb/include/lldb/Symbol/ObjectFile.h
+++ lldb/include/lldb/Symbol/ObjectFile.h
@@ -673,6 +673,7 @@
   virtual size_t ReadSectionData(Section *section,
  DataExtractor §ion_data);
 
+  /// Returns true if the object file exists only in memory.
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 
   // Strip linker annotations (such as @@VERSION) from symbol names.
@@ -736,6 +737,7 @@
   DataExtractor
   m_data; ///< The data for this object file so things can be parsed lazily.
   lldb::ProcessWP m_process_wp;
+  /// Set if the object file only exists in memory.
   const lldb::addr_t m_memory_addr;
   std::unique_ptr m_sections_up;
   std::unique_ptr m_symtab_up;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D129319: [lldb] Add comments to describe m_memory_addr and IsInMemory

[Jason Molenda via Phabricator via lldb-commits]

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

LGTM, this isn't clear right now reading the source.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129319

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

[Lldb-commits] [lldb] 65cac0e - Use StringRef to avoid unnecessary copies into std::strings

[David Blaikie via lldb-commits]

Author: David Blaikie
Date: 2022-07-07T19:50:12Z
New Revision: 65cac0ed9266e3551663358de677161ce25a25bf

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

LOG: Use StringRef to avoid unnecessary copies into std::strings

Added: 


Modified: 
lldb/include/lldb/Symbol/TypeList.h
lldb/include/lldb/Symbol/TypeMap.h
lldb/source/Core/Module.cpp
lldb/source/Symbol/TypeList.cpp
lldb/source/Symbol/TypeMap.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/TypeList.h 
b/lldb/include/lldb/Symbol/TypeList.h
index 03390858025b..403469c989f5 100644
--- a/lldb/include/lldb/Symbol/TypeList.h
+++ b/lldb/include/lldb/Symbol/TypeList.h
@@ -49,10 +49,11 @@ class TypeList {
 
   void ForEach(std::function const &callback);
 
-  void RemoveMismatchedTypes(const char *qualified_typename, bool exact_match);
+  void RemoveMismatchedTypes(llvm::StringRef qualified_typename,
+ bool exact_match);
 
-  void RemoveMismatchedTypes(const std::string &type_scope,
- const std::string &type_basename,
+  void RemoveMismatchedTypes(llvm::StringRef type_scope,
+ llvm::StringRef type_basename,
  lldb::TypeClass type_class, bool exact_match);
 
   void RemoveMismatchedTypes(lldb::TypeClass type_class);

diff  --git a/lldb/include/lldb/Symbol/TypeMap.h 
b/lldb/include/lldb/Symbol/TypeMap.h
index ede54c1a09d4..064e2cc948b6 100644
--- a/lldb/include/lldb/Symbol/TypeMap.h
+++ b/lldb/include/lldb/Symbol/TypeMap.h
@@ -53,10 +53,11 @@ class TypeMap {
 
   bool Remove(const lldb::TypeSP &type_sp);
 
-  void RemoveMismatchedTypes(const char *qualified_typename, bool exact_match);
+  void RemoveMismatchedTypes(llvm::StringRef qualified_typename,
+ bool exact_match);
 
-  void RemoveMismatchedTypes(const std::string &type_scope,
- const std::string &type_basename,
+  void RemoveMismatchedTypes(llvm::StringRef type_scope,
+ llvm::StringRef type_basename,
  lldb::TypeClass type_class, bool exact_match);
 
   void RemoveMismatchedTypes(lldb::TypeClass type_class);

diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 41c21e1dc326..c05b40c4362e 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1000,8 +1000,7 @@ void Module::FindTypes(
 FindTypes_Impl(type_basename_const_str, CompilerDeclContext(), max_matches,
searched_symbol_files, typesmap);
 if (typesmap.GetSize())
-  typesmap.RemoveMismatchedTypes(std::string(type_scope),
- std::string(type_basename), type_class,
+  typesmap.RemoveMismatchedTypes(type_scope, type_basename, type_class,
  exact_match);
   } else {
 // The type is not in a namespace/class scope, just search for it by
@@ -1011,15 +1010,13 @@ void Module::FindTypes(
   // class prefix (like "struct", "class", "union", "typedef" etc).
   FindTypes_Impl(ConstString(type_basename), CompilerDeclContext(),
  UINT_MAX, searched_symbol_files, typesmap);
-  typesmap.RemoveMismatchedTypes(std::string(type_scope),
- std::string(type_basename), type_class,
+  typesmap.RemoveMismatchedTypes(type_scope, type_basename, type_class,
  exact_match);
 } else {
   FindTypes_Impl(name, CompilerDeclContext(), UINT_MAX,
  searched_symbol_files, typesmap);
   if (exact_match) {
-std::string name_str(name.AsCString(""));
-typesmap.RemoveMismatchedTypes(std::string(type_scope), name_str,
+typesmap.RemoveMismatchedTypes(type_scope, name.AsCString(""),
type_class, exact_match);
   }
 }

diff  --git a/lldb/source/Symbol/TypeList.cpp b/lldb/source/Symbol/TypeList.cpp
index ace715d933ea..494e59e3a0fc 100644
--- a/lldb/source/Symbol/TypeList.cpp
+++ b/lldb/source/Symbol/TypeList.cpp
@@ -97,7 +97,7 @@ void TypeList::Dump(Stream *s, bool show_context) {
   }
 }
 
-void TypeList::RemoveMismatchedTypes(const char *qualified_typename,
+void TypeList::RemoveMismatchedTypes(llvm::StringRef qualified_typename,
  bool exact_match) {
   llvm::StringRef type_scope;
   llvm::StringRef type_basename;
@@ -107,13 +107,12 @@ void TypeList::RemoveMismatchedTypes(const char 
*qualified_typename,
 type_basename = qualified_typename;
 type_scope = "";
   }
-  return RemoveMismatchedTypes(std::string(type_scope),
-   std::string(type_basename), type_class,
+  return RemoveMismatchedTy

[Lldb-commits] [PATCH] D129261: [lldb/test] Add Shell/Expr/TestStringLiteralExpr.test

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM, assuming this isn't already covered (I couldn't immediately find a test 
that covers string literals)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129261

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

[Lldb-commits] [PATCH] D129319: [lldb] Add comments to describe m_memory_addr and IsInMemory

[Augusto Noronha via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ade38c28573: [lldb] Add comments to describe m_memory_addr 
and IsInMemory (authored by augusto2112).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129319

Files:
  lldb/include/lldb/Symbol/ObjectFile.h


Index: lldb/include/lldb/Symbol/ObjectFile.h
===
--- lldb/include/lldb/Symbol/ObjectFile.h
+++ lldb/include/lldb/Symbol/ObjectFile.h
@@ -673,6 +673,7 @@
   virtual size_t ReadSectionData(Section *section,
  DataExtractor §ion_data);
 
+  /// Returns true if the object file exists only in memory.
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 
   // Strip linker annotations (such as @@VERSION) from symbol names.
@@ -736,6 +737,7 @@
   DataExtractor
   m_data; ///< The data for this object file so things can be parsed 
lazily.
   lldb::ProcessWP m_process_wp;
+  /// Set if the object file only exists in memory.
   const lldb::addr_t m_memory_addr;
   std::unique_ptr m_sections_up;
   std::unique_ptr m_symtab_up;


Index: lldb/include/lldb/Symbol/ObjectFile.h
===
--- lldb/include/lldb/Symbol/ObjectFile.h
+++ lldb/include/lldb/Symbol/ObjectFile.h
@@ -673,6 +673,7 @@
   virtual size_t ReadSectionData(Section *section,
  DataExtractor §ion_data);
 
+  /// Returns true if the object file exists only in memory.
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 
   // Strip linker annotations (such as @@VERSION) from symbol names.
@@ -736,6 +737,7 @@
   DataExtractor
   m_data; ///< The data for this object file so things can be parsed lazily.
   lldb::ProcessWP m_process_wp;
+  /// Set if the object file only exists in memory.
   const lldb::addr_t m_memory_addr;
   std::unique_ptr m_sections_up;
   std::unique_ptr m_symtab_up;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5ade38c - [lldb] Add comments to describe m_memory_addr and IsInMemory

[Augusto Noronha via lldb-commits]

Author: Augusto Noronha
Date: 2022-07-07T13:11:50-07:00
New Revision: 5ade38c28573b92b8b0bfd1fe7feef2fbea76ddf

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

LOG: [lldb] Add comments to describe m_memory_addr and IsInMemory

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

Added: 


Modified: 
lldb/include/lldb/Symbol/ObjectFile.h

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index c61e3c138944..e51d50592c90 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -673,6 +673,7 @@ class ObjectFile : public 
std::enable_shared_from_this,
   virtual size_t ReadSectionData(Section *section,
  DataExtractor §ion_data);
 
+  /// Returns true if the object file exists only in memory.
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 
   // Strip linker annotations (such as @@VERSION) from symbol names.
@@ -736,6 +737,7 @@ class ObjectFile : public 
std::enable_shared_from_this,
   DataExtractor
   m_data; ///< The data for this object file so things can be parsed 
lazily.
   lldb::ProcessWP m_process_wp;
+  /// Set if the object file only exists in memory.
   const lldb::addr_t m_memory_addr;
   std::unique_ptr m_sections_up;
   std::unique_ptr m_symtab_up;



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

[Lldb-commits] [lldb] 856ebe9 - Retrieve as StringRef since that's how it'll be used

[David Blaikie via lldb-commits]

Author: David Blaikie
Date: 2022-07-07T20:13:36Z
New Revision: 856ebe9e8247698095a66f98647ee5d7cb12f337

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

LOG: Retrieve as StringRef since that's how it'll be used

Added: 


Modified: 
lldb/source/Core/Module.cpp

Removed: 




diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index c05b40c4362e..43d1cdb5bd38 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1016,7 +1016,7 @@ void Module::FindTypes(
   FindTypes_Impl(name, CompilerDeclContext(), UINT_MAX,
  searched_symbol_files, typesmap);
   if (exact_match) {
-typesmap.RemoveMismatchedTypes(type_scope, name.AsCString(""),
+typesmap.RemoveMismatchedTypes(type_scope, name.GetStringRef(),
type_class, exact_match);
   }
 }



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

[Lldb-commits] [lldb] 6edbde1 - Simplify some AsCString usage that was also explicitly handling default

[David Blaikie via lldb-commits]

Author: David Blaikie
Date: 2022-07-07T20:27:05Z
New Revision: 6edbde100132f5dc025bed64059d9fb770abd19e

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

LOG: Simplify some AsCString usage that was also explicitly handling default

Added: 


Modified: 
lldb/source/Core/Module.cpp

Removed: 




diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 43d1cdb5bd38..893e20837124 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -144,9 +144,7 @@ Module::Module(const ModuleSpec &module_spec)
   module_spec.GetArchitecture().GetArchitectureName(),
   module_spec.GetFileSpec().GetPath().c_str(),
   module_spec.GetObjectName().IsEmpty() ? "" : "(",
-  module_spec.GetObjectName().IsEmpty()
-  ? ""
-  : module_spec.GetObjectName().AsCString(""),
+  module_spec.GetObjectName().AsCString(""),
   module_spec.GetObjectName().IsEmpty() ? "" : ")");
 
   auto data_sp = module_spec.GetData();
@@ -254,8 +252,7 @@ Module::Module(const FileSpec &file_spec, const ArchSpec 
&arch,
 LLDB_LOGF(log, "%p Module::Module((%s) '%s%s%s%s')",
   static_cast(this), m_arch.GetArchitectureName(),
   m_file.GetPath().c_str(), m_object_name.IsEmpty() ? "" : "(",
-  m_object_name.IsEmpty() ? "" : m_object_name.AsCString(""),
-  m_object_name.IsEmpty() ? "" : ")");
+  m_object_name.AsCString(""), m_object_name.IsEmpty() ? "" : ")");
 }
 
 Module::Module() : m_file_has_changed(false), m_first_file_changed_log(false) {
@@ -283,8 +280,7 @@ Module::~Module() {
 LLDB_LOGF(log, "%p Module::~Module((%s) '%s%s%s%s')",
   static_cast(this), m_arch.GetArchitectureName(),
   m_file.GetPath().c_str(), m_object_name.IsEmpty() ? "" : "(",
-  m_object_name.IsEmpty() ? "" : m_object_name.AsCString(""),
-  m_object_name.IsEmpty() ? "" : ")");
+  m_object_name.AsCString(""), m_object_name.IsEmpty() ? "" : ")");
   // Release any auto pointers before we start tearing down our member
   // variables since the object file and symbol files might need to make
   // function calls back into this module object. The ordering is important



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

[Lldb-commits] [PATCH] D129327: [lldb] Add support for using integral const static data members in the expression evaluator

[Andy Yankovsky via Phabricator via lldb-commits]

werat created this revision.
Herald added a subscriber: mgorny.
Herald added a reviewer: shafik.
Herald added a project: All.
werat requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This adds support for using const static integral data members as described by 
C++11 [class.static.data]p3
to LLDB's expression evaluator.

So far LLDB treated these data members are normal static variables. They 
already work as intended when they are declared in the class definition and 
then defined in a namespace scope. However, if they are declared and 
initialised in the class definition but never defined in a namespace scope, all 
LLDB expressions that use them will fail to link when LLDB can't find the 
respective symbol for the variable.

The reason for this is that the data members which are only declared in the 
class are not emitted into any object file so LLDB can never resolve them. 
Expressions that use these variables are expected to directly use their 
constant value if possible. Clang can do this for us during codegen, but it 
requires that we add the constant value to the VarDecl we generate for these 
data members.

This patch implements this by:

- parsing the constant values from the debug info and adding it to variable 
declarations we encounter.
- ensuring that LLDB doesn't implicitly try to take the address of expressions 
that might be an lvalue that points to such a special data member.

The second change is caused by LLDB's way of storing lvalues in the expression 
parser. When LLDB parses an expression, it tries to keep the result around via 
two mechanisms:

1. For lvalues, LLDB generates a static pointer variable and stores the address 
of the last expression in it: `T *$__lldb_expr_result_ptr = &LastExpression`
2. For everything else, LLDB generates a static variable of the same type as 
the last expression and then direct initialises that variable: `T 
$__lldb_expr_result(LastExpression)`

If we try to print a special const static data member via something like `expr 
Class::Member`, then LLDB will try to take the address of this expression as 
it's an lvalue. This means LLDB will try to take the address of the variable 
which causes that Clang can't replace the use with the constant value. There 
isn't any good way to detect this case (as there a lot of different expressions 
that could yield an lvalue that points to such a data member), so this patch 
also changes that we only use the first way of capturing the result if the last 
expression does not have a type that could potentially indicate it's coming 
from such a special data member.

This change shouldn't break most workflows for users. The only observable side 
effect I could find is that the implicit persistent result variables for const 
int's now have their own memory address:

Before this change:

  (lldb) p i
  (const int) $0 = 123
  (lldb) p &$0
  (const int *) $1 = 0x7ffeefbff8e8
  (lldb) p &i
  (const int *) $2 = 0x7ffeefbff8e8

After this change we capture `i` by value so it has its own value.

  (lldb) p i
  (const int) $0 = 123
  (lldb) p &$0
  (const int *) $1 = 0x000100155320
  (lldb) p &i
  (const int *) $2 = 0x7ffeefbff8e8

Co-authored-by: Raphael Isemann 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129327

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/lang/cpp/const_static_integral_member/Makefile
  
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
  lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
  lldb/test/API/lang/cpp/const_static_integral_member_int128/Makefile
  
lldb/test/API/lang/cpp/const_static_integral_member_int128/TestConstStaticIntegralMemberInt128.py
  lldb/test/API/lang/cpp/const_static_integral_member_int128/main.cpp
  lldb/unittests/SymbolFile/DWARF/CMakeLists.txt
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
@@ -273,3 +273,128 @@
   };
   ASSERT_EQ(found_function_types, expected_function_types);
 }
+
+struct ExtractIntFromFormValueTest : public testing::Test {
+  SubsystemRAII subsystems;
+  TypeSystemClang ts;
+  DWARFASTParserClang parser;
+  ExtractIntFromFormValueTest()
+  : ts("dummy ASTContext", HostInfoBase::GetTargetTriple()), parser(ts) {}
+
+  /// Takes the given integer value, stores it in a DWARFFormValue and then
+  /// tries to extract the value back via
+  /// DWARFASTParserClang::ExtractIntFromFormValue.
+  /// Returns the string representation of the extracted value or the error
+  /// that w

[Lldb-commits] [PATCH] D81471: [lldb] Add support for using integral const static data members in the expression evaluator

[Andy Yankovsky via Phabricator via lldb-commits]

werat marked 6 inline comments as done.
werat added a comment.

In D81471#3632297 , @labath wrote:

> I guess the condition we really want to express here is "does this expression 
> refer to a constexpr variable (ideally one without a location)"? And the 
> problem is that clang does not give us the means to detect that?
>
> Is that really the case? Would it maybe be possible to use some c++ magic to 
> get clang to do that for us. Write something like `if constexpr 
> (__builtin_constant_p(user_expression)) do_something_rvalue_like(); else 
> assume_regular_lvalue();` ?

I think you're right here, but I don't know a better way to express this. My 
clangfu is not good enough for this yet :)

As I understand it, we can't express it purely in the expression (via 
`__builtin_constant_p` or something), because we need to know the answer before 
executing the expression (it changes the way we do 
materialization/dematerialization).

Could this be something to improve in the future?




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2689
+  // TODO: Support float/double static members as well.
+  if (!attrs.const_value_form && !ct.IsIntegerOrEnumerationType(unused))
+return;

Michael137 wrote:
> Should this be:
> 
> ```
> if (!attrs.const_value_form || !ct.IsIntegerOrEnumerationType(unused))
> ```
> ?
Whoops, yes, thanks!



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2696
+   "Failed to add const value to variable {1}: {0}",
+   v->getQualifiedNameAsString());
+return;

Michael137 wrote:
> Can `v` be `nullptr` here?
Honestly, not sure. Theoretically `AddVariableToRecordType` can return 
`nullptr`, but I've never seen such case. Maybe it only happens when the debug 
info is malformed? Would that be caught earlier at some stage?
I can add a safe guard here with a log message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81471

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

[Lldb-commits] [PATCH] D81471: [lldb] Add support for using integral const static data members in the expression evaluator

[Andy Yankovsky via Phabricator via lldb-commits]

werat updated this revision to Diff 443051.
werat marked 2 inline comments as done.
werat added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81471

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/lang/cpp/const_static_integral_member/Makefile
  
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
  lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
  lldb/test/API/lang/cpp/const_static_integral_member_int128/Makefile
  
lldb/test/API/lang/cpp/const_static_integral_member_int128/TestConstStaticIntegralMemberInt128.py
  lldb/test/API/lang/cpp/const_static_integral_member_int128/main.cpp
  lldb/unittests/SymbolFile/DWARF/CMakeLists.txt
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
@@ -273,3 +273,128 @@
   };
   ASSERT_EQ(found_function_types, expected_function_types);
 }
+
+struct ExtractIntFromFormValueTest : public testing::Test {
+  SubsystemRAII subsystems;
+  TypeSystemClang ts;
+  DWARFASTParserClang parser;
+  ExtractIntFromFormValueTest()
+  : ts("dummy ASTContext", HostInfoBase::GetTargetTriple()), parser(ts) {}
+
+  /// Takes the given integer value, stores it in a DWARFFormValue and then
+  /// tries to extract the value back via
+  /// DWARFASTParserClang::ExtractIntFromFormValue.
+  /// Returns the string representation of the extracted value or the error
+  /// that was returned from ExtractIntFromFormValue.
+  llvm::Expected Extract(clang::QualType qt, uint64_t value) {
+DWARFFormValue form_value;
+form_value.SetUnsigned(value);
+llvm::Expected result =
+parser.ExtractIntFromFormValue(ts.GetType(qt), form_value);
+if (!result)
+  return result.takeError();
+llvm::SmallString<16> result_str;
+result->toStringUnsigned(result_str);
+return std::string(result_str.str());
+  }
+
+  /// Same as ExtractIntFromFormValueTest::Extract but takes a signed integer
+  /// and treats the result as a signed integer.
+  llvm::Expected ExtractS(clang::QualType qt, int64_t value) {
+DWARFFormValue form_value;
+form_value.SetSigned(value);
+llvm::Expected result =
+parser.ExtractIntFromFormValue(ts.GetType(qt), form_value);
+if (!result)
+  return result.takeError();
+llvm::SmallString<16> result_str;
+result->toStringSigned(result_str);
+return std::string(result_str.str());
+  }
+};
+
+TEST_F(ExtractIntFromFormValueTest, TestBool) {
+  using namespace llvm;
+  clang::ASTContext &ast = ts.getASTContext();
+
+  EXPECT_THAT_EXPECTED(Extract(ast.BoolTy, 0), HasValue("0"));
+  EXPECT_THAT_EXPECTED(Extract(ast.BoolTy, 1), HasValue("1"));
+  EXPECT_THAT_EXPECTED(Extract(ast.BoolTy, 2), Failed());
+  EXPECT_THAT_EXPECTED(Extract(ast.BoolTy, 3), Failed());
+}
+
+TEST_F(ExtractIntFromFormValueTest, TestInt) {
+  using namespace llvm;
+
+  clang::ASTContext &ast = ts.getASTContext();
+
+  // Find the min/max values for 'int' on the current host target.
+  constexpr int64_t int_max = std::numeric_limits::max();
+  constexpr int64_t int_min = std::numeric_limits::min();
+
+  // Check that the bit width of int matches the int width in our type system.
+  ASSERT_EQ(sizeof(int) * 8, ast.getIntWidth(ast.IntTy));
+
+  // Check values around int_min.
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_min - 2), llvm::Failed());
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_min - 1), llvm::Failed());
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_min),
+   HasValue(std::to_string(int_min)));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_min + 1),
+   HasValue(std::to_string(int_min + 1)));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_min + 2),
+   HasValue(std::to_string(int_min + 2)));
+
+  // Check values around 0.
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, -128), HasValue("-128"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, -10), HasValue("-10"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, -1), HasValue("-1"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, 0), HasValue("0"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, 1), HasValue("1"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, 10), HasValue("10"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, 128), HasValue("128"));
+
+  // Check values around int_max.
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_max - 2),
+   HasValue(std::to_string(int_max - 2)));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_m

[Lldb-commits] [PATCH] D81471: [lldb] Add support for using integral const static data members in the expression evaluator

[Andy Yankovsky via Phabricator via lldb-commits]

werat updated this revision to Diff 443052.
werat added a comment.

Fix condition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81471

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2690,7 +2690,7 @@
 
   bool unused;
   // TODO: Support float/double static members as well.
-  if (!attrs.const_value_form && !ct.IsIntegerOrEnumerationType(unused))
+  if (!attrs.const_value_form || !ct.IsIntegerOrEnumerationType(unused))
 return;
   llvm::Expected const_value_or_err =
   ExtractIntFromFormValue(ct, *attrs.const_value_form);


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2690,7 +2690,7 @@
 
   bool unused;
   // TODO: Support float/double static members as well.
-  if (!attrs.const_value_form && !ct.IsIntegerOrEnumerationType(unused))
+  if (!attrs.const_value_form || !ct.IsIntegerOrEnumerationType(unused))
 return;
   llvm::Expected const_value_or_err =
   ExtractIntFromFormValue(ct, *attrs.const_value_form);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D81471: [lldb] Add support for using integral const static data members in the expression evaluator

[Andy Yankovsky via Phabricator via lldb-commits]

werat updated this revision to Diff 443053.
werat added a comment.

Fix condition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81471

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/lang/cpp/const_static_integral_member/Makefile
  
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
  lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
  lldb/test/API/lang/cpp/const_static_integral_member_int128/Makefile
  
lldb/test/API/lang/cpp/const_static_integral_member_int128/TestConstStaticIntegralMemberInt128.py
  lldb/test/API/lang/cpp/const_static_integral_member_int128/main.cpp
  lldb/unittests/SymbolFile/DWARF/CMakeLists.txt
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
@@ -273,3 +273,128 @@
   };
   ASSERT_EQ(found_function_types, expected_function_types);
 }
+
+struct ExtractIntFromFormValueTest : public testing::Test {
+  SubsystemRAII subsystems;
+  TypeSystemClang ts;
+  DWARFASTParserClang parser;
+  ExtractIntFromFormValueTest()
+  : ts("dummy ASTContext", HostInfoBase::GetTargetTriple()), parser(ts) {}
+
+  /// Takes the given integer value, stores it in a DWARFFormValue and then
+  /// tries to extract the value back via
+  /// DWARFASTParserClang::ExtractIntFromFormValue.
+  /// Returns the string representation of the extracted value or the error
+  /// that was returned from ExtractIntFromFormValue.
+  llvm::Expected Extract(clang::QualType qt, uint64_t value) {
+DWARFFormValue form_value;
+form_value.SetUnsigned(value);
+llvm::Expected result =
+parser.ExtractIntFromFormValue(ts.GetType(qt), form_value);
+if (!result)
+  return result.takeError();
+llvm::SmallString<16> result_str;
+result->toStringUnsigned(result_str);
+return std::string(result_str.str());
+  }
+
+  /// Same as ExtractIntFromFormValueTest::Extract but takes a signed integer
+  /// and treats the result as a signed integer.
+  llvm::Expected ExtractS(clang::QualType qt, int64_t value) {
+DWARFFormValue form_value;
+form_value.SetSigned(value);
+llvm::Expected result =
+parser.ExtractIntFromFormValue(ts.GetType(qt), form_value);
+if (!result)
+  return result.takeError();
+llvm::SmallString<16> result_str;
+result->toStringSigned(result_str);
+return std::string(result_str.str());
+  }
+};
+
+TEST_F(ExtractIntFromFormValueTest, TestBool) {
+  using namespace llvm;
+  clang::ASTContext &ast = ts.getASTContext();
+
+  EXPECT_THAT_EXPECTED(Extract(ast.BoolTy, 0), HasValue("0"));
+  EXPECT_THAT_EXPECTED(Extract(ast.BoolTy, 1), HasValue("1"));
+  EXPECT_THAT_EXPECTED(Extract(ast.BoolTy, 2), Failed());
+  EXPECT_THAT_EXPECTED(Extract(ast.BoolTy, 3), Failed());
+}
+
+TEST_F(ExtractIntFromFormValueTest, TestInt) {
+  using namespace llvm;
+
+  clang::ASTContext &ast = ts.getASTContext();
+
+  // Find the min/max values for 'int' on the current host target.
+  constexpr int64_t int_max = std::numeric_limits::max();
+  constexpr int64_t int_min = std::numeric_limits::min();
+
+  // Check that the bit width of int matches the int width in our type system.
+  ASSERT_EQ(sizeof(int) * 8, ast.getIntWidth(ast.IntTy));
+
+  // Check values around int_min.
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_min - 2), llvm::Failed());
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_min - 1), llvm::Failed());
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_min),
+   HasValue(std::to_string(int_min)));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_min + 1),
+   HasValue(std::to_string(int_min + 1)));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_min + 2),
+   HasValue(std::to_string(int_min + 2)));
+
+  // Check values around 0.
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, -128), HasValue("-128"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, -10), HasValue("-10"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, -1), HasValue("-1"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, 0), HasValue("0"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, 1), HasValue("1"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, 10), HasValue("10"));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, 128), HasValue("128"));
+
+  // Check values around int_max.
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_max - 2),
+   HasValue(std::to_string(int_max - 2)));
+  EXPECT_THAT_EXPECTED(ExtractS(ast.IntTy, int_max - 1),
+   HasValue(std::to_

[Lldb-commits] [lldb] b92c334 - Remove dead code: TypeMap::RemoveMismatchedTypes(TypeClass type_class)

[David Blaikie via lldb-commits]

Author: David Blaikie
Date: 2022-07-07T20:46:14Z
New Revision: b92c33495aeda7d7fa7f5d3f518c59cc5785fd9f

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

LOG: Remove dead code: TypeMap::RemoveMismatchedTypes(TypeClass type_class)

Added: 


Modified: 
lldb/include/lldb/Symbol/TypeMap.h
lldb/source/Symbol/TypeMap.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/TypeMap.h 
b/lldb/include/lldb/Symbol/TypeMap.h
index 064e2cc948b6..c200ccb9844f 100644
--- a/lldb/include/lldb/Symbol/TypeMap.h
+++ b/lldb/include/lldb/Symbol/TypeMap.h
@@ -53,15 +53,10 @@ class TypeMap {
 
   bool Remove(const lldb::TypeSP &type_sp);
 
-  void RemoveMismatchedTypes(llvm::StringRef qualified_typename,
- bool exact_match);
-
   void RemoveMismatchedTypes(llvm::StringRef type_scope,
  llvm::StringRef type_basename,
  lldb::TypeClass type_class, bool exact_match);
 
-  void RemoveMismatchedTypes(lldb::TypeClass type_class);
-
 private:
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;

diff  --git a/lldb/source/Symbol/TypeMap.cpp b/lldb/source/Symbol/TypeMap.cpp
index 7217a29fc002..0d5f6d53e5a0 100644
--- a/lldb/source/Symbol/TypeMap.cpp
+++ b/lldb/source/Symbol/TypeMap.cpp
@@ -127,20 +127,6 @@ void TypeMap::Dump(Stream *s, bool show_context, 
lldb::DescriptionLevel level) {
   }
 }
 
-void TypeMap::RemoveMismatchedTypes(llvm::StringRef qualified_typename,
-bool exact_match) {
-  llvm::StringRef type_scope;
-  llvm::StringRef type_basename;
-  TypeClass type_class = eTypeClassAny;
-  if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope,
- type_basename, type_class)) {
-type_basename = qualified_typename;
-type_scope = "";
-  }
-  return RemoveMismatchedTypes(type_scope, type_basename, type_class,
-   exact_match);
-}
-
 void TypeMap::RemoveMismatchedTypes(llvm::StringRef type_scope,
 llvm::StringRef type_basename,
 TypeClass type_class, bool exact_match) {
@@ -213,25 +199,3 @@ void TypeMap::RemoveMismatchedTypes(llvm::StringRef 
type_scope,
   }
   m_types.swap(matching_types);
 }
-
-void TypeMap::RemoveMismatchedTypes(TypeClass type_class) {
-  if (type_class == eTypeClassAny)
-return;
-
-  // Our "collection" type currently is a std::map which doesn't have any good
-  // way to iterate and remove items from the map so we currently just make a
-  // new list and add all of the matching types to it, and then swap it into
-  // m_types at the end
-  collection matching_types;
-
-  iterator pos, end = m_types.end();
-
-  for (pos = m_types.begin(); pos != end; ++pos) {
-Type *the_type = pos->second.get();
-TypeClass match_type_class =
-the_type->GetForwardCompilerType().GetTypeClass();
-if (match_type_class & type_class)
-  matching_types.insert(*pos);
-  }
-  m_types.swap(matching_types);
-}



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

[Lldb-commits] [PATCH] D129332: [trace][intel pt] Support dumping the trace info in json

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: jj10306, persona0220.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Thanks to ym...@fb.com for coming up with this change.

`thread trace dump info` can dump some metrics that can be useful for
analyzing the performance and quality of a trace. This diff adds a --json
option for dumping this information in json format that can be easily
understood my machines.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129332

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.cpp
  lldb/test/API/commands/trace/TestTraceDumpInfo.py
  lldb/test/API/commands/trace/TestTraceLoad.py

Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -20,6 +20,72 @@
   substrs=["67911: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
+self.expect("thread trace dump info --json",
+  substrs=['''{
+  "traceTechnology": "intel-pt",
+  "threadStats": {
+"tid": 3497234,
+"traceItemsCount": 0,
+"memoryUsage": {
+  "totalInBytes": "0",
+  "avgPerItemInBytes": null
+},
+"timingInSeconds": {},
+"events": {
+  "totalCount": 0,
+  "individualCounts": {}
+},
+"continuousExecutions": 0,
+"PSBBlocks": 0,
+"errorItems": {
+  "total": 0,
+  "individualErrors": {}
+}
+  },
+  "globalStats": {
+"timingInSeconds": {
+  "Context switch and Intel PT traces correlation": 0
+},
+"totalUnattributedPSBBlocks": 0,
+"totalCountinuosExecutions": 153,
+"totalPSBBlocks": 5,
+"totalContinuousExecutions": 153
+  }
+}'''])
+
+self.expect("thread trace dump info 2 --json",
+  substrs=['''{
+  "traceTechnology": "intel-pt",
+  "threadStats": {
+"tid": 3497496,
+"traceItemsCount": 19523,
+"memoryUsage": {
+  "totalInBytes": "175739",
+  "avgPerItemInBytes": 9.00''', '''},
+"timingInSeconds": {},
+"events": {
+  "totalCount": 1,
+  "individualCounts": {
+"software disabled tracing": 1
+  }
+},
+"continuousExecutions": 1,
+"PSBBlocks": 1,
+"errorItems": {
+  "total": 0,
+  "individualErrors": {}
+}
+  },
+  "globalStats": {
+"timingInSeconds": {
+  "Context switch and Intel PT traces correlation": 0''', '''},
+"totalUnattributedPSBBlocks": 0,
+"totalCountinuosExecutions": 153,
+"totalPSBBlocks": 5,
+"totalContinuousExecutions": 153
+  }
+}'''])
+
 @testSBAPIAndCommands
 def testLoadMultiCoreTraceWithStringNumbers(self):
 src_dir = self.getSourceDir()
@@ -71,9 +137,10 @@
 # check that the Process and Thread objects were created correctly
 self.expect("thread info", substrs=["tid = 3842849"])
 self.expect("thread list", substrs=["Process 1234 stopped", "tid = 3842849"])
-self.expect("thread trace dump info", substrs=['''Trace technology: intel-pt
+self.expect("thread trace dump info", substrs=['''thread #1: tid = 3842849
+
+  Trace technology: intel-pt
 
-thread #1: tid = 3842849
   Total number of trace items: 23
 
   Memory usage:
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -34,9 +34,10 @@
 substrs=["intel-pt"])
 
 self.expect("thread trace dump info",
-substrs=['''Trace technology: intel-pt
+substrs=['''thread #1: tid = 3842849
+
+  Trace technology: intel-pt
 
-thread #1: tid = 3842849
   Total number of trace items: 23
 
   Memory usage:
@@ -54,3 +55,37 @@
   Errors:
 Number of TSC decoding errors: 0'''],
 patterns=["Decoding instructions: \d.\d\ds"])
+
+def testDumpRawTraceSizeJSON(self):
+self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+substrs=["intel-pt"])
+
+self.expect("thread trace dump info --json ",
+substrs=['''{
+  "traceTechnology": "intel-pt",
+  "threadStats": {
+"tid": 3842849,
+"traceItemsCount": 23,
+"memoryUsage": {
+  "totalInBytes": "207",
+  "avgPerItemInBytes": 9
+},
+"timingInSeconds": {
+  "Decoding instructions": 0''', '''
+},
+"events": {
+  "totalCount": 2,
+  "individualCounts": {
+"software disable

[Lldb-commits] [lldb] 562c346 - [LLDB] Fix aggregate-indirect-arg.cpp failure introduced by 227dffd0b6d78154516ace45f6ed28259c7baa48

[Zequan Wu via lldb-commits]

Author: Zequan Wu
Date: 2022-07-07T15:01:07-07:00
New Revision: 562c3467a6738aa89203f72fc1d1343e5baadf3c

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

LOG: [LLDB] Fix aggregate-indirect-arg.cpp failure introduced by 
227dffd0b6d78154516ace45f6ed28259c7baa48

Added: 


Modified: 
lldb/source/Expression/DWARFExpressionList.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpressionList.cpp 
b/lldb/source/Expression/DWARFExpressionList.cpp
index e13a2931644b..68e3e8c611b9 100644
--- a/lldb/source/Expression/DWARFExpressionList.cpp
+++ b/lldb/source/Expression/DWARFExpressionList.cpp
@@ -204,8 +204,11 @@ bool DWARFExpressionList::Evaluate(ExecutionContext 
*exe_ctx,
 }
 addr_t addr = pc.GetFileAddress();
 const auto *entry = m_exprs.FindEntryThatContains(addr);
-if (!entry)
+if (!entry) {
+  if (error_ptr)
+error_ptr->SetErrorString("variable not available");
   return false;
+}
 expr = entry->data;
   }
   expr.GetExpressionData(data);



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

[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D129307#3636695 , @clayborg wrote:

> In D129307#3636573 , @jingham wrote:
>
>> I'm not entirely clear what problem this is solving.  Any actor setting 
>> breakpoints can already choose the match depth by simply providing that many 
>> directory components.  I.e. if I know I have subdir1/foo.cpp and 
>> subdir2/foo.cpp I can set a breakpoint on only one of them by doing:
>>
>>   (lldb) break set -f subdir1/foo.cpp -l 10
>>
>> So an IDE could implement this by simply passing either the base name or the 
>> base name plus one sub directory, etc.  As you say, that's what command line 
>> users do anyway, they seldom type out full paths.  So this is really about 
>> IDE's running lldb, and this seems more like a feature the IDE should offer, 
>> not lldb.
>
> Xcode has this same issue if you download a dSYM file from the Apple build 
> infrastructure if that dSYM doesn't contain path re-mappings in the UUID 
> plists. Xcode, the IDE, will send down full paths to the source files that it 
> knows about if you set breakpoints. Granted Xcode could work around this, but 
> we added the ability to remap things at the module level in LLDB to work 
> around this issue. So yes, it would be great if all IDEs would do this, but 
> currently none do.
>
>> It also seems awkward to do all this work as part of the breakpoint 
>> filtering - which as you have seen is somewhat delicate code.  After all, 
>> you can implement "only match the last N components" by only submitting the 
>> last N components when looking up the files.  So it would be much more 
>> straightforward to strip the leading path components from the file name when 
>> you create the breakpoint and you don't have to muck with the filtering at 
>> all.  That would also help remove some confusion when you see that the path 
>> submitted was /foo/bar/baz.cpp but I matched /some/other/bar/baz.cpp (but 
>> not /some/other/different/baz.cpp) because I had set the match suffix count 
>> to 1.  It would be nice to have break list show me exactly what it was going 
>> to match on.
>
> We might need to always store the path that was given in source file and line 
> breakpoints and then have the breakpoint update its locations when/if either 
> of these two settings are modified. This would also help with your example 
> below where you comment on breakpoint settings depend on the history of the 
> session (they shouldn't).

I took that bit out because in fact the implementation stores the depth in the 
resolver when made.  So any given breakpoint isn't history dependent, but 
rather to understand why some breakpoints take and some don't you have to know 
the history of your settings in the session - which is not recorded anywhere 
you can get your hands on.  That's a source of confusion for sure but  
probably not a huge one.

>> I am also not sure doing this as a general setting for values other than 0 
>> is really helpful.  So far as I can tell, you would use a value of 1 because 
>> you know that though you might have files with duplicate names they are 
>> always disambiguated by their parent directory name.
>
> Exactly the case I was worried about. Usually zero would be the default IMHO, 
> but if you have two different binaries that both have a "main.cpp" it could 
> help. We can remove the depth and have this setting only set breakpoints by 
> basename if needed if no one wants the extra complexity.

Certainly just doing base name will find all the matches regardless of what the 
debug info says about the path, and the only problem is extra matches from 
files with the same name in different directories.  That's only slightly 
annoying and not a fatal problem because you can always disable locations you 
don't want, though it would be good to have some way to handle this.  But it 
seems to me to disambiguate effectively, you really have to know something 
about the structure  of your project.  You could probably get away with 1 
directory as a global setting safely since most of time the sources move 
rigidly and the source root will be above everything.  But anything above that 
will require you know that all the source files are that many levels below the 
source root or your setting will throw out some matches when the source root is 
moved.   So it seems to me any higher setting is going to be too fiddly to be 
worth the effort.

>> Using a value of 2 says the names are disambiguated by the two parent 
>> directory names.  But there's no guarantee that one value is going to work 
>> for all the files in your project and having to change a global setting from 
>> breakpoint to breakpoint is awkward.
>>
>> I also don't like that this makes breakpoint settings depend on the history 
>> of the session.  For instance:
>>
>> a) Set the partial match to on and the depth to 2
>> b) Set a file & line breakpoint
>> c) Change the partial match depth

[Lldb-commits] [PATCH] D128956: make debugserver able to inspect mach-o binaries present in memory, but not yet registered with dyld

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py:46
+self.expect (both_gdb_packet, substrs=['response: 
{"images":[{"load_address":%d,' % macho_addr])
+self.expect (both_gdb_packet, substrs=['response: {"load_address":%d,' 
% invalid_macho_addr], matching=False)
+

DavidSpickett wrote:
> For these would it be more reliable to just look for the `load_address:` bits?
> 
> If the packet contains multiple `response`, then fine but if it's one 
> response then the second one wouldn't ever match even if debugserver did 
> somehow find the one at the invalid address. Also if it did would it put it 
> in a `images` section and therefore not match because of that too?
Yeah, good suggestion.  I wrote the two tests in the order they're in, and 
realized I couldn't match the `images` key in the second one; didn't go back 
and make the first one consistent.



Comment at: lldb/test/API/macosx/unregistered-macho/main.c:48
+  memcpy(p, &uuid, sizeof(uuid));
+  p += sizeof(uuid);
+

DavidSpickett wrote:
> This is redundant (the test uses `macho_buf`).
I needed to increment `p` so it points to the end of the mach-o image if I want 
to write it to disk and try sending it through `otool` for testing.  You're 
right though, this has no effect to the program itself.



Comment at: lldb/test/API/macosx/unregistered-macho/main.c:33
+  seg.vmsize = 0x1000;
+  seg.fileoff = 0;
+  seg.filesize = 0;

DavidSpickett wrote:
> Does this need to be `sizeof (struct mach_header_64)`? Probably doesn't 
> and/or it doesn't matter for this test.
I don't think it matters.  I'm saying that this segment doesn't exist in the 
file.  Quick check, I created a hello world program with a single BSS int, so 
it doesn't exist in the binary but is initialized to zero at runtime. 
```
Load command 2
  cmd LC_SEGMENT_64
  cmdsize 152
  segname __DATA
   vmaddr 0x00014000
   vmsize 0x4000
  fileoff 0
 filesize 0
```



Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:768
+return true;
+  };
+

DavidSpickett wrote:
> Personally I'd put this as a static function just before this one as it 
> doesn't capture anything, but up to you.
Yeah good point.  It's operating on either a mach_header or mach_header_64 so 
there wasn't a single variable to capture and test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128956

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

[Lldb-commits] [PATCH] D128956: make debugserver able to inspect mach-o binaries present in memory, but not yet registered with dyld

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 443083.
jasonmolenda added a comment.

One more patch update to address David's further feedback.  This is ready to 
land now, will give it a day in case anyone has comments or suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128956

Files:
  lldb/test/API/macosx/unregistered-macho/Makefile
  lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py
  lldb/test/API/macosx/unregistered-macho/main.c
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm

Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -746,6 +746,16 @@
   return nullptr;
 }
 
+static bool mach_header_validity_test (uint32_t magic, uint32_t cputype) {
+  if (magic != MH_MAGIC && magic != MH_CIGAM && magic != MH_MAGIC_64 &&
+  magic != MH_CIGAM_64)
+return false;
+  if (cputype != CPU_TYPE_X86_64 && cputype != CPU_TYPE_ARM &&
+  cputype != CPU_TYPE_ARM64 && cputype != CPU_TYPE_ARM64_32)
+return false;
+  return true;
+}
+
 // Given an address, read the mach-o header and load commands out of memory to
 // fill in
 // the mach_o_information "inf" object.
@@ -757,12 +767,16 @@
 uint32_t dyld_platform, nub_addr_t mach_o_header_addr, int wordsize,
 struct mach_o_information &inf) {
   uint64_t load_cmds_p;
+
   if (wordsize == 4) {
 struct mach_header header;
 if (ReadMemory(mach_o_header_addr, sizeof(struct mach_header), &header) !=
 sizeof(struct mach_header)) {
   return false;
 }
+if (!mach_header_validity_test(header.magic, header.cputype))
+  return false;
+
 load_cmds_p = mach_o_header_addr + sizeof(struct mach_header);
 inf.mach_header.magic = header.magic;
 inf.mach_header.cputype = header.cputype;
@@ -779,6 +793,8 @@
&header) != sizeof(struct mach_header_64)) {
   return false;
 }
+if (!mach_header_validity_test(header.magic, header.cputype))
+  return false;
 load_cmds_p = mach_o_header_addr + sizeof(struct mach_header_64);
 inf.mach_header.magic = header.magic;
 inf.mach_header.cputype = header.cputype;
@@ -896,6 +912,8 @@
   const size_t image_count = image_infos.size();
 
   for (size_t i = 0; i < image_count; i++) {
+if (!image_infos[i].is_valid)
+  continue;
 JSONGenerator::DictionarySP image_info_dict_sp(
 new JSONGenerator::Dictionary());
 image_info_dict_sp->AddIntegerItem("load_address",
@@ -970,7 +988,6 @@
   }
 
   JSONGenerator::DictionarySP reply_sp(new JSONGenerator::Dictionary());
-  ;
   reply_sp->AddItem("images", image_infos_array_sp);
 
   return reply_sp;
@@ -985,8 +1002,8 @@
 // information.
 JSONGenerator::ObjectSP MachProcess::GetLoadedDynamicLibrariesInfos(
 nub_process_t pid, nub_addr_t image_list_address, nub_addr_t image_count) {
-  JSONGenerator::DictionarySP reply_sp;
 
+  JSONGenerator::ObjectSP empty_reply_sp(new JSONGenerator::Dictionary());
   int pointer_size = GetInferiorAddrSize(pid);
 
   std::vector image_infos;
@@ -994,91 +1011,89 @@
 
   uint8_t *image_info_buf = (uint8_t *)malloc(image_infos_size);
   if (image_info_buf == NULL) {
-return reply_sp;
+return empty_reply_sp;
+  }
+  if (ReadMemory(image_list_address, image_infos_size, image_info_buf) !=
+  image_infos_size) {
+return empty_reply_sp;
   }
-if (ReadMemory(image_list_address, image_infos_size, image_info_buf) !=
-image_infos_size) {
-  return reply_sp;
-}
 
-  First the image_infos array with (load addr, pathname, mod date)
-///tuples
-
-for (size_t i = 0; i < image_count; i++) {
-  struct binary_image_information info;
-  nub_addr_t pathname_address;
-  if (pointer_size == 4) {
-uint32_t load_address_32;
-uint32_t pathname_address_32;
-uint32_t mod_date_32;
-::memcpy(&load_address_32, image_info_buf + (i * 3 * pointer_size), 4);
-::memcpy(&pathname_address_32,
- image_info_buf + (i * 3 * pointer_size) + pointer_size, 4);
-::memcpy(&mod_date_32, image_info_buf + (i * 3 * pointer_size) +
-   pointer_size + pointer_size,
- 4);
-info.load_address = load_address_32;
-info.mod_date = mod_date_32;
-pathname_address = pathname_address_32;
-  } else {
-uint64_t load_address_64;
-uint64_t pathname_address_64;
-uint64_t mod_date_64;
-::memcpy(&load_address_64, image_info_buf + (i * 3 * pointer_size), 8);
-::memcpy(&pathname_address_64,
- image_info_buf + (i * 3 * pointer_size) + pointer_size, 8);
-::memcpy(&mod_date_64, image_info_buf + (i * 3 * pointer_si

[Lldb-commits] [lldb] e4c5bca - Revert "[LLDB][NFC] Decouple dwarf location table from DWARFExpression."

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-07-07T16:36:10-07:00
New Revision: e4c5bca597a6f12e8f789a53e862387b9808ff48

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

LOG: Revert "[LLDB][NFC] Decouple dwarf location table from DWARFExpression."

This reverts commit 227dffd0b6d78154516ace45f6ed28259c7baa48 and its
follow up 562c3467a6738aa89203f72fc1d1343e5baadf3c because it breaks a
bunch of tests on GreenDragon:

https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45155/

Added: 


Modified: 
lldb/include/lldb/Expression/DWARFExpression.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/Variable.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Utility/RangeMap.h
lldb/include/lldb/lldb-forward.h
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/Expression/CMakeLists.txt
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Expression/Materializer.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Symbol/Variable.cpp
lldb/source/Target/RegisterContextUnwind.cpp
lldb/source/Target/StackFrame.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn

Removed: 
lldb/include/lldb/Expression/DWARFExpressionList.h
lldb/source/Expression/DWARFExpressionList.cpp



diff  --git a/lldb/include/lldb/Expression/DWARFExpression.h 
b/lldb/include/lldb/Expression/DWARFExpression.h
index 49e51d51f211c..96a0e8e02da13 100644
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -42,14 +42,49 @@ class DWARFExpression {
   /// \param[in] data
   /// A data extractor configured to read the DWARF location expression's
   /// bytecode.
-  DWARFExpression(const DataExtractor &data);
+  DWARFExpression(lldb::ModuleSP module, const DataExtractor &data,
+  const DWARFUnit *dwarf_cu);
 
   /// Destructor
   virtual ~DWARFExpression();
 
+  /// Print the description of the expression to a stream
+  ///
+  /// \param[in] s
+  /// The stream to print to.
+  ///
+  /// \param[in] level
+  /// The level of verbosity to use.
+  ///
+  /// \param[in] abi
+  /// An optional ABI plug-in that can be used to resolve register
+  /// names.
+  void GetDescription(Stream *s, lldb::DescriptionLevel level, ABI *abi) const;
+
   /// Return true if the location expression contains data
   bool IsValid() const;
 
+  /// Return true if a location list was provided
+  bool IsLocationList() const;
+
+  /// Search for a load address in the location list
+  ///
+  /// \param[in] func_load_addr
+  /// The actual address of the function containing this location list.
+  ///
+  /// \param[in] addr
+  /// The address to resolve
+  ///
+  /// \return
+  /// True if IsLocationList() is true and the address was found;
+  /// false otherwise.
+  //bool
+  //LocationListContainsLoadAddress (Process* process, const Address &addr)
+  //const;
+  //
+  bool LocationListContainsAddress(lldb::addr_t func_load_addr,
+   lldb::addr_t addr) const;
+
   /// If a location is not a location list, return true if the location
   /// contains a DW_OP_addr () opcode in the stream that matches \a file_addr.
   /// If file_addr is LLDB_INVALID_ADDRESS, the this function will return true
@@ -58,9 +93,6 @@ class DWARFExpression {
   /// static variable since there is no other indication from DWARF debug
   /// info.
   ///
-  /// \param[in] dwarf_cu
-  /// The dwarf unit this expression belongs to.
-  ///
   /// \param[in] op_addr_idx
   /// The DW_OP_addr index to retrieve in case there is more than
   /// one DW_OP_addr opcode in the location byte stream.
@@ -72,22 +104,36 @@ class DWARFExpression {
   /// \return
   /// LLDB_INVALID_ADDRESS if the l

[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D129307#3637310 , @jingham wrote:

> In D129307#3636695 , @clayborg 
> wrote:
>
>> In D129307#3636573 , @jingham 
>> wrote:
>>
>>> I'm not entirely clear what problem this is solving.  Any actor setting 
>>> breakpoints can already choose the match depth by simply providing that 
>>> many directory components.  I.e. if I know I have subdir1/foo.cpp and 
>>> subdir2/foo.cpp I can set a breakpoint on only one of them by doing:
>>>
>>>   (lldb) break set -f subdir1/foo.cpp -l 10
>>>
>>> So an IDE could implement this by simply passing either the base name or 
>>> the base name plus one sub directory, etc.  As you say, that's what command 
>>> line users do anyway, they seldom type out full paths.  So this is really 
>>> about IDE's running lldb, and this seems more like a feature the IDE should 
>>> offer, not lldb.
>>
>> Xcode has this same issue if you download a dSYM file from the Apple build 
>> infrastructure if that dSYM doesn't contain path re-mappings in the UUID 
>> plists. Xcode, the IDE, will send down full paths to the source files that 
>> it knows about if you set breakpoints. Granted Xcode could work around this, 
>> but we added the ability to remap things at the module level in LLDB to work 
>> around this issue. So yes, it would be great if all IDEs would do this, but 
>> currently none do.
>>
>>> It also seems awkward to do all this work as part of the breakpoint 
>>> filtering - which as you have seen is somewhat delicate code.  After all, 
>>> you can implement "only match the last N components" by only submitting the 
>>> last N components when looking up the files.  So it would be much more 
>>> straightforward to strip the leading path components from the file name 
>>> when you create the breakpoint and you don't have to muck with the 
>>> filtering at all.  That would also help remove some confusion when you see 
>>> that the path submitted was /foo/bar/baz.cpp but I matched 
>>> /some/other/bar/baz.cpp (but not /some/other/different/baz.cpp) because I 
>>> had set the match suffix count to 1.  It would be nice to have break list 
>>> show me exactly what it was going to match on.
>>
>> We might need to always store the path that was given in source file and 
>> line breakpoints and then have the breakpoint update its locations when/if 
>> either of these two settings are modified. This would also help with your 
>> example below where you comment on breakpoint settings depend on the history 
>> of the session (they shouldn't).
>
> I took that bit out because in fact the implementation stores the depth in 
> the resolver when made.  So any given breakpoint isn't history dependent, but 
> rather to understand why some breakpoints take and some don't you have to 
> know the history of your settings in the session - which is not recorded 
> anywhere you can get your hands on.  That's a source of confusion for sure 
> but  probably not a huge one.

I would love for people to be able to enable/disable this setting to see if it 
fixes their debug session. If we go the route you are suggesting, we will need 
to set the setting and then remove the breakpoint and try to set it again. But 
I would love it if people could just enable this setting and see if it fixes 
their breakpoint issues. If the breakpoints always stored the full path that 
was supplied, and if the breakpoint always listened to the target settings, 
then nothing would need to change on the breakpoint storage end. We might need 
to add a "resolved path" to the source file and line breakpoints which would be 
the path that was used to set the breakpoint which we could show if it differs 
from the specified path. The breakpoints would always re-resolve if the 
settings were changed.

>>> I am also not sure doing this as a general setting for values other than 0 
>>> is really helpful.  So far as I can tell, you would use a value of 1 
>>> because you know that though you might have files with duplicate names they 
>>> are always disambiguated by their parent directory name.
>>
>> Exactly the case I was worried about. Usually zero would be the default 
>> IMHO, but if you have two different binaries that both have a "main.cpp" it 
>> could help. We can remove the depth and have this setting only set 
>> breakpoints by basename if needed if no one wants the extra complexity.
>
> Certainly just doing base name will find all the matches regardless of what 
> the debug info says about the path, and the only problem is extra matches 
> from files with the same name in different directories.  That's only slightly 
> annoying and not a fatal problem because you can always disable locations you 
> don't want, though it would be good to have some way to handle this.  But it 
> seems to me to disambiguate effectively, you really have to know something 
> about the structure  of you

[Lldb-commits] [lldb] 72d9390 - Add a little extra test coverage for simple template names

[David Blaikie via lldb-commits]

Author: David Blaikie
Date: 2022-07-08T00:12:29Z
New Revision: 72d9390778966d4f87ec4b1de63c107b2fd46b9a

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

LOG: Add a little extra test coverage for simple template names

This would fail with an overly naive approach to simple template
name (clang's -gsimple-template-names) since the names wouldn't be
unique per specialization, creating ambiguity/chance that a query for
one specialization would find another.

Added: 


Modified: 
lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py

Removed: 




diff  --git 
a/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py 
b/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
index b596611b15f2f..81a8876743474 100644
--- a/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
+++ b/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -14,6 +14,11 @@ def assertComplete(self, typename):
 self.assertTrue(found_type.IsValid())
 self.assertTrue(found_type.IsTypeComplete())
 
+def assertIsNotPresent(self, typename):
+""" Asserts that the type with the given name is not found. """
+found_type = self.target().FindFirstType(typename)
+self.assertFalse(found_type.IsValid())
+
 def assertCompleteWithVar(self, typename):
 """ Asserts that the type with the given name is complete. """
 found_type = self.target().FindFirstType(typename)
@@ -41,6 +46,7 @@ def test_forward_declarations(self):
 self.assertCompleteWithVar("DefinedClass")
 self.assertCompleteWithVar("DefinedClassTypedef")
 self.assertCompleteWithVar("DefinedTemplateClass")
+self.assertIsNotPresent("DefinedTemplateClass")
 
 # Record types without a defining declaration are not complete.
 self.assertPointeeIncomplete("FwdClass *", "fwd_class")



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

[Lldb-commits] [PATCH] D129332: [trace][intel pt] Support dumping the trace info in json

[walter erquinigo via Phabricator via lldb-commits]

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

improve tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129332

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.cpp
  lldb/test/API/commands/trace/TestTraceDumpInfo.py
  lldb/test/API/commands/trace/TestTraceLoad.py

Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -20,6 +20,72 @@
   substrs=["67911: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
+self.expect("thread trace dump info --json",
+  substrs=['''{
+  "traceTechnology": "intel-pt",
+  "threadStats": {
+"tid": 3497234,
+"traceItemsCount": 0,
+"memoryUsage": {
+  "totalInBytes": "0",
+  "avgPerItemInBytes": null
+},
+"timingInSeconds": {},
+"events": {
+  "totalCount": 0,
+  "individualCounts": {}
+},
+"continuousExecutions": 0,
+"PSBBlocks": 0,
+"errorItems": {
+  "total": 0,
+  "individualErrors": {}
+}
+  },
+  "globalStats": {
+"timingInSeconds": {
+  "Context switch and Intel PT traces correlation": 0
+},
+"totalUnattributedPSBBlocks": 0,
+"totalCountinuosExecutions": 153,
+"totalPSBBlocks": 5,
+"totalContinuousExecutions": 153
+  }
+}'''])
+
+self.expect("thread trace dump info 2 --json",
+  substrs=['''{
+  "traceTechnology": "intel-pt",
+  "threadStats": {
+"tid": 3497496,
+"traceItemsCount": 19523,
+"memoryUsage": {
+  "totalInBytes": "175739",
+  "avgPerItemInBytes": 9.00''', '''},
+"timingInSeconds": {},
+"events": {
+  "totalCount": 1,
+  "individualCounts": {
+"software disabled tracing": 1
+  }
+},
+"continuousExecutions": 1,
+"PSBBlocks": 1,
+"errorItems": {
+  "total": 0,
+  "individualErrors": {}
+}
+  },
+  "globalStats": {
+"timingInSeconds": {
+  "Context switch and Intel PT traces correlation": 0''', '''},
+"totalUnattributedPSBBlocks": 0,
+"totalCountinuosExecutions": 153,
+"totalPSBBlocks": 5,
+"totalContinuousExecutions": 153
+  }
+}'''])
+
 @testSBAPIAndCommands
 def testLoadMultiCoreTraceWithStringNumbers(self):
 src_dir = self.getSourceDir()
@@ -71,9 +137,10 @@
 # check that the Process and Thread objects were created correctly
 self.expect("thread info", substrs=["tid = 3842849"])
 self.expect("thread list", substrs=["Process 1234 stopped", "tid = 3842849"])
-self.expect("thread trace dump info", substrs=['''Trace technology: intel-pt
+self.expect("thread trace dump info", substrs=['''thread #1: tid = 3842849
+
+  Trace technology: intel-pt
 
-thread #1: tid = 3842849
   Total number of trace items: 23
 
   Memory usage:
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -34,9 +34,10 @@
 substrs=["intel-pt"])
 
 self.expect("thread trace dump info",
-substrs=['''Trace technology: intel-pt
+substrs=['''thread #1: tid = 3842849
+
+  Trace technology: intel-pt
 
-thread #1: tid = 3842849
   Total number of trace items: 23
 
   Memory usage:
@@ -54,3 +55,37 @@
   Errors:
 Number of TSC decoding errors: 0'''],
 patterns=["Decoding instructions: \d.\d\ds"])
+
+def testDumpRawTraceSizeJSON(self):
+self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+substrs=["intel-pt"])
+
+self.expect("thread trace dump info --json ",
+substrs=['''{
+  "traceTechnology": "intel-pt",
+  "threadStats": {
+"tid": 3842849,
+"traceItemsCount": 23,
+"memoryUsage": {
+  "totalInBytes": "207",
+  "avgPerItemInBytes": 9
+},
+"timingInSeconds": {
+  "Decoding instructions": 0''', '''
+},
+"events": {
+  "totalCount": 2,
+  "individualCounts": {
+"software disabled tracing": 2
+  }
+},
+"errorItems": {
+  "total": 0,
+  "individualErrors": {}
+}
+  },
+  "globalStats": {
+"timingInSeconds": {}
+  }
+}'''])
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceInt

[Lldb-commits] [PATCH] D129338: Tell the user which pathname was invalid...

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added a reviewer: JDevlieghere.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When there's an error with the pathname in "command script import" lldb just 
says "error: module importing failed: invalid pathname" but doesn't actually 
tell you what the bad pathname was.  When you're loading some python module 
that imports a bunch of other modules, that makes it hard to figure out which 
one was bad.

I changed it to either say "empty path" if that was the error, or report the 
pathname otherwise.  I added a test as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129338

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/test/API/commands/command/script/import/TestImport.py


Index: lldb/test/API/commands/command/script/import/TestImport.py
===
--- lldb/test/API/commands/command/script/import/TestImport.py
+++ lldb/test/API/commands/command/script/import/TestImport.py
@@ -39,11 +39,10 @@
 self.runCmd("command script import ./bar/bar.py --allow-reload")
 
 self.expect("command script import ./nosuchfile.py",
-error=True, startstr='error: module importing failed')
+error=True, startstr="error: module importing failed: 
invalid pathname './nosuchfile.py'")
 self.expect("command script import ./nosuchfolder/",
 error=True, startstr='error: module importing failed')
 self.expect("command script import ./foo/foo.py", error=False)
-
 self.runCmd("command script import --allow-reload ./thepackage")
 self.expect("TPcommandA", substrs=["hello world A"])
 self.expect("TPcommandB", substrs=["hello world B"])
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2637,7 +2637,7 @@
  .SetSetLLDBGlobals(false);
 
   if (!pathname || !pathname[0]) {
-error.SetErrorString("invalid pathname");
+error.SetErrorString("empty path");
 return false;
   }
 
@@ -2707,7 +2707,7 @@
   // if not a valid file of any sort, check if it might be a filename still
   // dot can't be used but / and \ can, and if either is found, reject
   if (strchr(pathname, '\\') || strchr(pathname, '/')) {
-error.SetErrorString("invalid pathname");
+error.SetErrorStringWithFormatv("invalid pathname '{0}'", pathname);
 return false;
   }
   // Not a filename, probably a package of some sort, let it go through.


Index: lldb/test/API/commands/command/script/import/TestImport.py
===
--- lldb/test/API/commands/command/script/import/TestImport.py
+++ lldb/test/API/commands/command/script/import/TestImport.py
@@ -39,11 +39,10 @@
 self.runCmd("command script import ./bar/bar.py --allow-reload")
 
 self.expect("command script import ./nosuchfile.py",
-error=True, startstr='error: module importing failed')
+error=True, startstr="error: module importing failed: invalid pathname './nosuchfile.py'")
 self.expect("command script import ./nosuchfolder/",
 error=True, startstr='error: module importing failed')
 self.expect("command script import ./foo/foo.py", error=False)
-
 self.runCmd("command script import --allow-reload ./thepackage")
 self.expect("TPcommandA", substrs=["hello world A"])
 self.expect("TPcommandB", substrs=["hello world B"])
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2637,7 +2637,7 @@
  .SetSetLLDBGlobals(false);
 
   if (!pathname || !pathname[0]) {
-error.SetErrorString("invalid pathname");
+error.SetErrorString("empty path");
 return false;
   }
 
@@ -2707,7 +2707,7 @@
   // if not a valid file of any sort, check if it might be a filename still
   // dot can't be used but / and \ can, and if either is found, reject
   if (strchr(pathname, '\\') || strchr(pathname, '/')) {
-error.SetErrorString("invalid pathname");
+error.SetErrorStringWithFormatv("invalid pathname '{0}'", pathname);
 return false;
   }
   // Not a filename, probably a package of some sort, let it go through.
__

[Lldb-commits] [PATCH] D129340: [trace][intel pt] Create a CPU change event and expose it in the dumper

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: jj10306, persona0220.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Thanks to fredz...@fb.com for coming up with this feature.

When tracing in per-cpu mode, we have information of in which cpu we are 
execution each instruction, which comes from the context switch trace. This 
diff makes this information available as a `cpu changed event`, which an 
additional accessor in the cursor `GetCPU()`. As cpu changes are very 
infrequent, any consumer should listen to cpu change events instead of querying 
the actual cpu of a trace item. Once a cpu change event is seen, the consumer 
can invoke GetCPU() to get that information. Also, it's possible to invoke 
GetCPU() on an arbitrary instruction item, which will return the last cpu seen. 
However, this call is O(logn) and should be used sparingly.

Manually tested with a sample program that starts on cpu 52, then goes to 18, 
and then goes back to 52.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129340

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceDumper.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Target/TraceCursor.cpp
  lldb/source/Target/TraceDumper.cpp
  lldb/test/API/commands/trace/TestTraceEvents.py
  lldb/test/API/commands/trace/TestTraceLoad.py

Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -13,11 +13,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19522: [tsc=40450075478109270] (error) expected tracing enabled event",
+  substrs=["19523: [tsc=40450075478109270] (error) expected tracing enabled event",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "19520: [tsc=40450075477657246] 0x00400ba7jg 0x400bb3"])
+   "19521: [tsc=40450075477657246] 0x00400ba7jg 0x400bb3"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["67911: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+  substrs=["67912: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
 self.expect("thread trace dump info --json",
@@ -58,15 +58,16 @@
   "traceTechnology": "intel-pt",
   "threadStats": {
 "tid": 3497496,
-"traceItemsCount": 19523,
+"traceItemsCount": 19524,
 "memoryUsage": {
-  "totalInBytes": "175739",
+  "totalInBytes": "175760",
   "avgPerItemInBytes": 9.00''', '''},
 "timingInSeconds": {},
 "events": {
-  "totalCount": 1,
+  "totalCount": 2,
   "individualCounts": {
-"software disabled tracing": 1
+"software disabled tracing": 1,
+"CPU core changed": 1
   }
 },
 "continuousExecutions": 1,
@@ -92,11 +93,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_with_string_numbers.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19522: [tsc=40450075478109270] (error) expected tracing enabled event",
+  substrs=["19523: [tsc=40450075478109270] (error) expected tracing enabled event",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "19520: [tsc=40450075477657246] 0x00400ba7jg 0x400bb3"])
+   "19521: [tsc=40450075477657246] 0x00400ba7jg 0x400bb3"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["67911: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+  substrs=["67912: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
 @testSBAPIAndCommands
@@ -105,11 +106,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_missing_threads.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instruct

[Lldb-commits] [PATCH] D129338: Tell the user which pathname was invalid...

[Will Hawkins via Phabricator via lldb-commits]

hawkinsw added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2717
   if (module_file.GetDirectory().IsEmpty()) {
 error.SetErrorString("invalid directory name");
 return false;

Sorry for dropping in unsolicited, but would this be another error that we 
would want to add more specificity for?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129338

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

[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I'm out tomorrow, so I won't get a chance for a detailed reply till Monday.  
But my short reactions are:

1. Setting the breakpoint search to only check base names will of course make 
all your file and line breakpoints work, but they will also pick up extra hits. 
 In command line lldb that doesn't look nearly as weird as clicking on one 
source file window & seeing a breakpoint marker show up in a completely 
different window.  If we didn't care about that problem and expected people to 
manage these extra locations by hand, then indeed just saying "only set using 
base-names" is fine.  But it still seems weird to me that we would add an lldb 
setting and code to support that rather than have IDE's just only pass in the 
base name if that's their strategy.

2. I really don't want changing a setting to add or remove locations from an 
extant breakpoint.  That's not how breakpoints work.  If I go to the trouble of 
inputting a command on a location, and the I change a setting and the location 
goes away, I would rightly be ticked off.  So I really think the locations 
should record whatever search are going to when they are created, and stick to 
it.  However, I have no problem with having the breakpoint store "original 
path/matched path" pairs if that makes it easier to figure out what is going on.

3. The behind your back creation of source maps gives me the willies.   
Particularly if you have more than one module with debug information, all built 
originally in the phony locations so they all look like the original 
directories have the same root, it seems like you could start getting source 
maps that map everything to everything and after a while this will get hard to 
reason about.  Maybe I'm being pessimistic, however, and if you are excited to 
try, more power to you.

But I don't think you can drive this from breakpoints alone.  Suppose I set no 
breakpoints, and the program crashes at 
/OriginalDirectory/SourceRoot/MyProject/subdir/foo.c, lldb is still going to 
have to figure out what to show the user, so you're still going to have to come 
up with the real source file.  And you can't use your breakpoint tricks to help 
you because it's lldb providing the information and at the point where you are 
doing this it only knows the original location.

It seems like it would be cleaner to have some kind of callback when a binary 
with debug information gets loaded where the UI could have a look at the CU's 
in the module that got loaded, and see if it knows about any of those files 
locally (because it has them in in directory-equivalent places in its project). 
 The UI can then construct a source map from there.  That way this would happen 
predictably and for all consumers, rather than relying on the user's path 
through breakpoint setting to set the source mapping world back to rights.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129307

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