[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack added a comment.

@yinghuitan yup, `RetryAfterSignal::Open` helper functions look better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152712

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

[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack updated this revision to Diff 531186.
splhack added a comment.

Add lldb/include/lldb/Host/common/RetryAfterSignal.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152712

Files:
  lldb/include/lldb/Host/common/RetryAfterSignal.h
  lldb/source/Host/common/PseudoTerminal.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Host/posix/FileSystemPosix.cpp
  lldb/source/Host/posix/PipePosix.cpp
  lldb/source/Host/posix/ProcessLauncherPosixFork.cpp

Index: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
===
--- lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Host/Pipe.h"
 #include "lldb/Host/ProcessLaunchInfo.h"
+#include "lldb/Host/common/RetryAfterSignal.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/Support/Errno.h"
@@ -71,7 +72,7 @@
 }
 
 static void DupDescriptor(int error_fd, const char *file, int fd, int flags) {
-  int target_fd = llvm::sys::RetryAfterSignal(-1, ::open, file, flags, 0666);
+  int target_fd = RetryAfterSignal::Open(file, flags, 0666);
 
   if (target_fd == -1)
 ExitWithError(error_fd, "DupDescriptor-open");
Index: lldb/source/Host/posix/PipePosix.cpp
===
--- lldb/source/Host/posix/PipePosix.cpp
+++ lldb/source/Host/posix/PipePosix.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Host/posix/PipePosix.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Host/common/RetryAfterSignal.h"
 #include "lldb/Utility/SelectHelper.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Errno.h"
@@ -148,7 +149,7 @@
 flags |= O_CLOEXEC;
 
   Status error;
-  int fd = llvm::sys::RetryAfterSignal(-1, ::open, name.str().c_str(), flags);
+  int fd = RetryAfterSignal::Open(name.str().c_str(), flags);
   if (fd != -1)
 m_fds[READ] = fd;
   else
Index: lldb/source/Host/posix/FileSystemPosix.cpp
===
--- lldb/source/Host/posix/FileSystemPosix.cpp
+++ lldb/source/Host/posix/FileSystemPosix.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "lldb/Host/FileSystem.h"
+#include "lldb/Host/common/RetryAfterSignal.h"
 
 // C includes
 #include 
@@ -77,5 +78,5 @@
 }
 
 int FileSystem::Open(const char *path, int flags, int mode) {
-  return llvm::sys::RetryAfterSignal(-1, ::open, path, flags, mode);
+  return RetryAfterSignal::Open(path, flags, mode);
 }
Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -13,6 +13,7 @@
 #define _DARWIN_UNLIMITED_SELECT
 #endif
 
+#include "lldb/Host/common/RetryAfterSignal.h"
 #include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Host/Socket.h"
@@ -726,7 +727,7 @@
 #if LLDB_ENABLE_POSIX
   std::string addr_str = s.str();
   // file:///PATH
-  int fd = llvm::sys::RetryAfterSignal(-1, ::open, addr_str.c_str(), O_RDWR);
+  int fd = RetryAfterSignal::Open(addr_str.c_str(), O_RDWR);
   if (fd == -1) {
 if (error_ptr)
   error_ptr->SetErrorToErrno();
@@ -776,7 +777,7 @@
 return eConnectionStatusError;
   }
 
-  int fd = llvm::sys::RetryAfterSignal(-1, ::open, path.str().c_str(), O_RDWR);
+  int fd = RetryAfterSignal::Open(path.str().c_str(), O_RDWR);
   if (fd == -1) {
 if (error_ptr)
   error_ptr->SetErrorToErrno();
Index: lldb/source/Host/common/PseudoTerminal.cpp
===
--- lldb/source/Host/common/PseudoTerminal.cpp
+++ lldb/source/Host/common/PseudoTerminal.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/Config.h"
+#include "lldb/Host/common/RetryAfterSignal.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Errno.h"
 #include 
@@ -95,7 +96,7 @@
   CloseSecondaryFileDescriptor();
 
   std::string name = GetSecondaryName();
-  m_secondary_fd = llvm::sys::RetryAfterSignal(-1, ::open, name.c_str(), oflag);
+  m_secondary_fd = RetryAfterSignal::Open(name.c_str(), oflag);
   if (m_secondary_fd >= 0)
 return llvm::Error::success();
 
Index: lldb/include/lldb/Host/common/RetryAfterSignal.h
===
--- /dev/null
+++ lldb/include/lldb/Host/common/RetryAfterSignal.h
@@ -0,0 +1,35 @@
+//===-- RetryAfterSignal.h --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// 

[Lldb-commits] [PATCH] D152886: [lldb] Make it easier to spot if sources were resolved in crashlog output

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/examples/python/crashlog.py:340-343
+for key in path_remapping:
+source_path = os.path.expanduser(
+path_remapping[key]
+)





Comment at: lldb/examples/python/crashlog.py:344
+)
+if os.path.exists(source_path):
+self.resolved_source = True

I guess that checks that the user have access to the remapped path, is that 
right ?

If that's not the case, I think we should let the user know


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

https://reviews.llvm.org/D152886

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

[Lldb-commits] [PATCH] D152886: [lldb] Make it easier to spot if sources were resolved in crashlog output

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: mib, aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

It can be tricky to troubleshoot why the crashlog script can't show inline 
sources. The two most common causes are that we couldn't find the dSYM or, if 
we find the dSYM, that the path remapping included in the dSYMForUUID output 
isn't correct. The former is relatively easy to diagnose thanks to the messages 
printed by the crashlog script. The latter is harder, because you have to 
figure out the remapped source path. This patch tries to make it easier to 
diagnose the second issue by including whether the path in the source remapping 
is accessible. If at least one of the paths exists, we consider the image to 
have sources, and include that in the symbol resolution output.

Example output:

  Resolved symbols and sources for ---- 
/path/to/foo
  Resolved symbols s for ---- /path/to/bar




https://reviews.llvm.org/D152886

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/symbolication.py


Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -257,6 +257,7 @@
 self.resolved_path = None
 self.resolve = False
 self.resolved = False
+self.resolved_source = False
 self.unavailable = False
 self.uuid = uuid
 self.section_infos = list()
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -335,6 +335,15 @@
 plist["DBGSymbolRichExecutable"]
 )
 self.resolved_path = self.path
+if "DBGSourcePathRemapping" in plist:
+path_remapping = 
plist["DBGSourcePathRemapping"]
+for key in path_remapping:
+source_path = os.path.expanduser(
+path_remapping[key]
+)
+if os.path.exists(source_path):
+self.resolved_source = True
+
 if not self.resolved_path and os.path.exists(self.path):
 if not self.find_matching_slice():
 return False
@@ -372,7 +381,11 @@
 self.path and os.path.exists(self.path)
 ):
 with print_lock:
-print("Resolved symbols for %s %s..." % (uuid_str, 
self.path))
+source_resolved = "and sources " if self.resolved_source 
else ""
+print(
+"Resolved symbols %sfor %s %s..."
+% (source_resolved, uuid_str, self.path)
+)
 return True
 else:
 self.unavailable = True


Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -257,6 +257,7 @@
 self.resolved_path = None
 self.resolve = False
 self.resolved = False
+self.resolved_source = False
 self.unavailable = False
 self.uuid = uuid
 self.section_infos = list()
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -335,6 +335,15 @@
 plist["DBGSymbolRichExecutable"]
 )
 self.resolved_path = self.path
+if "DBGSourcePathRemapping" in plist:
+path_remapping = plist["DBGSourcePathRemapping"]
+for key in path_remapping:
+source_path = os.path.expanduser(
+path_remapping[key]
+)
+if os.path.exists(source_path):
+self.resolved_source = True
+
 if not self.resolved_path and os.path.exists(self.path):
 if not self.find_matching_slice():
 return False
@@ -372,7 +381,11 @@
 self.path and os.path.exists(self.path)
 ):
 with print_lock:
-print("Resolved symbols for %s %s..." % (uuid_str, self.path))
+source_resolved = "and sources " if self.resolved_source 

