[Lldb-commits] [PATCH] D118473: [lldb] [Commands] Implement "thread siginfo"

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

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



Comment at: lldb/source/Commands/CommandObjectThread.cpp:1323
 
+// CommandObjectThreadSiginfo
+

JDevlieghere wrote:
>  I know you're just being consistent with the rest of this file, but we've 
> been slowly phasing out these kind of comments.
I'm all for it ;-).


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

https://reviews.llvm.org/D118473

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

[Lldb-commits] [PATCH] D118473: [lldb] [Commands] Implement "thread siginfo"

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

mgorny updated this revision to Diff 405249.
mgorny marked an inline comment as done.
mgorny added a comment.

Removed the comment and added `thread info`-style status above siginfos.


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

https://reviews.llvm.org/D118473

Files:
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/test/Shell/Commands/Inputs/sigchld.c
  lldb/test/Shell/Commands/command-thread-siginfo.test

Index: lldb/test/Shell/Commands/command-thread-siginfo.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-thread-siginfo.test
@@ -0,0 +1,19 @@
+# REQUIRES: system-linux
+# RUN: %clang_host -g %S/Inputs/sigchld.c -o %t
+# RUN: %lldb %t -b -s %s | FileCheck %s
+
+process launch -s
+process handle SIGCHLD -s true
+process continue
+# CHECK: signo = [[SIGNO:[0-9]+]]
+# CHECK: code = [[CODE:[0-9]+]]
+# CHECK: child_pid = [[PID:[0-9]+]]
+# CHECK: uid = [[UID:[0-9]+]]
+# CHECK: stop reason = signal SIGCHLD
+thread siginfo
+# CHECK-DAG: si_signo = [[SIGNO]]
+# CHECK-DAG: si_errno = 0
+# CHECK-DAG: si_code = [[CODE]]
+# CHECK-DAG: si_pid = [[PID]]
+# CHECK-DAG: si_uid = [[UID]]
+# CHECK-DAG: si_status = 14
Index: lldb/test/Shell/Commands/Inputs/sigchld.c
===
--- /dev/null
+++ lldb/test/Shell/Commands/Inputs/sigchld.c
@@ -0,0 +1,31 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+void handler(int signo) {
+  printf("SIGCHLD\n");
+}
+
+int main() {
+  void *ret = signal(SIGINT, handler);
+  assert (ret != SIG_ERR);
+
+  pid_t child_pid = fork();
+  assert (child_pid != -1);
+
+  if (child_pid == 0) {
+sleep(1);
+_exit(14);
+  }
+
+  printf("signo = %d\n", SIGCHLD);
+  printf("code = %d\n", CLD_EXITED);
+  printf("child_pid = %d\n", child_pid);
+  printf("uid = %d\n", getuid());
+  pid_t waited = wait(NULL);
+  assert(waited == child_pid);
+
+  return 0;
+}
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -1320,6 +1320,53 @@
   }
 };
 
+class CommandObjectThreadSiginfo : public CommandObjectIterateOverThreads {
+public:
+  CommandObjectThreadSiginfo(CommandInterpreter &interpreter)
+  : CommandObjectIterateOverThreads(
+interpreter, "thread siginfo",
+"Display the current siginfo object for a thread. Defaults to "
+"the current thread.",
+"thread siginfo",
+eCommandRequiresProcess | eCommandTryTargetAPILock |
+eCommandProcessMustBeLaunched | eCommandProcessMustBePaused) {}
+
+  ~CommandObjectThreadSiginfo() override = default;
+
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), CommandCompletions::eThreadIndexCompletion,
+request, nullptr);
+  }
+
+  bool HandleOneThread(lldb::tid_t tid, CommandReturnObject &result) override {
+ThreadSP thread_sp =
+m_exe_ctx.GetProcessPtr()->GetThreadList().FindThreadByID(tid);
+if (!thread_sp) {
+  result.AppendErrorWithFormat("thread no longer exists: 0x%" PRIx64 "\n",
+   tid);
+  return false;
+}
+
+Stream &strm = result.GetOutputStream();
+if (!thread_sp->GetDescription(strm, eDescriptionLevelFull, false, false)) {
+  result.AppendErrorWithFormat("error displaying info for thread: \"%d\"\n",
+   thread_sp->GetIndexID());
+  return false;
+}
+ValueObjectSP exception_object_sp = thread_sp->GetSiginfoValue();
+if (exception_object_sp)
+  exception_object_sp->Dump(strm);
+else
+  strm.Printf("(no siginfo)\n");
+strm.PutChar('\n');
+
+return true;
+  }
+};
+
 // CommandObjectThreadReturn
 #define LLDB_OPTIONS_thread_return
 #include "CommandOptions.inc"
@@ -2293,6 +2340,8 @@
  CommandObjectSP(new CommandObjectThreadInfo(interpreter)));
   LoadSubCommand("exception", CommandObjectSP(new CommandObjectThreadException(
   interpreter)));
