[Lldb-commits] [PATCH] D115951: Cache the manual DWARF index out to the LLDB cache directory when the LLDB index cache is enabled.

2021-12-29 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid added a comment.

@clayborg

This breaks LLDB Linux buildbots:

https://lab.llvm.org/buildbot/#/builders/17/builds/15166
https://lab.llvm.org/buildbot/#/builders/96/builds/16114
https://lab.llvm.org/buildbot/#/builders/68/builds/23796


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115951

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

[Lldb-commits] [lldb] caa7e76 - [lldb] Make ProcessLauncherPosixFork (mostly) async-signal-safe

2021-12-29 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2021-12-29T09:45:47+01:00
New Revision: caa7e765e5ae250c67eab3edd7cd324d3634f779

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

LOG: [lldb] Make ProcessLauncherPosixFork (mostly) async-signal-safe

Multithreaded applications using fork(2) need to be extra careful about
what they do in the fork child. Without any special precautions (which
only really work if you can fully control all threads) they can only
safely call async-signal-safe functions. This is because the forked
child will contain snapshot of the parents memory at a random moment in
the execution of all of the non-forking threads (this is where the
similarity with signals comes in).

For example, the other threads could have been holding locks that can
now never be released in the child process and any attempt to obtain
them would block. This is what sometimes happen when using tcmalloc --
our fork child ends up hanging in the memory allocation routine. It is
also what happened with our logging code, which is why we added a
pthread_atfork hackaround.

This patch implements a proper fix to the problem, by which is to make
the child code async-signal-safe. The ProcessLaunchInfo structure is
transformed into a simpler ForkLaunchInfo representation, one which can
be read without allocating memory and invoking complex library
functions.

Strictly speaking this implementation is not async-signal-safe, as it
still invokes library functions outside of the posix-blessed set of
entry points. Strictly adhering to the spec would mean reimplementing a
lot of the functionality in pure C, so instead I rely on the fact that
any reasonable implementation of some functions (e.g.,
basic_string::c_str()) will not start allocating memory or doing other
unsafe things.

The new child code does not call into our logging infrastructure, which
enables us to remove the pthread_atfork call from there.

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

Added: 


Modified: 
lldb/include/lldb/Utility/Log.h
lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
lldb/source/Utility/Log.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 01edec0445650..2684783939bd8 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -211,8 +211,6 @@ class Log final {
   static uint32_t GetFlags(llvm::raw_ostream &stream, const 
ChannelMap::value_type &entry,
llvm::ArrayRef categories);
 
-  static void DisableLoggingChild();
-
   Log(const Log &) = delete;
   void operator=(const Log &) = delete;
 };

diff  --git a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp 
b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
index 2f08b9fa8857a..635dbb14a0271 100644
--- a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -38,43 +38,40 @@
 using namespace lldb;
 using namespace lldb_private;
 