[Lldb-commits] [lldb] 1ae6a78 - [lldb] Fix Debugger whitespace and formatting (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-06-13T20:50:30-07:00
New Revision: 1ae6a782f096a9870c3f5bd7c3489abeaa238d0f

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

LOG: [lldb] Fix Debugger whitespace and formatting (NFC)

Remove trailing whitespace and fix formatting.

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/source/Core/Debugger.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 4005bdbddacca..64d9fad8de20d 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -52,7 +52,7 @@
 namespace llvm {
 class raw_ostream;
 class ThreadPool;
-}
+} // namespace llvm
 
 namespace lldb_private {
 class Address;
@@ -377,19 +377,19 @@ class Debugger : public 
std::enable_shared_from_this,
   bool IsHandlingEvents() const { return m_event_handler_thread.IsJoinable(); }
 
   Status RunREPL(lldb::LanguageType language, const char *repl_options);
-  
+
   /// Interruption in LLDB:
-  /// 
+  ///
   /// This is a voluntary interruption mechanism, not preemptive.  Parts of 
lldb
-  /// that do work that can be safely interrupted call 
+  /// that do work that can be safely interrupted call
   /// Debugger::InterruptRequested and if that returns true, they should return
   /// at a safe point, shortcutting the rest of the work they were to do.
-  ///  
-  /// lldb clients can both offer a CommandInterpreter (through 
+  ///
+  /// lldb clients can both offer a CommandInterpreter (through
   /// RunCommandInterpreter) and use the SB API's for their own purposes, so it
   /// is convenient to separate "interrupting the CommandInterpreter execution"
-  /// and interrupting the work it is doing with the SB API's.  So there are 
two 
-  /// ways to cause an interrupt: 
+  /// and interrupting the work it is doing with the SB API's.  So there are 
two
+  /// ways to cause an interrupt:
   ///   * CommandInterpreter::InterruptCommand: Interrupts the command 
currently
   /// running in the command interpreter IOHandler thread
   ///   * Debugger::RequestInterrupt: Interrupts are active on anything but the
@@ -398,7 +398,6 @@ class Debugger : public 
std::enable_shared_from_this,
   /// Since the two checks are mutually exclusive, however, it's also 
convenient
   /// to have just one function to check the interrupt state.
 
-
   /// Bump the "interrupt requested" count on the debugger to support
   /// cooperative interruption.  If this is non-zero, InterruptRequested will
   /// return true.  Interruptible operations are expected to query the
@@ -406,13 +405,13 @@ class Debugger : public 
std::enable_shared_from_this,
   /// if it returns \b true.
   ///
   void RequestInterrupt();
-  
+
   /// Decrement the "interrupt requested" counter.
   void CancelInterruptRequest();
-  
+
   /// This is the correct way to query the state of Interruption.
-  /// If you are on the RunCommandInterpreter thread, it will check the 
-  /// command interpreter state, and if it is on another thread it will 
+  /// If you are on the RunCommandInterpreter thread, it will check the
+  /// command interpreter state, and if it is on another thread it will
   /// check the debugger Interrupt Request state.
   ///
   /// \return
@@ -574,13 +573,13 @@ class Debugger : public 
std::enable_shared_from_this,
   bool StartIOHandlerThread();
 
   void StopIOHandlerThread();
-  
+
   // Sets the IOHandler thread to the new_thread, and returns
   // the previous IOHandler thread.
   HostThread SetIOHandlerThread(HostThread _thread);
 
   void JoinIOHandlerThread();
-  
+
   bool IsIOHandlerThreadCurrentThread() const;
 
   lldb::thread_result_t IOHandlerThread();

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 6f43cc608c2ce..2487304dfc199 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -236,8 +236,10 @@ Status Debugger::SetPropertyValue(const ExecutionContext 
*exe_ctx,
   // use-color changed. Ping the prompt so it can reset the ansi terminal
   // codes.
   SetPrompt(GetPrompt());
-} else if (property_path == 
g_debugger_properties[ePropertyUseSourceCache].name) {
-  // use-source-cache changed. Wipe out the cache contents if it was 
disabled.
+} else if (property_path ==
+   g_debugger_properties[ePropertyUseSourceCache].name) {
+  // use-source-cache changed. Wipe out the cache contents if it was
+  // disabled.
   if (!GetUseSourceCache()) {
 m_source_file_cache.Clear();
   }
@@ -1707,7 +1709,7 @@ void Debugger::HandleProcessEvent(const EventSP 
_sp) {
 
 // Display running state changes first before any STDIO
 if (got_state_changed && !state_is_stopped) {
-  // This is a 

[Lldb-commits] [lldb] d132b85 - [lldb] Include in LLDBAssert

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-06-13T20:50:29-07:00
New Revision: d132b854b3cbd094a06023f394d9b3dfd44e9b4f

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

LOG: [lldb] Include  in LLDBAssert

Added: 


Modified: 
lldb/source/Utility/LLDBAssert.cpp

Removed: 




diff  --git a/lldb/source/Utility/LLDBAssert.cpp 
b/lldb/source/Utility/LLDBAssert.cpp
index 29bcacb6ad24c..4ecd6043e8ea6 100644
--- a/lldb/source/Utility/LLDBAssert.cpp
+++ b/lldb/source/Utility/LLDBAssert.cpp
@@ -16,6 +16,8 @@
 #include 
 #endif
 
+#include 
+
 namespace lldb_private {
 
 static void DefaultAssertCallback(llvm::StringRef message,



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

[Lldb-commits] [PATCH] D152866: [lldb] Print lldbassert to debugger diagnostics

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG83a6a0a62047: [lldb] Print lldbassert to debugger 
diagnostics (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D152866?vs=531126=531161#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152866

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/LLDBAssert.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/LLDBAssert.cpp

Index: lldb/source/Utility/LLDBAssert.cpp
===
--- lldb/source/Utility/LLDBAssert.cpp
+++ lldb/source/Utility/LLDBAssert.cpp
@@ -8,7 +8,7 @@
 
 #include "lldb/Utility/LLDBAssert.h"
 #include "llvm/Config/llvm-config.h"
-#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -16,12 +16,21 @@
 #include 
 #endif
 
-using namespace llvm;
-using namespace lldb_private;
+namespace lldb_private {
 
-void lldb_private::lldb_assert(bool expression, const char *expr_text,
-   const char *func, const char *file,
-   unsigned int line) {
+static void DefaultAssertCallback(llvm::StringRef message,
+  llvm::StringRef backtrace,
+  llvm::StringRef prompt) {
+  llvm::errs() << message << '\n';
+  llvm::errs() << backtrace; // Backtrace includes a newline.
+  llvm::errs() << prompt << '\n';
+}
+
+static std::atomic g_lldb_assert_callback =
+
+
+void lldb_assert(bool expression, const char *expr_text, const char *func,
+ const char *file, unsigned int line) {
   if (LLVM_LIKELY(expression))
 return;
 
@@ -35,10 +44,21 @@
 
   // Print a warning and encourage the user to file a bug report, similar to
   // LLVM’s crash handler, and then return execution.
-  errs() << format("Assertion failed: (%s), function %s, file %s, line %u\n",
-   expr_text, func, file, line);
-  errs() << "backtrace leading to the failure:\n";
-  llvm::sys::PrintStackTrace(errs());
-  errs() << "please file a bug report against lldb reporting this failure "
-"log, and as many details as possible\n";
+  std::string buffer;
+  llvm::raw_string_ostream backtrace(buffer);
+  llvm::sys::PrintStackTrace(backtrace);
+
+  (*g_lldb_assert_callback.load())(
+  llvm::formatv("Assertion failed: ({0}), function {1}, file {2}, line {3}",
+expr_text, func, file, line)
+  .str(),
+  backtrace.str(),
+  "Please file a bug report against lldb reporting this failure log, and "
+  "as many details as possible");
 }
+
+void SetLLDBAssertCallback(LLDBAssertCallback callback) {
+  g_lldb_assert_callback.exchange(callback);
+}
+
+} // namespace lldb_private
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1352,6 +1352,13 @@
   function_changed, initial_function);
 }
 
+void Debugger::AssertCallback(llvm::StringRef message,
+  llvm::StringRef backtrace,
+  llvm::StringRef prompt) {
+  Debugger::ReportError(
+  llvm::formatv("{0}\n{1}{2}", message, backtrace, prompt).str());
+}
+
 void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
   void *baton) {
   // For simplicity's sake, I am not going to deal with how to close down any
Index: lldb/source/API/SystemInitializerFull.cpp
===
--- lldb/source/API/SystemInitializerFull.cpp
+++ lldb/source/API/SystemInitializerFull.cpp
@@ -78,6 +78,9 @@
   // Settings must be initialized AFTER PluginManager::Initialize is called.
   Debugger::SettingsInitialize();
 
+  // Use the Debugger's LLDBAssert callback.
+  SetLLDBAssertCallback(Debugger::AssertCallback);
+
   return llvm::Error::success();
 }
 
Index: lldb/include/lldb/Utility/LLDBAssert.h
===
--- lldb/include/lldb/Utility/LLDBAssert.h
+++ lldb/include/lldb/Utility/LLDBAssert.h
@@ -9,6 +9,8 @@
 #ifndef LLDB_UTILITY_LLDBASSERT_H
 #define LLDB_UTILITY_LLDBASSERT_H
 
+#include "llvm/ADT/StringRef.h"
+
 #ifndef NDEBUG
 #define lldbassert(x) assert(x)
 #else
@@ -29,6 +31,12 @@
 namespace lldb_private {
 void lldb_assert(bool expression, const char *expr_text, const char *func,
  const char *file, unsigned int line);
+
+typedef void (*LLDBAssertCallback)(llvm::StringRef message,
+   llvm::StringRef backtrace,
+   llvm::StringRef 

[Lldb-commits] [lldb] 83a6a0a - [lldb] Print lldbassert to debugger diagnostics

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-06-13T20:46:33-07:00
New Revision: 83a6a0a620476d2fa181cb663ccbf06cc9d7a24f

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

LOG: [lldb] Print lldbassert to debugger diagnostics

When hitting an lldbassert in a non-assert build, we emit a blurb
including the assertion, the triggering file and line and a pretty
backtrace leading up to the issue. Currently, this is all printed to
stderr. That's fine on the command line, but when used as library, for
example from Xcode, this information doesn't make it to the user. This
patch uses the diagnostic infrastructure to report LLDB asserts as
diagnostic events.

The patch is slightly more complicated than I would've liked because of
layering. lldbassert is part of Utility while the debugger diagnostics
are implemented in Core.

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

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Utility/LLDBAssert.h
lldb/source/API/SystemInitializerFull.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Utility/LLDBAssert.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index b63597fc71b4c..4005bdbddacca 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -128,6 +128,9 @@ class Debugger : public 
std::enable_shared_from_this,
 const ExecutionContext *exe_ctx,
 const Address *addr, Stream );
 
+  static void AssertCallback(llvm::StringRef message, llvm::StringRef 
backtrace,
+ llvm::StringRef prompt);
+
   void Clear();
 
   bool GetAsyncExecution();

diff  --git a/lldb/include/lldb/Utility/LLDBAssert.h 
b/lldb/include/lldb/Utility/LLDBAssert.h
index 524f56fd77c80..aeef3e51e20a8 100644
--- a/lldb/include/lldb/Utility/LLDBAssert.h
+++ b/lldb/include/lldb/Utility/LLDBAssert.h
@@ -9,6 +9,8 @@
 #ifndef LLDB_UTILITY_LLDBASSERT_H
 #define LLDB_UTILITY_LLDBASSERT_H
 
+#include "llvm/ADT/StringRef.h"
+
 #ifndef NDEBUG
 #define lldbassert(x) assert(x)
 #else
@@ -29,6 +31,12 @@
 namespace lldb_private {
 void lldb_assert(bool expression, const char *expr_text, const char *func,
  const char *file, unsigned int line);
+
+typedef void (*LLDBAssertCallback)(llvm::StringRef message,
+   llvm::StringRef backtrace,
+   llvm::StringRef prompt);
+
+void SetLLDBAssertCallback(LLDBAssertCallback callback);
 } // namespace lldb_private
 
 #endif // LLDB_UTILITY_LLDBASSERT_H

diff  --git a/lldb/source/API/SystemInitializerFull.cpp 
b/lldb/source/API/SystemInitializerFull.cpp
index 521aecf36fd8b..27319debc8582 100644
--- a/lldb/source/API/SystemInitializerFull.cpp
+++ b/lldb/source/API/SystemInitializerFull.cpp
@@ -78,6 +78,9 @@ llvm::Error SystemInitializerFull::Initialize() {
   // Settings must be initialized AFTER PluginManager::Initialize is called.
   Debugger::SettingsInitialize();
 
+  // Use the Debugger's LLDBAssert callback.
+  SetLLDBAssertCallback(Debugger::AssertCallback);
+
   return llvm::Error::success();
 }
 

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index ad177637f45b4..6f43cc608c2ce 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1352,6 +1352,13 @@ bool Debugger::FormatDisassemblerAddress(const 
FormatEntity::Entry *format,
   function_changed, initial_function);
 }
 
+void Debugger::AssertCallback(llvm::StringRef message,
+  llvm::StringRef backtrace,
+  llvm::StringRef prompt) {
+  Debugger::ReportError(
+  llvm::formatv("{0}\n{1}{2}", message, backtrace, prompt).str());
+}
+
 void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
   void *baton) {
   // For simplicity's sake, I am not going to deal with how to close down any

diff  --git a/lldb/source/Utility/LLDBAssert.cpp 
b/lldb/source/Utility/LLDBAssert.cpp
index 17689582cdc58..29bcacb6ad24c 100644
--- a/lldb/source/Utility/LLDBAssert.cpp
+++ b/lldb/source/Utility/LLDBAssert.cpp
@@ -8,7 +8,7 @@
 
 #include "lldb/Utility/LLDBAssert.h"
 #include "llvm/Config/llvm-config.h"
-#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -16,12 +16,21 @@
 #include 
 #endif
 
-using namespace llvm;
-using namespace lldb_private;
+namespace lldb_private {
 
-void lldb_private::lldb_assert(bool expression, const char *expr_text,
-   const char *func, const char *file,
-   

[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan accepted this revision.
yinghuitan added a comment.

Can we wrap this as a helper function and shared to avoid duplication? 
Otherwise looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152712

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

[Lldb-commits] [PATCH] D152757: [lldb][ObjectFileELF] Set ModuleSpec file offset and size

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack updated this revision to Diff 531147.
splhack added a comment.

fix EXPECT_EQ warnings


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152757

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/unittests/ObjectFile/ELF/CMakeLists.txt
  lldb/unittests/ObjectFile/ELF/Inputs/liboffset-test.so
  lldb/unittests/ObjectFile/ELF/Inputs/offset-test.bin
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,35 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 0UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 3600UL);
+}
+
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithOffsetFile) {
+  std::string SO = GetInputFilePath("offset-test.bin");
+  ModuleSpecList Specs;
+  ASSERT_EQ(
+  1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 1024, 3600, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 1024UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 4640UL);
+}
+
 TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
   /*
   // nosym-entrypoint-arm-thumb.s
Index: lldb/unittests/ObjectFile/ELF/CMakeLists.txt
===
--- lldb/unittests/ObjectFile/ELF/CMakeLists.txt
+++ lldb/unittests/ObjectFile/ELF/CMakeLists.txt
@@ -11,5 +11,7 @@
 
 set(test_inputs
   early-section-headers.so
+  liboffset-test.so
+  offset-test.bin
   )
 add_unittest_inputs(ObjectFileELFTests "${test_inputs}")
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -555,6 +555,8 @@
 if (header.Parse(data, _offset)) {
   if (data_sp) {
 ModuleSpec spec(file);
+spec.SetObjectOffset(file_offset);
+spec.SetObjectSize(length);
 
 const uint32_t sub_type = subTypeFromElfHeader(header);
 spec.GetArchitecture().SetArchitecture(
@@ -587,7 +589,7 @@
   }
 
   if (data_sp->GetByteSize() < length)
-data_sp = MapFileData(file, -1, file_offset);
+data_sp = MapFileData(file, length, file_offset);
   if (data_sp)
 data.SetData(data_sp);
   // In case there is header extension in the section #0, the header we


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,35 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 0UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 3600UL);
+}
+
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithOffsetFile) {
+  std::string SO = GetInputFilePath("offset-test.bin");
+  ModuleSpecList Specs;
+  ASSERT_EQ(
+  1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 1024, 3600, Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 1024UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 4640UL);
+}
+
 TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
   /*
   // nosym-entrypoint-arm-thumb.s
Index: 

[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack added inline comments.



Comment at: lldb/source/Host/CMakeLists.txt:109-121
+
+set(ANDROID_SOURCES
+  android/ZipFile.cpp
+  android/HostInfoAndroid.cpp
+  )
 if (CMAKE_SYSTEM_NAME MATCHES "Android")
+  list(APPEND ANDROID_SOURCES

bulbazord wrote:
> splhack wrote:
> > bulbazord wrote:
> > > I don't think this is correct to do. lldbHost is different than other 
> > > libraries in that it's meant to provide functionality that lldb and 
> > > lldb-server needs to work on the host system. Unconditionally adding a 
> > > host subdirectory for android even when we're on Linux doesn't make sense 
> > > to do.
> > I agree with that, however, I think this is pretty much only way to unblock 
> > writing and running unit tests for the Android host system, which has been 
> > no tests at all. The AndroidPlatformTest D152855 requires this to run the 
> > tests on Linux, and Android is basically Linux, so, hope it still makes 
> > sense for the unit testing capability. (only android/LibcGlue.cpp is not 
> > buildable for Linux target.)
> I see. You want to be able to run the android host tests but that's not easy 
> to do right now. I think this is a reasonable thing to want to do (especially 
> since so much of android support in lldb is not well tested AFAIK).
> 
> Instead of making this functionality specific to android hosts, why not make 
> it possible to do on all platforms? This would do a few things:
> - It would make it easier to test on more than just Linux and Android 
> machines.
> - It would open up the possibility of being able to use an apk on the host 
> machine instead of needing to fetch it from the remote device via `adb shell 
> dd`. An optimization for sure, but for large shared objects this may be able 
> to improve performance.
> 
> What do you think?
Yeah, sounds great to me! will update move things around.
- include/lldb/Utility/ZipFile.h
- source/Utility/ZipFile.cpp
- `HostInfoAndroid::ResolveZipPath` -> `ZipFile::ResolveBionicZipPath` (this is 
bionic libc specific)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152759

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

[Lldb-commits] [PATCH] D152866: [lldb] Print lldbassert to debugger diagnostics

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Separate out message, backtrace and prompt. This works better for a custom 
(downstream) callback that prompts the user to automatically file a bug report.


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

https://reviews.llvm.org/D152866

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/LLDBAssert.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/LLDBAssert.cpp

Index: lldb/source/Utility/LLDBAssert.cpp
===
--- lldb/source/Utility/LLDBAssert.cpp
+++ lldb/source/Utility/LLDBAssert.cpp
@@ -8,7 +8,7 @@
 
 #include "lldb/Utility/LLDBAssert.h"
 #include "llvm/Config/llvm-config.h"
-#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -16,12 +16,21 @@
 #include 
 #endif
 
-using namespace llvm;
-using namespace lldb_private;
+namespace lldb_private {
 
-void lldb_private::lldb_assert(bool expression, const char *expr_text,
-   const char *func, const char *file,
-   unsigned int line) {
+static void DefaultLLDBAssertCallback(std::string message,
+  std::string backtrace,
+  std::string prompt) {
+  llvm::errs() << message << '\n';
+  llvm::errs() << backtrace; // Backtrace includes a newline.
+  llvm::errs() << prompt << '\n';
+}
+
+static std::atomic g_lldb_assert_callback =
+
+
+void lldb_assert(bool expression, const char *expr_text, const char *func,
+ const char *file, unsigned int line) {
   if (LLVM_LIKELY(expression))
 return;
 
@@ -35,10 +44,21 @@
 
   // Print a warning and encourage the user to file a bug report, similar to
   // LLVM’s crash handler, and then return execution.
-  errs() << format("Assertion failed: (%s), function %s, file %s, line %u\n",
-   expr_text, func, file, line);
-  errs() << "backtrace leading to the failure:\n";
-  llvm::sys::PrintStackTrace(errs());
-  errs() << "please file a bug report against lldb reporting this failure "
-"log, and as many details as possible\n";
+  std::string buffer;
+  llvm::raw_string_ostream backtrace(buffer);
+  llvm::sys::PrintStackTrace(backtrace);
+
+  (*g_lldb_assert_callback.load())(
+  llvm::formatv("Assertion failed: ({0}), function {1}, file {2}, line {3}",
+expr_text, func, file, line)
+  .str(),
+  backtrace.str(),
+  "Please file a bug report against lldb reporting this failure log, and "
+  "as many details as possible");
 }
+
+void SetLLDBAssertCallback(LLDBAssertCallback callback) {
+  g_lldb_assert_callback.exchange(callback);
+}
+
+} // namespace lldb_private
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1352,6 +1352,11 @@
   function_changed, initial_function);
 }
 
+void Debugger::AssertCallback(std::string message, std::string backtrace,
+  std::string prompt) {
+  Debugger::ReportError(message + "\n" + backtrace + prompt);
+}
+
 void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
   void *baton) {
   // For simplicity's sake, I am not going to deal with how to close down any
Index: lldb/source/API/SystemInitializerFull.cpp
===
--- lldb/source/API/SystemInitializerFull.cpp
+++ lldb/source/API/SystemInitializerFull.cpp
@@ -78,6 +78,9 @@
   // Settings must be initialized AFTER PluginManager::Initialize is called.
   Debugger::SettingsInitialize();
 
+  // Use the Debugger's LLDBAssert callback.
+  SetLLDBAssertCallback(Debugger::AssertCallback);
+
   return llvm::Error::success();
 }
 
Index: lldb/include/lldb/Utility/LLDBAssert.h
===
--- lldb/include/lldb/Utility/LLDBAssert.h
+++ lldb/include/lldb/Utility/LLDBAssert.h
@@ -9,6 +9,8 @@
 #ifndef LLDB_UTILITY_LLDBASSERT_H
 #define LLDB_UTILITY_LLDBASSERT_H
 
+#include 
+
 #ifndef NDEBUG
 #define lldbassert(x) assert(x)
 #else
@@ -29,6 +31,11 @@
 namespace lldb_private {
 void lldb_assert(bool expression, const char *expr_text, const char *func,
  const char *file, unsigned int line);
+
+typedef void (*LLDBAssertCallback)(std::string message, std::string backtrace,
+   std::string prompt);
+
+void SetLLDBAssertCallback(LLDBAssertCallback callback);
 } // namespace lldb_private
 
 #endif // LLDB_UTILITY_LLDBASSERT_H
Index: lldb/include/lldb/Core/Debugger.h

[Lldb-commits] [PATCH] D152872: Add support for __debug_line_str in Mach-O

[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


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

https://reviews.llvm.org/D152872

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

[Lldb-commits] [PATCH] D152872: Add support for __debug_line_str in Mach-O

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: bulbazord, fdeazeve, rastogishubham, JDevlieghere.
Herald added a project: All.
aprantl requested review of this revision.

This patch resolves an issue that currently accounts for the vast majority of 
failures on the matrix bot.


https://reviews.llvm.org/D152872

Files:
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/dwarf5-macho.c


Index: lldb/test/Shell/SymbolFile/DWARF/x86/dwarf5-macho.c
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dwarf5-macho.c
@@ -0,0 +1,15 @@
+// Test that the file names in the __debug_line_str section can be decoded.
+
+// REQUIRES: system-darwin
+
+// RUN: %clang -target x86_64-apple-darwin %s -c -o %t.o -gdwarf-5
+// RUN: llvm-readobj --sections %t.o | FileCheck %s --check-prefix NAMES
+// RUN: xcrun %clang -target x86_64-apple-darwin -o %t.exe %t.o
+// RUN: %lldb %t.exe -b -o "target modules dump line-table %s" | FileCheck %s
+
+// NAMES: Name: __debug_line_str
+
+int main(int argc, char **argv) {
+  // CHECK: dwarf5-macho.c:[[@LINE+1]]
+  return 0;
+}
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1443,6 +1443,7 @@
   static ConstString g_sect_name_dwarf_debug_frame("__debug_frame");
   static ConstString g_sect_name_dwarf_debug_info("__debug_info");
   static ConstString g_sect_name_dwarf_debug_line("__debug_line");
+  static ConstString g_sect_name_dwarf_debug_line_str("__debug_line_str");
   static ConstString g_sect_name_dwarf_debug_loc("__debug_loc");
   static ConstString g_sect_name_dwarf_debug_loclists("__debug_loclists");
   static ConstString g_sect_name_dwarf_debug_macinfo("__debug_macinfo");
@@ -1472,6 +1473,8 @@
 return eSectionTypeDWARFDebugInfo;
   if (section_name == g_sect_name_dwarf_debug_line)
 return eSectionTypeDWARFDebugLine;
+  if (section_name == g_sect_name_dwarf_debug_line_str)
+return eSectionTypeDWARFDebugLineStr;
   if (section_name == g_sect_name_dwarf_debug_loc)
 return eSectionTypeDWARFDebugLoc;
   if (section_name == g_sect_name_dwarf_debug_loclists)


Index: lldb/test/Shell/SymbolFile/DWARF/x86/dwarf5-macho.c
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dwarf5-macho.c
@@ -0,0 +1,15 @@
+// Test that the file names in the __debug_line_str section can be decoded.
+
+// REQUIRES: system-darwin
+
+// RUN: %clang -target x86_64-apple-darwin %s -c -o %t.o -gdwarf-5
+// RUN: llvm-readobj --sections %t.o | FileCheck %s --check-prefix NAMES
+// RUN: xcrun %clang -target x86_64-apple-darwin -o %t.exe %t.o
+// RUN: %lldb %t.exe -b -o "target modules dump line-table %s" | FileCheck %s
+
+// NAMES: Name: __debug_line_str
+
+int main(int argc, char **argv) {
+  // CHECK: dwarf5-macho.c:[[@LINE+1]]
+  return 0;
+}
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1443,6 +1443,7 @@
   static ConstString g_sect_name_dwarf_debug_frame("__debug_frame");
   static ConstString g_sect_name_dwarf_debug_info("__debug_info");
   static ConstString g_sect_name_dwarf_debug_line("__debug_line");
+  static ConstString g_sect_name_dwarf_debug_line_str("__debug_line_str");
   static ConstString g_sect_name_dwarf_debug_loc("__debug_loc");
   static ConstString g_sect_name_dwarf_debug_loclists("__debug_loclists");
   static ConstString g_sect_name_dwarf_debug_macinfo("__debug_macinfo");
@@ -1472,6 +1473,8 @@
 return eSectionTypeDWARFDebugInfo;
   if (section_name == g_sect_name_dwarf_debug_line)
 return eSectionTypeDWARFDebugLine;
+  if (section_name == g_sect_name_dwarf_debug_line_str)
+return eSectionTypeDWARFDebugLineStr;
   if (section_name == g_sect_name_dwarf_debug_loc)
 return eSectionTypeDWARFDebugLoc;
   if (section_name == g_sect_name_dwarf_debug_loclists)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152870: [lldb][NFCI] Remove StructuredData::Array::GetItemAtIndexAsString overloads with ConstString

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jingham.
Herald added a subscriber: arphaman.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This is the next step in removing ConstString from StructuredData. There
are StringRef overloads already, let's use those where we can.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152870

Files:
  lldb/include/lldb/Utility/StructuredData.h
  lldb/source/Target/DynamicRegisterInfo.cpp


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -137,21 +137,20 @@
   uint32_t composite_offset = UINT32_MAX;
   for (uint32_t composite_idx = 0; composite_idx < num_composite_regs;
++composite_idx) {
-ConstString composite_reg_name;
-if (!composite_reg_list.GetItemAtIndexAsString(composite_idx,
-   composite_reg_name, 
nullptr))
+llvm::StringRef composite_reg_name;
+if (!composite_reg_list.GetItemAtIndexAsString(composite_idx, 
composite_reg_name))
   return llvm::createStringError(
   llvm::inconvertibleErrorCode(),
   "\"composite\" list value is not a Python string at index %d",
   composite_idx);
 
 const RegisterInfo *composite_reg_info =
-GetRegisterInfo(composite_reg_name.GetStringRef());
+GetRegisterInfo(composite_reg_name);
 if (!composite_reg_info)
   return llvm::createStringError(
   llvm::inconvertibleErrorCode(),
   "failed to find composite register by name: \"%s\"",
-  composite_reg_name.GetCString());
+  composite_reg_name.str().c_str());
 
 composite_offset =
 std::min(composite_offset, composite_reg_info->byte_offset);
@@ -199,9 +198,10 @@
   if (dict.GetValueForKeyAsArray("sets", sets)) {
 const uint32_t num_sets = sets->GetSize();
 for (uint32_t i = 0; i < num_sets; ++i) {
-  ConstString set_name;
-  if (sets->GetItemAtIndexAsString(i, set_name) && !set_name.IsEmpty()) {
-m_sets.push_back({set_name.AsCString(), nullptr, 0, nullptr});
+  llvm::StringRef set_name;
+  if (sets->GetItemAtIndexAsString(i, set_name) && !set_name.empty()) {
+m_sets.push_back(
+{ConstString(set_name).AsCString(), nullptr, 0, nullptr});
   } else {
 Clear();
 printf("error: register sets must have valid names\n");
@@ -338,12 +338,12 @@
   const size_t num_regs = invalidate_reg_list->GetSize();
   if (num_regs > 0) {
 for (uint32_t idx = 0; idx < num_regs; ++idx) {
-  ConstString invalidate_reg_name;
+  llvm::StringRef invalidate_reg_name;
   uint64_t invalidate_reg_num;
   if (invalidate_reg_list->GetItemAtIndexAsString(
   idx, invalidate_reg_name)) {
 const RegisterInfo *invalidate_reg_info =
-GetRegisterInfo(invalidate_reg_name.GetStringRef());
+GetRegisterInfo(invalidate_reg_name);
 if (invalidate_reg_info) {
   m_invalidate_regs_map[i].push_back(
   invalidate_reg_info->kinds[eRegisterKindLLDB]);
@@ -352,7 +352,7 @@
   // format
   printf("error: failed to find a 'invalidate-regs' register for "
  "\"%s\" while parsing register \"%s\"\n",
- invalidate_reg_name.GetCString(), reg_info.name);
+ invalidate_reg_name.str().c_str(), reg_info.name);
 }
   } else if (invalidate_reg_list->GetItemAtIndexAsInteger(
  idx, invalidate_reg_num)) {
Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -266,25 +266,6 @@
   return success;
 }
 
-bool GetItemAtIndexAsString(size_t idx, ConstString ) const {
-  ObjectSP value_sp = GetItemAtIndex(idx);
-  if (value_sp.get()) {
-if (auto string_value = value_sp->GetAsString()) {
-  result = ConstString(string_value->GetValue());
-  return true;
-}
-  }
-  return false;
-}
-
-bool GetItemAtIndexAsString(size_t idx, ConstString ,
-const char *default_val) const {
-  bool success = GetItemAtIndexAsString(idx, result);
-  if (!success)
-result.SetCString(default_val);
-  return success;
-}
-
 bool GetItemAtIndexAsDictionary(size_t idx, Dictionary *) const {
   result = nullptr;
   ObjectSP value_sp = GetItemAtIndex(idx);


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- 

[Lldb-commits] [PATCH] D152757: [lldb][ObjectFileELF] Set ModuleSpec file offset and size

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack updated this revision to Diff 531118.
splhack added a comment.
Herald added a subscriber: JDevlieghere.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152757

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/unittests/ObjectFile/ELF/CMakeLists.txt
  lldb/unittests/ObjectFile/ELF/Inputs/liboffset-test.so
  lldb/unittests/ObjectFile/ELF/Inputs/offset-test.bin
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,35 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 0);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 3600);
+}
+
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithOffsetFile) {
+  std::string SO = GetInputFilePath("offset-test.bin");
+  ModuleSpecList Specs;
+  ASSERT_EQ(
+  1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 1024, 3600, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 1024);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 4640);
+}
+
 TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
   /*
   // nosym-entrypoint-arm-thumb.s
Index: lldb/unittests/ObjectFile/ELF/CMakeLists.txt
===
--- lldb/unittests/ObjectFile/ELF/CMakeLists.txt
+++ lldb/unittests/ObjectFile/ELF/CMakeLists.txt
@@ -11,5 +11,7 @@
 
 set(test_inputs
   early-section-headers.so
+  liboffset-test.so
+  offset-test.bin
   )
 add_unittest_inputs(ObjectFileELFTests "${test_inputs}")
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -555,6 +555,8 @@
 if (header.Parse(data, _offset)) {
   if (data_sp) {
 ModuleSpec spec(file);
+spec.SetObjectOffset(file_offset);
+spec.SetObjectSize(length);
 
 const uint32_t sub_type = subTypeFromElfHeader(header);
 spec.GetArchitecture().SetArchitecture(
@@ -587,7 +589,7 @@
   }
 
   if (data_sp->GetByteSize() < length)
-data_sp = MapFileData(file, -1, file_offset);
+data_sp = MapFileData(file, length, file_offset);
   if (data_sp)
 data.SetData(data_sp);
   // In case there is header extension in the section #0, the header we


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,35 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 0);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 3600);
+}
+
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithOffsetFile) {
+  std::string SO = GetInputFilePath("offset-test.bin");
+  ModuleSpecList Specs;
+  ASSERT_EQ(
+  1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 1024, 3600, Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 1024);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 4640);
+}
+
 TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
   /*
   // nosym-entrypoint-arm-thumb.s
Index: 

[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Host/CMakeLists.txt:109-121
+
+set(ANDROID_SOURCES
+  android/ZipFile.cpp
+  android/HostInfoAndroid.cpp
+  )
 if (CMAKE_SYSTEM_NAME MATCHES "Android")
+  list(APPEND ANDROID_SOURCES

splhack wrote:
> bulbazord wrote:
> > I don't think this is correct to do. lldbHost is different than other 
> > libraries in that it's meant to provide functionality that lldb and 
> > lldb-server needs to work on the host system. Unconditionally adding a host 
> > subdirectory for android even when we're on Linux doesn't make sense to do.
> I agree with that, however, I think this is pretty much only way to unblock 
> writing and running unit tests for the Android host system, which has been no 
> tests at all. The AndroidPlatformTest D152855 requires this to run the tests 
> on Linux, and Android is basically Linux, so, hope it still makes sense for 
> the unit testing capability. (only android/LibcGlue.cpp is not buildable for 
> Linux target.)
I see. You want to be able to run the android host tests but that's not easy to 
do right now. I think this is a reasonable thing to want to do (especially 
since so much of android support in lldb is not well tested AFAIK).

Instead of making this functionality specific to android hosts, why not make it 
possible to do on all platforms? This would do a few things:
- It would make it easier to test on more than just Linux and Android machines.
- It would open up the possibility of being able to use an apk on the host 
machine instead of needing to fetch it from the remote device via `adb shell 
dd`. An optimization for sure, but for large shared objects this may be able to 
improve performance.

What do you think?



Comment at: lldb/source/Host/android/ZipFile.cpp:1
+//===-- ZipFile.cpp 
---===//
+//

BTW, I haven't looked at this file too closely yet, but I think a better 
candidate for its location would be `lldbUtility`. It doesn't rely on anything 
else from lldb other than something else in `lldbUtility`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152759

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

[Lldb-commits] [PATCH] D152866: [lldb] Print lldbassert to debugger diagnostics

[Alex Langford via Phabricator via lldb-commits]

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

In D152866#4419413 , @JDevlieghere 
wrote:

> In D152866#4419406 , @bulbazord 
> wrote:
>
>> I like this idea quite a bit! I assume that when somebody hits a bug, they 
>> can give us their diagnostics file and we will hopefully see the assertion 
>> somewhere (if there was one)?
>
> Unfortunate naming, but these are different diagnostics: these are errors and 
> warnings that are emitted as events. What you're thinking of is the 
> Diagnostic class.
>
> - I should really rename one of them to avoid confusion.
> - We could totally combine the two and emit a file with the lldbassert in the 
> "diagnostics dir" that we generate on crash. I'll tackle that in a follow-up 
> commit.

Oh I see. Yeah, the fact that we have multiple things named `Diagnostics` is 
confusing. :( Unfortunate but still better than nothing. Let's definitely 
figure out a resolution as a follow-up.


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

https://reviews.llvm.org/D152866

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

[Lldb-commits] [PATCH] D152866: [lldb] Print lldbassert to debugger diagnostics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 53.

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

https://reviews.llvm.org/D152866

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/LLDBAssert.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/LLDBAssert.cpp

Index: lldb/source/Utility/LLDBAssert.cpp
===
--- lldb/source/Utility/LLDBAssert.cpp
+++ lldb/source/Utility/LLDBAssert.cpp
@@ -8,7 +8,7 @@
 
 #include "lldb/Utility/LLDBAssert.h"
 #include "llvm/Config/llvm-config.h"
-#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -16,12 +16,17 @@
 #include 
 #endif
 
-using namespace llvm;
-using namespace lldb_private;
+namespace lldb_private {
 
-void lldb_private::lldb_assert(bool expression, const char *expr_text,
-   const char *func, const char *file,
-   unsigned int line) {
+static void DefaultLLDBAssertCallback(std::string message) {
+  llvm::errs() << message << '\n';
+}
+
+static std::atomic g_lldb_assert_callback =
+
+
+void lldb_assert(bool expression, const char *expr_text, const char *func,
+ const char *file, unsigned int line) {
   if (LLVM_LIKELY(expression))
 return;
 
@@ -35,10 +40,22 @@
 
   // Print a warning and encourage the user to file a bug report, similar to
   // LLVM’s crash handler, and then return execution.
-  errs() << format("Assertion failed: (%s), function %s, file %s, line %u\n",
-   expr_text, func, file, line);
-  errs() << "backtrace leading to the failure:\n";
-  llvm::sys::PrintStackTrace(errs());
-  errs() << "please file a bug report against lldb reporting this failure "
-"log, and as many details as possible\n";
+  std::string buffer;
+  llvm::raw_string_ostream backtrace(buffer);
+  llvm::sys::PrintStackTrace(backtrace);
+
+  std::string message =
+  llvm::formatv("Assertion failed: ({0}), function {1}, file {2}, line "
+"{3}\n{4}Please file a bug report against lldb reporting "
+"this failure log, and as many details as possible",
+expr_text, func, file, line, backtrace.str())
+  .str();
+
+  (*g_lldb_assert_callback.load())(std::move(message));
+}
+
+void SetLLDBAssertCallback(LLDBAssertCallback callback) {
+  g_lldb_assert_callback.exchange(callback);
 }
+
+} // namespace lldb_private
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1352,6 +1352,10 @@
   function_changed, initial_function);
 }
 
+void Debugger::AssertCallback(std::string message) {
+  Debugger::ReportError(std::move(message));
+}
+
 void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
   void *baton) {
   // For simplicity's sake, I am not going to deal with how to close down any
Index: lldb/source/API/SystemInitializerFull.cpp
===
--- lldb/source/API/SystemInitializerFull.cpp
+++ lldb/source/API/SystemInitializerFull.cpp
@@ -78,6 +78,9 @@
   // Settings must be initialized AFTER PluginManager::Initialize is called.
   Debugger::SettingsInitialize();
 
+  // Use the Debugger's LLDBAssert callback.
+  SetLLDBAssertCallback(Debugger::AssertCallback);
+
   return llvm::Error::success();
 }
 
Index: lldb/include/lldb/Utility/LLDBAssert.h
===
--- lldb/include/lldb/Utility/LLDBAssert.h
+++ lldb/include/lldb/Utility/LLDBAssert.h
@@ -9,6 +9,8 @@
 #ifndef LLDB_UTILITY_LLDBASSERT_H
 #define LLDB_UTILITY_LLDBASSERT_H
 
+#include 
+
 #ifndef NDEBUG
 #define lldbassert(x) assert(x)
 #else
@@ -29,6 +31,10 @@
 namespace lldb_private {
 void lldb_assert(bool expression, const char *expr_text, const char *func,
  const char *file, unsigned int line);
+
+typedef void (*LLDBAssertCallback)(std::string message);
+
+void SetLLDBAssertCallback(LLDBAssertCallback callback);
 } // namespace lldb_private
 
 #endif // LLDB_UTILITY_LLDBASSERT_H
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -128,6 +128,8 @@
 const ExecutionContext *exe_ctx,
 const Address *addr, Stream );
 
+  static void AssertCallback(std::string message);
+
   void Clear();
 
   bool GetAsyncExecution();
@@ -395,7 +397,6 @@
   /// Since the two checks are mutually exclusive, however, it's also convenient
   /// to have just one function to check the interrupt 

[Lldb-commits] [PATCH] D152866: [lldb] Print lldbassert to debugger diagnostics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 531110.
JDevlieghere marked an inline comment as done.

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

https://reviews.llvm.org/D152866

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/LLDBAssert.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/LLDBAssert.cpp

Index: lldb/source/Utility/LLDBAssert.cpp
===
--- lldb/source/Utility/LLDBAssert.cpp
+++ lldb/source/Utility/LLDBAssert.cpp
@@ -8,7 +8,7 @@
 
 #include "lldb/Utility/LLDBAssert.h"
 #include "llvm/Config/llvm-config.h"
-#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -16,12 +16,17 @@
 #include 
 #endif
 
-using namespace llvm;
-using namespace lldb_private;
+namespace lldb_private {
 
-void lldb_private::lldb_assert(bool expression, const char *expr_text,
-   const char *func, const char *file,
-   unsigned int line) {
+static void DefaultLLDBAssertCallback(std::string message) {
+  llvm::errs() << message << '\n';
+}
+
+static std::atomic g_lldb_assert_callback =
+
+
+void lldb_assert(bool expression, const char *expr_text, const char *func,
+ const char *file, unsigned int line) {
   if (LLVM_LIKELY(expression))
 return;
 
@@ -35,10 +40,22 @@
 
   // Print a warning and encourage the user to file a bug report, similar to
   // LLVM’s crash handler, and then return execution.
-  errs() << format("Assertion failed: (%s), function %s, file %s, line %u\n",
-   expr_text, func, file, line);
-  errs() << "backtrace leading to the failure:\n";
-  llvm::sys::PrintStackTrace(errs());
-  errs() << "please file a bug report against lldb reporting this failure "
-"log, and as many details as possible\n";
+  std::string buffer;
+  llvm::raw_string_ostream backtrace(buffer);
+  llvm::sys::PrintStackTrace(backtrace);
+
+  std::string message =
+  llvm::formatv("Assertion failed: ({0}), function {1}, file {2}, line "
+"{3}\n{4}Please file a bug report against lldb reporting "
+"this failure log, and as many details as possible",
+expr_text, func, file, line, backtrace.str())
+  .str();
+
+  (*g_lldb_assert_callback.load())(std::move(message));
+}
+
+void SetLLDBAssertCallback(LLDBAssertCallback callback) {
+  g_lldb_assert_callback.exchange(callback);
 }
+
+} // namespace lldb_private
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -112,6 +112,7 @@
   SetEventName(eBroadcastBitWatchpointChanged, "watchpoint-changed");
   SetEventName(eBroadcastBitSymbolsLoaded, "symbols-loaded");
 
+  lldbassert(false && "foo");
   CheckInWithManager();
 
   LLDB_LOG(GetLog(LLDBLog::Object), "{0} Target::Target()",
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1352,6 +1352,10 @@
   function_changed, initial_function);
 }
 
+void Debugger::AssertCallback(std::string message) {
+  Debugger::ReportError(std::move(message));
+}
+
 void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
   void *baton) {
   // For simplicity's sake, I am not going to deal with how to close down any
Index: lldb/source/API/SystemInitializerFull.cpp
===
--- lldb/source/API/SystemInitializerFull.cpp
+++ lldb/source/API/SystemInitializerFull.cpp
@@ -78,6 +78,9 @@
   // Settings must be initialized AFTER PluginManager::Initialize is called.
   Debugger::SettingsInitialize();
 
+  // Use the Debugger's LLDBAssert callback.
+  SetLLDBAssertCallback(Debugger::AssertCallback);
+
   return llvm::Error::success();
 }
 
Index: lldb/include/lldb/Utility/LLDBAssert.h
===
--- lldb/include/lldb/Utility/LLDBAssert.h
+++ lldb/include/lldb/Utility/LLDBAssert.h
@@ -9,6 +9,8 @@
 #ifndef LLDB_UTILITY_LLDBASSERT_H
 #define LLDB_UTILITY_LLDBASSERT_H
 
+#include 
+
 #ifndef NDEBUG
 #define lldbassert(x) assert(x)
 #else
@@ -29,6 +31,10 @@
 namespace lldb_private {
 void lldb_assert(bool expression, const char *expr_text, const char *func,
  const char *file, unsigned int line);
+
+typedef void (*LLDBAssertCallback)(std::string message);
+
+void SetLLDBAssertCallback(LLDBAssertCallback callback);
 } // namespace lldb_private
 
 #endif // LLDB_UTILITY_LLDBASSERT_H
Index: lldb/include/lldb/Core/Debugger.h

[Lldb-commits] [PATCH] D152866: [lldb] Print lldbassert to debugger diagnostics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D152866#4419406 , @bulbazord wrote:

> I like this idea quite a bit! I assume that when somebody hits a bug, they 
> can give us their diagnostics file and we will hopefully see the assertion 
> somewhere (if there was one)?

Unfortunate naming, but these are different diagnostics: these are errors and 
warnings that are emitted as events. What you're thinking of is the Diagnostic 
class.

- I should really rename one of them to avoid confusion.
- We could totally combine the two and emit a file with the lldbassert in the 
"diagnostics dir" that we generate on crash. I'll tackle that in a follow-up 
commit.


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

https://reviews.llvm.org/D152866

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

[Lldb-commits] [PATCH] D152866: [lldb] Print lldbassert to debugger diagnostics

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

I like this idea quite a bit! I assume that when somebody hits a bug, they can 
give us their diagnostics file and we will hopefully see the assertion 
somewhere (if there was one)?




Comment at: lldb/source/Target/Target.cpp:115
 
+  lldbassert(false && "foo");
   CheckInWithManager();

.


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

https://reviews.llvm.org/D152866

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

[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack added a comment.

@bulbazord thanks for reviewing! will address the types, formats.




Comment at: lldb/source/Host/CMakeLists.txt:109-121
+
+set(ANDROID_SOURCES
+  android/ZipFile.cpp
+  android/HostInfoAndroid.cpp
+  )
 if (CMAKE_SYSTEM_NAME MATCHES "Android")
+  list(APPEND ANDROID_SOURCES

bulbazord wrote:
> I don't think this is correct to do. lldbHost is different than other 
> libraries in that it's meant to provide functionality that lldb and 
> lldb-server needs to work on the host system. Unconditionally adding a host 
> subdirectory for android even when we're on Linux doesn't make sense to do.
I agree with that, however, I think this is pretty much only way to unblock 
writing and running unit tests for the Android host system, which has been no 
tests at all. The AndroidPlatformTest D152855 requires this to run the tests on 
Linux, and Android is basically Linux, so, hope it still makes sense for the 
unit testing capability. (only android/LibcGlue.cpp is not buildable for Linux 
target.)



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:261-264
+  if (const char *run_as = std::getenv("ANDROID_PLATFORM_RUN_AS"))
+snprintf(run_as_cmd, sizeof(run_as_cmd), "run-as '%s' ", run_as);
+  else
+run_as_cmd[0] = '\0';

bulbazord wrote:
> Maybe it would be a good idea to centralize the `run-as` logic somewhere, 
> right now it's pretty ad-hoc.
Will look into these run-as things with D152494 centralize and if 
plugin.platform.android.platform-run-as possible.



Comment at: lldb/unittests/Host/CMakeLists.txt:42
+
+if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
+  add_subdirectory(android)

bulbazord wrote:
> Why not just match on `"Android"`?
The reason is, as commented above, to run AndroidPlatformTest D152855 on Linux.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152759

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

[Lldb-commits] [PATCH] D152866: [lldb] Print lldbassert to debugger diagnostics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, rupprecht, bulbazord, aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

When hitting an lldbassert in a non-assert build, we emit a blurb including the 
assertion, the triggering file and line and a pretty backtrace leading up to 
the issue. Currently, this emitted to stderr. That's fine for the command line, 
but when used as library, for example from Xcode or VSCode, this information 
doesn't make it to the user. This patch redirects these messages to use the 
diagnostic infrastructure so they're emitted as diagnostic events.

The patch is slightly more complicated than I would've liked because of 
layering. `lldbassert` is part of `Utility` while the diagnostics are 
implemented in `Core`.


https://reviews.llvm.org/D152866

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/LLDBAssert.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/LLDBAssert.cpp

Index: lldb/source/Utility/LLDBAssert.cpp
===
--- lldb/source/Utility/LLDBAssert.cpp
+++ lldb/source/Utility/LLDBAssert.cpp
@@ -8,7 +8,7 @@
 
 #include "lldb/Utility/LLDBAssert.h"
 #include "llvm/Config/llvm-config.h"
-#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -16,12 +16,17 @@
 #include 
 #endif
 
-using namespace llvm;
-using namespace lldb_private;
+namespace lldb_private {
 
-void lldb_private::lldb_assert(bool expression, const char *expr_text,
-   const char *func, const char *file,
-   unsigned int line) {
+static void DefaultLLDBAssertCallback(std::string message) {
+  llvm::errs() << message << '\n';
+}
+
+static std::atomic g_lldb_assert_callback =
+
+
+void lldb_assert(bool expression, const char *expr_text, const char *func,
+ const char *file, unsigned int line) {
   if (LLVM_LIKELY(expression))
 return;
 
@@ -35,10 +40,22 @@
 
   // Print a warning and encourage the user to file a bug report, similar to
   // LLVM’s crash handler, and then return execution.
-  errs() << format("Assertion failed: (%s), function %s, file %s, line %u\n",
-   expr_text, func, file, line);
-  errs() << "backtrace leading to the failure:\n";
-  llvm::sys::PrintStackTrace(errs());
-  errs() << "please file a bug report against lldb reporting this failure "
-"log, and as many details as possible\n";
+  std::string buffer;
+  llvm::raw_string_ostream backtrace(buffer);
+  llvm::sys::PrintStackTrace(backtrace);
+
+  std::string message =
+  llvm::formatv("Assertion failed: ({0}), function {1}, file {2}, line "
+"{3}\n{4}Please file a bug report against lldb reporting "
+"this failure log, and as many details as possible",
+expr_text, func, file, line, backtrace.str())
+  .str();
+
+  (*g_lldb_assert_callback.load())(std::move(message));
+}
+
+void SetLLDBAssertCallback(LLDBAssertCallback callback) {
+  g_lldb_assert_callback.exchange(callback);
 }
+
+} // namespace lldb_private
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -112,6 +112,7 @@
   SetEventName(eBroadcastBitWatchpointChanged, "watchpoint-changed");
   SetEventName(eBroadcastBitSymbolsLoaded, "symbols-loaded");
 
+  lldbassert(false && "foo");
   CheckInWithManager();
 
   LLDB_LOG(GetLog(LLDBLog::Object), "{0} Target::Target()",
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1352,6 +1352,10 @@
   function_changed, initial_function);
 }
 
+void Debugger::AssertCallback(std::string message) {
+  Debugger::ReportError(std::move(message));
+}
+
 void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
   void *baton) {
   // For simplicity's sake, I am not going to deal with how to close down any
Index: lldb/source/API/SystemInitializerFull.cpp
===
--- lldb/source/API/SystemInitializerFull.cpp
+++ lldb/source/API/SystemInitializerFull.cpp
@@ -78,6 +78,9 @@
   // Settings must be initialized AFTER PluginManager::Initialize is called.
   Debugger::SettingsInitialize();
 
+  // Use the Debugger's LLDBAssert callback.
+  SetLLDBAssertCallback(Debugger::AssertCallback);
+
   return llvm::Error::success();
 }
 
Index: lldb/include/lldb/Utility/LLDBAssert.h
===
--- lldb/include/lldb/Utility/LLDBAssert.h

[Lldb-commits] [PATCH] D152331: [lldb][NFCI] Platforms should own their SDKBuild and SDKRootDirectory strings

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 531104.
bulbazord added a comment.

Avoid construction of temporary std::string


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152331

Files:
  lldb/include/lldb/Interpreter/OptionGroupPlatform.h
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Interpreter/OptionGroupPlatform.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Target/Platform.cpp

Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -210,11 +210,10 @@
 Status error(eErrorTypeGeneric);
 ModuleSpec resolved_spec;
 // Check if we have sysroot set.
-if (m_sdk_sysroot) {
+if (!m_sdk_sysroot.empty()) {
   // Prepend sysroot to module spec.
   resolved_spec = spec;
-  resolved_spec.GetFileSpec().PrependPathComponent(
-  m_sdk_sysroot.GetStringRef());
+  resolved_spec.GetFileSpec().PrependPathComponent(m_sdk_sysroot);
   // Try to get shared module with resolved spec.
   error = ModuleList::GetSharedModule(resolved_spec, module_sp,
   module_search_paths_ptr, old_modules,
@@ -312,9 +311,9 @@
 strm.Printf(" Connected: %s\n", is_connected ? "yes" : "no");
   }
 
-  if (GetSDKRootDirectory()) {
-strm.Format("   Sysroot: {0}\n", GetSDKRootDirectory());
-  }
+  if (const std::string _root = GetSDKRootDirectory(); !sdk_root.empty())
+strm.Format("   Sysroot: {0}\n", sdk_root);
+
   if (GetWorkingDirectory()) {
 strm.Printf("WorkingDir: %s\n", GetWorkingDirectory().GetPath().c_str());
   }
Index: lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
===
--- lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -191,8 +191,8 @@
   launch_info.SetArguments(args, true);
 
   Environment emulator_env = Host::GetEnvironment();
-  if (ConstString sysroot = GetSDKRootDirectory())
-emulator_env["QEMU_LD_PREFIX"] = sysroot.GetStringRef().str();
+  if (const std::string  = GetSDKRootDirectory(); !sysroot.empty())
+emulator_env["QEMU_LD_PREFIX"] = sysroot;
   for (const auto  : GetGlobalProperties().GetEmulatorEnvVars())
 emulator_env[KV.first()] = KV.second;
   launch_info.GetEnvironment() = ComputeLaunchEnvironment(
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -34,8 +34,8 @@
   std::lock_guard guard(m_sdk_dir_mutex);
   if (m_sdk_directory_infos.empty()) {
 // A --sysroot option was supplied - add it to our list of SDKs to check
-if (m_sdk_sysroot) {
-  FileSpec sdk_sysroot_fspec(m_sdk_sysroot.GetCString());
+if (!m_sdk_sysroot.empty()) {
+  FileSpec sdk_sysroot_fspec(m_sdk_sysroot.c_str());
   FileSystem::Instance().Resolve(sdk_sysroot_fspec);
   const SDKDirectoryInfo sdk_sysroot_directory_info(sdk_sysroot_fspec);
   m_sdk_directory_infos.push_back(sdk_sysroot_directory_info);
@@ -43,7 +43,7 @@
 LLDB_LOGF(log,
   "PlatformDarwinDevice::UpdateSDKDirectoryInfosIfNeeded added "
   "--sysroot SDK directory %s",
-  m_sdk_sysroot.GetCString());
+  m_sdk_sysroot.c_str());
   }
   return true;
 }
@@ -152,20 +152,20 @@
 std::vector check_sdk_info(num_sdk_infos, true);
 
 // Prefer the user SDK build string.
-ConstString build = GetSDKBuild();
+std::string build = GetSDKBuild();
 
 // Fall back to the platform's build string.
-if (!build) {
-  if (std::optional os_build_str = GetOSBuildString()) {
-build = ConstString(*os_build_str);
-  }
+if (build.empty()) {
+  if (std::optional os_build_str = GetOSBuildString())
+build.assign(*os_build_str);
 }
 
 // If we have a build string, only check platforms for which the build
 // string matches.
-if (build) {
+if (!build.empty()) {
   for (i = 0; i < num_sdk_infos; ++i)
-check_sdk_info[i] = m_sdk_directory_infos[i].build == build;
+check_sdk_info[i] = m_sdk_directory_infos[i].build.GetStringRef() ==
+llvm::StringRef(build);
 }
 
 // If we are connected we can find the version of the OS the platform us
@@ -201,7 +201,7 @@
   }
 }
   }
-} else if (build) {
+} else if (!build.empty()) {
   // No version, just a build number, return the first one that matches.
   for (i = 0; i < num_sdk_infos; ++i)

[Lldb-commits] [PATCH] D152331: [lldb][NFCI] Platforms should own their SDKBuild and SDKRootDirectory strings

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 531103.
bulbazord added a comment.

Update other 2 functions (and one callsite)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152331

Files:
  lldb/include/lldb/Interpreter/OptionGroupPlatform.h
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Interpreter/OptionGroupPlatform.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Target/Platform.cpp

Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -210,11 +210,10 @@
 Status error(eErrorTypeGeneric);
 ModuleSpec resolved_spec;
 // Check if we have sysroot set.
-if (m_sdk_sysroot) {
+if (!m_sdk_sysroot.empty()) {
   // Prepend sysroot to module spec.
   resolved_spec = spec;
-  resolved_spec.GetFileSpec().PrependPathComponent(
-  m_sdk_sysroot.GetStringRef());
+  resolved_spec.GetFileSpec().PrependPathComponent(m_sdk_sysroot);
   // Try to get shared module with resolved spec.
   error = ModuleList::GetSharedModule(resolved_spec, module_sp,
   module_search_paths_ptr, old_modules,
@@ -312,9 +311,9 @@
 strm.Printf(" Connected: %s\n", is_connected ? "yes" : "no");
   }
 
-  if (GetSDKRootDirectory()) {
-strm.Format("   Sysroot: {0}\n", GetSDKRootDirectory());
-  }
+  if (const std::string _root = GetSDKRootDirectory(); !sdk_root.empty())
+strm.Format("   Sysroot: {0}\n", sdk_root);
+
   if (GetWorkingDirectory()) {
 strm.Printf("WorkingDir: %s\n", GetWorkingDirectory().GetPath().c_str());
   }
Index: lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
===
--- lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -191,8 +191,8 @@
   launch_info.SetArguments(args, true);
 
   Environment emulator_env = Host::GetEnvironment();
-  if (ConstString sysroot = GetSDKRootDirectory())
-emulator_env["QEMU_LD_PREFIX"] = sysroot.GetStringRef().str();
+  if (const std::string  = GetSDKRootDirectory(); !sysroot.empty())
+emulator_env["QEMU_LD_PREFIX"] = sysroot;
   for (const auto  : GetGlobalProperties().GetEmulatorEnvVars())
 emulator_env[KV.first()] = KV.second;
   launch_info.GetEnvironment() = ComputeLaunchEnvironment(
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -34,8 +34,8 @@
   std::lock_guard guard(m_sdk_dir_mutex);
   if (m_sdk_directory_infos.empty()) {
 // A --sysroot option was supplied - add it to our list of SDKs to check
-if (m_sdk_sysroot) {
-  FileSpec sdk_sysroot_fspec(m_sdk_sysroot.GetCString());
+if (!m_sdk_sysroot.empty()) {
+  FileSpec sdk_sysroot_fspec(m_sdk_sysroot.c_str());
   FileSystem::Instance().Resolve(sdk_sysroot_fspec);
   const SDKDirectoryInfo sdk_sysroot_directory_info(sdk_sysroot_fspec);
   m_sdk_directory_infos.push_back(sdk_sysroot_directory_info);
@@ -43,7 +43,7 @@
 LLDB_LOGF(log,
   "PlatformDarwinDevice::UpdateSDKDirectoryInfosIfNeeded added "
   "--sysroot SDK directory %s",
-  m_sdk_sysroot.GetCString());
+  m_sdk_sysroot.c_str());
   }
   return true;
 }
@@ -152,20 +152,20 @@
 std::vector check_sdk_info(num_sdk_infos, true);
 
 // Prefer the user SDK build string.
-ConstString build = GetSDKBuild();
+std::string build = GetSDKBuild();
 
 // Fall back to the platform's build string.
-if (!build) {
-  if (std::optional os_build_str = GetOSBuildString()) {
-build = ConstString(*os_build_str);
-  }
+if (build.empty()) {
+  if (std::optional os_build_str = GetOSBuildString())
+build.assign(*os_build_str);
 }
 
 // If we have a build string, only check platforms for which the build
 // string matches.
-if (build) {
+if (!build.empty()) {
   for (i = 0; i < num_sdk_infos; ++i)
-check_sdk_info[i] = m_sdk_directory_infos[i].build == build;
+check_sdk_info[i] = m_sdk_directory_infos[i].build.GetStringRef() ==
+llvm::StringRef(build);
 }
 
 // If we are connected we can find the version of the OS the platform us
@@ -201,7 +201,7 @@
   }
 }
   }
-} else if (build) {
+} else if (!build.empty()) {
   // No version, just a build number, return the first one that matches.
   for (i = 0; i < num_sdk_infos; ++i)

[Lldb-commits] [PATCH] D152597: [lldb][NFCI] Remove StructuredData::Dictionary::GetValueForKeyAsString overloads involving ConstString

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe669011c3cca: [lldb][NFCI] Remove 
StructuredData::Dictionary::GetValueForKeyAsString… (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152597

Files:
  lldb/include/lldb/Utility/StructuredData.h
  lldb/source/Target/DynamicRegisterInfo.cpp


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -238,17 +238,20 @@
 std::vector invalidate_regs;
 memset(_info, 0, sizeof(reg_info));
 
-ConstString name_val;
-ConstString alt_name_val;
-if (!reg_info_dict->GetValueForKeyAsString("name", name_val, nullptr)) {
+llvm::StringRef name_val;
+if (!reg_info_dict->GetValueForKeyAsString("name", name_val)) {
   Clear();
   printf("error: registers must have valid names and offsets\n");
   reg_info_dict->DumpToStdout();
   return 0;
 }
-reg_info.name = name_val.GetCString();
-reg_info_dict->GetValueForKeyAsString("alt-name", alt_name_val, nullptr);
-reg_info.alt_name = alt_name_val.GetCString();
+reg_info.name = ConstString(name_val).GetCString();
+
+llvm::StringRef alt_name_val;
+if (reg_info_dict->GetValueForKeyAsString("alt-name", alt_name_val))
+  reg_info.alt_name = ConstString(alt_name_val).GetCString();
+else
+  reg_info.alt_name = nullptr;
 
 llvm::Expected byte_offset =
 ByteOffsetFromRegInfoDict(i, *reg_info_dict, byte_order);
Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -536,26 +536,6 @@
   return success;
 }
 
-bool GetValueForKeyAsString(llvm::StringRef key,
-ConstString ) const {
-  ObjectSP value_sp = GetValueForKey(key);
-  if (value_sp.get()) {
-if (auto string_value = value_sp->GetAsString()) {
-  result = ConstString(string_value->GetValue());
-  return true;
-}
-  }
-  return false;
-}
-
-bool GetValueForKeyAsString(llvm::StringRef key, ConstString ,
-const char *default_val) const {
-  bool success = GetValueForKeyAsString(key, result);
-  if (!success)
-result.SetCString(default_val);
-  return success;
-}
-
 bool GetValueForKeyAsDictionary(llvm::StringRef key,
 Dictionary *) const {
   result = nullptr;


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -238,17 +238,20 @@
 std::vector invalidate_regs;
 memset(_info, 0, sizeof(reg_info));
 
-ConstString name_val;
-ConstString alt_name_val;
-if (!reg_info_dict->GetValueForKeyAsString("name", name_val, nullptr)) {
+llvm::StringRef name_val;
+if (!reg_info_dict->GetValueForKeyAsString("name", name_val)) {
   Clear();
   printf("error: registers must have valid names and offsets\n");
   reg_info_dict->DumpToStdout();
   return 0;
 }
-reg_info.name = name_val.GetCString();
-reg_info_dict->GetValueForKeyAsString("alt-name", alt_name_val, nullptr);
-reg_info.alt_name = alt_name_val.GetCString();
+reg_info.name = ConstString(name_val).GetCString();
+
+llvm::StringRef alt_name_val;
+if (reg_info_dict->GetValueForKeyAsString("alt-name", alt_name_val))
+  reg_info.alt_name = ConstString(alt_name_val).GetCString();
+else
+  reg_info.alt_name = nullptr;
 
 llvm::Expected byte_offset =
 ByteOffsetFromRegInfoDict(i, *reg_info_dict, byte_order);
Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -536,26 +536,6 @@
   return success;
 }
 
-bool GetValueForKeyAsString(llvm::StringRef key,
-ConstString ) const {
-  ObjectSP value_sp = GetValueForKey(key);
-  if (value_sp.get()) {
-if (auto string_value = value_sp->GetAsString()) {
-  result = ConstString(string_value->GetValue());
-  return true;
-}
-  }
-  return false;
-}
-
-bool GetValueForKeyAsString(llvm::StringRef key, ConstString ,
-const char *default_val) const {
-  bool success = GetValueForKeyAsString(key, result);
-  if (!success)
-result.SetCString(default_val);
-  return success;
-}
-
 bool 

[Lldb-commits] [lldb] e669011 - [lldb][NFCI] Remove StructuredData::Dictionary::GetValueForKeyAsString overloads involving ConstString

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-06-13T15:51:56-07:00
New Revision: e669011c3cca4b7b4a1c2e8d96467cecbc05df4d

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

LOG: [lldb][NFCI] Remove StructuredData::Dictionary::GetValueForKeyAsString 
overloads involving ConstString

In an effort to keep the ConstString StringPool small, I plan on
removing use of ConstString in StructuredData. The only class that
really uses it is StructuredData::Dictionary.

This one was fairly easy to remove, I plan on removing the others in
follow-up changes.

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

Added: 


Modified: 
lldb/include/lldb/Utility/StructuredData.h
lldb/source/Target/DynamicRegisterInfo.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/StructuredData.h 
b/lldb/include/lldb/Utility/StructuredData.h
index d6b51f7c58c12..28c71a8ca827a 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -536,26 +536,6 @@ class StructuredData {
   return success;
 }
 
-bool GetValueForKeyAsString(llvm::StringRef key,
-ConstString ) const {
-  ObjectSP value_sp = GetValueForKey(key);
-  if (value_sp.get()) {
-if (auto string_value = value_sp->GetAsString()) {
-  result = ConstString(string_value->GetValue());
-  return true;
-}
-  }
-  return false;
-}
-
-bool GetValueForKeyAsString(llvm::StringRef key, ConstString ,
-const char *default_val) const {
-  bool success = GetValueForKeyAsString(key, result);
-  if (!success)
-result.SetCString(default_val);
-  return success;
-}
-
 bool GetValueForKeyAsDictionary(llvm::StringRef key,
 Dictionary *) const {
   result = nullptr;

diff  --git a/lldb/source/Target/DynamicRegisterInfo.cpp 
b/lldb/source/Target/DynamicRegisterInfo.cpp
index d20f7c0bf98a0..700619959f5c8 100644
--- a/lldb/source/Target/DynamicRegisterInfo.cpp
+++ b/lldb/source/Target/DynamicRegisterInfo.cpp
@@ -238,17 +238,20 @@ DynamicRegisterInfo::SetRegisterInfo(const 
StructuredData::Dictionary ,
 std::vector invalidate_regs;
 memset(_info, 0, sizeof(reg_info));
 
-ConstString name_val;
-ConstString alt_name_val;
-if (!reg_info_dict->GetValueForKeyAsString("name", name_val, nullptr)) {
+llvm::StringRef name_val;
+if (!reg_info_dict->GetValueForKeyAsString("name", name_val)) {
   Clear();
   printf("error: registers must have valid names and offsets\n");
   reg_info_dict->DumpToStdout();
   return 0;
 }
-reg_info.name = name_val.GetCString();
-reg_info_dict->GetValueForKeyAsString("alt-name", alt_name_val, nullptr);
-reg_info.alt_name = alt_name_val.GetCString();
+reg_info.name = ConstString(name_val).GetCString();
+
+llvm::StringRef alt_name_val;
+if (reg_info_dict->GetValueForKeyAsString("alt-name", alt_name_val))
+  reg_info.alt_name = ConstString(alt_name_val).GetCString();
+else
+  reg_info.alt_name = nullptr;
 
 llvm::Expected byte_offset =
 ByteOffsetFromRegInfoDict(i, *reg_info_dict, byte_order);



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

[Lldb-commits] [PATCH] D152331: [lldb][NFCI] Platforms should own their SDKBuild and SDKRootDirectory strings

[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 with the other two functions updated as well.




Comment at: lldb/include/lldb/Target/Platform.h:453
 
-  void SetSDKRootDirectory(ConstString dir) { m_sdk_sysroot = dir; }
+  void SetSDKRootDirectory(llvm::StringRef dir) { m_sdk_sysroot = dir.str(); }
 





Comment at: lldb/include/lldb/Target/Platform.h:457
 
-  void SetSDKBuild(ConstString sdk_build) { m_sdk_build = sdk_build; }
+  void SetSDKBuild(llvm::StringRef sdk_build) { m_sdk_build = sdk_build.str(); 
}
 

std::string


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152331

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

[Lldb-commits] [lldb] a1a74f7 - [lldb] Default can_create to true in GetChildAtIndex (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-06-13T15:51:32-07:00
New Revision: a1a74f7cdea3db6e1a557eb1466f56b59e2d7fec

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

LOG: [lldb] Default can_create to true in GetChildAtIndex (NFC)

Existing callers of `GetChildAtIndex` pass true for can_create. This change
makes true the default value, callers don't have to pass an opaque true.

See also D151966 for the same change to `GetChildMemberWithName`.

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

Added: 


Modified: 
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
lldb/source/API/SBValue.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectSyntheticFilter.cpp
lldb/source/DataFormatters/FormatManager.cpp
lldb/source/DataFormatters/ValueObjectPrinter.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp

lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
lldb/source/Plugins/Language/ObjC/Cocoa.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
lldb/source/Target/StackFrame.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index b3eca5064a7b2..2edd1e019d922 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -469,7 +469,8 @@ class ValueObject {
   /// Returns a unique id for this ValueObject.
   lldb::user_id_t GetID() const { return m_id.GetID(); }
 
-  virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx, bool can_create);
+  virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx,
+  bool can_create = true);
 
   // this will always create the children if necessary
   lldb::ValueObjectSP GetChildAtIndexPath(llvm::ArrayRef idxs,

diff  --git a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h 
b/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
index 9df322ae6bc3c..67596232eafd1 100644
--- a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
+++ b/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
@@ -51,7 +51,8 @@ class ValueObjectSynthetic : public ValueObject {
 
   lldb::ValueType GetValueType() const override;
 
-  lldb::ValueObjectSP GetChildAtIndex(size_t idx, bool can_create) override;
+  lldb::ValueObjectSP GetChildAtIndex(size_t idx,
+  bool can_create = true) override;
 
   lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
  bool can_create = true) override;

diff  --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 4df64689d2d93..e8ed36380d377 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -668,7 +668,7 @@ SBValue SBValue::GetChildAtIndex(uint32_t idx,
   lldb::ValueObjectSP value_sp(GetSP(locker));
   if (value_sp) {
 const bool can_create = true;
-child_sp = value_sp->GetChildAtIndex(idx, can_create);
+child_sp = value_sp->GetChildAtIndex(idx);
 if (can_create_synthetic && !child_sp) {
   child_sp = value_sp->GetSyntheticArrayMember(idx, can_create);
 }

diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index e1bc56de4cdd7..79d3da63059cb 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -4522,7 +4522,7 @@ struct Row {
   if (valobj) {
 const size_t num_children = valobj->GetNumChildren();
 for (size_t i = 0; i < num_children; ++i) {
-  children.push_back(Row(valobj->GetChildAtIndex(i, true), this));
+  children.push_back(Row(valobj->GetChildAtIndex(i), this));
 }
   }
 }

diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 38b15b87e189f..9ff980db96d23 100644
--- 

[Lldb-commits] [PATCH] D152031: [lldb] Default can_create to true in GetChildAtIndex (NFC)

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1a74f7cdea3: [lldb] Default can_create to true in 
GetChildAtIndex (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152031

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/source/API/SBValue.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectSyntheticFilter.cpp
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
  lldb/source/Target/StackFrame.cpp

Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -814,7 +814,7 @@
   // extract bit low out of it. reading array item low would be done by
   // saying arr[low], without a deref * sign
   Status error;
-  ValueObjectSP temp(valobj_sp->GetChildAtIndex(0, true));
+  ValueObjectSP temp(valobj_sp->GetChildAtIndex(0));
   if (error.Fail()) {
 valobj_sp->GetExpressionPath(var_expr_path_strm);
 error.SetErrorStringWithFormat(
@@ -868,7 +868,7 @@
   valobj_sp->GetTypeName().AsCString(""),
   var_expr_path_strm.GetData());
 } else {
-  child_valobj_sp = synthetic->GetChildAtIndex(child_index, true);
+  child_valobj_sp = synthetic->GetChildAtIndex(child_index);
   if (!child_valobj_sp) {
 valobj_sp->GetExpressionPath(var_expr_path_strm);
 error.SetErrorStringWithFormat(
@@ -894,7 +894,7 @@
nullptr, nullptr, _incomplete_array)) {
   // Pass false to dynamic_value here so we can tell the difference
   // between no dynamic value and no member of this type...
-  child_valobj_sp = valobj_sp->GetChildAtIndex(child_index, true);
+  child_valobj_sp = valobj_sp->GetChildAtIndex(child_index);
   if (!child_valobj_sp && (is_incomplete_array || !no_synth_child))
 child_valobj_sp =
 valobj_sp->GetSyntheticArrayMember(child_index, true);
@@ -940,7 +940,7 @@
 valobj_sp->GetTypeName().AsCString(""),
 var_expr_path_strm.GetData());
   } else {
-child_valobj_sp = synthetic->GetChildAtIndex(child_index, true);
+child_valobj_sp = synthetic->GetChildAtIndex(child_index);
 if (!child_valobj_sp) {
   valobj_sp->GetExpressionPath(var_expr_path_strm);
   error.SetErrorStringWithFormat(
@@ -1012,7 +1012,7 @@
 // extract bits low thru high out of it. reading array items low thru
 // high would be done by saying arr[low-high], without a deref * sign
 Status error;
-ValueObjectSP temp(valobj_sp->GetChildAtIndex(0, true));
+ValueObjectSP temp(valobj_sp->GetChildAtIndex(0));
 if (error.Fail()) {
   valobj_sp->GetExpressionPath(var_expr_path_strm);
   error.SetErrorStringWithFormat(
@@ -1400,8 +1400,7 @@
   }
 
   for (int ci = 0, ce = parent->GetNumChildren(); ci != ce; ++ci) {
-const bool can_create = true;
-ValueObjectSP child_sp = parent->GetChildAtIndex(ci, can_create);
+ValueObjectSP child_sp = parent->GetChildAtIndex(ci);
 
 if (!child_sp) {
   return ValueObjectSP();
Index: lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
===
--- lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
+++ lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
@@ -120,7 +120,7 @@
 
   std::vector pcs;
   for (int i = 0; i < count; i++) {
-addr_t pc 

[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

[Alex Langford via Phabricator via lldb-commits]

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

This is definitely useful to support, I had a few comments mostly about 
matching lldb's coding style.

Also, are you running clang-format on your patches? There are a few places 
where I'm not sure if the formatting is right.




Comment at: lldb/source/Host/CMakeLists.txt:109-121
+
+set(ANDROID_SOURCES
+  android/ZipFile.cpp
+  android/HostInfoAndroid.cpp
+  )
 if (CMAKE_SYSTEM_NAME MATCHES "Android")
+  list(APPEND ANDROID_SOURCES

I don't think this is correct to do. lldbHost is different than other libraries 
in that it's meant to provide functionality that lldb and lldb-server needs to 
work on the host system. Unconditionally adding a host subdirectory for android 
even when we're on Linux doesn't make sense to do.



Comment at: lldb/source/Host/android/HostInfoAndroid.cpp:107
+  // #opening-shared-libraries-directly-from-an-apk
+  const std::string kZipSeparator = "!/";
+  auto path = file_spec.GetPath();

You can avoid using `std::string` here (and invoking the constructor) by using 
an llvm::StringLiteral. Like so:
```
static constexpr llvm::StringLiteral k_zip_separator("!/");
```



Comment at: lldb/source/Host/android/HostInfoAndroid.cpp:108
+  const std::string kZipSeparator = "!/";
+  auto path = file_spec.GetPath();
+  auto pos = path.find(kZipSeparator);

It's not obvious what the type of `path` is, please write out the type instead 
of using `auto`.



Comment at: lldb/source/Host/android/HostInfoAndroid.cpp:119-126
+  auto zip_path = path.substr(0, pos);
+  auto so_path = path.substr(pos + kZipSeparator.size());
+
+  auto zip_file_spec = FileSpec(zip_path);
+  auto zip_file_size = FileSystem::Instance().GetByteSize(zip_file_spec);
+  auto zip_data = FileSystem::Instance().CreateDataBuffer(zip_file_spec,
+  zip_file_size,

It's not obvious what the types of these are (except for `zip_file_spec`), 
please write out the types explicitly.



Comment at: lldb/source/Host/android/HostInfoAndroid.cpp:126
+  zip_file_size,
+  0);
+  if (ZipFile::Find(zip_data, so_path, file_offset, file_size)) {

It's not obvious what 0 is as the parameter here, please make it look something 
like this:
`/* offset = */ 0);`



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:261-264
+  if (const char *run_as = std::getenv("ANDROID_PLATFORM_RUN_AS"))
+snprintf(run_as_cmd, sizeof(run_as_cmd), "run-as '%s' ", run_as);
+  else
+run_as_cmd[0] = '\0';

Maybe it would be a good idea to centralize the `run-as` logic somewhere, right 
now it's pretty ad-hoc.



Comment at: lldb/unittests/Host/CMakeLists.txt:42
+
+if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
+  add_subdirectory(android)

Why not just match on `"Android"`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152759

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

[Lldb-commits] [PATCH] D152861: Clear non-addressable bits from fp/sp/lr/pc values in RegisterContextUnwind

[Jason Molenda via Phabricator via lldb-commits]

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

Address Alex's feedback of not extracting the ABI pointer out of the shared 
pointer; simply use the shared pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152861

Files:
  lldb/source/Target/RegisterContextUnwind.cpp

Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -137,9 +137,8 @@
   // (which would be a no-op in frame 0 where we get it from the register set,
   // but still a good idea to make the call here for other ABIs that may
   // exist.)
-  ABI *abi = process->GetABI().get();
-  if (abi)
-current_pc = abi->FixCodeAddress(current_pc);
+  if (ABISP abi_sp = process->GetABI())
+current_pc = abi_sp->FixCodeAddress(current_pc);
 
   UnwindPlanSP lang_runtime_plan_sp = LanguageRuntime::GetRuntimeUnwindPlan(
   m_thread, this, m_behaves_like_zeroth_frame);
@@ -355,17 +354,23 @@
 
   // Let ABIs fixup code addresses to make sure they are valid. In ARM ABIs
   // this will strip bit zero in case we read a PC from memory or from the LR.
-  ABI *abi = process->GetABI().get();
-  if (abi)
-pc = abi->FixCodeAddress(pc);
+  ABISP abi_sp = process->GetABI();
+  if (abi_sp)
+pc = abi_sp->FixCodeAddress(pc);
 
   if (log) {
 UnwindLogMsg("pc = 0x%" PRIx64, pc);
 addr_t reg_val;
-if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_FP, reg_val))
+if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_FP, reg_val)) {
+  if (abi_sp)
+reg_val = abi_sp->FixDataAddress(reg_val);
   UnwindLogMsg("fp = 0x%" PRIx64, reg_val);
-if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP, reg_val))
+}
+if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP, reg_val)) {
+  if (abi_sp)
+reg_val = abi_sp->FixDataAddress(reg_val);
   UnwindLogMsg("sp = 0x%" PRIx64, reg_val);
+}
   }
 
   // A pc of 0x0 means it's the end of the stack crawl unless we're above a trap
@@ -424,11 +429,11 @@
   }
 }
 
-if (abi) {
+if (abi_sp) {
   m_fast_unwind_plan_sp.reset();
   m_full_unwind_plan_sp =
   std::make_shared(lldb::eRegisterKindGeneric);
-  abi->CreateDefaultUnwindPlan(*m_full_unwind_plan_sp);
+  abi_sp->CreateDefaultUnwindPlan(*m_full_unwind_plan_sp);
   if (m_frame_type != eSkipFrame) // don't override eSkipFrame
   {
 m_frame_type = eNormalFrame;
@@ -1751,8 +1756,8 @@
   if (ReadRegisterValueFromRegisterLocation(regloc, reg_info, reg_value)) {
 old_caller_pc_value = reg_value.GetAsUInt64();
 if (ProcessSP process_sp = m_thread.GetProcess()) {
-  if (ABISP abi = process_sp->GetABI())
-old_caller_pc_value = abi->FixCodeAddress(old_caller_pc_value);
+  if (ABISP abi_sp = process_sp->GetABI())
+old_caller_pc_value = abi_sp->FixCodeAddress(old_caller_pc_value);
 }
   }
 }
@@ -1811,8 +1816,8 @@
   reg_value)) {
   new_caller_pc_value = reg_value.GetAsUInt64();
   if (ProcessSP process_sp = m_thread.GetProcess()) {
-if (ABISP abi = process_sp->GetABI())
-  new_caller_pc_value = abi->FixCodeAddress(new_caller_pc_value);
+if (ABISP abi_sp = process_sp->GetABI())
+  new_caller_pc_value = abi_sp->FixCodeAddress(new_caller_pc_value);
   }
 }
   }
@@ -1953,6 +1958,7 @@
 
   address = LLDB_INVALID_ADDRESS;
   addr_t cfa_reg_contents;
+  ABISP abi_sp = m_thread.GetProcess()->GetABI();
 
   switch (fa.GetValueType()) {
   case UnwindPlan::Row::FAValue::isRegisterDereferenced: {
@@ -1963,11 +1969,13 @@
   GetRegisterInfoAtIndex(cfa_reg.GetAsKind(eRegisterKindLLDB));
   RegisterValue reg_value;
   if (reg_info) {
+if (abi_sp)
+  cfa_reg_contents = abi_sp->FixDataAddress(cfa_reg_contents);
 Status error = ReadRegisterValueFromMemory(
 reg_info, cfa_reg_contents, reg_info->byte_size, reg_value);
 if (error.Success()) {
   address = reg_value.GetAsUInt64();
-  if (ABISP abi_sp = m_thread.GetProcess()->GetABI())
+  if (abi_sp)
 address = abi_sp->FixCodeAddress(address);
   UnwindLogMsg(
   "CFA value via dereferencing reg %s (%d): reg has val 0x%" PRIx64
@@ -1989,6 +1997,8 @@
 RegisterNumber cfa_reg(m_thread, row_register_kind,
fa.GetRegisterNumber());
 if (ReadGPRValue(cfa_reg, cfa_reg_contents)) {
+  if (abi_sp)
+cfa_reg_contents = abi_sp->FixDataAddress(cfa_reg_contents);
   if (cfa_reg_contents == LLDB_INVALID_ADDRESS || cfa_reg_contents == 0 ||
   cfa_reg_contents == 1) {

[Lldb-commits] [PATCH] D152494: [lldb][Android] Fix adb shell cat

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

An environment variable isn't the worst thing in the world, but what would be 
better is an lldb setting so we can change it at runtime instead of needing to 
restart the debugging session. That may happen if you make a typo or make a 
mistake. Something like `plugin.platform.android.platform-run-as`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152494

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

[Lldb-commits] [PATCH] D152863: [lldb] NFC Add Process methods to Fix.*Address, to simplify this bit clearing across the codebase

[Jason Molenda via Phabricator via lldb-commits]

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

Right now to clear unaddressable bits (AArch64 TBI or ptrauth, armv7 0th bit 
setting for arm/thumb) everyone needs to take a Process, get the ABI, then call 
the method.  As David and I had discussed in another patch, this is unnecessary 
friction that should be simplified.

This patch adds those same methods to Process, so the callers can skip the 
extra step of getting the ABI when working with these values.

I'm only adding the Process methods in this patch, leaving all the existing 
callers as-is, I'll look at updating those separately, but new users can start 
using these methods once they're in place.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152863

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -5688,6 +5688,24 @@
   return GetDataAddressMask();
 }
 
+addr_t Process::FixCodeAddress(addr_t addr) {
+  if (ABISP abi_sp = GetABI())
+addr = abi_sp->FixCodeAddress(addr);
+  return addr;
+}
+
+addr_t Process::FixDataAddress(addr_t addr) {
+  if (ABISP abi_sp = GetABI())
+addr = abi_sp->FixDataAddress(addr);
+  return addr;
+}
+
+addr_t Process::FixAnyAddress(addr_t addr) {
+  if (ABISP abi_sp = GetABI())
+addr = abi_sp->FixAnyAddress(addr);
+  return addr;
+}
+
 void Process::DidExec() {
   Log *log = GetLog(LLDBLog::Process);
   LLDB_LOGF(log, "Process::%s()", __FUNCTION__);
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -1392,6 +1392,22 @@
 m_highmem_data_address_mask = data_address_mask;
   }
 
+  /// Some targets might use bits in a code address to indicate a mode switch,
+  /// ARM uses bit zero to signify a code address is thumb, so any ARM ABI
+  /// plug-ins would strip those bits.
+  /// Or use the high bits to authenticate a pointer value.
+  lldb::addr_t FixCodeAddress(lldb::addr_t pc);
+  lldb::addr_t FixDataAddress(lldb::addr_t pc);
+
+  /// Use this method when you do not know, or do not care what kind of address
+  /// you are fixing. On platforms where there would be a difference between 
the
+  /// two types, it will pick the safest option.
+  ///
+  /// Its purpose is to signal that no specific choice was made and provide an
+  /// alternative to randomly picking FixCode/FixData address. Which could 
break
+  /// platforms where there is a difference (only Arm Thumb at this time).
+  lldb::addr_t FixAnyAddress(lldb::addr_t pc);
+
   /// Get the Modification ID of the process.
   ///
   /// \return


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -5688,6 +5688,24 @@
   return GetDataAddressMask();
 }
 
+addr_t Process::FixCodeAddress(addr_t addr) {
+  if (ABISP abi_sp = GetABI())
+addr = abi_sp->FixCodeAddress(addr);
+  return addr;
+}
+
+addr_t Process::FixDataAddress(addr_t addr) {
+  if (ABISP abi_sp = GetABI())
+addr = abi_sp->FixDataAddress(addr);
+  return addr;
+}
+
+addr_t Process::FixAnyAddress(addr_t addr) {
+  if (ABISP abi_sp = GetABI())
+addr = abi_sp->FixAnyAddress(addr);
+  return addr;
+}
+
 void Process::DidExec() {
   Log *log = GetLog(LLDBLog::Process);
   LLDB_LOGF(log, "Process::%s()", __FUNCTION__);
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -1392,6 +1392,22 @@
 m_highmem_data_address_mask = data_address_mask;
   }
 
+  /// Some targets might use bits in a code address to indicate a mode switch,
+  /// ARM uses bit zero to signify a code address is thumb, so any ARM ABI
+  /// plug-ins would strip those bits.
+  /// Or use the high bits to authenticate a pointer value.
+  lldb::addr_t FixCodeAddress(lldb::addr_t pc);
+  lldb::addr_t FixDataAddress(lldb::addr_t pc);
+
+  /// Use this method when you do not know, or do not care what kind of address
+  /// you are fixing. On platforms where there would be a difference between the
+  /// two types, it will pick the safest option.
+  ///
+  /// Its purpose is to signal that no specific choice was made and provide an
+  /// alternative to randomly picking FixCode/FixData address. Which could break
+  /// platforms where there is a difference (only Arm Thumb at this time).
+  lldb::addr_t FixAnyAddress(lldb::addr_t pc);
+
   /// Get the Modification ID of the process.
   

[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

[Alex Langford via Phabricator via lldb-commits]

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

I don't really work on android-related code anymore but this change seems 
alright to me. There's prior art in LLVM which makes it a lot easier to accept. 
It's unfortunate that we need this workaround because of an issue with Bionic, 
but that's life.

You may want to wait until another person has a chance to look at this though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152712

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

[Lldb-commits] [PATCH] D152855: [lldb][Android] Add PlatformAndroidTest

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack created this revision.
Herald added a subscriber: danielkiss.
Herald added a project: All.
splhack added reviewers: clayborg, labath, lanza, srhines.
splhack edited the summary of this revision.
splhack updated this revision to Diff 531075.
splhack added a comment.
splhack updated this revision to Diff 531082.
splhack published this revision for review.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

update depends


splhack added a comment.

.


splhack added a comment.

(not sure why build status got merge conflict, probably landing D152494 
 may resolve)


To test

- D152494  [lldb][Android] Fix adb shell cat
- D152759  [lldb][Android] Support zip .so 
file

introduce PlatformAndroidTest with the capability of mocking adb client.

Depends on D152494  and D152712 
 and D152757 
 and D152759 



Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152855

Files:
  lldb/source/Plugins/Platform/Android/AdbClient.h
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.h
  lldb/unittests/Platform/Android/CMakeLists.txt
  lldb/unittests/Platform/Android/PlatformAndroidTest.cpp

Index: lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
@@ -0,0 +1,217 @@
+//===-- PlatformAndroidTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Utility/Connection.h"
+#include "Plugins/Platform/Android/PlatformAndroid.h"
+#include "Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "gmock/gmock.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::platform_android;
+using namespace testing;
+
+namespace {
+
+class MockSyncService : public AdbClient::SyncService {
+public:
+  MockSyncService() : SyncService(std::move(m_mock_conn)) {}
+
+  MOCK_METHOD2(PullFile, Status(const FileSpec _file,
+const FileSpec _file));
+  MOCK_METHOD4(Stat, Status(const FileSpec _file, uint32_t ,
+uint32_t , uint32_t ));
+
+private:
+  std::unique_ptr m_mock_conn;
+};
+
+typedef std::unique_ptr SyncServiceSP;
+
+class MockAdbClient : public AdbClient {
+public:
+  explicit MockAdbClient() : AdbClient("mock") {}
+
+  MOCK_METHOD3(ShellToFile, Status(const char *command,
+   std::chrono::milliseconds timeout,
+   const FileSpec _file_spec));
+  MOCK_METHOD1(GetSyncService, SyncServiceSP(Status ));
+};
+
+static void set_env(const char *var, const char *value) {
+#ifdef _WIN32
+  _putenv_s(var, value);
+#else
+  if (strlen(value) == 0)
+unsetenv(var);
+  else
+setenv(var, value, true);
+#endif
+}
+
+class PlatformAndroidTest : public PlatformAndroid, public ::testing::Test {
+public:
+  PlatformAndroidTest() : PlatformAndroid(false) {
+m_remote_platform_sp = PlatformSP(new PlatformAndroidRemoteGDBServer());
+  }
+
+  MOCK_METHOD0(GetAdbClient, AdbClientSP());
+
+  void SetUp() override { set_env("ANDROID_PLATFORM_RUN_AS", ""); }
+
+  void TearDown() override { set_env("ANDROID_PLATFORM_RUN_AS", ""); }
+};
+
+} // namespace
+
+TEST_F(PlatformAndroidTest, DownloadModuleSliceWithNormalFile) {
+  auto sync_service = new MockSyncService();
+  EXPECT_CALL(*sync_service,
+  Stat(FileSpec("/system/lib64/libc.so"), _, _, _))
+  .Times(1).WillOnce(DoAll(SetArgReferee<1>(1), Return(Status(;
+  EXPECT_CALL(*sync_service,
+  PullFile(FileSpec("/system/lib64/libc.so"), _))
+  .Times(1).WillOnce(Return(Status()));
+
+  auto adb_client = new MockAdbClient();
+  EXPECT_CALL(*adb_client,
+   GetSyncService(_))
+   .Times(1).WillOnce(
+   Return(ByMove(std::move(SyncServiceSP(sync_service);
+
+  EXPECT_CALL(*this, GetAdbClient())
+  .Times(1).WillOnce(Return(ByMove(std::move(AdbClientSP(adb_client);
+
+  EXPECT_TRUE(
+  DownloadModuleSlice(FileSpec("/system/lib64/libc.so"), 0, 0, FileSpec())
+  .Success());
+}
+
+TEST_F(PlatformAndroidTest, DownloadModuleSliceWithZipFile) {
+  auto adb_client = new MockAdbClient();
+  EXPECT_CALL(*adb_client,
+   ShellToFile(
+   StrEq("dd if='/system/app/Test/Test.apk' "
+ 

[Lldb-commits] [PATCH] D152861: Clear non-addressable bits from fp/sp/lr/pc values in RegisterContextUnwind

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D152861#4419197 , @bulbazord wrote:

> I noticed you're pulling a lot of raw pointers out of shared pointers. Is 
> there a particular reason you're doing that? `operator bool` on 
> `std::shared_ptr` returns true if the held pointer is non-null.

Nah, I didn't care much in these contexts whether I was holding a shared 
pointer from the lifetime point of view.  You're right it's not going to be 
more efficient to do it this way, I might as well use the shared pointers I got 
back instead of extracting the pointer out of the SP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152861

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

[Lldb-commits] [PATCH] D152331: [lldb][NFCI] Platforms should own their SDKBuild and SDKRootDirectory strings

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 531084.
bulbazord added a comment.

Implement Jonas's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152331

Files:
  lldb/include/lldb/Interpreter/OptionGroupPlatform.h
  lldb/include/lldb/Target/Platform.h
  lldb/source/Interpreter/OptionGroupPlatform.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Target/Platform.cpp

Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -210,11 +210,10 @@
 Status error(eErrorTypeGeneric);
 ModuleSpec resolved_spec;
 // Check if we have sysroot set.
-if (m_sdk_sysroot) {
+if (!m_sdk_sysroot.empty()) {
   // Prepend sysroot to module spec.
   resolved_spec = spec;
-  resolved_spec.GetFileSpec().PrependPathComponent(
-  m_sdk_sysroot.GetStringRef());
+  resolved_spec.GetFileSpec().PrependPathComponent(m_sdk_sysroot);
   // Try to get shared module with resolved spec.
   error = ModuleList::GetSharedModule(resolved_spec, module_sp,
   module_search_paths_ptr, old_modules,
@@ -312,9 +311,9 @@
 strm.Printf(" Connected: %s\n", is_connected ? "yes" : "no");
   }
 
-  if (GetSDKRootDirectory()) {
-strm.Format("   Sysroot: {0}\n", GetSDKRootDirectory());
-  }
+  if (const std::string _root = GetSDKRootDirectory(); !sdk_root.empty())
+strm.Format("   Sysroot: {0}\n", sdk_root);
+
   if (GetWorkingDirectory()) {
 strm.Printf("WorkingDir: %s\n", GetWorkingDirectory().GetPath().c_str());
   }
Index: lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
===
--- lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -191,8 +191,8 @@
   launch_info.SetArguments(args, true);
 
   Environment emulator_env = Host::GetEnvironment();
-  if (ConstString sysroot = GetSDKRootDirectory())
-emulator_env["QEMU_LD_PREFIX"] = sysroot.GetStringRef().str();
+  if (const std::string  = GetSDKRootDirectory(); !sysroot.empty())
+emulator_env["QEMU_LD_PREFIX"] = sysroot;
   for (const auto  : GetGlobalProperties().GetEmulatorEnvVars())
 emulator_env[KV.first()] = KV.second;
   launch_info.GetEnvironment() = ComputeLaunchEnvironment(
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -34,8 +34,8 @@
   std::lock_guard guard(m_sdk_dir_mutex);
   if (m_sdk_directory_infos.empty()) {
 // A --sysroot option was supplied - add it to our list of SDKs to check
-if (m_sdk_sysroot) {
-  FileSpec sdk_sysroot_fspec(m_sdk_sysroot.GetCString());
+if (!m_sdk_sysroot.empty()) {
+  FileSpec sdk_sysroot_fspec(m_sdk_sysroot.c_str());
   FileSystem::Instance().Resolve(sdk_sysroot_fspec);
   const SDKDirectoryInfo sdk_sysroot_directory_info(sdk_sysroot_fspec);
   m_sdk_directory_infos.push_back(sdk_sysroot_directory_info);
@@ -43,7 +43,7 @@
 LLDB_LOGF(log,
   "PlatformDarwinDevice::UpdateSDKDirectoryInfosIfNeeded added "
   "--sysroot SDK directory %s",
-  m_sdk_sysroot.GetCString());
+  m_sdk_sysroot.c_str());
   }
   return true;
 }
@@ -152,20 +152,20 @@
 std::vector check_sdk_info(num_sdk_infos, true);
 
 // Prefer the user SDK build string.
-ConstString build = GetSDKBuild();
+std::string build = GetSDKBuild();
 
 // Fall back to the platform's build string.
-if (!build) {
-  if (std::optional os_build_str = GetOSBuildString()) {
-build = ConstString(*os_build_str);
-  }
+if (build.empty()) {
+  if (std::optional os_build_str = GetOSBuildString())
+build.assign(*os_build_str);
 }
 
 // If we have a build string, only check platforms for which the build
 // string matches.
-if (build) {
+if (!build.empty()) {
   for (i = 0; i < num_sdk_infos; ++i)
-check_sdk_info[i] = m_sdk_directory_infos[i].build == build;
+check_sdk_info[i] = m_sdk_directory_infos[i].build.GetStringRef() ==
+llvm::StringRef(build);
 }
 
 // If we are connected we can find the version of the OS the platform us
@@ -201,7 +201,7 @@
   }
 }
   }
-} else if (build) {
+} else if (!build.empty()) {
   // No version, just a build number, return the first one that matches.
   for (i = 0; i < num_sdk_infos; ++i)
 if (check_sdk_info[i])
@@ -248,8 +248,8 @@

[Lldb-commits] [PATCH] D152861: Clear non-addressable bits from fp/sp/lr/pc values in RegisterContextUnwind

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

I noticed you're pulling a lot of raw pointers out of shared pointers. Is there 
a particular reason you're doing that? `operator bool` on `std::shared_ptr` 
returns true if the held pointer is non-null.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152861

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

[Lldb-commits] [PATCH] D151951: [lldb][NFCI] Change return type of Properties::GetExperimentalSettingsName

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3cff6ca81335: [lldb][NFCI] Change return type of 
Properties::GetExperimentalSettingsName (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151951

Files:
  lldb/include/lldb/Core/UserSettingsController.h
  lldb/source/Core/UserSettingsController.cpp
  lldb/source/Interpreter/OptionValueProperties.cpp


Index: lldb/source/Interpreter/OptionValueProperties.cpp
===
--- lldb/source/Interpreter/OptionValueProperties.cpp
+++ lldb/source/Interpreter/OptionValueProperties.cpp
@@ -88,8 +88,8 @@
 value_sp->GetSubValue(exe_ctx, sub_name.drop_front(), error);
 if (!return_val_sp) {
   if (Properties::IsSettingExperimental(sub_name.drop_front())) {
-size_t experimental_len =
-strlen(Properties::GetExperimentalSettingsName());
+const size_t experimental_len =
+Properties::GetExperimentalSettingsName().size();
 if (sub_name[experimental_len + 1] == '.')
   return_val_sp = value_sp->GetSubValue(
   exe_ctx, sub_name.drop_front(experimental_len + 2), error);
Index: lldb/source/Core/UserSettingsController.cpp
===
--- lldb/source/Core/UserSettingsController.cpp
+++ lldb/source/Core/UserSettingsController.cpp
@@ -107,7 +107,10 @@
   return lldb::OptionValuePropertiesSP();
 }
 
-const char *Properties::GetExperimentalSettingsName() { return "experimental"; 
}
+llvm::StringRef Properties::GetExperimentalSettingsName() {
+  static constexpr llvm::StringLiteral g_experimental("experimental");
+  return g_experimental;
+}
 
 bool Properties::IsSettingExperimental(llvm::StringRef setting) {
   if (setting.empty())
Index: lldb/include/lldb/Core/UserSettingsController.h
===
--- lldb/include/lldb/Core/UserSettingsController.h
+++ lldb/include/lldb/Core/UserSettingsController.h
@@ -79,7 +79,7 @@
   // don't find the name will not be treated as errors.  Also, if you decide to
   // keep the settings just move them into the containing properties, and we
   // will auto-forward the experimental settings to the real one.
-  static const char *GetExperimentalSettingsName();
+  static llvm::StringRef GetExperimentalSettingsName();
 
   static bool IsSettingExperimental(llvm::StringRef setting);
 


Index: lldb/source/Interpreter/OptionValueProperties.cpp
===
--- lldb/source/Interpreter/OptionValueProperties.cpp
+++ lldb/source/Interpreter/OptionValueProperties.cpp
@@ -88,8 +88,8 @@
 value_sp->GetSubValue(exe_ctx, sub_name.drop_front(), error);
 if (!return_val_sp) {
   if (Properties::IsSettingExperimental(sub_name.drop_front())) {
-size_t experimental_len =
-strlen(Properties::GetExperimentalSettingsName());
+const size_t experimental_len =
+Properties::GetExperimentalSettingsName().size();
 if (sub_name[experimental_len + 1] == '.')
   return_val_sp = value_sp->GetSubValue(
   exe_ctx, sub_name.drop_front(experimental_len + 2), error);
Index: lldb/source/Core/UserSettingsController.cpp
===
--- lldb/source/Core/UserSettingsController.cpp
+++ lldb/source/Core/UserSettingsController.cpp
@@ -107,7 +107,10 @@
   return lldb::OptionValuePropertiesSP();
 }
 
-const char *Properties::GetExperimentalSettingsName() { return "experimental"; }
+llvm::StringRef Properties::GetExperimentalSettingsName() {
+  static constexpr llvm::StringLiteral g_experimental("experimental");
+  return g_experimental;
+}
 
 bool Properties::IsSettingExperimental(llvm::StringRef setting) {
   if (setting.empty())
Index: lldb/include/lldb/Core/UserSettingsController.h
===
--- lldb/include/lldb/Core/UserSettingsController.h
+++ lldb/include/lldb/Core/UserSettingsController.h
@@ -79,7 +79,7 @@
   // don't find the name will not be treated as errors.  Also, if you decide to
   // keep the settings just move them into the containing properties, and we
   // will auto-forward the experimental settings to the real one.
-  static const char *GetExperimentalSettingsName();
+  static llvm::StringRef GetExperimentalSettingsName();
 
   static bool IsSettingExperimental(llvm::StringRef setting);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 3cff6ca - [lldb][NFCI] Change return type of Properties::GetExperimentalSettingsName

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-06-13T14:53:24-07:00
New Revision: 3cff6ca81335ceff4ddd823b87daa38eedba5130

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

LOG: [lldb][NFCI] Change return type of Properties::GetExperimentalSettingsName

Most users of this stick it into a StringRef. The one user that doesn't
just tries to get the length out of it, which we can precompute by
putting it in a constexpr StringLiteral.

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

Added: 


Modified: 
lldb/include/lldb/Core/UserSettingsController.h
lldb/source/Core/UserSettingsController.cpp
lldb/source/Interpreter/OptionValueProperties.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/UserSettingsController.h 
b/lldb/include/lldb/Core/UserSettingsController.h
index f320057ef120d..26d5d37383ade 100644
--- a/lldb/include/lldb/Core/UserSettingsController.h
+++ b/lldb/include/lldb/Core/UserSettingsController.h
@@ -79,7 +79,7 @@ class Properties {
   // don't find the name will not be treated as errors.  Also, if you decide to
   // keep the settings just move them into the containing properties, and we
   // will auto-forward the experimental settings to the real one.
-  static const char *GetExperimentalSettingsName();
+  static llvm::StringRef GetExperimentalSettingsName();
 
   static bool IsSettingExperimental(llvm::StringRef setting);
 

diff  --git a/lldb/source/Core/UserSettingsController.cpp 
b/lldb/source/Core/UserSettingsController.cpp
index bc03b7891b052..30bb468357443 100644
--- a/lldb/source/Core/UserSettingsController.cpp
+++ b/lldb/source/Core/UserSettingsController.cpp
@@ -107,7 +107,10 @@ Properties::GetSubProperty(const ExecutionContext *exe_ctx,
   return lldb::OptionValuePropertiesSP();
 }
 
-const char *Properties::GetExperimentalSettingsName() { return "experimental"; 
}
+llvm::StringRef Properties::GetExperimentalSettingsName() {
+  static constexpr llvm::StringLiteral g_experimental("experimental");
+  return g_experimental;
+}
 
 bool Properties::IsSettingExperimental(llvm::StringRef setting) {
   if (setting.empty())

diff  --git a/lldb/source/Interpreter/OptionValueProperties.cpp 
b/lldb/source/Interpreter/OptionValueProperties.cpp
index 62cf408e229f8..8719a2b63656e 100644
--- a/lldb/source/Interpreter/OptionValueProperties.cpp
+++ b/lldb/source/Interpreter/OptionValueProperties.cpp
@@ -88,8 +88,8 @@ OptionValueProperties::GetSubValue(const ExecutionContext 
*exe_ctx,
 value_sp->GetSubValue(exe_ctx, sub_name.drop_front(), error);
 if (!return_val_sp) {
   if (Properties::IsSettingExperimental(sub_name.drop_front())) {
-size_t experimental_len =
-strlen(Properties::GetExperimentalSettingsName());
+const size_t experimental_len =
+Properties::GetExperimentalSettingsName().size();
 if (sub_name[experimental_len + 1] == '.')
   return_val_sp = value_sp->GetSubValue(
   exe_ctx, sub_name.drop_front(experimental_len + 2), error);



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

[Lldb-commits] [PATCH] D152861: Clear non-addressable bits from fp/sp/lr/pc values in RegisterContextUnwind

[Jason Molenda via Phabricator via lldb-commits]

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

The darwin kernel may have authentication bits on fields like $pc or $sp when 
the register state is thread_get_state'd with macOS Sonoma.  debugserver clears 
these bits before handing the values to lldb  arm64 corefiles created by gcore 
on MacOS Sonoma (macOS 14) will include these signed value as-is.

This patch changes RegisterContextUnwind to clear the unaddressable bits from 
sp/pc/fp/lr -- these must point to stack or code in memory.

We already clear the bits from spilled lr's because that's frequently signed 
with an ABI using ARMv8.3 ptrauth.  This patch extends that same behavior to sp 
and fp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152861

Files:
  lldb/source/Target/RegisterContextUnwind.cpp

Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -362,10 +362,16 @@
   if (log) {
 UnwindLogMsg("pc = 0x%" PRIx64, pc);
 addr_t reg_val;
-if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_FP, reg_val))
+if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_FP, reg_val)) {
+  if (abi)
+reg_val = abi->FixDataAddress(reg_val);
   UnwindLogMsg("fp = 0x%" PRIx64, reg_val);
-if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP, reg_val))
+}
+if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP, reg_val)) {
+  if (abi)
+reg_val = abi->FixDataAddress(reg_val);
   UnwindLogMsg("sp = 0x%" PRIx64, reg_val);
+}
   }
 
   // A pc of 0x0 means it's the end of the stack crawl unless we're above a trap
@@ -1953,6 +1959,7 @@
 
   address = LLDB_INVALID_ADDRESS;
   addr_t cfa_reg_contents;
+  ABI *abi = m_thread.GetProcess()->GetABI().get();
 
   switch (fa.GetValueType()) {
   case UnwindPlan::Row::FAValue::isRegisterDereferenced: {
@@ -1963,12 +1970,14 @@
   GetRegisterInfoAtIndex(cfa_reg.GetAsKind(eRegisterKindLLDB));
   RegisterValue reg_value;
   if (reg_info) {
+if (abi)
+  cfa_reg_contents = abi->FixDataAddress(cfa_reg_contents);
 Status error = ReadRegisterValueFromMemory(
 reg_info, cfa_reg_contents, reg_info->byte_size, reg_value);
 if (error.Success()) {
   address = reg_value.GetAsUInt64();
-  if (ABISP abi_sp = m_thread.GetProcess()->GetABI())
-address = abi_sp->FixCodeAddress(address);
+  if (abi)
+address = abi->FixCodeAddress(address);
   UnwindLogMsg(
   "CFA value via dereferencing reg %s (%d): reg has val 0x%" PRIx64
   ", CFA value is 0x%" PRIx64,
@@ -1989,6 +1998,8 @@
 RegisterNumber cfa_reg(m_thread, row_register_kind,
fa.GetRegisterNumber());
 if (ReadGPRValue(cfa_reg, cfa_reg_contents)) {
+  if (abi)
+cfa_reg_contents = abi->FixDataAddress(cfa_reg_contents);
   if (cfa_reg_contents == LLDB_INVALID_ADDRESS || cfa_reg_contents == 0 ||
   cfa_reg_contents == 1) {
 UnwindLogMsg(
@@ -2024,8 +2035,8 @@
 if (dwarfexpr.Evaluate(_ctx, this, 0, nullptr, nullptr, result,
)) {
   address = result.GetScalar().ULongLong();
-  if (ABISP abi_sp = m_thread.GetProcess()->GetABI())
-address = abi_sp->FixCodeAddress(address);
+  if (ABI *abi = m_thread.GetProcess()->GetABI().get())
+address = abi->FixCodeAddress(address);
 
   UnwindLogMsg("CFA value set by DWARF expression is 0x%" PRIx64,
address);
@@ -2076,6 +2087,8 @@
 return LLDB_INVALID_ADDRESS;
   if (!m_sym_ctx.module_sp || !m_sym_ctx.symbol)
 return LLDB_INVALID_ADDRESS;
+  if (ABI *abi = m_thread.GetProcess()->GetABI().get())
+hint = abi->FixCodeAddress(hint);
 
   hint += plan_offset;
 
@@ -2133,28 +2146,38 @@
 return false;
   }
 
+  uint32_t generic_regnum = LLDB_INVALID_REGNUM;
+  if (register_kind == eRegisterKindGeneric)
+generic_regnum = regnum;
+  else
+m_thread.GetRegisterContext()->ConvertBetweenRegisterKinds(
+register_kind, regnum, eRegisterKindGeneric, generic_regnum);
+  ABI *abi = m_thread.GetProcess()->GetABI().get();
+
   RegisterValue reg_value;
   // if this is frame 0 (currently executing frame), get the requested reg
   // contents from the actual thread registers
   if (IsFrameZero()) {
 if (m_thread.GetRegisterContext()->ReadRegister(reg_info, reg_value)) {
   value = reg_value.GetAsUInt64();
+  if (abi && generic_regnum != LLDB_INVALID_REGNUM) {
+if (generic_regnum == LLDB_REGNUM_GENERIC_PC ||
+ 

[Lldb-commits] [lldb] 68d964d - Revert "[lldb] Fix failure in TestStackCoreScriptedProcess on x86_64"

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2023-06-13T13:45:59-07:00
New Revision: 68d964d02418576c7b9ecbe5e8426d9681a1d3a8

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

LOG: Revert "[lldb] Fix failure in TestStackCoreScriptedProcess on x86_64"

This reverts commit 4177b490358432a457935ba5d6d076ae60de588f, since I
landed it by mistake.

Added: 


Modified: 
lldb/include/lldb/Utility/StructuredData.h
lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py

Removed: 




diff  --git a/lldb/include/lldb/Utility/StructuredData.h 
b/lldb/include/lldb/Utility/StructuredData.h
index a9df052605478..d6b51f7c58c12 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -484,7 +484,6 @@ class StructuredData {
   }
   return success;
 }
-  
 template 
 bool GetValueForKeyAsInteger(llvm::StringRef key, IntType ) const {
   ObjectSP value_sp = GetValueForKey(key);

diff  --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp 
b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index 920c66f153ba4..ac707ffb21711 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -250,12 +250,10 @@ bool ScriptedThread::CalculateStopInfo() {
 StopInfo::CreateStopReasonWithBreakpointSiteID(*this, break_id);
   } break;
   case lldb::eStopReasonSignal: {
-unsigned int signal;
+int signal;
 llvm::StringRef description;
-if (!data_dict->GetValueForKeyAsInteger("signal", signal)) {
-signal = LLDB_INVALID_SIGNAL_NUMBER;
-return false;
-}
+data_dict->GetValueForKeyAsInteger("signal", signal,
+   LLDB_INVALID_SIGNAL_NUMBER);
 data_dict->GetValueForKeyAsString("desc", description);
 stop_info_sp =
 StopInfo::CreateStopReasonWithSignal(*this, signal, 
description.data());

diff  --git 
a/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
 
b/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
index 28f8630ba8611..bf9681ad678b6 100644
--- 
a/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
+++ 
b/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
@@ -35,6 +35,7 @@ def get_module_with_name(self, target, name):
 @skipIfOutOfTreeDebugserver
 @skipIfRemote
 @skipIfAsan  # On ASAN builds, this test times-out (rdar://98678134)
+@skipIfDarwin
 def test_launch_scripted_process_stack_frames(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""



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

[Lldb-commits] [lldb] ec05cf5 - [lldb] Improve corefile saving ergonomics

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2023-06-13T13:44:51-07:00
New Revision: ec05cf50daece35e0b01d3761e64ae54efe8a78e

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

LOG: [lldb] Improve corefile saving ergonomics

This patch improves the way the user can save the process state into a
corefile by adding completion handler that would provide tab completion
for the corefile path and also resolves the corefile path to expand
relative path.

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/source/API/SBProcess.cpp
lldb/source/Commands/CommandObjectProcess.cpp

Removed: 




diff  --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp
index a5491c6fa6997..6050628a2e1f3 100644
--- a/lldb/source/API/SBProcess.cpp
+++ b/lldb/source/API/SBProcess.cpp
@@ -1183,6 +1183,7 @@ lldb::SBError SBProcess::SaveCore(const char *file_name,
   }
 
   FileSpec core_file(file_name);
+  FileSystem::Instance().Resolve(core_file);
   error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
 flavor);
 

diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp 
b/lldb/source/Commands/CommandObjectProcess.cpp
index 82d11687d377a..ab047ee926c99 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -1306,6 +1306,13 @@ class CommandObjectProcessSaveCore : public 
CommandObjectParsed {
 
   Options *GetOptions() override { return _options; }
 
+  void
+  HandleArgumentCompletion(CompletionRequest ,
+   OptionElementVector _element_vector) override {
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), lldb::eDiskFileCompletion, request, nullptr);
+  }
+
   class CommandOptions : public Options {
   public:
 CommandOptions() = default;
@@ -1354,6 +1361,7 @@ class CommandObjectProcessSaveCore : public 
CommandObjectParsed {
 if (process_sp) {
   if (command.GetArgumentCount() == 1) {
 FileSpec output_file(command.GetArgumentAtIndex(0));
+FileSystem::Instance().Resolve(output_file);
 SaveCoreStyle corefile_style = m_options.m_requested_save_core_style;
 Status error =
 PluginManager::SaveCore(process_sp, output_file, corefile_style,



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

[Lldb-commits] [lldb] 4177b49 - [lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2023-06-13T13:44:51-07:00
New Revision: 4177b490358432a457935ba5d6d076ae60de588f

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

LOG: [lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

This patch should address the failure of TestStackCoreScriptedProcess
that is happening specifically on x86_64.

It turns out that in 1370a1cb5b97, I changed the way we extract integers
from a `StructuredData::Dictionary` and in order to get a stop info from
the scripted process, we call a method that returns a `SBStructuredData`
containing the stop reason data.

TestStackCoreScriptedProcess` was failing specifically on x86_64 because
the stop info dictionary contains the signal number, that the `Scripted
Thread` was trying to extract as a signed integer where it was actually
parsed as an unsigned integer. That caused `GetValueForKeyAsInteger` to
return the default value parameter, `LLDB_INVALID_SIGNAL_NUMBER`.

This patch address the issue by extracting the signal number with the
appropriate type and re-enables the test.

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/include/lldb/Utility/StructuredData.h
lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py

Removed: 




diff  --git a/lldb/include/lldb/Utility/StructuredData.h 
b/lldb/include/lldb/Utility/StructuredData.h
index d6b51f7c58c12..a9df052605478 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -484,6 +484,7 @@ class StructuredData {
   }
   return success;
 }
+  
 template 
 bool GetValueForKeyAsInteger(llvm::StringRef key, IntType ) const {
   ObjectSP value_sp = GetValueForKey(key);

diff  --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp 
b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index ac707ffb21711..920c66f153ba4 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -250,10 +250,12 @@ bool ScriptedThread::CalculateStopInfo() {
 StopInfo::CreateStopReasonWithBreakpointSiteID(*this, break_id);
   } break;
   case lldb::eStopReasonSignal: {
-int signal;
+unsigned int signal;
 llvm::StringRef description;
-data_dict->GetValueForKeyAsInteger("signal", signal,
-   LLDB_INVALID_SIGNAL_NUMBER);
+if (!data_dict->GetValueForKeyAsInteger("signal", signal)) {
+signal = LLDB_INVALID_SIGNAL_NUMBER;
+return false;
+}
 data_dict->GetValueForKeyAsString("desc", description);
 stop_info_sp =
 StopInfo::CreateStopReasonWithSignal(*this, signal, 
description.data());

diff  --git 
a/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
 
b/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
index bf9681ad678b6..28f8630ba8611 100644
--- 
a/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
+++ 
b/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
@@ -35,7 +35,6 @@ def get_module_with_name(self, target, name):
 @skipIfOutOfTreeDebugserver
 @skipIfRemote
 @skipIfAsan  # On ASAN builds, this test times-out (rdar://98678134)
-@skipIfDarwin
 def test_launch_scripted_process_stack_frames(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""



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

[Lldb-commits] [PATCH] D152842: [lldb] Improve corefile saving ergonomics

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGec05cf50daec: [lldb] Improve corefile saving ergonomics 
(authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152842

Files:
  lldb/source/API/SBProcess.cpp
  lldb/source/Commands/CommandObjectProcess.cpp


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -1306,6 +1306,13 @@
 
   Options *GetOptions() override { return _options; }
 
+  void
+  HandleArgumentCompletion(CompletionRequest ,
+   OptionElementVector _element_vector) override {
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), lldb::eDiskFileCompletion, request, nullptr);
+  }
+
   class CommandOptions : public Options {
   public:
 CommandOptions() = default;
@@ -1354,6 +1361,7 @@
 if (process_sp) {
   if (command.GetArgumentCount() == 1) {
 FileSpec output_file(command.GetArgumentAtIndex(0));
+FileSystem::Instance().Resolve(output_file);
 SaveCoreStyle corefile_style = m_options.m_requested_save_core_style;
 Status error =
 PluginManager::SaveCore(process_sp, output_file, corefile_style,
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1183,6 +1183,7 @@
   }
 
   FileSpec core_file(file_name);
+  FileSystem::Instance().Resolve(core_file);
   error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
 flavor);
 


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -1306,6 +1306,13 @@
 
   Options *GetOptions() override { return _options; }
 
+  void
+  HandleArgumentCompletion(CompletionRequest ,
+   OptionElementVector _element_vector) override {
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), lldb::eDiskFileCompletion, request, nullptr);
+  }
+
   class CommandOptions : public Options {
   public:
 CommandOptions() = default;
@@ -1354,6 +1361,7 @@
 if (process_sp) {
   if (command.GetArgumentCount() == 1) {
 FileSpec output_file(command.GetArgumentAtIndex(0));
+FileSystem::Instance().Resolve(output_file);
 SaveCoreStyle corefile_style = m_options.m_requested_save_core_style;
 Status error =
 PluginManager::SaveCore(process_sp, output_file, corefile_style,
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1183,6 +1183,7 @@
   }
 
   FileSpec core_file(file_name);
+  FileSystem::Instance().Resolve(core_file);
   error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
 flavor);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152842: [lldb] Improve corefile saving ergonomics

[Alex Langford via Phabricator via lldb-commits]

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

LGTM, what do you think @jasonmolenda ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152842

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

[Lldb-commits] [PATCH] D152846: [lldb][NFCI] Remove custom matcher classes in Listener in favor of lambdas

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/source/Utility/Listener.cpp:214
   } else {
-pos = std::find_if(m_events.begin(), m_events.end(),
-   EventMatcher(broadcaster, event_type_mask));
+pos = std::find_if(m_events.begin(), m_events.end(), event_matcher);
   }

Since you used `llvm::erased_it` above, could you use `llvm::find_if` here as 
well to stay consistent. It will also make this line shorter since you can just 
pass the container instead of passing the beginning and ending iterators.



Comment at: lldb/source/Utility/Listener.cpp:372
+iter =
+std::find_if(m_broadcaster_managers.begin(), end_iter, 
manager_matcher);
 if (iter == end_iter)

ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152846

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

[Lldb-commits] [PATCH] D152848: [lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

[Alex Langford via Phabricator via lldb-commits]

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

Makes sense.




Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:253
   case lldb::eStopReasonSignal: {
-int signal;
+unsigned int signal;
 llvm::StringRef description;

Is there a more specific type we can use other than `unsigned int`? Maybe 
`uint32_t`?


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

https://reviews.llvm.org/D152848

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

[Lldb-commits] [PATCH] D152331: [lldb][NFCI] Platforms should own their SDKBuild and SDKRootDirectory strings

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Interpreter/OptionGroupPlatform.h:52
 
-  void SetSDKRootDirectory(ConstString sdk_root_directory) {
-m_sdk_sysroot = sdk_root_directory;
+  void SetSDKRootDirectory(llvm::StringRef sdk_root_directory) {
+m_sdk_sysroot = sdk_root_directory.str();

bulbazord wrote:
> JDevlieghere wrote:
> > This should take a std::string by value and move it.
> Is there a specific reason you want to do that instead of using 
> `llvm::StringRef::str()`?
Yes, it saves a copy when calling `SetSDKRootDirectory` with an rvalue 
reference or a moved std::string. There's no downside if you're going to store 
it as a std::string anyway. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152331

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

[Lldb-commits] [PATCH] D152848: [lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 531017.
mib added a comment.

Re-enable test


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

https://reviews.llvm.org/D152848

Files:
  lldb/include/lldb/Utility/StructuredData.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py


Index: 
lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
===
--- 
lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
+++ 
lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
@@ -35,7 +35,6 @@
 @skipIfOutOfTreeDebugserver
 @skipIfRemote
 @skipIfAsan  # On ASAN builds, this test times-out (rdar://98678134)
-@skipIfDarwin
 def test_launch_scripted_process_stack_frames(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""
Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -250,10 +250,12 @@
 StopInfo::CreateStopReasonWithBreakpointSiteID(*this, break_id);
   } break;
   case lldb::eStopReasonSignal: {
-int signal;
+unsigned int signal;
 llvm::StringRef description;
-data_dict->GetValueForKeyAsInteger("signal", signal,
-   LLDB_INVALID_SIGNAL_NUMBER);
+if (!data_dict->GetValueForKeyAsInteger("signal", signal)) {
+signal = LLDB_INVALID_SIGNAL_NUMBER;
+return false;
+}
 data_dict->GetValueForKeyAsString("desc", description);
 stop_info_sp =
 StopInfo::CreateStopReasonWithSignal(*this, signal, 
description.data());
Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -484,6 +484,7 @@
   }
   return success;
 }
+  
 template 
 bool GetValueForKeyAsInteger(llvm::StringRef key, IntType ) const {
   ObjectSP value_sp = GetValueForKey(key);


Index: lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
@@ -35,7 +35,6 @@
 @skipIfOutOfTreeDebugserver
 @skipIfRemote
 @skipIfAsan  # On ASAN builds, this test times-out (rdar://98678134)
-@skipIfDarwin
 def test_launch_scripted_process_stack_frames(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""
Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -250,10 +250,12 @@
 StopInfo::CreateStopReasonWithBreakpointSiteID(*this, break_id);
   } break;
   case lldb::eStopReasonSignal: {
-int signal;
+unsigned int signal;
 llvm::StringRef description;
-data_dict->GetValueForKeyAsInteger("signal", signal,
-   LLDB_INVALID_SIGNAL_NUMBER);
+if (!data_dict->GetValueForKeyAsInteger("signal", signal)) {
+signal = LLDB_INVALID_SIGNAL_NUMBER;
+return false;
+}
 data_dict->GetValueForKeyAsString("desc", description);
 stop_info_sp =
 StopInfo::CreateStopReasonWithSignal(*this, signal, description.data());
Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -484,6 +484,7 @@
   }
   return success;
 }
+  
 template 
 bool GetValueForKeyAsInteger(llvm::StringRef key, IntType ) const {
   ObjectSP value_sp = GetValueForKey(key);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152848: [lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: bulbazord, jingham, JDevlieghere.
mib added a project: LLDB.
Herald added a subscriber: pengfei.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch should address the failure of TestStackCoreScriptedProcess
that is happening specifically on x86_64.

It turns out that in 1370a1cb5b97 
, I 
changed the way we extract integers
from a `StructuredData::Dictionary` and in order to get a stop info from
the scripted process, we call a method that returns a `SBStructuredData`
containing the stop reason data.

TestStackCoreScriptedProcess` was failing specifically on x86_64 because
the stop info dictionary contains the signal number, that the `Scripted
Thread` was trying to extract as a signed integer where it was actually
parsed as an unsigned integer. That caused `GetValueForKeyAsInteger` to
return the default value parameter, `LLDB_INVALID_SIGNAL_NUMBER`.

This patch address the issue by extracting the signal number with the
appropriate type and re-enables the test.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152848

Files:
  lldb/include/lldb/Utility/StructuredData.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp


Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -250,10 +250,12 @@
 StopInfo::CreateStopReasonWithBreakpointSiteID(*this, break_id);
   } break;
   case lldb::eStopReasonSignal: {
-int signal;
+unsigned int signal;
 llvm::StringRef description;
-data_dict->GetValueForKeyAsInteger("signal", signal,
-   LLDB_INVALID_SIGNAL_NUMBER);
+if (!data_dict->GetValueForKeyAsInteger("signal", signal)) {
+signal = LLDB_INVALID_SIGNAL_NUMBER;
+return false;
+}
 data_dict->GetValueForKeyAsString("desc", description);
 stop_info_sp =
 StopInfo::CreateStopReasonWithSignal(*this, signal, 
description.data());
Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -484,6 +484,7 @@
   }
   return success;
 }
+  
 template 
 bool GetValueForKeyAsInteger(llvm::StringRef key, IntType ) const {
   ObjectSP value_sp = GetValueForKey(key);


Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -250,10 +250,12 @@
 StopInfo::CreateStopReasonWithBreakpointSiteID(*this, break_id);
   } break;
   case lldb::eStopReasonSignal: {
-int signal;
+unsigned int signal;
 llvm::StringRef description;
-data_dict->GetValueForKeyAsInteger("signal", signal,
-   LLDB_INVALID_SIGNAL_NUMBER);
+if (!data_dict->GetValueForKeyAsInteger("signal", signal)) {
+signal = LLDB_INVALID_SIGNAL_NUMBER;
+return false;
+}
 data_dict->GetValueForKeyAsString("desc", description);
 stop_info_sp =
 StopInfo::CreateStopReasonWithSignal(*this, signal, description.data());
Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -484,6 +484,7 @@
   }
   return success;
 }
+  
 template 
 bool GetValueForKeyAsInteger(llvm::StringRef key, IntType ) const {
   ObjectSP value_sp = GetValueForKey(key);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d4fcd411b3d: [lldb] Default can_create to true in 
GetChildMemberWithName (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151966

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectRegister.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/source/API/SBValue.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  lldb/source/Target/StackFrame.cpp

Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -604,7 +604,7 @@
   valobj_sp = GetValueObjectForFrameVariable(variable_sp, use_dynamic);
   if (!valobj_sp)
 return valobj_sp;
-  valobj_sp = valobj_sp->GetChildMemberWithName(name_const_string, true);
+  valobj_sp = valobj_sp->GetChildMemberWithName(name_const_string);
   if (valobj_sp)
 break;
 }
@@ -705,13 +705,13 @@
   return ValueObjectSP();
 }
   }
-  child_valobj_sp = valobj_sp->GetChildMemberWithName(child_name, true);
+  child_valobj_sp = valobj_sp->GetChildMemberWithName(child_name);
   if (!child_valobj_sp) {
 if (!no_synth_child) {
   child_valobj_sp = valobj_sp->GetSyntheticValue();
   if (child_valobj_sp)
 child_valobj_sp =
-child_valobj_sp->GetChildMemberWithName(child_name, true);
+child_valobj_sp->GetChildMemberWithName(child_name);
 }
 
 if (no_synth_child || !child_valobj_sp) {
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -514,7 +514,7 @@
 ThreadSP AppleObjCRuntime::GetBacktraceThreadFromException(
 lldb::ValueObjectSP exception_sp) {
   ValueObjectSP reserved_dict =
-  exception_sp->GetChildMemberWithName("reserved", true);
+  exception_sp->GetChildMemberWithName("reserved");
   if (!reserved_dict)
 return FailExceptionParsing("Failed to get 'reserved' member.");
 
@@ -567,15 +567,15 @@
 
   if (!return_addresses)
 return FailExceptionParsing("Failed to get return addresses.");
-  auto frames_value = return_addresses->GetChildMemberWithName("_frames", true);
+  auto frames_value = return_addresses->GetChildMemberWithName("_frames");
   if (!frames_value)
 return FailExceptionParsing("Failed to get frames_value.");
   addr_t frames_addr = frames_value->GetValueAsUnsigned(0);
-  auto count_value = return_addresses->GetChildMemberWithName("_cnt", true);
+  auto count_value = return_addresses->GetChildMemberWithName("_cnt");
   if (!count_value)
 return FailExceptionParsing("Failed to get count_value.");
   size_t count = count_value->GetValueAsUnsigned(0);
-  auto ignore_value = return_addresses->GetChildMemberWithName("_ignore", true);
+  auto ignore_value = return_addresses->GetChildMemberWithName("_ignore");
   if (!ignore_value)
 return FailExceptionParsing("Failed to get ignore_value.");
   size_t ignore = ignore_value->GetValueAsUnsigned(0);
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -138,11 

[Lldb-commits] [lldb] 7d4fcd4 - [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-06-13T11:37:41-07:00
New Revision: 7d4fcd411b3d790e5347c302e6a610ed793ece77

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

LOG: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

It turns out all existing callers of `GetChildMemberWithName` pass true for 
`can_create`.
This change makes `true` the default value, callers don't have to pass an 
opaque true.

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

Added: 


Modified: 
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectRegister.h
lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
lldb/source/API/SBValue.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/source/Target/StackFrame.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index 9f7bdf61e402b..b3eca5064a7b2 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -487,7 +487,7 @@ class ValueObject {
  ConstString *name_of_error = nullptr);
 
   virtual lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
- bool can_create);
+ bool can_create = true);
 
   virtual size_t GetIndexOfChildWithName(llvm::StringRef name);
 

diff  --git a/lldb/include/lldb/Core/ValueObjectRegister.h 
b/lldb/include/lldb/Core/ValueObjectRegister.h
index 9859b9e8d645f..2e47eee3d7f79 100644
--- a/lldb/include/lldb/Core/ValueObjectRegister.h
+++ b/lldb/include/lldb/Core/ValueObjectRegister.h
@@ -53,7 +53,7 @@ class ValueObjectRegisterSet : public ValueObject {
   int32_t synthetic_index) override;
 
   lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
- bool can_create) override;
+ bool can_create = true) override;
 
   size_t GetIndexOfChildWithName(llvm::StringRef name) override;
 

diff  --git a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h 
b/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
index 8e20e05c27522..9df322ae6bc3c 100644
--- a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
+++ b/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
@@ -54,7 +54,7 @@ class ValueObjectSynthetic : public ValueObject {
   lldb::ValueObjectSP GetChildAtIndex(size_t idx, bool can_create) override;
 
   lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
- bool can_create) override;
+ bool can_create = true) override;
 
   size_t GetIndexOfChildWithName(llvm::StringRef name) override;
 

diff  --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index aee9e86fdde12..4df64689d2d93 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -715,7 +715,7 @@ SBValue::GetChildMemberWithName(const char *name,
   ValueLocker locker;
   lldb::ValueObjectSP value_sp(GetSP(locker));
   if (value_sp) {
-child_sp = value_sp->GetChildMemberWithName(name, true);
+child_sp = value_sp->GetChildMemberWithName(name);
   }
 
   SBValue sb_value;

diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 1e18210a72379..38b15b87e189f 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -432,7 

[Lldb-commits] [PATCH] D152846: [lldb][NFCI] Remove custom matcher classes in Listener in favor of lambdas

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jingham, fdeazeve.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Instead of writing boilerplate classes to serve as matchers for things
like `find_if` and `erase_if`, we can use lambdas. I believe this
simplifies the Listener class.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152846

Files:
  lldb/source/Utility/Listener.cpp

Index: lldb/source/Utility/Listener.cpp
===
--- lldb/source/Utility/Listener.cpp
+++ lldb/source/Utility/Listener.cpp
@@ -18,20 +18,6 @@
 using namespace lldb;
 using namespace lldb_private;
 
-namespace {
-class BroadcasterManagerWPMatcher {
-public:
-  BroadcasterManagerWPMatcher(BroadcasterManagerSP manager_sp)
-  : m_manager_sp(std::move(manager_sp)) {}
-  bool operator()(const BroadcasterManagerWP _wp) const {
-BroadcasterManagerSP input_sp = input_wp.lock();
-return (input_sp && input_sp == m_manager_sp);
-  }
-
-  BroadcasterManagerSP m_manager_sp;
-};
-} // anonymous namespace
-
 Listener::Listener(const char *name)
 : m_name(name), m_broadcasters(), m_broadcasters_mutex(), m_events(),
   m_events_mutex(), m_is_shadow() {
@@ -181,17 +167,12 @@
 }
 
 void Listener::BroadcasterManagerWillDestruct(BroadcasterManagerSP manager_sp) {
-  // Just need to remove this broadcast manager from the list of managers:
-  broadcaster_manager_collection::iterator iter,
-  end_iter = m_broadcaster_managers.end();
-  BroadcasterManagerWP manager_wp;
-
-  BroadcasterManagerWPMatcher matcher(std::move(manager_sp));
-  iter = std::find_if(
-  m_broadcaster_managers.begin(), end_iter, matcher);
-  if (iter != end_iter)
-m_broadcaster_managers.erase(iter);
+  const auto manager_matcher =
+  [_sp](const BroadcasterManagerWP _wp) -> bool {
+BroadcasterManagerSP input_sp = input_wp.lock();
+return (input_sp && input_sp == manager_sp);
+  };
+  llvm::erase_if(m_broadcaster_managers, manager_matcher);
 }
 
 void Listener::AddEvent(EventSP _sp) {
@@ -206,36 +187,6 @@
   m_events_condition.notify_all();
 }
 
-class EventBroadcasterMatches {
-public:
-  EventBroadcasterMatches(Broadcaster *broadcaster)
-  : m_broadcaster(broadcaster) {}
-
-  bool operator()(const EventSP _sp) const {
-return event_sp->BroadcasterIs(m_broadcaster);
-  }
-
-private:
-  Broadcaster *m_broadcaster;
-};
-
-class EventMatcher {
-public:
-  EventMatcher(Broadcaster *broadcaster, uint32_t event_type_mask)
-  : m_broadcaster(broadcaster), m_event_type_mask(event_type_mask) {}
-
-  bool operator()(const EventSP _sp) const {
-if (m_broadcaster && !event_sp->BroadcasterIs(m_broadcaster))
-  return false;
-
-return m_event_type_mask == 0 || m_event_type_mask & event_sp->GetType();
-  }
-
-private:
-  Broadcaster *m_broadcaster;
-  const uint32_t m_event_type_mask;
-};
-
 bool Listener::FindNextEventInternal(
 std::unique_lock ,
 Broadcaster *broadcaster, // nullptr for any broadcaster
@@ -249,13 +200,18 @@
   if (m_events.empty())
 return false;
 
+  const auto event_matcher =
+  [broadcaster, event_type_mask](const EventSP _sp) -> bool {
+if (broadcaster && !event_sp->BroadcasterIs(broadcaster))
+  return false;
+return event_type_mask == 0 || event_type_mask & event_sp->GetType();
+  };
   Listener::event_collection::iterator pos = m_events.end();
 
   if (broadcaster == nullptr && event_type_mask == 0) {
 pos = m_events.begin();
   } else {
-pos = std::find_if(m_events.begin(), m_events.end(),
-   EventMatcher(broadcaster, event_type_mask));
+pos = std::find_if(m_events.begin(), m_events.end(), event_matcher);
   }
 
   if (pos != m_events.end()) {
@@ -395,6 +351,11 @@
   if (!manager_sp)
 return 0;
 
+  const auto manager_matcher =
+  [_sp](const BroadcasterManagerWP _wp) -> bool {
+BroadcasterManagerSP input_sp = input_wp.lock();
+return (input_sp && input_sp == manager_sp);
+  };
   // The BroadcasterManager mutex must be locked before m_broadcasters_mutex to
   // avoid violating the lock hierarchy (manager before broadcasters).
   std::lock_guard manager_guard(
@@ -407,10 +368,8 @@
 broadcaster_manager_collection::iterator iter,
 end_iter = m_broadcaster_managers.end();
 BroadcasterManagerWP manager_wp(manager_sp);
-BroadcasterManagerWPMatcher matcher(manager_sp);
-iter = std::find_if(
-m_broadcaster_managers.begin(), end_iter, matcher);
+iter =
+std::find_if(m_broadcaster_managers.begin(), end_iter, manager_matcher);
 if (iter == end_iter)
   m_broadcaster_managers.push_back(manager_wp);
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added a comment.

I'm alright with this if Jim is. I think we should remove this parameter since 
it's never set to anything other than `true` from what I understand, but that 
can be done in a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151966

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

[Lldb-commits] [PATCH] D152409: [lldb] Never print children if the max depth has been reached

[Augusto Noronha via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf94c7ffe4640: [lldb] Never print children if the max depth 
has been reached (authored by augusto2112).

Changed prior to commit:
  https://reviews.llvm.org/D152409?vs=529673=530994#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152409

Files:
  lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  
lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/TestFrameVarDepthAndElemCount.py
  lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/main.cpp

Index: lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/main.cpp
===
--- lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/main.cpp
+++ lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/main.cpp
@@ -15,5 +15,5 @@
 int main() {
   C *c = new C[5];
   puts("break here");
-  return 0; // break here
+  return 0;
 }
Index: lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/TestFrameVarDepthAndElemCount.py
===
--- lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/TestFrameVarDepthAndElemCount.py
+++ lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/TestFrameVarDepthAndElemCount.py
@@ -12,7 +12,7 @@
 """Test that bool types work in the expression parser"""
 self.build()
 lldbutil.run_to_source_breakpoint(
-self, "// break here", lldb.SBFileSpec("main.cpp")
+self, "break here", lldb.SBFileSpec("main.cpp")
 )
 
 # Check that we print 5 elements but only 2 levels deep.
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -502,7 +502,6 @@
 }
 
 bool ValueObjectPrinter::ShouldPrintChildren(
-bool is_failed_description,
 DumpValueObjectOptions::PointerDepth _ptr_depth) {
   const bool is_ref = IsRef();
   const bool is_ptr = IsPtr();
@@ -511,6 +510,10 @@
   if (is_uninit)
 return false;
 
+  // If we have reached the maximum depth we shouldn't print any more children.
+  if (HasReachedMaximumDepth())
+return false;
+
   // if the user has specified an element count, always print children as it is
   // explicit user demand being honored
   if (m_options.m_pointer_as_array)
@@ -523,37 +526,34 @@
   if (TypeSummaryImpl *type_summary = GetSummaryFormatter())
 print_children = type_summary->DoesPrintChildren(m_valobj);
 
-  if (is_failed_description || !HasReachedMaximumDepth()) {
-// We will show children for all concrete types. We won't show pointer
-// contents unless a pointer depth has been specified. We won't reference
-// contents unless the reference is the root object (depth of zero).
+  // We will show children for all concrete types. We won't show pointer
+  // contents unless a pointer depth has been specified. We won't reference
+  // contents unless the reference is the root object (depth of zero).
 
-// Use a new temporary pointer depth in case we override the current
-// pointer depth below...
+  // Use a new temporary pointer depth in case we override the current
+  // pointer depth below...
 
-if (is_ptr || is_ref) {
-  // We have a pointer or reference whose value is an address. Make sure
-  // that address is not NULL
-  AddressType ptr_address_type;
-  if (m_valobj->GetPointerValue(_address_type) == 0)
-return false;
+  if (is_ptr || is_ref) {
+// We have a pointer or reference whose value is an address. Make sure
+// that address is not NULL
+AddressType ptr_address_type;
+if (m_valobj->GetPointerValue(_address_type) == 0)
+  return false;
 
-  const bool is_root_level = m_curr_depth == 0;
+const bool is_root_level = m_curr_depth == 0;
 
-  if (is_ref && is_root_level && print_children) {
-// If this is the root object (depth is zero) that we are showing and
-// it is a reference, and no pointer depth has been supplied print out
-// what it references. Don't do this at deeper depths otherwise we can
-// end up with infinite recursion...
-return true;
-  }
-
-  return curr_ptr_depth.CanAllowExpansion();
+if (is_ref && is_root_level && print_children) {
+  // If this is the root object (depth is zero) that we are showing and
+  // it is a reference, and no pointer depth has been supplied print out
+  // what it references. Don't do this at deeper depths otherwise we can
+  // end up with infinite recursion...
+  return true;
 }
 
-return print_children || m_summary.empty();
+return curr_ptr_depth.CanAllowExpansion();
   }
-  return false;
+
+  return 

[Lldb-commits] [lldb] f94c7ff - [lldb] Never print children if the max depth has been reached

[Augusto Noronha via lldb-commits]

Author: Augusto Noronha
Date: 2023-06-13T11:03:04-07:00
New Revision: f94c7ffe46400559740e7f81d7c60d8c0fe7df82

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

LOG: [lldb] Never print children if the max depth has been reached

When formatting a variable, the max depth would potentially be ignored
if the current value object failed to print itself. Change that to
always respect the max depth, even if failure occurs.

rdar://109855463

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

Added: 


Modified: 
lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
lldb/source/DataFormatters/ValueObjectPrinter.cpp

lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/TestFrameVarDepthAndElemCount.py
lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/main.cpp

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h 
b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index a4946a20591a5..2b3936eaa707f 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -91,8 +91,7 @@ class ValueObjectPrinter {
   bool PrintObjectDescriptionIfNeeded(bool value_printed, bool 
summary_printed);
 
   bool
-  ShouldPrintChildren(bool is_failed_description,
-  DumpValueObjectOptions::PointerDepth _ptr_depth);
+  ShouldPrintChildren(DumpValueObjectOptions::PointerDepth _ptr_depth);
 
   bool ShouldExpandEmptyAggregates();
 

diff  --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index 16aeff13791c5..8e89b0bd4797e 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -502,7 +502,6 @@ bool 
DumpValueObjectOptions::PointerDepth::CanAllowExpansion() const {
 }
 
 bool ValueObjectPrinter::ShouldPrintChildren(
-bool is_failed_description,
 DumpValueObjectOptions::PointerDepth _ptr_depth) {
   const bool is_ref = IsRef();
   const bool is_ptr = IsPtr();
@@ -511,6 +510,10 @@ bool ValueObjectPrinter::ShouldPrintChildren(
   if (is_uninit)
 return false;
 
+  // If we have reached the maximum depth we shouldn't print any more children.
+  if (HasReachedMaximumDepth())
+return false;
+
   // if the user has specified an element count, always print children as it is
   // explicit user demand being honored
   if (m_options.m_pointer_as_array)
@@ -523,37 +526,34 @@ bool ValueObjectPrinter::ShouldPrintChildren(
   if (TypeSummaryImpl *type_summary = GetSummaryFormatter())
 print_children = type_summary->DoesPrintChildren(m_valobj);
 
-  if (is_failed_description || !HasReachedMaximumDepth()) {
-// We will show children for all concrete types. We won't show pointer
-// contents unless a pointer depth has been specified. We won't reference
-// contents unless the reference is the root object (depth of zero).
+  // We will show children for all concrete types. We won't show pointer
+  // contents unless a pointer depth has been specified. We won't reference
+  // contents unless the reference is the root object (depth of zero).
 
-// Use a new temporary pointer depth in case we override the current
-// pointer depth below...
+  // Use a new temporary pointer depth in case we override the current
+  // pointer depth below...
 
-if (is_ptr || is_ref) {
-  // We have a pointer or reference whose value is an address. Make sure
-  // that address is not NULL
-  AddressType ptr_address_type;
-  if (m_valobj->GetPointerValue(_address_type) == 0)
-return false;
+  if (is_ptr || is_ref) {
+// We have a pointer or reference whose value is an address. Make sure
+// that address is not NULL
+AddressType ptr_address_type;
+if (m_valobj->GetPointerValue(_address_type) == 0)
+  return false;
 
-  const bool is_root_level = m_curr_depth == 0;
+const bool is_root_level = m_curr_depth == 0;
 
-  if (is_ref && is_root_level && print_children) {
-// If this is the root object (depth is zero) that we are showing and
-// it is a reference, and no pointer depth has been supplied print out
-// what it references. Don't do this at deeper depths otherwise we can
-// end up with infinite recursion...
-return true;
-  }
-
-  return curr_ptr_depth.CanAllowExpansion();
+if (is_ref && is_root_level && print_children) {
+  // If this is the root object (depth is zero) that we are showing and
+  // it is a reference, and no pointer depth has been supplied print out
+  // what it references. Don't do this at deeper depths otherwise we can
+  // end up with infinite recursion...
+  return true;
 }
 
-return 

[Lldb-commits] [PATCH] D152837: [lldb] Identify Swift-implemented ObjC classes

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Core/ValueObjectDynamicValue.cpp:169
+  if (known_type == lldb::eLanguageTypeObjC &&
+  UseSwiftRuntime(*m_parent, exe_ctx)) {
+runtime = process->GetLanguageRuntime(lldb::eLanguageTypeSwift);

This is a bit of a layering violation. Could the ObjC language runtime perform 
this check in `GetDynamicTypeAndAddress` and dispatch to the Swift runtime 
there?

If you do this, we need to ensure we can't get into an infinite loop between 
the runtimes.

Alternatively, we could add a virtual GetPreferredLanguageRuntime() method to 
LanguageRuntime that we call here, maybe?



Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h:89
 
+virtual bool IsSwift() const { return false; }
+

Can you add a Doxygen commen?
/// Determine whether this class is implemented in Swift.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152837

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

[Lldb-commits] [PATCH] D152837: [lldb] Identify Swift-implemented ObjC classes

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Core/ValueObjectDynamicValue.cpp:119-123
+if (auto *runtime = llvm::dyn_cast_or_null(
+process->GetLanguageRuntime(lldb::eLanguageTypeObjC)))
+  if (auto class_sp = runtime->GetClassDescriptor(valobj))
+if (class_sp->IsSwift())
+  return true;

@bulbazord this is where it's used, to work with `ClassDescriptor` which is 
part of the `ObjCLanguageRuntime`. I don't think it would make sense to lift 
that into the generic `LangaugeRuntime`. What other courses might there be?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152837

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

[Lldb-commits] [PATCH] D152842: [lldb] Improve corefile saving ergonomics

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: bulbazord, jasonmolenda.
mib added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch improves the way the user can save the process state into a
corefile by adding completion handler that would provide tab completion
for the corefile path and also resolves the corefile path to expand
relative path.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152842

Files:
  lldb/source/API/SBProcess.cpp
  lldb/source/Commands/CommandObjectProcess.cpp


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -1306,6 +1306,13 @@
 
   Options *GetOptions() override { return _options; }
 
+  void
+  HandleArgumentCompletion(CompletionRequest ,
+   OptionElementVector _element_vector) override {
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), lldb::eDiskFileCompletion, request, nullptr);
+  }
+
   class CommandOptions : public Options {
   public:
 CommandOptions() = default;
@@ -1354,6 +1361,7 @@
 if (process_sp) {
   if (command.GetArgumentCount() == 1) {
 FileSpec output_file(command.GetArgumentAtIndex(0));
+FileSystem::Instance().Resolve(output_file);
 SaveCoreStyle corefile_style = m_options.m_requested_save_core_style;
 Status error =
 PluginManager::SaveCore(process_sp, output_file, corefile_style,
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1183,6 +1183,7 @@
   }
 
   FileSpec core_file(file_name);
+  FileSystem::Instance().Resolve(core_file);
   error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
 flavor);
 


Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -1306,6 +1306,13 @@
 
   Options *GetOptions() override { return _options; }
 
+  void
+  HandleArgumentCompletion(CompletionRequest ,
+   OptionElementVector _element_vector) override {
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), lldb::eDiskFileCompletion, request, nullptr);
+  }
+
   class CommandOptions : public Options {
   public:
 CommandOptions() = default;
@@ -1354,6 +1361,7 @@
 if (process_sp) {
   if (command.GetArgumentCount() == 1) {
 FileSpec output_file(command.GetArgumentAtIndex(0));
+FileSystem::Instance().Resolve(output_file);
 SaveCoreStyle corefile_style = m_options.m_requested_save_core_style;
 Status error =
 PluginManager::SaveCore(process_sp, output_file, corefile_style,
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1183,6 +1183,7 @@
   }
 
   FileSpec core_file(file_name);
+  FileSystem::Instance().Resolve(core_file);
   error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
 flavor);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152837: [lldb] Identify Swift-implemented ObjC classes

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: lldb/source/Core/ValueObjectDynamicValue.cpp:10
 #include "lldb/Core/ValueObjectDynamicValue.h"
+#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
 #include "lldb/Core/Value.h"

Is there a way we can avoid using a plugin in non-plugin code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152837

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

[Lldb-commits] [PATCH] D151951: [lldb][NFCI] Change return type of Properties::GetExperimentalSettingsName

[Jim Ingham via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151951

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

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Jim Ingham via Phabricator via lldb-commits]

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

It's OK to retain this as the default, and as you say, taking it out would be a 
trivial patch after this work.  The control does allow you to do "Have I 
already made this child" before setting about to make it, though I can't 
currently see anywhere where we could use this information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151966

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

[Lldb-commits] [PATCH] D152331: [lldb][NFCI] Platforms should own their SDKBuild and SDKRootDirectory strings

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/include/lldb/Interpreter/OptionGroupPlatform.h:52
 
-  void SetSDKRootDirectory(ConstString sdk_root_directory) {
-m_sdk_sysroot = sdk_root_directory;
+  void SetSDKRootDirectory(llvm::StringRef sdk_root_directory) {
+m_sdk_sysroot = sdk_root_directory.str();

JDevlieghere wrote:
> This should take a std::string by value and move it.
Is there a specific reason you want to do that instead of using 
`llvm::StringRef::str()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152331

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

[Lldb-commits] [PATCH] D152837: [lldb] Identify Swift-implemented ObjC classes

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: aprantl, jingham, augusto2112.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Classes implemented in Swift can be exposed to ObjC. For those classes, the ObjC
metadata is incomplete (the types of the ivars are incomplete), but as one 
would expect
the Swift metadata is complete. In such cases, the Swift runtime should be 
consulted
first when determining the dynamic type of a value.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152837

Files:
  lldb/source/Core/ValueObjectDynamicValue.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h

Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -86,6 +86,8 @@
   return (m_is_cf == eLazyBoolYes);
 }
 
+virtual bool IsSwift() const { return false; }
+
 virtual bool IsValid() = 0;
 
 /// There are two routines in the ObjC runtime that tagged pointer clients
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
@@ -34,6 +34,8 @@
 return true; // any Objective-C v2 runtime class descriptor we vend is valid
   }
 
+  bool IsSwift() const override;
+
   // a custom descriptor is used for tagged pointers
   bool GetTaggedPointerInfo(uint64_t *info_bits = nullptr,
 uint64_t *value_bits = nullptr,
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -530,6 +530,18 @@
   return 0;
 }
 
+// From the ObjC runtime.
+static uint8_t IS_SWIFT_STABLE = 1U << 1;
+
+bool ClassDescriptorV2::IsSwift() const {
+  std::unique_ptr objc_class;
+  if (auto *process = m_runtime.GetProcess())
+if (Read_objc_class(process, objc_class))
+  return objc_class->m_flags & IS_SWIFT_STABLE;
+
+  return false;
+}
+
 ClassDescriptorV2::iVarsStorage::iVarsStorage() : m_ivars(), m_mutex() {}
 
 size_t ClassDescriptorV2::iVarsStorage::size() { return m_ivars.size(); }
Index: lldb/source/Core/ValueObjectDynamicValue.cpp
===
--- lldb/source/Core/ValueObjectDynamicValue.cpp
+++ lldb/source/Core/ValueObjectDynamicValue.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "lldb/Core/ValueObjectDynamicValue.h"
+#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Symbol/CompilerType.h"
@@ -108,6 +109,22 @@
   return m_parent->GetValueType();
 }
 
+static bool UseSwiftRuntime(ValueObject ,
+const ExecutionContext _ctx) {
+  if (auto *frame = exe_ctx.GetFramePtr())
+if (frame->GetLanguage() == lldb::eLanguageTypeSwift)
+  return true;
+
+  if (auto *process = exe_ctx.GetProcessPtr())
+if (auto *runtime = llvm::dyn_cast_or_null(
+process->GetLanguageRuntime(lldb::eLanguageTypeObjC)))
+  if (auto class_sp = runtime->GetClassDescriptor(valobj))
+if (class_sp->IsSwift())
+  return true;
+
+  return false;
+}
+
 bool ValueObjectDynamicValue::UpdateValue() {
   SetValueIsValid(false);
   m_error.Clear();
@@ -146,6 +163,17 @@
   LanguageRuntime *runtime = nullptr;
 
   lldb::LanguageType known_type = m_parent->GetObjectRuntimeLanguage();
+
+  // An ObjC object in a Swift context, or a ObjC object implemented in Swift.
+  if (known_type == lldb::eLanguageTypeObjC &&
+  UseSwiftRuntime(*m_parent, exe_ctx)) {
+runtime = process->GetLanguageRuntime(lldb::eLanguageTypeSwift);
+if (runtime)
+  found_dynamic_type = runtime->GetDynamicTypeAndAddress(
+  *m_parent, m_use_dynamic, class_type_or_name, dynamic_address,
+  value_type);
+  }
+
   if (known_type != lldb::eLanguageTypeUnknown &&
   known_type != lldb::eLanguageTypeC) {
 runtime = process->GetLanguageRuntime(known_type);
___
lldb-commits mailing list

[Lldb-commits] [PATCH] D152031: [lldb] Default can_create to true in GetChildAtIndex (NFC)

[Jim Ingham via Phabricator via lldb-commits]

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

I can dream up a few speculative uses of `can_create => false` but they are all 
a little far-fetched.  Certainly passing `true` is the dominant mode, so making 
it a default seems fine to me and reduces a bunch of noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152031

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

[Lldb-commits] [PATCH] D152579: [lldb] Symtab::SectionFileAddressesChanged should clear the file address map instead of name map

[Jim Ingham via Phabricator via lldb-commits]

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

That does look like a thinko.  Nothing about a file's section addresses 
changing will change the name to symbol map as that isn't dependent on load 
addresses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152579

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

[Lldb-commits] [PATCH] D152573: [lldb][NFCI] Remove use of ConstString from Listener

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG35419651c53c: [lldb][NFCI] Remove use of ConstString from 
Listener (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152573

Files:
  lldb/include/lldb/Utility/Listener.h
  lldb/source/Utility/Listener.cpp

Index: lldb/source/Utility/Listener.cpp
===
--- lldb/source/Utility/Listener.cpp
+++ lldb/source/Utility/Listener.cpp
@@ -8,7 +8,6 @@
 
 #include "lldb/Utility/Listener.h"
 #include "lldb/Utility/Broadcaster.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBLog.h"
 
@@ -222,46 +221,25 @@
 
 class EventMatcher {
 public:
-  EventMatcher(Broadcaster *broadcaster, const ConstString *broadcaster_names,
-   uint32_t num_broadcaster_names, uint32_t event_type_mask)
-  : m_broadcaster(broadcaster), m_broadcaster_names(broadcaster_names),
-m_num_broadcaster_names(num_broadcaster_names),
-m_event_type_mask(event_type_mask) {}
+  EventMatcher(Broadcaster *broadcaster, uint32_t event_type_mask)
+  : m_broadcaster(broadcaster), m_event_type_mask(event_type_mask) {}
 
   bool operator()(const EventSP _sp) const {
 if (m_broadcaster && !event_sp->BroadcasterIs(m_broadcaster))
   return false;
 
-if (m_broadcaster_names) {
-  bool found_source = false;
-  const llvm::StringRef event_broadcaster_name =
-  event_sp->GetBroadcaster()->GetBroadcasterName();
-  for (uint32_t i = 0; i < m_num_broadcaster_names; ++i) {
-if (m_broadcaster_names[i] == event_broadcaster_name) {
-  found_source = true;
-  break;
-}
-  }
-  if (!found_source)
-return false;
-}
-
 return m_event_type_mask == 0 || m_event_type_mask & event_sp->GetType();
   }
 
 private:
   Broadcaster *m_broadcaster;
-  const ConstString *m_broadcaster_names;
-  const uint32_t m_num_broadcaster_names;
   const uint32_t m_event_type_mask;
 };
 
 bool Listener::FindNextEventInternal(
 std::unique_lock ,
-Broadcaster *broadcaster, // nullptr for any broadcaster
-const ConstString *broadcaster_names, // nullptr for any event
-uint32_t num_broadcaster_names, uint32_t event_type_mask, EventSP _sp,
-bool remove) {
+Broadcaster *broadcaster, // nullptr for any broadcaster
+uint32_t event_type_mask, EventSP _sp, bool remove) {
   // NOTE: callers of this function must lock m_events_mutex using a
   // Mutex::Locker
   // and pass the locker as the first argument. m_events_mutex is no longer
@@ -273,13 +251,11 @@
 
   Listener::event_collection::iterator pos = m_events.end();
 
-  if (broadcaster == nullptr && broadcaster_names == nullptr &&
-  event_type_mask == 0) {
+  if (broadcaster == nullptr && event_type_mask == 0) {
 pos = m_events.begin();
   } else {
 pos = std::find_if(m_events.begin(), m_events.end(),
-   EventMatcher(broadcaster, broadcaster_names,
-num_broadcaster_names, event_type_mask));
+   EventMatcher(broadcaster, event_type_mask));
   }
 
   if (pos != m_events.end()) {
@@ -288,12 +264,10 @@
 if (log != nullptr)
   LLDB_LOGF(log,
 "%p '%s' Listener::FindNextEventInternal(broadcaster=%p, "
-"broadcaster_names=%p[%u], event_type_mask=0x%8.8x, "
+"event_type_mask=0x%8.8x, "
 "remove=%i) event %p",
 static_cast(this), GetName(),
-static_cast(broadcaster),
-static_cast(broadcaster_names),
-num_broadcaster_names, event_type_mask, remove,
+static_cast(broadcaster), event_type_mask, remove,
 static_cast(event_sp.get()));
 
 if (remove) {
@@ -315,7 +289,7 @@
 Event *Listener::PeekAtNextEvent() {
   std::unique_lock guard(m_events_mutex);
   EventSP event_sp;
-  if (FindNextEventInternal(guard, nullptr, nullptr, 0, 0, event_sp, false))
+  if (FindNextEventInternal(guard, nullptr, 0, event_sp, false))
 return event_sp.get();
   return nullptr;
 }
@@ -323,7 +297,7 @@
 Event *Listener::PeekAtNextEventForBroadcaster(Broadcaster *broadcaster) {
   std::unique_lock guard(m_events_mutex);
   EventSP event_sp;
-  if (FindNextEventInternal(guard, broadcaster, nullptr, 0, 0, event_sp, false))
+  if (FindNextEventInternal(guard, broadcaster, 0, event_sp, false))
 return event_sp.get();
   return nullptr;
 }
@@ -333,26 +307,23 @@
 uint32_t event_type_mask) {
   std::unique_lock guard(m_events_mutex);
   EventSP event_sp;
-  if (FindNextEventInternal(guard, broadcaster, nullptr, 0, event_type_mask,
-event_sp, false))
+  if (FindNextEventInternal(guard, broadcaster, 

[Lldb-commits] [lldb] 3541965 - [lldb][NFCI] Remove use of ConstString from Listener

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-06-13T10:14:38-07:00
New Revision: 35419651c53c9f9a58d2b86a4ec61d47c2105c55

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

LOG: [lldb][NFCI] Remove use of ConstString from Listener

The only place ConstString was used in Listener was for filtering
broadcasters by name when looking for the next event. This functionality
is completely unused from what I can tell (even in downstream forks).

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

Added: 


Modified: 
lldb/include/lldb/Utility/Listener.h
lldb/source/Utility/Listener.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Listener.h 
b/lldb/include/lldb/Utility/Listener.h
index 8d50fe77d6a29..daa7deb345f30 100644
--- a/lldb/include/lldb/Utility/Listener.h
+++ b/lldb/include/lldb/Utility/Listener.h
@@ -27,7 +27,6 @@
 #include 
 
 namespace lldb_private {
-class ConstString;
 class Event;
 }
 
@@ -119,15 +118,12 @@ class Listener : public 
std::enable_shared_from_this {
   bool
   FindNextEventInternal(std::unique_lock ,
 Broadcaster *broadcaster, // nullptr for any 
broadcaster
-const ConstString *sources, // nullptr for any event
-uint32_t num_sources, uint32_t event_type_mask,
-lldb::EventSP _sp, bool remove);
+uint32_t event_type_mask, lldb::EventSP _sp,
+bool remove);
 
   bool GetEventInternal(const Timeout ,
 Broadcaster *broadcaster, // nullptr for any 
broadcaster
-const ConstString *sources, // nullptr for any event
-uint32_t num_sources, uint32_t event_type_mask,
-lldb::EventSP _sp);
+uint32_t event_type_mask, lldb::EventSP _sp);
 
   std::string m_name;
   broadcaster_collection m_broadcasters;

diff  --git a/lldb/source/Utility/Listener.cpp 
b/lldb/source/Utility/Listener.cpp
index 060fc2b1cedf1..34bfde401f586 100644
--- a/lldb/source/Utility/Listener.cpp
+++ b/lldb/source/Utility/Listener.cpp
@@ -8,7 +8,6 @@
 
 #include "lldb/Utility/Listener.h"
 #include "lldb/Utility/Broadcaster.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBLog.h"
 
@@ -222,46 +221,25 @@ class EventBroadcasterMatches {
 
 class EventMatcher {
 public:
-  EventMatcher(Broadcaster *broadcaster, const ConstString *broadcaster_names,
-   uint32_t num_broadcaster_names, uint32_t event_type_mask)
-  : m_broadcaster(broadcaster), m_broadcaster_names(broadcaster_names),
-m_num_broadcaster_names(num_broadcaster_names),
-m_event_type_mask(event_type_mask) {}
+  EventMatcher(Broadcaster *broadcaster, uint32_t event_type_mask)
+  : m_broadcaster(broadcaster), m_event_type_mask(event_type_mask) {}
 
   bool operator()(const EventSP _sp) const {
 if (m_broadcaster && !event_sp->BroadcasterIs(m_broadcaster))
   return false;
 
-if (m_broadcaster_names) {
-  bool found_source = false;
-  const llvm::StringRef event_broadcaster_name =
-  event_sp->GetBroadcaster()->GetBroadcasterName();
-  for (uint32_t i = 0; i < m_num_broadcaster_names; ++i) {
-if (m_broadcaster_names[i] == event_broadcaster_name) {
-  found_source = true;
-  break;
-}
-  }
-  if (!found_source)
-return false;
-}
-
 return m_event_type_mask == 0 || m_event_type_mask & event_sp->GetType();
   }
 
 private:
   Broadcaster *m_broadcaster;
-  const ConstString *m_broadcaster_names;
-  const uint32_t m_num_broadcaster_names;
   const uint32_t m_event_type_mask;
 };
 
 bool Listener::FindNextEventInternal(
 std::unique_lock ,
-Broadcaster *broadcaster, // nullptr for any broadcaster
-const ConstString *broadcaster_names, // nullptr for any event
-uint32_t num_broadcaster_names, uint32_t event_type_mask, EventSP 
_sp,
-bool remove) {
+Broadcaster *broadcaster, // nullptr for any broadcaster
+uint32_t event_type_mask, EventSP _sp, bool remove) {
   // NOTE: callers of this function must lock m_events_mutex using a
   // Mutex::Locker
   // and pass the locker as the first argument. m_events_mutex is no longer
@@ -273,13 +251,11 @@ bool Listener::FindNextEventInternal(
 
   Listener::event_collection::iterator pos = m_events.end();
 
-  if (broadcaster == nullptr && broadcaster_names == nullptr &&
-  event_type_mask == 0) {
+  if (broadcaster == nullptr && event_type_mask == 0) {
 pos = m_events.begin();
   } else {
 pos = std::find_if(m_events.begin(), m_events.end(),
-   EventMatcher(broadcaster, broadcaster_names,
-

[Lldb-commits] [PATCH] D152806: [lldb][test] Re-XFAIL prefer-debug-over-eh-frame.test

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfae704bad900: [lldb][test] Re-XFAIL 
prefer-debug-over-eh-frame.test (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152806

Files:
  lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test


Index: lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
===
--- lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
+++ lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
@@ -5,6 +5,7 @@
 # be thrown.
 
 # UNSUPPORTED: system-windows
+# XFAIL: system-darwin
 # REQUIRES: target-x86_64, native
 
 # RUN: %clang_host -g %p/Inputs/call-asm.c 
%p/Inputs/prefer-debug-over-eh-frame.s -o %t


Index: lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
===
--- lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
+++ lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
@@ -5,6 +5,7 @@
 # be thrown.
 
 # UNSUPPORTED: system-windows
+# XFAIL: system-darwin
 # REQUIRES: target-x86_64, native
 
 # RUN: %clang_host -g %p/Inputs/call-asm.c %p/Inputs/prefer-debug-over-eh-frame.s -o %t
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] fae704b - [lldb][test] Re-XFAIL prefer-debug-over-eh-frame.test

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2023-06-13T15:08:21+01:00
New Revision: fae704bad90077a22e4534222903a6b03407650e

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

LOG: [lldb][test] Re-XFAIL prefer-debug-over-eh-frame.test

This was un-XFAILed in `83cb2123be487302070562c45e6eb4955b22c2b4`
due to D144999. Since then D152540 fixed emission of eh_frame's
on Darwin, causing this test to fail again.

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

Added: 


Modified: 
lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test

Removed: 




diff  --git a/lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test 
b/lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
index 0d3f34ec86f56..19e3ae18c25f3 100644
--- a/lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
+++ b/lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
@@ -5,6 +5,7 @@
 # be thrown.
 
 # UNSUPPORTED: system-windows
+# XFAIL: system-darwin
 # REQUIRES: target-x86_64, native
 
 # RUN: %clang_host -g %p/Inputs/call-asm.c 
%p/Inputs/prefer-debug-over-eh-frame.s -o %t



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

[Lldb-commits] [PATCH] D152806: [lldb][test] Re-XFAIL prefer-debug-over-eh-frame.test

[Vy Nguyen via Phabricator via lldb-commits]

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

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152806

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

[Lldb-commits] [PATCH] D152516: [lldb][AArch64] Add thread local storage tpidr register

[David Spickett via Phabricator via lldb-commits]

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

Correct size getter in read method. This just happened to work because the MTE
set also contains one register.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152516

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/test/API/linux/aarch64/tls_register/Makefile
  lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
  lldb/test/API/linux/aarch64/tls_register/main.c

Index: lldb/test/API/linux/aarch64/tls_register/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_register/main.c
@@ -0,0 +1,25 @@
+#include 
+#include 
+
+struct __attribute__((__packed__)) TestStruct {
+  uint64_t one;
+  uint64_t two;
+};
+
+int main() {
+  static __thread struct TestStruct test_data;
+  test_data.one = 0xABABABABCDCDCDCD;
+#define TWO_VALUE 0xEFEFEFEF01010101
+  test_data.two = TWO_VALUE;
+
+  // Barrier to ensure the above writes are done first.
+  __asm__ volatile("" ::: "memory"); // Set break point at this line.
+
+  // Here lldb moves the TLS pointer 8 bytes forward.
+  // So this actually reads test_data.two instead of test_data.one.
+  volatile bool tls_has_moved = test_data.one == TWO_VALUE;
+
+  return 0; // Set break point 2 at this line.
+  // lldb will reset the thread pointer here so we don't potentially confuse
+  // libc.
+}
Index: lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
@@ -0,0 +1,77 @@
+"""
+Test lldb's ability to read and write the AArch64 TLS register tpidr.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxTLSRegister(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_tls(self):
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point at this line."),
+num_expected_locations=1,
+)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point 2 at this line."),
+num_expected_locations=1,
+)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+# We can't know what the value should be since it moves about between
+# runs. So we'll check that we can read it, and by modifying it, see
+# changes in the program.
+
+regs = self.thread().GetSelectedFrame().GetRegisters()
+tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
+self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
+tpidr = tls_regs.GetChildMemberWithName("tpidr")
+self.assertTrue(tpidr.IsValid(), "No tpidr register found.")
+tpidr = tpidr.GetValueAsUnsigned()
+
+# We should be able to find a known value in the TLS area it points to.
+# test_data should be very soon after the header, 64 bytes seems to work fine.
+self.expect("memory find -e 0xABABABABCDCDCDCD $tpidr $tpidr+64",
+substrs=["data found at location: 0x"])
+
+# Now modify the TLS pointer so that anyone reading test_data.one actually
+# reads test_data.two.
+self.expect("register write tpidr 0x{:x}".format(tpidr+8))
+
+# Let the program read test_data.one.
+self.expect("continue")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+self.expect("p tls_has_moved", substrs=["true"])
+
+# Reset TLS so we don't potentially confuse the libc.
+self.expect("register write tpidr 0x{:x}".format(tpidr))
\ No newline at end of file
Index: lldb/test/API/linux/aarch64/tls_register/Makefile
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_register/Makefile
@@ -0,0 

[Lldb-commits] [PATCH] D152806: [lldb][test] Re-XFAIL prefer-debug-over-eh-frame.test

[Michael Buch via Phabricator via lldb-commits]

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

This was un-XFAILed in `83cb2123be487302070562c45e6eb4955b22c2b4`
due to D144999 . Since then D152540 
 fixed emission of eh_frame's
on Darwin, causing this test to fail again.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152806

Files:
  lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test


Index: lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
===
--- lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
+++ lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
@@ -5,6 +5,7 @@
 # be thrown.
 
 # UNSUPPORTED: system-windows
+# XFAIL: system-darwin
 # REQUIRES: target-x86_64, native
 
 # RUN: %clang_host -g %p/Inputs/call-asm.c 
%p/Inputs/prefer-debug-over-eh-frame.s -o %t


Index: lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
===
--- lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
+++ lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
@@ -5,6 +5,7 @@
 # be thrown.
 
 # UNSUPPORTED: system-windows
+# XFAIL: system-darwin
 # REQUIRES: target-x86_64, native
 
 # RUN: %clang_host -g %p/Inputs/call-asm.c %p/Inputs/prefer-debug-over-eh-frame.s -o %t
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

[Hans Wennborg via Phabricator via lldb-commits]

hans edited reviewers, added: labath; removed: hans.
hans added a comment.
Herald added a subscriber: JDevlieghere.

+labath


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152712

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

[Lldb-commits] [PATCH] D152708: [RFC][Draft] Enable primitive support for Two-Level Line Tables in LLVM

[Stephen Tozer via Phabricator via lldb-commits]

StephenTozer added a comment.

In D152708#4414402 , @dblaikie wrote:

> It is unfortunate to hear that TLLT are a significant size increase, though 
> not entirely surprising - it's a bunch of extra info to encode. I'll be glad 
> to have this example to experiment with.

For what it's worth, we haven't tested this with any larger programs, so this 
is more of a rough estimate of size, but the .debug_line section increased in 
size on the order of 50% for our small test cases. On the other hand, I think 
that in larger programs the .debug_line section is likely be significantly 
smaller than the .debug_info section anyway, so if you are interested in 
producing inline frames without using .debug_info it is probably an improvement 
in most respects - as an example with a single-source input, `flops.c` (taken 
from the LLVM test suite repo), the .debug_line section increased from 3338 to 
4758 bytes, and the overall size of the DWARF output from 9467 to 10893.

> Are there any known (or vague/unknown) limitations on the implementation with 
> respect to the actual output/on-disk representation?

Possibly, but not outside of the proposal itself - this patch was written over 
the span of 4 days so doubtless there are inefficiencies in the code itself and 
there may be bugs in the output (especially if you try it on a larger 
codebase), but in theory the output follows the spec faithfully and shouldn't 
be wasting up space. With that said, the main cause of the size increase isn't 
really the additional information, but the fact that the TLLT does repeat 
itself a lot by design; generally speaking most instructions are not inlined 
callsites, and so the additional information for those lines (the context and 
subprogram fields) has a much smaller impact on size than the fact that we are 
emitting the `(InstructionAddress, SourceLocation)` pair twice for every 
instruction.

We didn't dig deeper into why the representation needs to be split into two 
separate line number programs rather than either a single line table with some 
additional column to convey the Instruction->SourceLocation attribution, or 
keeping two line tables but using a single line number program that can either 
emit Logical and Actual rows simultaneously. Either of these methods would 
reduce the repetition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152708

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