+  LoadSubCommand("siginfo",
+ CommandObjectSP(new CommandObjectThreadSiginfo(interpreter)));
   LoadSubCommand("step-in",
  CommandObjectSP(new CommandObjectThreadStepWithTypeAndScope(
  interpreter, "thread step-in",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D118794: [lldb][AArch64] Remove non-address bits from addresses passed to ptrace on Linux

[David Spickett via Phabricator via lldb-commits]

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

This commit makes lldb-server on Linux strip non-address bits from addresses
passed to the following ptrace operations:

- POKEDATA (memory write)
- PEEKDATA (memory read)
- process_vm_readv (memory read)
- POKEMTETAGS (memory tag write)
- PEEKMTETAGS (memory tag read)

(anything that needs a virtual address of the tracee)

When we only had the top byte set, we were getting away with it
because the hardware did the masking for us. Though according to
https://www.kernel.org/doc/html/latest/arm64/tagged-address-abi.html
we should be removing them.

"When the AArch64 Tagged Address ABI is enabled for a thread, the
following behaviours are guaranteed:

All syscalls except the cases mentioned in section 3 can accept any
valid tagged pointer."

Since we don't enable the tagged address ABI, the implication
is that the opposite is true. Even if certain ptrace calls happen
to work now, they might not in future.

For pointer signatures usually they would be authenticated before
use and ptrace doesn't know to do that. So we must remove the
signature before use.

I've used the data_mask here for all addresses because Linux
currently does not set different code and data masks. Since AArch64's
"data_mask" is the pointer signature mask only, the top byte is removed
manually.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118794

Files:
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/test/API/tools/lldb-server/memory-non-address-bits/Makefile
  
lldb/test/API/tools/lldb-server/memory-non-address-bits/TestGdbRemoteMemoryNonAddressBits.py
  lldb/test/API/tools/lldb-server/memory-non-address-bits/main.c

Index: lldb/test/API/tools/lldb-server/memory-non-address-bits/main.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/memory-non-address-bits/main.c
@@ -0,0 +1,45 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int print_pointers(char *buf, char *buf_with_non_address) {
+  // This output is picked up by the test.
+  printf("buf: %p buf_with_non_address: %p\n", buf, buf_with_non_address);
+
+  // Exit after some time, so we don't leave a zombie process
+  // if the test framework lost track of us.
+  sleep(60);
+  return 0;
+}
+
+int main(int argc, char const *argv[]) {
+  // We need to allocate a memory tagged page to know whether
+  // our memory tag read/write packets succeeded.
+  if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC, 0,
+0, 0)) {
+return print_pointers(NULL, NULL);
+  }
+
+  size_t page_size = sysconf(_SC_PAGESIZE);
+  char *buf = mmap(0, page_size, PROT_READ | PROT_WRITE | PROT_MTE,
+   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (buf == MAP_FAILED)
+return print_pointers(NULL, NULL);
+
+  // Set top byte including where memory tag bits would go.
+  char *buf_with_non_address = (char *)((size_t)buf | (size_t)0xff << 56);
+  // Sign it using a hint space instruction in case the core doesn't have
+  // pointer authentication.
+  __asm__ __volatile__("pacdza %0"
+   : "=r"(buf_with_non_address)
+   : "r"(buf_with_non_address));
+  // Address is now:
+  // <4 bit user tag><4 bit memory tag>
+
+  return print_pointers(buf, buf_with_non_address);
+}
Index: lldb/test/API/tools/lldb-server/memory-non-address-bits/TestGdbRemoteMemoryNonAddressBits.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/memory-non-address-bits/TestGdbRemoteMemoryNonAddressBits.py
@@ -0,0 +1,86 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+"""
+Check that lldb-server correctly removes the non-address bits
+required to make ptrace calls on Linux.
+"""
+
+class TestGdbRemoteMemoryNonAddressBits(gdbremote_testcase.GdbRemoteTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def prep_test(self):
+self.build()
+self.set_inferior_startup_launch()
+procs = self.prep_debug_monitor_and_inferior()
+
+# We don't use isAArch64MTE here because we cannot do runCmd in an
+# lldb-server test. Instead we run the example and skip if it fails
+# to allocate an MTE buffer.
+# Pointer signing is done with a hint space instruction so the test
+# can run on pre armv8.3-a CPUs. (though it will not be doing any useful
+# checking in that case)
+
+# Run the process
+self.test_sequence.add_log_lines(
+[
+# Start running after initial stop
+"read p

[Lldb-commits] [PATCH] D118794: [lldb][AArch64] Remove non-address bits from addresses passed to ptrace on Linux

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: omjavaid.
DavidSpickett added a comment.

Without this change any memory read/write of a pointer with a signature would 
fail. I found it trying to extend memory find and using a pointer with both a 
signature and a memory tag.

There's a couple of things I could do but not sure if worth it right now given 
that AArch64 Linux is the only user of this aside from Mac OS (which has 
debugserver so that may already be doing this). Those are:

- Sink the application of top byte ignore into 
NativeRegisterContextLinux_AArch64
- Raise the whole process up to NativeProcessProtocol

The latter would implement this logic for every NativeProcess at once but has 
the issues that:

- most won't have a named register like linux has, so we end up adding an OS 
specific branch anyway
- Many more functions at NativeProcessProtocol level take an address. The 
current way lets us fix the few ptrace calls all those member functions 
actually end up using.

Or in other words: this is a bit hacky but perhaps it should stay that way 
until it's more widely needed.

I also thought about fixing this from the lldb end but it seems logical to make 
the server act correctly even without having lldb clean the addresses up front. 
(though in a some cases, like memory tags, it will do but for its own reasons)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118794

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

[Lldb-commits] [PATCH] D118794: [lldb][AArch64] Remove non-address bits from addresses passed to ptrace on Linux

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Also if anyone does know how or if debugserver handles this situation, that 
would be good to know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118794

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

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, clayborg, labath.
JDevlieghere requested review of this revision.

Libraries which rely heavily on templates (e.g. boost) can generate extremely 
long symbol names, with mangled names in the 10 000 of characters. These 
symbols take a long time to demangle and can results in unmangled names that 
are several megabytes in size. This patch adds a setting to skip past these 
symbols when indexing the symbol table to speed up launch/attach times and keep 
memory usage in check.

I arbitrarily picked 1 as the default value which seems large enough to not 
affect most workflows.


https://reviews.llvm.org/D118812

Files:
  lldb/include/lldb/Core/ModuleList.h
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/ModuleList.cpp
  lldb/source/Symbol/Symtab.cpp


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -297,11 +297,19 @@
 std::vector> backlog;
 backlog.reserve(num_symbols / 2);
 
+const uint64_t mangling_limit =
+ModuleList::GetGlobalModuleListProperties().GetDemanglingLimit();
+
 // Instantiation of the demangler is expensive, so better use a single one
 // for all entries during batch processing.
 RichManglingContext rmc;
 for (uint32_t value = 0; value < num_symbols; ++value) {
   Symbol *symbol = &m_symbols[value];
+  Mangled &mangled = symbol->GetMangled();
+
+  if (mangling_limit)
+if (mangled.GetMangledName().GetLength() > mangling_limit)
+  continue;
 
   // Don't let trampolines get into the lookup by name map If we ever need
   // the trampoline symbols to be searchable by name we can remove this and
@@ -314,7 +322,6 @@
 
   // If the symbol's name string matched a Mangled::ManglingScheme, it is
   // stored in the mangled field.
-  Mangled &mangled = symbol->GetMangled();
   if (ConstString name = mangled.GetMangledName()) {
 name_to_index.Append(name, value);
 
Index: lldb/source/Core/ModuleList.cpp
===
--- lldb/source/Core/ModuleList.cpp
+++ lldb/source/Core/ModuleList.cpp
@@ -125,6 +125,12 @@
   ->GetCurrentValue();
 }
 
+uint64_t ModuleListProperties::GetDemanglingLimit() const {
+  const uint32_t idx = ePropertyDemanglingLimit;
+  return m_collection_sp->GetPropertyAtIndexAsUInt64(
+  nullptr, idx, g_modulelist_properties[idx].default_uint_value);
+}
+
 bool ModuleListProperties::SetLLDBIndexCachePath(const FileSpec &path) {
   return m_collection_sp->SetPropertyAtIndexAsFileSpec(
   nullptr, ePropertyLLDBIndexCachePath, path);
Index: lldb/source/Core/CoreProperties.td
===
--- lldb/source/Core/CoreProperties.td
+++ lldb/source/Core/CoreProperties.td
@@ -9,6 +9,10 @@
 Global,
 DefaultStringValue<"">,
 Desc<"The path to the clang modules cache directory 
(-fmodules-cache-path).">;
+  def DemanglingLimit: Property<"demangling-max-length", "UInt64">,
+Global,
+DefaultUnsignedValue<1000>,
+Desc<"The maximum length of the mangled symbol name. Mangled symbols that 
exceed this threshold will not be demangled when indexing the symbol table. A 
value of 0 means no limit.">;
   def SymLinkPaths: Property<"debug-info-symlink-paths", "FileSpecList">,
 Global,
 DefaultStringValue<"">,
Index: lldb/include/lldb/Core/ModuleList.h
===
--- lldb/include/lldb/Core/ModuleList.h
+++ lldb/include/lldb/Core/ModuleList.h
@@ -58,6 +58,7 @@
 
   FileSpec GetClangModulesCachePath() const;
   bool SetClangModulesCachePath(const FileSpec &path);
+  uint64_t GetDemanglingLimit() const;
   bool GetEnableExternalLookup() const;
   bool SetEnableExternalLookup(bool new_value);
   bool GetEnableLLDBIndexCache() const;


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -297,11 +297,19 @@
 std::vector> backlog;
 backlog.reserve(num_symbols / 2);
 
+const uint64_t mangling_limit =
+ModuleList::GetGlobalModuleListProperties().GetDemanglingLimit();
+
 // Instantiation of the demangler is expensive, so better use a single one
 // for all entries during batch processing.
 RichManglingContext rmc;
 for (uint32_t value = 0; value < num_symbols; ++value) {
   Symbol *symbol = &m_symbols[value];
+  Mangled &mangled = symbol->GetMangled();
+
+  if (mangling_limit)
+if (mangled.GetMangledName().GetLength() > mangling_limit)
+  continue;
 
   // Don't let trampolines get into the lookup by name map If we ever need
   // the trampoline symbols to be searchable by name we can remove this and
@@ -314,7 +322,6 @@
 
   

[Lldb-commits] [PATCH] D118814: [lldb] Don't keep demangled names in memory after indexing

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, clayborg, labath.
Herald added a subscriber: arphaman.
JDevlieghere requested review of this revision.

The symbol table needs to demangle all symbols when building its index. 
However, the index doesn't need the demangeld symbols after it's done. We keep 
the demangled symbols in memory for caching purposes, so that if we do need to 
demangle them again. Most of the time you don't need (all) the demangled 
symbols again.

This patch add a setting to prefer using less memory over p and not cache all 
the demangled strings that are generated for indexing. If the symbol does get 
demangled later on, it is cached as usual. This change only impacts the batch 
demangled operation when building the symbol table index.

I was going to keep this setting default to "on" to keep the existing behavior, 
but Jim argued that the amount of memory we save here is well worth the small 
performance hit of having to demangle a subset of symbols twice.


https://reviews.llvm.org/D118814

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/include/lldb/Core/ModuleList.h
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Mangled.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Symbol/Symtab.cpp

Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -299,6 +299,8 @@
 
 const uint64_t mangling_limit =
 ModuleList::GetGlobalModuleListProperties().GetDemanglingLimit();
+const bool store_mangled_name =
+ModuleList::GetGlobalModuleListProperties().GetCacheDemangledNames();
 
 // Instantiation of the demangler is expensive, so better use a single one
 // for all entries during batch processing.
@@ -335,7 +337,8 @@
 
 const SymbolType type = symbol->GetType();
 if (type == eSymbolTypeCode || type == eSymbolTypeResolver) {
-  if (mangled.DemangleWithRichManglingInfo(rmc, lldb_skip_name))
+  if (mangled.DemangleWithRichManglingInfo(rmc, lldb_skip_name,
+   store_mangled_name))
 RegisterMangledNameEntry(value, class_contexts, backlog, rmc);
 }
   }
Index: lldb/source/Core/ModuleList.cpp
===
--- lldb/source/Core/ModuleList.cpp
+++ lldb/source/Core/ModuleList.cpp
@@ -131,6 +131,12 @@
   nullptr, idx, g_modulelist_properties[idx].default_uint_value);
 }
 
+bool ModuleListProperties::GetCacheDemangledNames() const {
+  const uint32_t idx = ePropertyCacheDemangledNames;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_modulelist_properties[idx].default_uint_value != 0);
+}
+
 bool ModuleListProperties::SetLLDBIndexCachePath(const FileSpec &path) {
   return m_collection_sp->SetPropertyAtIndexAsFileSpec(
   nullptr, ePropertyLLDBIndexCachePath, path);
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -195,8 +195,9 @@
 
 // Explicit demangling for scheduled requests during batch processing. This
 // makes use of ItaniumPartialDemangler's rich demangle info
-bool Mangled::DemangleWithRichManglingInfo(
-RichManglingContext &context, SkipMangledNameFn *skip_mangled_name) {
+bool Mangled::DemangleWithRichManglingInfo(RichManglingContext &context,
+   SkipMangledNameFn *skip_mangled_name,
+   bool store_demangled_name) {
   // Others are not meant to arrive here. ObjC names or C's main() for example
   // have their names stored in m_demangled, while m_mangled is empty.
   assert(m_mangled);
@@ -218,8 +219,9 @@
   // If we got an info, we have a name. Copy to string pool and connect the
   // counterparts to accelerate later access in GetDemangledName().
   context.ParseFullName();
-  m_demangled.SetStringWithMangledCounterpart(context.GetBufferRef(),
-  m_mangled);
+  if (store_demangled_name)
+m_demangled.SetStringWithMangledCounterpart(context.GetBufferRef(),
+m_mangled);
   return true;
 } else {
   m_demangled.SetCString("");
@@ -233,8 +235,9 @@
   if (char *d = GetMSVCDemangledStr(m_mangled.GetCString())) {
 // If we got an info, we have a name. Copy to string pool and connect
 // the counterparts to accelerate later access in GetDemangledName().
-m_demangled.SetStringWithMangledCounterpart(llvm::StringRef(d),
-m_mangled);
+if (store_demangled_name)
+  m_demangled.SetStringWithMangledCounterpart(llvm::StringRef(d),
+  m_mangle

[Lldb-commits] [PATCH] D118473: [lldb] [Commands] Implement "thread siginfo"

[Jim Ingham via Phabricator via lldb-commits]

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

This looks fine to me.


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

https://reviews.llvm.org/D118473

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

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

Any chance you might want a limit on the size of the demangled name too? (might 
be worth considering what the most densely encoded mangled name is (ie: what's 
the longest name that could be produced by a 10k long mangled name? and see if 
that's worth having another cutoff for)


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

https://reviews.llvm.org/D118812

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

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

Is it worth trying to come up with a limit that's not arbitrarily picked?




Comment at: lldb/source/Symbol/Symtab.cpp:300
 
+const uint64_t mangling_limit =
+ModuleList::GetGlobalModuleListProperties().GetDemanglingLimit();

technically `demangling_limit`


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

https://reviews.llvm.org/D118812

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

[Lldb-commits] [PATCH] D118814: [lldb] Don't keep demangled names in memory after indexing

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

> the amount of memory we save here is well worth the small performance hit

any numbers to share?


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

https://reviews.llvm.org/D118814

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

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D118812#3291109 , @dblaikie wrote:

> Any chance you might want a limit on the size of the demangled name too? 
> (might be worth considering what the most densely encoded mangled name is 
> (ie: what's the longest name that could be produced by a 10k long mangled 
> name? and see if that's worth having another cutoff for)

Ironically, lldb seldom cares about most of the goo in these long demangled 
names.  At this point, we are building up our fast-lookup "name indexes".  We 
really only care about extracting the fully scoped names of the methods.  When 
we get around to doing smart matching on overloads, we can still pull out all 
the matches to the method name, and then do the overload match on the results.  
That should be sufficiently efficient, and obviate the need to do any fancy 
indexing based on overloads.  So most of the work of demangling these names is 
not being used anyway.

So what would be the better solution for lldb on the demangling front would be 
a way to tell the demangler "only extract the full method name, and don't 
bother producing the argument list or return values".  But I have no idea how 
easy that would be in the demangler.


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

https://reviews.llvm.org/D118812

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

[Lldb-commits] [PATCH] D118814: [lldb] Don't keep demangled names in memory after indexing

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D118814#3291174 , @kastiglione 
wrote:

>> the amount of memory we save here is well worth the small performance hit
>
> any numbers to share?

We have one scenario where this bring memory usage after attach down from 7gb 
to 1.8gb. (rdar://86413848) This is definitely an extreme example though.


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

https://reviews.llvm.org/D118814

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

[Lldb-commits] [lldb] 287ce6b - [lldb] [Commands] Implement "thread siginfo"

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-02-02T19:31:44+01:00
New Revision: 287ce6b51675aee43870bebfff68bb144d1ab90e

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

LOG: [lldb] [Commands] Implement "thread siginfo"

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

Added: 
lldb/test/Shell/Commands/Inputs/sigchld.c
lldb/test/Shell/Commands/command-thread-siginfo.test

Modified: 
lldb/source/Commands/CommandObjectThread.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index 137aaa81c61a0..f6042937a4ff1 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -1320,6 +1320,53 @@ class CommandObjectThreadException : public 
CommandObjectIterateOverThreads {
   }
 };
 
+class CommandObjectThreadSiginfo : public CommandObjectIterateOverThreads {
+public:
+  CommandObjectThreadSiginfo(CommandInterpreter &interpreter)
+  : CommandObjectIterateOverThreads(
+interpreter, "thread siginfo",
+"Display the current siginfo object for a thread. Defaults to "
+"the current thread.",
+"thread siginfo",
+eCommandRequiresProcess | eCommandTryTargetAPILock |
+eCommandProcessMustBeLaunched | eCommandProcessMustBePaused) {}
+
+  ~CommandObjectThreadSiginfo() override = default;
+
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), CommandCompletions::eThreadIndexCompletion,
+request, nullptr);
+  }
+
+  bool HandleOneThread(lldb::tid_t tid, CommandReturnObject &result) override {
+ThreadSP thread_sp =
+m_exe_ctx.GetProcessPtr()->GetThreadList().FindThreadByID(tid);
+if (!thread_sp) {
+  result.AppendErrorWithFormat("thread no longer exists: 0x%" PRIx64 "\n",
+   tid);
+  return false;
+}
+
+Stream &strm = result.GetOutputStream();
+if (!thread_sp->GetDescription(strm, eDescriptionLevelFull, false, false)) 
{
+  result.AppendErrorWithFormat("error displaying info for thread: 
\"%d\"\n",
+   thread_sp->GetIndexID());
+  return false;
+}
+ValueObjectSP exception_object_sp = thread_sp->GetSiginfoValue();
+if (exception_object_sp)
+  exception_object_sp->Dump(strm);
+else
+  strm.Printf("(no siginfo)\n");
+strm.PutChar('\n');
+
+return true;
+  }
+};
+
 // CommandObjectThreadReturn
 #define LLDB_OPTIONS_thread_return
 #include "CommandOptions.inc"
@@ -2293,6 +2340,8 @@ 
CommandObjectMultiwordThread::CommandObjectMultiwordThread(
  CommandObjectSP(new CommandObjectThreadInfo(interpreter)));
   LoadSubCommand("exception", CommandObjectSP(new CommandObjectThreadException(
   interpreter)));
+  LoadSubCommand("siginfo",
+ CommandObjectSP(new CommandObjectThreadSiginfo(interpreter)));
   LoadSubCommand("step-in",
  CommandObjectSP(new CommandObjectThreadStepWithTypeAndScope(
  interpreter, "thread step-in",

diff  --git a/lldb/test/Shell/Commands/Inputs/sigchld.c 
b/lldb/test/Shell/Commands/Inputs/sigchld.c
new file mode 100644
index 0..ba8c5ef45365b
--- /dev/null
+++ b/lldb/test/Shell/Commands/Inputs/sigchld.c
@@ -0,0 +1,31 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+void handler(int signo) {
+  printf("SIGCHLD\n");
+}
+
+int main() {
+  void *ret = signal(SIGINT, handler);
+  assert (ret != SIG_ERR);
+
+  pid_t child_pid = fork();
+  assert (child_pid != -1);
+
+  if (child_pid == 0) {
+sleep(1);
+_exit(14);
+  }
+
+  printf("signo = %d\n", SIGCHLD);
+  printf("code = %d\n", CLD_EXITED);
+  printf("child_pid = %d\n", child_pid);
+  printf("uid = %d\n", getuid());
+  pid_t waited = wait(NULL);
+  assert(waited == child_pid);
+
+  return 0;
+}

diff  --git a/lldb/test/Shell/Commands/command-thread-siginfo.test 
b/lldb/test/Shell/Commands/command-thread-siginfo.test
new file mode 100644
index 0..92829f3dcb0c4
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-thread-siginfo.test
@@ -0,0 +1,19 @@
+# REQUIRES: system-linux
+# RUN: %clang_host -g %S/Inputs/sigchld.c -o %t
+# RUN: %lldb %t -b -s %s | FileCheck %s
+
+process launch -s
+process handle SIGCHLD -s true
+process continue
+# CHECK: signo = [[SIGNO:[0-9]+]]
+# CHECK: code = [[CODE:[0-9]+]]
+# CHECK: child_pid = [[PID:[0-9]+]]
+# CHECK: uid = [[UID:[0-9]+]]
+# CHECK: stop reason = signal SIGCHLD
+thread siginfo
+# CHECK-DAG: si_signo = [[SIGNO]]
+# CHECK-DAG: si_errno = 

[Lldb-commits] [PATCH] D118473: [lldb] [Commands] Implement "thread siginfo"

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG287ce6b51675: [lldb] [Commands] Implement "thread 
siginfo" (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118473

Files:
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/test/Shell/Commands/Inputs/sigchld.c
  lldb/test/Shell/Commands/command-thread-siginfo.test

Index: lldb/test/Shell/Commands/command-thread-siginfo.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-thread-siginfo.test
@@ -0,0 +1,19 @@
+# REQUIRES: system-linux
+# RUN: %clang_host -g %S/Inputs/sigchld.c -o %t
+# RUN: %lldb %t -b -s %s | FileCheck %s
+
+process launch -s
+process handle SIGCHLD -s true
+process continue
+# CHECK: signo = [[SIGNO:[0-9]+]]
+# CHECK: code = [[CODE:[0-9]+]]
+# CHECK: child_pid = [[PID:[0-9]+]]
+# CHECK: uid = [[UID:[0-9]+]]
+# CHECK: stop reason = signal SIGCHLD
+thread siginfo
+# CHECK-DAG: si_signo = [[SIGNO]]
+# CHECK-DAG: si_errno = 0
+# CHECK-DAG: si_code = [[CODE]]
+# CHECK-DAG: si_pid = [[PID]]
+# CHECK-DAG: si_uid = [[UID]]
+# CHECK-DAG: si_status = 14
Index: lldb/test/Shell/Commands/Inputs/sigchld.c
===
--- /dev/null
+++ lldb/test/Shell/Commands/Inputs/sigchld.c
@@ -0,0 +1,31 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+void handler(int signo) {
+  printf("SIGCHLD\n");
+}
+
+int main() {
+  void *ret = signal(SIGINT, handler);
+  assert (ret != SIG_ERR);
+
+  pid_t child_pid = fork();
+  assert (child_pid != -1);
+
+  if (child_pid == 0) {
+sleep(1);
+_exit(14);
+  }
+
+  printf("signo = %d\n", SIGCHLD);
+  printf("code = %d\n", CLD_EXITED);
+  printf("child_pid = %d\n", child_pid);
+  printf("uid = %d\n", getuid());
+  pid_t waited = wait(NULL);
+  assert(waited == child_pid);
+
+  return 0;
+}
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -1320,6 +1320,53 @@
   }
 };
 
+class CommandObjectThreadSiginfo : public CommandObjectIterateOverThreads {
+public:
+  CommandObjectThreadSiginfo(CommandInterpreter &interpreter)
+  : CommandObjectIterateOverThreads(
+interpreter, "thread siginfo",
+"Display the current siginfo object for a thread. Defaults to "
+"the current thread.",
+"thread siginfo",
+eCommandRequiresProcess | eCommandTryTargetAPILock |
+eCommandProcessMustBeLaunched | eCommandProcessMustBePaused) {}
+
+  ~CommandObjectThreadSiginfo() override = default;
+
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), CommandCompletions::eThreadIndexCompletion,
+request, nullptr);
+  }
+
+  bool HandleOneThread(lldb::tid_t tid, CommandReturnObject &result) override {
+ThreadSP thread_sp =
+m_exe_ctx.GetProcessPtr()->GetThreadList().FindThreadByID(tid);
+if (!thread_sp) {
+  result.AppendErrorWithFormat("thread no longer exists: 0x%" PRIx64 "\n",
+   tid);
+  return false;
+}
+
+Stream &strm = result.GetOutputStream();
+if (!thread_sp->GetDescription(strm, eDescriptionLevelFull, false, false)) {
+  result.AppendErrorWithFormat("error displaying info for thread: \"%d\"\n",
+   thread_sp->GetIndexID());
+  return false;
+}
+ValueObjectSP exception_object_sp = thread_sp->GetSiginfoValue();
+if (exception_object_sp)
+  exception_object_sp->Dump(strm);
+else
+  strm.Printf("(no siginfo)\n");
+strm.PutChar('\n');
+
+return true;
+  }
+};
+
 // CommandObjectThreadReturn
 #define LLDB_OPTIONS_thread_return
 #include "CommandOptions.inc"
@@ -2293,6 +2340,8 @@
  CommandObjectSP(new CommandObjectThreadInfo(interpreter)));
   LoadSubCommand("exception", CommandObjectSP(new CommandObjectThreadException(
   interpreter)));
+  LoadSubCommand("siginfo",
+ CommandObjectSP(new CommandObjectThreadSiginfo(interpreter)));
   LoadSubCommand("step-in",
  CommandObjectSP(new CommandObjectThreadStepWithTypeAndScope(
  interpreter, "thread step-in",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D118473: [lldb] [Commands] Implement "thread siginfo"

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

mgorny added a comment.

Thanks for all the reviews and helpful suggestions!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118473

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

[Lldb-commits] [PATCH] D118814: [lldb] Don't keep demangled names in memory after indexing

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

This patch does have one side-effect that I know of, which is that if we don't 
keep demangled names in the symbol table, then breaking on an overload by 
demangled name won't work.  OTOH, that only worked if you exactly matched the 
demangler's output - no extra spaces, etc.   We don't have a smart matcher for 
overloads, so I'm not sure that requiring you to supply the mangled name 
instead in this case is much of a burden.

And if we want to actually have a nice way to break on overloads, it would 
involve parsing the incoming specification, extracting the method name and 
arguments, finding the method name matches and then doing some kind of fuzzy 
match against the arguments.  So we wouldn't need to keep all the demangled 
strings for that purpose either.

IMO the UE for breaking on overloads is not currently good enough that it 
outweighs the savings we get from not storing all the demangled names.  And if 
this is important to somebody's workflow, they can always get the old behavior 
back by reversing the setting value.


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

https://reviews.llvm.org/D118814

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

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a subscriber: rsmith.
dblaikie added a comment.

In D118812#3291303 , @jingham wrote:

> In D118812#3291109 , @dblaikie 
> wrote:
>
>> Any chance you might want a limit on the size of the demangled name too? 
>> (might be worth considering what the most densely encoded mangled name is 
>> (ie: what's the longest name that could be produced by a 10k long mangled 
>> name? and see if that's worth having another cutoff for)
>
> Ironically, lldb seldom cares about most of the goo in these long demangled 
> names.  At this point, we are building up our fast-lookup "name indexes".  We 
> really only care about extracting the fully scoped names of the methods.  
> When we get around to doing smart matching on overloads, we can still pull 
> out all the matches to the method name, and then do the overload match on the 
> results.  That should be sufficiently efficient, and obviate the need to do 
> any fancy indexing based on overloads.  So most of the work of demangling 
> these names is not being used anyway.
>
> So what would be the better solution for lldb on the demangling front would 
> be a way to tell the demangler "only extract the full method name, and don't 
> bother producing the argument list or return values".  But I have no idea how 
> easy that would be in the demangler.

I think there's an API level of the demangler in LLVM designed for rewriting 
demangled names (@rsmith created/implemented that, I think) - I'm not sure if 
it's structured to allow lazy parsing/stopping after you get the base name, for 
instance, but maybe...


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

https://reviews.llvm.org/D118812

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

[Lldb-commits] [PATCH] D118842: Small fixes to platform packet documentation

[Jason Molenda via Phabricator via lldb-commits]

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

I have a standalone implementation of the lldb platform packets for Darwin 
systems, and in the process of updating it to match the fixes from Michał in 
https://reviews.llvm.org/D107475 , I found a few places where the documentation 
needed to be updated to match lldb/gdb's actual behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118842

Files:
  lldb/docs/lldb-platform-packets.txt


Index: lldb/docs/lldb-platform-packets.txt
===
--- lldb/docs/lldb-platform-packets.txt
+++ lldb/docs/lldb-platform-packets.txt
@@ -94,7 +94,7 @@
 // 2. file path in ascii-hex encoding
 //
 //  response is F followed by the return value of the mkdir() call,
-//  base 10 encoded.
+//  base 16 encoded.
 
 //--
 // qPlatform_shell:
@@ -270,7 +270,7 @@
 //  send:Fc008
 //
 //  response is "F" followed by the file size in base 16.
-//  "F-1,errno" with the errno if an error occurs.
+//  "F-1,errno" with the errno if an error occurs, base 16.
 
 
 //--
@@ -286,7 +286,7 @@
 //
 //  response is "F" followed by the mode bits in base 16, this 0x1ed would 
 //  correspond to 0755 in octal.  
-//  "F-1,errno" with the errno if an error occurs.
+//  "F-1,errno" with the errno if an error occurs, base 16.
 
 //--
 // vFile:unlink:
@@ -300,7 +300,9 @@
 //  send:F0
 //
 //  Argument is a file path in ascii-hex encoding.
-//  Response is "F" plus the return value of unlink(), base 10 encoding.
+//  Response is "F" plus the return value of unlink(), base 16 encoding.
+//  Return value may optionally be followed by a comma and the base16
+//  value of errno if unlink failed.
 
 //--
 // vFile:symlink:
@@ -314,7 +316,8 @@
 //  send:F0,0
 //
 //  Argument file paths are in ascii-hex encoding.
-//  Response is "F" plus the return value of symlink(), base 10 encoding, 
twice.
+//  Response is "F" plus the return value of symlink(), base 16 encoding, 
+//  optionally followed by the value of errno if it failed, also base 16.
 
 //--
 // vFile:chmod:
@@ -330,7 +333,7 @@
 //
 //  Arguments are the mode bits to set, base 16, and a file path in 
 //  ascii-hex encoding.
-//  Response is "F" plus the return value of chmod(), base 10 encoding.
+//  Response is "F" plus the return value of chmod(), base 16 encoding.
 //
 //  I don't know why there are two packets for the same thing, v.
 //  vFile:chmod:.
@@ -370,7 +373,7 @@
 // 3. mode bits, base 16
 //  
 //  response is F followed by the opened file descriptor in base 16.
-//  "F-1,errno" with the errno if an error occurs.
+//  "F-1,errno" with the errno if an error occurs, base 16.
 //
 //--
 // vFile:close:
@@ -384,7 +387,8 @@
 //  send:F0
 //
 //  File descriptor is in base 16.
-//  "F-1,errno" with the errno if an error occurs.
+//  "F-1,errno" with the errno if an error occurs,
+//  errno is base 16.
 
 
 //--


Index: lldb/docs/lldb-platform-packets.txt
===
--- lldb/docs/lldb-platform-packets.txt
+++ lldb/docs/lldb-platform-packets.txt
@@ -94,7 +94,7 @@
 // 2. file path in ascii-hex encoding
 //
 //  response is F followed by the return value of the mkdir() call,
-//  base 10 encoded.
+//  base 16 encoded.
 
 //--
 // qPlatform_shell:
@@ -270,7 +270,7 @@
 //  send:Fc008
 //
 //  response is "F" followed by the file size in base 16.
-//  "F-1,errno" with the errno if an error occurs.
+//  "F-1,errno" with the errno if an error occurs, base 16.
 
 
 //--
@@ -286,7 +286,7 @@
 //
 //  response is "F" followed by the mode bits in base 16, this 0x1ed would 
 //  correspond to 0755 in octal.  
-//  "F-1,errno" with the errno if an error occurs.
+//  "F-1,errno" with the errno if an error occurs, base 16.
 
 //--
 // vFile:unlink:
@@ -300,7 +300,9 @@
 //  send:F0
 //
 //  Argument is a file path in ascii-hex encoding.
-//  Response is "F" plus the return value of unlink(), base 10 encoding.
+//  Response is "F" plus the return value of unlink(), base 16 encoding.
+//  Return value may optionally be followed by a com

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Can we put a limit on the length of the kinds of names we are willing to 
demangle in the first place? How long are some of these names _prior_ to 
demangling? It would be great if we could skip demangling names that are too 
long to begin with. That would allow us to skip trying to create the demangled 
name in the first place which is part of the memory problem right? Once the 
mangled name has been added to the ConstString pool it is already too late and 
we won't save any memory.


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

https://reviews.llvm.org/D118812

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

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D118812#3291937 , @clayborg wrote:

> Can we put a limit on the length of the kinds of names we are willing to 
> demangle in the first place? How long are some of these names _prior_ to 
> demangling? It would be great if we could skip demangling names that are too 
> long to begin with. That would allow us to skip trying to create the 
> demangled name in the first place which is part of the memory problem right? 
> Once the mangled name has been added to the ConstString pool it is already 
> too late and we won't save any memory.

Yep, that's exactly what this patch does: it checks the length of the mangled 
name and skips demangling if they are too long. It only does this while 
building the index though, so that if we need the demangled name elsewere we'll 
still cache it in the string pool.


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

https://reviews.llvm.org/D118812

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

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D118812#3291482 , @dblaikie wrote:

> In D118812#3291303 , @jingham wrote:
>
>> In D118812#3291109 , @dblaikie 
>> wrote:
>>
>>> Any chance you might want a limit on the size of the demangled name too? 
>>> (might be worth considering what the most densely encoded mangled name is 
>>> (ie: what's the longest name that could be produced by a 10k long mangled 
>>> name? and see if that's worth having another cutoff for)
>>
>> Ironically, lldb seldom cares about most of the goo in these long demangled 
>> names.  At this point, we are building up our fast-lookup "name indexes".  
>> We really only care about extracting the fully scoped names of the methods.  
>> When we get around to doing smart matching on overloads, we can still pull 
>> out all the matches to the method name, and then do the overload match on 
>> the results.  That should be sufficiently efficient, and obviate the need to 
>> do any fancy indexing based on overloads.  So most of the work of demangling 
>> these names is not being used anyway.
>>
>> So what would be the better solution for lldb on the demangling front would 
>> be a way to tell the demangler "only extract the full method name, and don't 
>> bother producing the argument list or return values".  But I have no idea 
>> how easy that would be in the demangler.
>
> I think there's an API level of the demangler in LLVM designed for rewriting 
> demangled names (@rsmith created/implemented that, I think) - I'm not sure if 
> it's structured to allow lazy parsing/stopping after you get the base name, 
> for instance, but maybe...

We should definitely look into that as a general optimization for indexing the 
string table and would make sense in combination with D118814 
. For this particular patch, we're trying to 
avoid demangling at all if the symbol is too long, so unless a partial demangle 
is really cheap (it might be) we'd still want to exclude symbols based on their 
mangled length.


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

https://reviews.llvm.org/D118812

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

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D118812#3291950 , @JDevlieghere 
wrote:

> In D118812#3291937 , @clayborg 
> wrote:
>
>> Can we put a limit on the length of the kinds of names we are willing to 
>> demangle in the first place? How long are some of these names _prior_ to 
>> demangling? It would be great if we could skip demangling names that are too 
>> long to begin with. That would allow us to skip trying to create the 
>> demangled name in the first place which is part of the memory problem right? 
>> Once the mangled name has been added to the ConstString pool it is already 
>> too late and we won't save any memory.
>
> Yep, that's exactly what this patch does: it checks the length of the mangled 
> name and skips demangling if they are too long. It only does this while 
> building the index though, so that if we need the demangled name elsewere 
> we'll still cache it in the string pool.

Yikes sorry, I read line Symtab.cpp:311 as if it was demangling the name first 
and then checking _its_ length...

The settings leads one to believe that we don't ever demangle names longer than 
X characters, but it really means "don't auto demangle when indexing the symbol 
table". Should we actually stop the names from ever being demangled in 
"Mangled::GetDemangledName()" to force us to never try and demangle these kinds 
of names ever? We would need to have the setting modify a static value in that 
Mangled class when the setting gets changed so that the 
Mangled::GetDemangledName() function wouldn't have to try and access the 
settings each time someone wanted to demangle something. Then we will still 
display the mangled name in stack traces, but we won't ever waste time trying 
to demangle these names. When they are that long, they are really not useful at 
all for me at least.


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

https://reviews.llvm.org/D118812

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

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D118812#3291969 , @clayborg wrote:

> Yikes sorry, I read line Symtab.cpp:311 as if it was demangling the name 
> first and then checking _its_ length...
>
> The settings leads one to believe that we don't ever demangle names longer 
> than X characters, but it really means "don't auto demangle when indexing the 
> symbol table". Should we actually stop the names from ever being demangled in 
> "Mangled::GetDemangledName()" to force us to never try and demangle these 
> kinds of names ever? We would need to have the setting modify a static value 
> in that Mangled class when the setting gets changed so that the 
> Mangled::GetDemangledName() function wouldn't have to try and access the 
> settings each time someone wanted to demangle something. Then we will still 
> display the mangled name in stack traces, but we won't ever waste time trying 
> to demangle these names. When they are that long, they are really not useful 
> at all for me at least.

Yeah, I considered that but I noticed some uses of the demangled names in the 
language plugins and I wasn't sure if that was going to break. If we think that 
it never makes sense to demangle these name, I'm happy to move it into 
Demangled and make it so that we never demangle these.


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

https://reviews.llvm.org/D118812

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

[Lldb-commits] [lldb] f3e1ba1 - [LLDB] add sub regigter enums on x64 Windows

[Zequan Wu via lldb-commits]

Author: Zequan Wu
Date: 2022-02-02T14:21:08-08:00
New Revision: f3e1ba1d03927a35c23c27c896a13f326f28864a

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

LOG: [LLDB] add sub regigter enums on x64 Windows

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

Added: 


Modified: 

lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
lldb/test/Shell/Register/x86-64-gp-read.test

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp 
b/lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
index 41e0d429fe52..3230e477f14d 100644
--- 
a/lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
+++ 
b/lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
@@ -24,7 +24,14 @@
 using namespace lldb;
 using namespace lldb_private;
 
-#define DEFINE_GPR(reg, alt) #reg, alt, 8, 0, eEncodingUint, 
eFormatHexUppercase
+#define DEFINE_GPR(reg, alt, generic)  
\
+{  
\
+  #reg, alt, 8, 0, eEncodingUint, eFormatHexUppercase, 
\
+  {dwarf_##reg##_x86_64, dwarf_##reg##_x86_64, generic,
\
+LLDB_INVALID_REGNUM, lldb_##reg##_x86_64 },
\
+nullptr, nullptr,  
\
+}
+
 #define DEFINE_GPR_BIN(reg, alt) #reg, alt, 8, 0, eEncodingUint, eFormatBinary
 #define DEFINE_FPU_XMM(reg)
\
   #reg, NULL, 16, 0, eEncodingUint, eFormatVectorOfUInt64, 
\
@@ -32,6 +39,30 @@ using namespace lldb_private;
LLDB_INVALID_REGNUM, lldb_##reg##_x86_64},  
\
   nullptr, nullptr
 
+#define DEFINE_GPR_PSEUDO_32(reg)  
\
+{  
\
+  #reg, nullptr, 4, 0, eEncodingUint, eFormatHexUppercase, 
\
+  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,  
\
+LLDB_INVALID_REGNUM, lldb_##reg##_x86_64 },
\
+nullptr, nullptr   
\
+}
+
+#define DEFINE_GPR_PSEUDO_16(reg)  
\
+{  
\
+  #reg, nullptr, 2, 0, eEncodingUint, eFormatHexUppercase, 
\
+  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,  
\
+LLDB_INVALID_REGNUM, lldb_##reg##_x86_64 },
\
+nullptr, nullptr   
\
+}
+
+#define DEFINE_GPR_PSEUDO_8(reg)   
\
+{  
\
+  #reg, nullptr, 1, 0, eEncodingUint, eFormatHexUppercase, 
\
+  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,  
\
+LLDB_INVALID_REGNUM, lldb_##reg##_x86_64 },
\
+nullptr, nullptr   
\
+}
+
 namespace {
 
 // This enum defines the layout of the global RegisterInfo array.  This is
@@ -59,6 +90,59 @@ enum RegisterIndex {
   eRegisterIndexR15,
   eRegisterIndexRip,
   eRegisterIndexRflags,
+  eRegisterIndexEax,
+  eRegisterIndexEbx,
+  eRegisterIndexEcx,
+  eRegisterIndexEdx,
+  eRegisterIndexEdi,
+  eRegisterIndexEsi,
+  eRegisterIndexEbp,
+  eRegisterIndexEsp,
+  eRegisterIndexR8d,
+  eRegisterIndexR9d,
+  eRegisterIndexR10d,
+  eRegisterIndexR11d,
+  eRegisterIndexR12d,
+  eRegisterIndexR13d,
+  eRegisterIndexR14d,
+  eRegisterIndexR15d,
+  eRegisterIndexAx,
+  eRegisterIndexBx,
+  eRegisterIndexCx,
+  eRegisterIndexDx,
+  eRegisterIndexDi,
+  eRegisterIndexSi,
+  eRegisterIndexBp,
+  eRegisterIndexSp,
+  eRegisterIndexR8w,
+  eRegisterIndexR9w,
+  eRegisterIndexR10w,
+  eRegisterIndexR11w,
+  eRegisterIndexR12w,
+  eRegisterIndexR13w,
+  eRegisterIndexR14w,
+  eRegisterIndexR15w,
+  eRegisterIndexAh,
+  eRegisterIndexBh,
+  eRegisterIndexCh,
+  eRegisterIndexDh,
+  eRegisterIndexAl,
+  eRegisterIndexBl,
+  eRegisterIndexCl,
+  eRegisterIndexDl,
+  eRegisterIndexDil,
+  eRegisterIndexSil,
+  eRegisterIndexBpl,
+  eRegisterIndexSpl,
+  eRegisterIndexR8l,
+  eRegisterIndexR9l,
+  eRegisterIndexR10l,
+  eRegisterIndexR11l,
+  eRegisterIndexR12l,
+  eRegisterIndexR13l,
+  eRegisterIndexR14l,
+  eRegisterIndexR15l,
+  eRegisterIndexR16l,
 
   eRegisterIn

[Lldb-commits] [PATCH] D118750: [LLDB] add sub register enums on x64 Windows

[Zequan Wu via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf3e1ba1d0392: [LLDB] add sub regigter enums on x64 Windows 
(authored by zequanwu).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118750

Files:
  lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
  lldb/test/Shell/Register/x86-64-gp-read.test

Index: lldb/test/Shell/Register/x86-64-gp-read.test
===
--- lldb/test/Shell/Register/x86-64-gp-read.test
+++ lldb/test/Shell/Register/x86-64-gp-read.test
@@ -1,4 +1,3 @@
-# XFAIL: system-windows
 # REQUIRES: native && target-x86_64
 # RUN: %clangxx_host -fomit-frame-pointer %p/Inputs/x86-64-gp-read.cpp -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
Index: lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
+++ lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
@@ -24,7 +24,14 @@
 using namespace lldb;
 using namespace lldb_private;
 
-#define DEFINE_GPR(reg, alt) #reg, alt, 8, 0, eEncodingUint, eFormatHexUppercase
+#define DEFINE_GPR(reg, alt, generic)  \
+{  \
+  #reg, alt, 8, 0, eEncodingUint, eFormatHexUppercase, \
+  {dwarf_##reg##_x86_64, dwarf_##reg##_x86_64, generic,\
+LLDB_INVALID_REGNUM, lldb_##reg##_x86_64 },\
+nullptr, nullptr,  \
+}
+
 #define DEFINE_GPR_BIN(reg, alt) #reg, alt, 8, 0, eEncodingUint, eFormatBinary
 #define DEFINE_FPU_XMM(reg)\
   #reg, NULL, 16, 0, eEncodingUint, eFormatVectorOfUInt64, \
@@ -32,6 +39,30 @@
LLDB_INVALID_REGNUM, lldb_##reg##_x86_64},  \
   nullptr, nullptr
 
+#define DEFINE_GPR_PSEUDO_32(reg)  \
+{  \
+  #reg, nullptr, 4, 0, eEncodingUint, eFormatHexUppercase, \
+  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,  \
+LLDB_INVALID_REGNUM, lldb_##reg##_x86_64 },\
+nullptr, nullptr   \
+}
+
+#define DEFINE_GPR_PSEUDO_16(reg)  \
+{  \
+  #reg, nullptr, 2, 0, eEncodingUint, eFormatHexUppercase, \
+  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,  \
+LLDB_INVALID_REGNUM, lldb_##reg##_x86_64 },\
+nullptr, nullptr   \
+}
+
+#define DEFINE_GPR_PSEUDO_8(reg)   \
+{  \
+  #reg, nullptr, 1, 0, eEncodingUint, eFormatHexUppercase, \
+  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,  \
+LLDB_INVALID_REGNUM, lldb_##reg##_x86_64 },\
+nullptr, nullptr   \
+}
+
 namespace {
 
 // This enum defines the layout of the global RegisterInfo array.  This is
@@ -59,6 +90,59 @@
   eRegisterIndexR15,
   eRegisterIndexRip,
   eRegisterIndexRflags,
+  eRegisterIndexEax,
+  eRegisterIndexEbx,
+  eRegisterIndexEcx,
+  eRegisterIndexEdx,
+  eRegisterIndexEdi,
+  eRegisterIndexEsi,
+  eRegisterIndexEbp,
+  eRegisterIndexEsp,
+  eRegisterIndexR8d,
+  eRegisterIndexR9d,
+  eRegisterIndexR10d,
+  eRegisterIndexR11d,
+  eRegisterIndexR12d,
+  eRegisterIndexR13d,
+  eRegisterIndexR14d,
+  eRegisterIndexR15d,
+  eRegisterIndexAx,
+  eRegisterIndexBx,
+  eRegisterIndexCx,
+  eRegisterIndexDx,
+  eRegisterIndexDi,
+  eRegisterIndexSi,
+  eRegisterIndexBp,
+  eRegisterIndexSp,
+  eRegisterIndexR8w,
+  eRegisterIndexR9w,
+  eRegisterIndexR10w,
+  eRegisterIndexR11w,
+  eRegisterIndexR12w,
+  eRegisterIndexR13w,
+  eRegisterIndexR14w,
+  eRegisterIndexR15w,
+  eRegisterIndexAh,
+  eRegisterIndexBh,
+  eRegisterIndexCh,
+  eRegisterIndexDh,
+  eRegisterIndexAl,
+  eRegisterIndexBl,
+  eRegisterIndexCl,
+  eRegisterIndexDl,
+  eRegisterIndexDil,
+  eRegisterIndexSil,
+  eRegisterIndexBpl,
+  eRegisterIndexSpl,
+  eRegisterIndexR8l,
+  eRegisterIndexR9l,
+  eRegisterIndexR10l,
+  eRegisterIndexR11l

[Lldb-commits] [PATCH] D118750: [LLDB] add sub register enums on x64 Windows

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.
Herald added a subscriber: JDevlieghere.

This broke the windows bot in the best way possible - by making some tests pass 
now that didn't use to. Could you un-xfail those too?

https://lab.llvm.org/buildbot/#/builders/83/builds/14736


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118750

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

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

If we can distinguish between "when we handle all mangled names" and "when we 
handle one" I think we should continue to demangle names in the "when we handle 
one" case, since you never know when somebody really might need to look at the 
whole name.  But OTOH that's probably seldom, so if there's no good way to 
distinguish, I'm also fine with never demangling over some limit.


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

https://reviews.llvm.org/D118812

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

[Lldb-commits] [PATCH] D118866: [LLDB] Fix window bot failure

[Zequan Wu via Phabricator via lldb-commits]

zequanwu created this revision.
zequanwu added a reviewer: stella.stamenova.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This attempts to fix this bot failure: 
https://lab.llvm.org/buildbot/#/builders/83/builds/14736 caused by D118750 
 by un-xfail those expected failed tests.

I failed to find following tests:
test_inferior_crashing_register_dwarf
test_recursive_inferior_crashing_register_dwarf
test_inferior_crashing_register_dwarf
test_inferior_crashing_step_dwarf
test_recursive_inferior_crashing_step_dwarf

Do you know where those tests are?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118866

Files:
  lldb/test/API/commands/register/register/register_command/TestRegisters.py


Index: 
lldb/test/API/commands/register/register/register_command/TestRegisters.py
===
--- lldb/test/API/commands/register/register/register_command/TestRegisters.py
+++ lldb/test/API/commands/register/register/register_command/TestRegisters.py
@@ -120,7 +120,6 @@
 
 @skipIfiOSSimulator
 @skipIf(archs=no_match(['amd64', 'x86_64']))
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr37683")
 def test_convenience_registers_with_process_attach(self):
 """Test convenience registers after a 'process attach'."""
 self.build()
@@ -128,7 +127,6 @@
 
 @skipIfiOSSimulator
 @skipIf(archs=no_match(['amd64', 'x86_64']))
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr37683")
 def test_convenience_registers_16bit_with_process_attach(self):
 """Test convenience registers after a 'process attach'."""
 self.build()


Index: lldb/test/API/commands/register/register/register_command/TestRegisters.py
===
--- lldb/test/API/commands/register/register/register_command/TestRegisters.py
+++ lldb/test/API/commands/register/register/register_command/TestRegisters.py
@@ -120,7 +120,6 @@
 
 @skipIfiOSSimulator
 @skipIf(archs=no_match(['amd64', 'x86_64']))
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr37683")
 def test_convenience_registers_with_process_attach(self):
 """Test convenience registers after a 'process attach'."""
 self.build()
@@ -128,7 +127,6 @@
 
 @skipIfiOSSimulator
 @skipIf(archs=no_match(['amd64', 'x86_64']))
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr37683")
 def test_convenience_registers_16bit_with_process_attach(self):
 """Test convenience registers after a 'process attach'."""
 self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D118866: [LLDB] Fix window bot failure

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

They should correspond to these files:

  lldb-api :: functionalities/inferior-crashing/TestInferiorCrashing.py => 
test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
  lldb-api :: functionalities/inferior-crashing/TestInferiorCrashingStep.py => 
test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
  lldb-api :: 
functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferior.py 
=> 
test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferior.py
  lldb-api :: 
functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
 => 
test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118866

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

[Lldb-commits] [PATCH] D118866: [LLDB] Fix window bot failure

[Zequan Wu via Phabricator via lldb-commits]

zequanwu updated this revision to Diff 405505.
zequanwu added a comment.

Un-xfail more tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118866

Files:
  lldb/test/API/commands/register/register/register_command/TestRegisters.py
  lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
  lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
  
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferior.py
  
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py


Index: 
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
===
--- 
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
+++ 
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
@@ -12,7 +12,6 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
 def test_recursive_inferior_crashing_step(self):
 """Test that stepping after a crash behaves correctly."""
 self.build()
Index: 
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferior.py
===
--- 
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferior.py
+++ 
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferior.py
@@ -20,7 +20,6 @@
 self.build()
 self.recursive_inferior_crashing()
 
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
 def test_recursive_inferior_crashing_register(self):
 """Test that lldb reliably reads registers from the inferior after 
crashing (command)."""
 self.build()
Index: 
lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
===
--- lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
+++ lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
@@ -19,7 +19,6 @@
 self.build()
 self.inferior_crashing()
 
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
 def test_inferior_crashing_register(self):
 """Test that lldb reliably reads registers from the inferior after 
crashing (command)."""
 self.build()
@@ -36,7 +35,6 @@
 self.build()
 self.inferior_crashing_expr()
 
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
 def test_inferior_crashing_step(self):
 """Test that stepping after a crash behaves correctly."""
 self.build()
Index: lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
===
--- lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
+++ lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
@@ -20,7 +20,6 @@
 self.build()
 self.inferior_crashing()
 
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
 def test_inferior_crashing_register(self):
 """Test that lldb reliably reads registers from the inferior after 
crashing (command)."""
 self.build()
Index: 
lldb/test/API/commands/register/register/register_command/TestRegisters.py
===
--- lldb/test/API/commands/register/register/register_command/TestRegisters.py
+++ lldb/test/API/commands/register/register/register_command/TestRegisters.py
@@ -120,7 +120,6 @@
 
 @skipIfiOSSimulator
 @skipIf(archs=no_match(['amd64', 'x86_64']))
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr37683")
 def test_convenience_registers_with_process_attach(self):
 """Test convenience registers after a 'process attach'."""
 self.build()
@@ -128,7 +127,6 @@
 
 @skipIfiOSSimulator
 @skipIf(archs=no_match(['amd64', 'x86_64']))
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr37683")
 def test_convenience_registers_16bit_with_process_attach(self):
 """Test convenience registers after a 'process attach'."""
 self.build()


Index: lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
===
--- lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
+++ lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
@@ -12,7 +12,6 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
-@expectedFailu

[Lldb-commits] [PATCH] D118842: Small fixes to platform packet documentation

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

FWIW I started updating my implementation by reading through all the changes to 
the docs, and implementing them.  Then I went back and looked at the patches 
and found some small changes that were made but without documentation being 
updated, or the documentation not being specific about expected details.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118842

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