-static void FixupEnvironment(Environment &env) {
-#ifdef __ANDROID__
-  // If there is no PATH variable specified inside the environment then set the
-  // path to /system/bin. It is required because the default path used by
-  // execve() is wrong on android.
-  env.try_emplace("PATH", "/system/bin");
-#endif
+// Begin code running in the child process
+// NB: This code needs to be async-signal safe, since we're invoking fork from
+// multithreaded contexts.
+
+static void write_string(int error_fd, const char *str) {
+  int r = write(error_fd, str, strlen(str));
+  (void)r;
 }
 
 [[noreturn]] static void ExitWithError(int error_fd,
const char *operation) {
   int err = errno;
-  llvm::raw_fd_ostream os(error_fd, true);
-  os << operation << " failed: " << llvm::sys::StrError(err);
-  os.flush();
+  write_string(error_fd, operation);
+  write_string(error_fd, " failed: ");
+  // strerror is not guaranteed to be async-signal safe, but it usually is.
+  write_string(error_fd, strerror(err));
   _exit(1);
 }
 
-static void DisableASLRIfRequested(int error_fd, const ProcessLaunchInfo 
&info) {
+static void DisableASLR(int error_fd) {
 #if defined(__linux__)
-  if (info.GetFlags().Test(lldb::eLaunchFlagDisableASLR)) {
-const unsigned long personality_get_current = 0x;
-int value = personality(personality_get_current);
-if (value == -1)
-  ExitWithError(error_fd, "personality get");
-
-value = personality(ADDR_NO_RANDOMIZE | value);
-if (value == -1)
-  ExitWithError(error_fd, "personality set");
-  }
+  const unsigned long personality_get_current = 0x;
+  int value = personality(personality_get_current);
+  if (value == -1)
+ExitWithError(error_fd, "personality get");
+
+  value = pe

[Lldb-commits] [lldb] daed479 - [lldb] Adjust TestModuleCacheSimple for D115951

2021-12-29 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2021-12-29T10:00:00+01:00
New Revision: daed4797fee4a5f1985388265f5af209b5cb3b10

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

LOG: [lldb] Adjust TestModuleCacheSimple for D115951

Now that we are caching the dwarf index as well, we will always have
more than one cache file (when not using accelerator tables). I have
adjusted the test to check for the presence of one _symtab_ index.

Added: 


Modified: 

lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py
 
b/lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py
index 35e96fb584ed1..6180203c4bb37 100644
--- 
a/lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py
+++ 
b/lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py
@@ -26,7 +26,8 @@ def setUp(self):
 
 
 def get_module_cache_files(self, basename):
-module_file_glob = os.path.join(self.cache_dir, "llvmcache-*%s*" % 
(basename))
+module_file_glob = os.path.join(self.cache_dir,
+"llvmcache-*%s*-symtab-*" % (basename))
 return glob.glob(module_file_glob)
 
 # Doesn't depend on any specific debug information.



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

[Lldb-commits] [PATCH] D116165: [lldb] Make ProcessLauncherPosixFork (mostly) async-signal-safe

2021-12-29 Thread Pavel Labath via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcaa7e765e5ae: [lldb] Make ProcessLauncherPosixFork (mostly) 
async-signal-safe (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D116165?vs=395859&id=396500#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116165

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
  lldb/source/Utility/Log.cpp

Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -30,7 +30,6 @@
 #include 
 #else
 #include 
-#include 
 #endif
 
 using namespace lldb_private;
@@ -180,9 +179,6 @@
 }
 
 void Log::Initialize() {
-#ifdef LLVM_ON_UNIX
-  pthread_atfork(nullptr, nullptr, &Log::DisableLoggingChild);
-#endif
   InitializeLldbChannel();
 }
 
@@ -346,11 +342,3 @@
   message << payload << "\n";
   WriteMessage(message.str());
 }
-
-void Log::DisableLoggingChild() {
-  // Disable logging by clearing out the atomic variable after forking -- if we
-  // forked while another thread held the channel mutex, we would deadlock when
-  // trying to write to the log.
-  for (auto &c: *g_channel_map)
-c.second.m_channel.log_ptr.store(nullptr, std::memory_order_relaxed);
-}
Index: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
===
--- lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -38,43 +38,40 @@
 using namespace lldb;
 using namespace lldb_private;
 
-static void FixupEnvironment(Environment &env) {
-#ifdef __ANDROID__
-  // If there is no PATH variable specified inside the environment then set the
-  // path to /system/bin. It is required because the default path used by
-  // execve() is wrong on android.
-  env.try_emplace("PATH", "/system/bin");
-#endif
+// Begin code running in the child process
+// NB: This code needs to be async-signal safe, since we're invoking fork from
+// multithreaded contexts.
+
+static void write_string(int error_fd, const char *str) {
+  int r = write(error_fd, str, strlen(str));
+  (void)r;
 }
 
 [[noreturn]] static void ExitWithError(int error_fd,
const char *operation) {
   int err = errno;
-  llvm::raw_fd_ostream os(error_fd, true);
-  os << operation << " failed: " << llvm::sys::StrError(err);
-  os.flush();
+  write_string(error_fd, operation);
+  write_string(error_fd, " failed: ");
+  // strerror is not guaranteed to be async-signal safe, but it usually is.
+  write_string(error_fd, strerror(err));
   _exit(1);
 }
 
-static void DisableASLRIfRequested(int error_fd, const ProcessLaunchInfo &info) {
+static void DisableASLR(int error_fd) {
 #if defined(__linux__)
-  if (info.GetFlags().Test(lldb::eLaunchFlagDisableASLR)) {
-const unsigned long personality_get_current = 0x;
-int value = personality(personality_get_current);
-if (value == -1)
-  ExitWithError(error_fd, "personality get");
-
-value = personality(ADDR_NO_RANDOMIZE | value);
-if (value == -1)
-  ExitWithError(error_fd, "personality set");
-  }
+  const unsigned long personality_get_current = 0x;
+  int value = personality(personality_get_current);
+  if (value == -1)
+ExitWithError(error_fd, "personality get");
+
+  value = personality(ADDR_NO_RANDOMIZE | value);
+  if (value == -1)
+ExitWithError(error_fd, "personality set");
 #endif
 }
 
-static void DupDescriptor(int error_fd, const FileSpec &file_spec, int fd,
-  int flags) {
-  int target_fd = llvm::sys::RetryAfterSignal(-1, ::open,
-  file_spec.GetCString(), flags, 0666);
+static void DupDescriptor(int error_fd, const char *file, int fd, int flags) {
+  int target_fd = llvm::sys::RetryAfterSignal(-1, ::open, file, flags, 0666);
 
   if (target_fd == -1)
 ExitWithError(error_fd, "DupDescriptor-open");
@@ -88,44 +85,67 @@
   ::close(target_fd);
 }
 
-[[noreturn]] static void ChildFunc(int error_fd,
-   const ProcessLaunchInfo &info) {
-  if (info.GetFlags().Test(eLaunchFlagLaunchInSeparateProcessGroup)) {
+namespace {
+struct ForkFileAction {
+  ForkFileAction(const FileAction &act);
+
+  FileAction::Action action;
+  int fd;
+  std::string path;
+  int arg;
+};
+
+struct ForkLaunchInfo {
+  ForkLaunchInfo(const ProcessLaunchInfo &info);
+
+  bool separate_process_group;
+  bool debug;
+  bool disable_aslr;
+  std::string wd;
+  const char **argv;
+  Environment::Envp envp;
+  std::vector actions;
+
+  bool has_action(int fd) const {
+for (const ForkFileAction &action : actions) {
+  if (action.fd == fd)
+return true;
+}
+return false;
+  }
+};
+} // namespace
+
+[[noreturn]] static void ChildFunc(int error_fd, const Fork

[Lldb-commits] [PATCH] D115951: Cache the manual DWARF index out to the LLDB cache directory when the LLDB index cache is enabled.

2021-12-29 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

The problem was that (without accelerator tables) we would always have more 
than one cache file for a given module. I have now adjusted the test (daed479 
) to check 
for the presence of one _symtab_ index, but other solutions are possible as 
well. Feel free to change it to something else if you think they're better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115951

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

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing bool conversion of Mangled

2021-12-29 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.

Cool. Thanks for fixing this. Do you need someone to commit this for you?


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

https://reviews.llvm.org/D116217

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

[Lldb-commits] [lldb] 633b002 - [lldb] Fix PR52702 by fixing bool conversion of Mangled

2021-12-29 Thread PoYao Chang via lldb-commits 

Author: PoYao Chang
Date: 2021-12-29T17:17:52+08:00
New Revision: 633b002944b966ddb64c85f4a8c017a858afb4fc

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

LOG: [lldb] Fix PR52702 by fixing bool conversion of Mangled

Remove the Mangled::operator! and Mangled::operator void* where the
comments in header and implementation files disagree and replace them
with operator bool.

This fix PR52702 as https://reviews.llvm.org/D106837 used the buggy
Mangled::operator! in Symbol::SynthesizeNameIfNeeded. For example,
consider the symbol "puts" in a hello world C program:

// Inside Symbol::SynthesizeNameIfNeeded
(lldb) p m_mangled
(lldb_private::Mangled) $0 = (m_mangled = None, m_demangled = "puts")
(lldb) p !m_mangled
(bool) $1 = true  # should be false!!
This leads to Symbol::SynthesizeNameIfNeeded overwriting m_demangled
part of Mangled (in this case "puts").

In conclusion, this patch turns
callq  0x401030  ; symbol stub for: ___lldb_unnamed_symbol36
back into
callq  0x401030  ; symbol stub for: puts .

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

Added: 


Modified: 
lldb/include/lldb/Core/Mangled.h
lldb/source/Core/Mangled.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/unittests/Core/MangledTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Mangled.h 
b/lldb/include/lldb/Core/Mangled.h
index 6c92591a0881..35705b0319ab 100644
--- a/lldb/include/lldb/Core/Mangled.h
+++ b/lldb/include/lldb/Core/Mangled.h
@@ -72,10 +72,10 @@ class Mangled {
 return !(*this == rhs);
   }
 
-  /// Convert to pointer operator.
+  /// Convert to bool operator.
   ///
-  /// This allows code to check a Mangled object to see if it contains a valid
-  /// mangled name using code such as:
+  /// This allows code to check any Mangled objects to see if they contain
+  /// anything valid using code such as:
   ///
   /// \code
   /// Mangled mangled(...);
@@ -84,25 +84,9 @@ class Mangled {
   /// \endcode
   ///
   /// \return
-  /// A pointer to this object if either the mangled or unmangled
-  /// name is set, NULL otherwise.
-  operator void *() const;
-
-  /// Logical NOT operator.
-  ///
-  /// This allows code to check a Mangled object to see if it contains an
-  /// empty mangled name using code such as:
-  ///
-  /// \code
-  /// Mangled mangled(...);
-  /// if (!mangled)
-  /// { ...
-  /// \endcode
-  ///
-  /// \return
-  /// Returns \b true if the object has an empty mangled and
-  /// unmangled name, \b false otherwise.
-  bool operator!() const;
+  /// Returns \b true if either the mangled or unmangled name is set,
+  /// \b false if the object has an empty mangled and unmangled name.
+  explicit operator bool() const;
 
   /// Clear the mangled and demangled values.
   void Clear();

diff  --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index c8aacdefefa2..4e10324401dc 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -70,23 +70,13 @@ Mangled::Mangled(llvm::StringRef name) {
 SetValue(ConstString(name));
 }
 
-// Convert to pointer operator. This allows code to check any Mangled objects
+// Convert to bool operator. This allows code to check any Mangled objects
 // to see if they contain anything valid using code such as:
 //
 //  Mangled mangled(...);
 //  if (mangled)
 //  { ...
-Mangled::operator void *() const {
-  return (m_mangled) ? const_cast(this) : nullptr;
-}
-
-// Logical NOT operator. This allows code to check any Mangled objects to see
-// if they are invalid using code such as:
-//
-//  Mangled mangled(...);
-//  if (!file_spec)
-//  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+Mangled::operator bool() const { return m_mangled || m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 8c995ef2eb2a..ca701c6f2fcc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2140,7 +2140,8 @@ void SymbolFileDWARF::FindGlobalVariables(
 
   llvm::StringRef basename;
   llvm::StringRef context;
-  bool name_is_mangled = (bool)Mangled(name);
+  bool name_is_mangled = Mangled::GetManglingScheme(name.GetStringRef()) !=
+ Mangled::eManglingSchemeNone;
 
   if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
   context, basename))

diff  --git a/lldb/unittests/Core/MangledTest.cpp 
b/lldb/unittests/Core/MangledTest.cpp
index 4c1bb0cc45c2..284c2f21aadd 100644
--- a/lldb/uni

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing bool conversion of Mangled

2021-12-29 Thread PoYao Chang via Phabricator via lldb-commits 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG633b002944b9: [lldb] Fix PR52702 by fixing bool conversion 
of Mangled (authored by rZhBoYao).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116217

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/unittests/Core/MangledTest.cpp

Index: lldb/unittests/Core/MangledTest.cpp
===
--- lldb/unittests/Core/MangledTest.cpp
+++ lldb/unittests/Core/MangledTest.cpp
@@ -89,6 +89,25 @@
   EXPECT_STREQ("", the_demangled.GetCString());
 }
 
+TEST(MangledTest, BoolConversionOperator) {
+  {
+ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+Mangled TheMangled(MangledName);
+EXPECT_EQ(true, bool(TheMangled));
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+ConstString UnmangledName("puts");
+Mangled TheMangled(UnmangledName);
+EXPECT_EQ(true, bool(TheMangled));
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+Mangled TheMangled{};
+EXPECT_EQ(false, bool(TheMangled));
+EXPECT_EQ(true, !TheMangled);
+  }
+}
 
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   SubsystemRAII
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2140,7 +2140,8 @@
 
   llvm::StringRef basename;
   llvm::StringRef context;
-  bool name_is_mangled = (bool)Mangled(name);
+  bool name_is_mangled = Mangled::GetManglingScheme(name.GetStringRef()) !=
+ Mangled::eManglingSchemeNone;
 
   if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
   context, basename))
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -70,23 +70,13 @@
 SetValue(ConstString(name));
 }
 
-// Convert to pointer operator. This allows code to check any Mangled objects
+// Convert to bool operator. This allows code to check any Mangled objects
 // to see if they contain anything valid using code such as:
 //
 //  Mangled mangled(...);
 //  if (mangled)
 //  { ...
-Mangled::operator void *() const {
-  return (m_mangled) ? const_cast(this) : nullptr;
-}
-
-// Logical NOT operator. This allows code to check any Mangled objects to see
-// if they are invalid using code such as:
-//
-//  Mangled mangled(...);
-//  if (!file_spec)
-//  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+Mangled::operator bool() const { return m_mangled || m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {
Index: lldb/include/lldb/Core/Mangled.h
===
--- lldb/include/lldb/Core/Mangled.h
+++ lldb/include/lldb/Core/Mangled.h
@@ -72,10 +72,10 @@
 return !(*this == rhs);
   }
 
-  /// Convert to pointer operator.
+  /// Convert to bool operator.
   ///
-  /// This allows code to check a Mangled object to see if it contains a valid
-  /// mangled name using code such as:
+  /// This allows code to check any Mangled objects to see if they contain
+  /// anything valid using code such as:
   ///
   /// \code
   /// Mangled mangled(...);
@@ -84,25 +84,9 @@
   /// \endcode
   ///
   /// \return
-  /// A pointer to this object if either the mangled or unmangled
-  /// name is set, NULL otherwise.
-  operator void *() const;
-
-  /// Logical NOT operator.
-  ///
-  /// This allows code to check a Mangled object to see if it contains an
-  /// empty mangled name using code such as:
-  ///
-  /// \code
-  /// Mangled mangled(...);
-  /// if (!mangled)
-  /// { ...
-  /// \endcode
-  ///
-  /// \return
-  /// Returns \b true if the object has an empty mangled and
-  /// unmangled name, \b false otherwise.
-  bool operator!() const;
+  /// Returns \b true if either the mangled or unmangled name is set,
+  /// \b false if the object has an empty mangled and unmangled name.
+  explicit operator bool() const;
 
   /// Clear the mangled and demangled values.
   void Clear();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing bool conversion of Mangled

2021-12-29 Thread PoYao Chang via Phabricator via lldb-commits 
rZhBoYao added a comment.

Thank you all for spending time reviewing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116217

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

[Lldb-commits] [lldb] fdd741d - [lldb/linux] Fix a bug in wait status handling

2021-12-29 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2021-12-29T11:06:30+01:00
New Revision: fdd741dd31814d3d4e5c29185f27a529943050d2

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

LOG: [lldb/linux] Fix a bug in wait status handling

The MonitorCallback function was assuming that the "exited" argument is
set whenever a thread exits, but the caller was only setting that flag
for the main thread.

This patch deletes the argument altogether, and lets MonitorCallback
compute what it needs itself.

This is almost NFC, since previously we would end up in the
"GetSignalInfo failed for unknown reasons" branch, which was doing the
same thing -- forgetting about the thread.

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index d7651ce71da0b..8f5496d9f4e5a 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -426,8 +426,7 @@ Status NativeProcessLinux::SetDefaultPtraceOpts(lldb::pid_t 
pid) {
 }
 
 // Handles all waitpid events from the inferior process.
-void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, bool exited,
- WaitStatus status) {
+void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, WaitStatus status) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
 
   // Certain activities 
diff er based on whether the pid is the tid of the main
@@ -435,7 +434,7 @@ void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, 
bool exited,
   const bool is_main_thread = (pid == GetID());
 
   // Handle when the thread exits.
-  if (exited) {
+  if (status.type == WaitStatus::Exit || status.type == WaitStatus::Signal) {
 LLDB_LOG(log,
  "got exit status({0}) , tid = {1} ({2} main thread), process "
  "state = {3}",
@@ -485,7 +484,7 @@ void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, 
bool exited,
 if (info.si_signo == SIGTRAP)
   MonitorSIGTRAP(info, *thread_sp);
 else
-  MonitorSignal(info, *thread_sp, exited);
+  MonitorSignal(info, *thread_sp);
   } else {
 if (info_err.GetError() == EINVAL) {
   // This is a group stop reception for this tid. We can reach here if we
@@ -753,7 +752,7 @@ void NativeProcessLinux::MonitorSIGTRAP(const siginfo_t 
&info,
   default:
 LLDB_LOG(log, "received unknown SIGTRAP stop event ({0}, pid {1} tid {2}",
  info.si_code, GetID(), thread.GetID());
-MonitorSignal(info, thread, false);
+MonitorSignal(info, thread);
 break;
   }
 }
@@ -801,7 +800,7 @@ void 
NativeProcessLinux::MonitorWatchpoint(NativeThreadLinux &thread,
 }
 
 void NativeProcessLinux::MonitorSignal(const siginfo_t &info,
-   NativeThreadLinux &thread, bool exited) 
{
+   NativeThreadLinux &thread) {
   const int signo = info.si_signo;
   const bool is_from_llgs = info.si_pid == getpid();
 
@@ -1962,16 +1961,11 @@ void NativeProcessLinux::SigchldHandler() {
 }
 
 WaitStatus wait_status = WaitStatus::Decode(status);
-bool exited = wait_status.type == WaitStatus::Exit ||
-  (wait_status.type == WaitStatus::Signal &&
-   wait_pid == static_cast<::pid_t>(GetID()));
 
-LLDB_LOG(
-log,
-"waitpid (-1, &status, _) => pid = {0}, status = {1}, exited = {2}",
-wait_pid, wait_status, exited);
+LLDB_LOG(log, "waitpid (-1, &status, _) => pid = {0}, status = {1}",
+ wait_pid, wait_status);
 
-MonitorCallback(wait_pid, exited, wait_status);
+MonitorCallback(wait_pid, wait_status);
   }
 }
 

diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
index 902afb6aa98b9..5d33c4753ca8c 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -164,7 +164,7 @@ class NativeProcessLinux : public NativeProcessELF,
 
   static Status SetDefaultPtraceOpts(const lldb::pid_t);
 
-  void MonitorCallback(lldb::pid_t pid, bool exited, WaitStatus status);
+  void MonitorCallback(lldb::pid_t pid, WaitStatus status);
 
   void WaitForCloneNotification(::pid_t pid);
 
@@ -176,8 +176,7 @@ class NativeProcessLinux : public NativeProcessELF,
 
   void MonitorWatchpoint(NativeThreadLinux &thread, uint32_t wp_index);
 
-  void MonitorSignal(const siginfo_t &info, NativeThreadLinux &thread,
- bool exited);
+  void MonitorSignal(const siginfo_t &info, NativeThreadLinux &th

[Lldb-commits] [PATCH] D116255: [lldb] [Process/FreeBSDKernel] Support finding all processes

2021-12-29 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 396512.
mgorny marked 3 inline comments as done.
mgorny added a comment.

Update the code per suggestions, update the first test case.

TODO: update the remaining tests, document the code better


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

https://reviews.llvm.org/D116255

Files:
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/TestFreeBSDKernelVMCore.py
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-minidump.bz2

Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
===
--- lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
@@ -23,6 +23,8 @@
 
 for l in sys.stdin:
 m = line_re.match(l)
+if m is None:
+continue
 offset, size = [int(x) for x in m.groups()]
 
 inf.seek(offset)
Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
===
--- lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
@@ -14,6 +14,12 @@
 AddressAlign:0x80
 Offset:  0x17BA348
 Size:0x445C80
+  - Name:.rodata
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_MERGE, SHF_STRINGS ]
+Address: 0x81152D30
+AddressAlign:0x10
+Size:0x800
 Symbols:
   - Name:kernbase
 Index:   SHN_ABS
@@ -36,3 +42,87 @@
 Binding: STB_GLOBAL
 Value:   0x81CD4C0C
 Size:0x4
+  - Name:proc_off_p_comm
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA624
+Size:0x4
+  - Name:proc_off_p_hash
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA62C
+Size:0x4
+  - Name:proc_off_p_list
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA628
+Size:0x4
+  - Name:proc_off_p_pid
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA620
+Size:0x4
+  - Name:proc_off_p_threads
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA630
+Size:0x4
+  - Name:thread_off_td_name
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA638
+Size:0x4
+  - Name:thread_off_td_oncpu
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA63C
+Size:0x4
+  - Name:thread_off_td_pcb
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA640
+Size:0x4
+  - Name:thread_off_td_plist
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA644
+Size:0x4
+  - Name:thread_off_td_tid
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA634
+Size:0x4
+  - Name:dumptid
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x81CA69A8
+Size:0x4
+  - Name:pcb_size
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA590
+Size:0x4
+  - Name:stoppcbs
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x81D23E20
+Size:0x14000
+  - Name:allproc
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+

[Lldb-commits] [PATCH] D116255: [lldb] [Process/FreeBSDKernel] Support finding all processes

2021-12-29 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 396514.
mgorny added a comment.

Add code comments


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

https://reviews.llvm.org/D116255

Files:
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/TestFreeBSDKernelVMCore.py
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-minidump.bz2

Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
===
--- lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
@@ -23,6 +23,8 @@
 
 for l in sys.stdin:
 m = line_re.match(l)
+if m is None:
+continue
 offset, size = [int(x) for x in m.groups()]
 
 inf.seek(offset)
Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
===
--- lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
@@ -14,6 +14,12 @@
 AddressAlign:0x80
 Offset:  0x17BA348
 Size:0x445C80
+  - Name:.rodata
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_MERGE, SHF_STRINGS ]
+Address: 0x81152D30
+AddressAlign:0x10
+Size:0x800
 Symbols:
   - Name:kernbase
 Index:   SHN_ABS
@@ -36,3 +42,87 @@
 Binding: STB_GLOBAL
 Value:   0x81CD4C0C
 Size:0x4
+  - Name:proc_off_p_comm
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA624
+Size:0x4
+  - Name:proc_off_p_hash
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA62C
+Size:0x4
+  - Name:proc_off_p_list
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA628
+Size:0x4
+  - Name:proc_off_p_pid
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA620
+Size:0x4
+  - Name:proc_off_p_threads
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA630
+Size:0x4
+  - Name:thread_off_td_name
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA638
+Size:0x4
+  - Name:thread_off_td_oncpu
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA63C
+Size:0x4
+  - Name:thread_off_td_pcb
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA640
+Size:0x4
+  - Name:thread_off_td_plist
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA644
+Size:0x4
+  - Name:thread_off_td_tid
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA634
+Size:0x4
+  - Name:dumptid
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x81CA69A8
+Size:0x4
+  - Name:pcb_size
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x815CA590
+Size:0x4
+  - Name:stoppcbs
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x81D23E20
+Size:0x14000
+  - Name:allproc
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x81C9A2F0
+Size:0x8
Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/TestFreeBSDKerne

[Lldb-commits] [PATCH] D116372: [lldb-server/linux] Fix waitpid for multithreaded forks

2021-12-29 Thread Pavel Labath via Phabricator via lldb-commits 
labath created this revision.
labath added reviewers: mgorny, DavidSpickett.
labath requested review of this revision.
Herald added a project: LLDB.

The lldb-server code is currently set up in a way that each
NativeProcess instance does its own waitpid handling. This works fine
for BSDs, where the code can do a waitpid(process_id), and get
information for all threads in that process.

The situation is trickier on linux, because waitpid(pid) will only
return information for the main thread of the process (one whose tid ==
pid). For this reason the linux code does a waitpid(-1), to get
information for all threads. This was fine while we were supporting just
a single process, but becomes a problem when we have multiple processes
as they end up stealing each others events.

There are two possible solutions to this problem:

- call waitpid(-1) centrally, and then dispatch the events to the appropriate 
process
- have each process call waitpid(tid) for all the threads it manages

This patch implements the second approach. Besides fitting better into
the existing design, it also has the added benefit of ensuring
predictable ordering for thread/process creation events (which come in
pairs -- one for the parent and one for the child). The first approach
OTOH, would make this ordering even more complicated since we would
have to keep the half-threads hanging in mid-air until we find the
process we should attach them to.

The downside to this approach is an increased number of syscalls (one
waitpid for each thread), but I think we're pretty far from optimizing
things like this, and so the cleanliness of the design is worth it.

The included test reproduces the circumstances which should demonstrate
the bug (which manifests as a hung test), but I have not been able to
get it to fail. The only place I've seen this failure modes are very
rare hangs in the thread sanitizer tests (tsan forks an addr2line
process to produce its error messages).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116372

Files:
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -6,6 +6,40 @@
 class TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
 mydir = TestBase.compute_mydir(__file__)
 
+@add_test_categories(["fork"])
+def test_fork_multithreaded(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["thread:new"]*2 + ["fork"])
+self.add_qSupported_packets(["multiprocess+", "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+fork_regex = "[$]T.*;fork:p([0-9a-f]+)[.]([0-9a-f]+).*"
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": fork_regex,
+ "capture": {1: "pid", 2: "tid"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+pid = int(ret["pid"], 16)
+self.reset_test_sequence()
+
+# detach the forked child
+self.test_sequence.add_log_lines([
+"read packet: $D;{:x}#00".format(pid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+], True)
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $k#00",
+], True)
+self.expect_gdbremote_sequence()
+
 def fork_and_detach_test(self, variant):
 self.build()
 self.prep_debug_monitor_and_inferior(inferior_args=[variant])
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -164,7 +164,7 @@
 
   static Status SetDefaultPtraceOpts(const lldb::pid_t);
 
-  void MonitorCallback(lldb::pid_t pid, WaitStatus status);
+  void MonitorCallback(NativeThreadLinux &thread, WaitStatus status);
 
   void WaitForCloneNotification(::pid_t pid);
 
@@ -180,7 +180,7 @@
 
   bool HasThreadNoLock(lldb::tid_t thread_id);
 
-  bool StopTrackingThread(lldb::tid_t thread_id);
+  void StopTrackingThread(NativeThreadLinux &thread);
 
   /// Create a new thread.
   ///
@@ -243,20 +243,9 @@
   /// Manages Intel PT process and thread traces.
   IntelPTManager m_intel_pt_manager;
 
-  struct CloneInfo {
-int event;
-lldb::tid_t parent_tid;
-  };
-
-  // Map of child processes that have been signaled on

[Lldb-commits] [PATCH] D116255: [lldb] [Process/FreeBSDKernel] Support finding all processes

2021-12-29 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 396530.
mgorny added a comment.

Update all tests.

Phab doesn't like git binary deltas for some reason, so I've uploaded the patch 
with `--no-binary`.

New file sizes:

  -rw-r--r-- 1 mgorny mgorny 120K 12-29 15:36 
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-full.bz2
  -rw-r--r-- 1 mgorny mgorny 132K 12-29 12:45 
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-minidump.bz2
  -rw-r--r-- 1 mgorny mgorny  37K 12-29 15:45 
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-arm64-minidump.bz2
  -rw-r--r-- 1 mgorny mgorny 136K 12-29 15:38 
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-i386-minidump.bz2


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

https://reviews.llvm.org/D116255

Files:
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/TestFreeBSDKernelVMCore.py
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-full.bz2
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-minidump.bz2
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-arm64-minidump.bz2
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-i386-minidump.bz2

Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
===
--- lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
@@ -23,6 +23,8 @@
 
 for l in sys.stdin:
 m = line_re.match(l)
+if m is None:
+continue
 offset, size = [int(x) for x in m.groups()]
 
 inf.seek(offset)
Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
===
--- lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
@@ -14,6 +14,12 @@
 AddressAlign:0x80
 Offset:  0x12B7AB0
 Size:0x2D48D8
+  - Name:.rodata
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_MERGE, SHF_STRINGS ]
+Address: 0x1400290
+AddressAlign:0x10
+Size:0x800
 Symbols:
   - Name:kernbase
 Index:   SHN_ABS
@@ -36,3 +42,87 @@
 Binding: STB_GLOBAL
 Value:   0x1D8B044
 Size:0x4
+  - Name:proc_off_p_comm
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x1809ABC
+Size:0x4
+  - Name:proc_off_p_hash
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x1809AC4
+Size:0x4
+  - Name:proc_off_p_list
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x1809AC0
+Size:0x4
+  - Name:proc_off_p_pid
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x1809AB8
+Size:0x4
+  - Name:proc_off_p_threads
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x1809AC8
+Size:0x4
+  - Name:thread_off_td_name
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x1809AD0
+Size:0x4
+  - Name:thread_off_td_oncpu
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x1809AD4
+Size:0x4
+  - Name:thread_off_td_pcb
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x1809AD8
+Size:0x4
+  - Name:thread_off_td_plist
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_GLOBAL
+Value:   0x1809ADC
+Size:0x4
+  - Name:thread_off_td_tid
+Type:STT_OBJECT
+Section: .rodata
+Binding: STB_G

[Lldb-commits] [PATCH] D116372: [lldb-server/linux] Fix waitpid for multithreaded forks

2021-12-29 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

Thank you a lot for doing this. I love how the code becomes simpler, and the 
cost doesn't seem much.




Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1909
+ wait_status);
+tid_events.try_emplace(thread_up->GetID(), wait_status);
+  }

Hmm, I'm a bit confused here. Why would the key already exist? I mean, you 
start with empty `tid_events` and catch only one event for every thread, 
correct?



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1918
+} else {
+  // This can happen if one of the events is an main thread exit.
+  LLDB_LOG(log, "... but the thread has disappeared");

Are we talking about some kind of race here? Or some thread that appears in 
`m_threads` but is not returned by `GetThreadByID()`?

I was wondering if you could use thread pointers as keys.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116372

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

Re: [Lldb-commits] [PATCH] D115951: Cache the manual DWARF index out to the LLDB cache directory when the LLDB index cache is enabled.

2021-12-29 Thread Greg Clayton via lldb-commits 
Thanks Pavel, that fix is good.

> On Dec 29, 2021, at 1:05 AM, Pavel Labath via Phabricator 
>  wrote:
> 
> labath added a comment.
> 
> The problem was that (without accelerator tables) we would always have more 
> than one cache file for a given module. I have now adjusted the test (daed479 
> ) to 
> check for the presence of one _symtab_ index, but other solutions are 
> possible as well. Feel free to change it to something else if you think 
> they're better.
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D115951/new/
> 
> https://reviews.llvm.org/D115951
> 

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

[Lldb-commits] [PATCH] D116195: [LLDB] Allows overwriting the existing line entry at given file address when inserting

2021-12-29 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

In D116195#3211906 , @zequanwu wrote:

> In D116195#3211346 , @labath wrote:
>
>> In D116195#3208476 , @zequanwu 
>> wrote:
>>
>>> In D116195#3207672 , @labath 
>>> wrote:
>>>
 How are you planning to make use of this functionality?

 I'm asking because I'm wondering if it wouldn't be better to do this kind 
 of processing in the PDB code, and then hand this class a finished list of 
 line entries. Inserting entries into the middle of a vector is expensive, 
 which is why our dwarf code no longer uses this function (it uses the 
 vector constructor instead). If we could get pdb to do 
 something similar, then we could get rid of this function altogether.
>>>
>>> My plan was to insert entries when parsing inlined call site(`S_INLINESITE 
>>> `) in ParseBlocksRecursive, which usually happens after ParseLineTable. In 
>>> PDB, line entries in inlined call site often have same file addresses as 
>>> line entries in line table, and the former is better since it describes 
>>> lines inside inlined function rather than lines in caller. And we could 
>>> have nested inlined call sites. The line entries from inner call sites 
>>> would always overwrite the line entries from the outer call sites if they 
>>> have the same file address.
>>
>> So, I'm afraid that won't work. You shouldn't be modifying the line tables 
>> after ParseLineTable returns. Lldb (currently) considers the line tables to 
>> be either fully parsed or not parsed at all. There's no way to "partially" 
>> parse a line table. Attempting to do so will result in various strange 
>> effects, including file+line breakpoints resolving differently depending on 
>> which blocks have been parsed.
>>
>> I think you'll need to decide whether you want the inline info to be 
>> reflected in the line table or not. If yes, then you'll need to parse it 
>> from within ParseLineTable, at which point, you can probably do the 
>> deduplication outside of the LineTable class -- we can talk about how to 
>> make that easier.
>
> I think the inline info should be reflected in the line table. Otherwise 
> `image lookup -a ...` cannot correctly show the file and line info for 
> inlined functions. My plan is to keep a sorted (by address) set of line 
> entries before adding into line sequence. Every time we add a new line entry 
> into the set while parsing inline info, replace the existing entry with new 
> entry if they have same address.

Line tables in LLDB are currently encoded such that they only need the deepest 
line table entry for a given address. Line tables in DWARF encode only these 
line table entries. The inlined call stack is decoded by looking at the 
lldb_private::Block objects and looking at their inline information using:

  const InlineFunctionInfo *Block::GetInlinedFunctionInfo() const;

The inline function info has a "Declaration" object that describes the call 
site which is accessed via:

  Declaration &InlineFunctionInfo::GetCallSite();

A declarations a file + line + column. So maybe the best thing to do would be 
to make sure that we only emit the deepest line table entries and possibly 
encode the call site info into the Block objects?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116195

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