date:20220502

[Lldb-commits] [PATCH] D124801: [lldb] Make GetSharedModuleWithLocalCache consider the device support directory

2022-05-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe53019a8ff77: [lldb] Make GetSharedModuleWithLocalCache 
consider the device support directory (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124801

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -328,6 +328,29 @@
 return err;
   }
 }
+
+// We failed to find the module in our shared cache. Let's see if we have a
+// copy in our device support directory.
+FileSpec device_support_spec(GetDeviceSupportDirectoryForOSVersion());
+device_support_spec.AppendPathComponent("Symbols");
+device_support_spec.AppendPathComponent(
+module_spec.GetFileSpec().GetPath());
+FileSystem::Instance().Resolve(device_support_spec);
+if (FileSystem::Instance().Exists(device_support_spec)) {
+  ModuleSpec local_spec(device_support_spec, module_spec.GetUUID());
+  err = ModuleList::GetSharedModule(local_spec, module_sp,
+module_search_paths_ptr, old_modules,
+did_create_ptr);
+  if (module_sp) {
+LLDB_LOGF(log,
+  "[%s] module %s was found in Device Support "
+  "directory: %s",
+  (IsHost() ? "host" : "remote"),
+  module_spec.GetFileSpec().GetPath().c_str(),
+  local_spec.GetFileSpec().GetPath().c_str());
+return err;
+  }
+}
   }
 
   err = ModuleList::GetSharedModule(module_spec, module_sp,


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -328,6 +328,29 @@
 return err;
   }
 }
+
+// We failed to find the module in our shared cache. Let's see if we have a
+// copy in our device support directory.
+FileSpec device_support_spec(GetDeviceSupportDirectoryForOSVersion());
+device_support_spec.AppendPathComponent("Symbols");
+device_support_spec.AppendPathComponent(
+module_spec.GetFileSpec().GetPath());
+FileSystem::Instance().Resolve(device_support_spec);
+if (FileSystem::Instance().Exists(device_support_spec)) {
+  ModuleSpec local_spec(device_support_spec, module_spec.GetUUID());
+  err = ModuleList::GetSharedModule(local_spec, module_sp,
+module_search_paths_ptr, old_modules,
+did_create_ptr);
+  if (module_sp) {
+LLDB_LOGF(log,
+  "[%s] module %s was found in Device Support "
+  "directory: %s",
+  (IsHost() ? "host" : "remote"),
+  module_spec.GetFileSpec().GetPath().c_str(),
+  local_spec.GetFileSpec().GetPath().c_str());
+return err;
+  }
+}
   }
 
   err = ModuleList::GetSharedModule(module_spec, module_sp,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e53019a - [lldb] Make GetSharedModuleWithLocalCache consider the device support directory

2022-05-02 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-05-02T21:07:11-07:00
New Revision: e53019a8ff778048dd83aee29dd659af8b771920

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

LOG: [lldb] Make GetSharedModuleWithLocalCache consider the device support 
directory

Make GetSharedModuleWithLocalCache consider the device support
directory. In the past we only needed the device support directory to
debug remote processes. Since the introduction of Apple Silicon and
Rosetta this stopped being true.

When debugging a Rosetta process on macOS we need to consider the
Rosetta expanded shared cache. This patch and it dependencies move that
logic out of PlatfromRemoteDarwinDevice into a new abstract class called
PlatfromDarwinDevice. The new platform sit in between PlatformDarwin and
PlatformMacOSX and PlatformRemoteDarwinDevice and has all the necessary
logic to deal with the device support directory.

Technically I could have moved everything in PlatfromDarwinDevice into
PlatfromDarwin but decided that this logic is sufficiently self
contained that it warrants its own abstraction.

rdar://91966349

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

Added: 


Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
index d4173e4c812b9..7feaef95ea8c3 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -328,6 +328,29 @@ lldb_private::Status 
PlatformDarwinDevice::GetSharedModuleWithLocalCache(
 return err;
   }
 }
+
+// We failed to find the module in our shared cache. Let's see if we have a
+// copy in our device support directory.
+FileSpec device_support_spec(GetDeviceSupportDirectoryForOSVersion());
+device_support_spec.AppendPathComponent("Symbols");
+device_support_spec.AppendPathComponent(
+module_spec.GetFileSpec().GetPath());
+FileSystem::Instance().Resolve(device_support_spec);
+if (FileSystem::Instance().Exists(device_support_spec)) {
+  ModuleSpec local_spec(device_support_spec, module_spec.GetUUID());
+  err = ModuleList::GetSharedModule(local_spec, module_sp,
+module_search_paths_ptr, old_modules,
+did_create_ptr);
+  if (module_sp) {
+LLDB_LOGF(log,
+  "[%s] module %s was found in Device Support "
+  "directory: %s",
+  (IsHost() ? "host" : "remote"),
+  module_spec.GetFileSpec().GetPath().c_str(),
+  local_spec.GetFileSpec().GetPath().c_str());
+return err;
+  }
+}
   }
 
   err = ModuleList::GetSharedModule(module_spec, module_sp,



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

[Lldb-commits] [PATCH] D124814: Fix debugserver translation check

2022-05-02 Thread Alexandre Perez via Phabricator via lldb-commits

aperez added a comment.

In D124814#3487179 , @jasonmolenda 
wrote:

> Ah, I see what you're doing.  You've got a shell running x86_64 or something 
> (maybe started lldb as x86_64) so everything that is spawned from that is 
> x86_64 -- debugserver and the inferior process -- and so you hit this.  let 
> me look more closely later tonight, but I didn't take that scenario into 
> account.  Normally when people are running debugserver as x86_64 it's because 
> they unintentionally ran a parent as x86_64 and are paying an unintended perf 
> hit across the debug session and part of this error reporting was to say 
> "hey, you probably didn't mean to do this".

In our case we are currently distributing an x86_64-only version of the 
toolchain (including lldb and debugserver) across many machines. Some of these 
machines are m1 laptops, and we started observing failures to attach on these 
machines once 0c443e92d3b9 
 was 
included.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124814

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

[Lldb-commits] [PATCH] D124814: Fix debugserver translation check

2022-05-02 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

(tbh every case I've seen, it's someone who checked "Run in Rosetta" on the 
Xcode.app bundle with Finder, and are running their entire IDE as x86_64)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124814

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

[Lldb-commits] [PATCH] D124814: Fix debugserver translation check

2022-05-02 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

Ah, I see what you're doing.  You've got a shell running x86_64 or something 
(maybe started lldb as x86_64) so everything that is spawned from that is 
x86_64 -- debugserver and the inferior process -- and so you hit this.  let me 
look more closely later tonight, but I didn't take that scenario into account.  
Normally when people are running debugserver as x86_64 it's because they 
unintentionally ran a parent as x86_64 and are paying an unintended perf hit 
across the debug session and part of this error reporting was to say "hey, you 
probably didn't mean to do this".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124814

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

[Lldb-commits] [PATCH] D124800: [lldb] Move GetSharedModuleWithLocalCache to PlatformDarwinDevice (NFC)

2022-05-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG322b4130415a: [lldb] Move GetSharedModuleWithLocalCache to 
PlatformDarwinDevice (NFC) (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124800

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h

Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
@@ -9,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMMACOSX_H
 #define LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMMACOSX_H
 
-#include "PlatformDarwin.h"
+#include "PlatformDarwinDevice.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
@@ -28,7 +28,7 @@
 class Process;
 class Target;
 
-class PlatformMacOSX : public PlatformDarwin {
+class PlatformMacOSX : public PlatformDarwinDevice {
 public:
   PlatformMacOSX();
 
@@ -69,6 +69,10 @@
 return PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
 target, options, XcodeSDK::Type::MacOSX);
   }
+
+protected:
+  llvm::StringRef GetDeviceSupportDirectoryName() override;
+  llvm::StringRef GetPlatformName() override;
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -96,7 +96,7 @@
 }
 
 /// Default Constructor
-PlatformMacOSX::PlatformMacOSX() : PlatformDarwin(true) {}
+PlatformMacOSX::PlatformMacOSX() : PlatformDarwinDevice(true) {}
 
 ConstString PlatformMacOSX::GetSDKDirectory(lldb_private::Target ) {
   ModuleSP exe_module_sp(target.GetExecutableModule());
@@ -211,3 +211,9 @@
   }
   return error;
 }
+
+llvm::StringRef PlatformMacOSX::GetDeviceSupportDirectoryName() {
+  return "macOS DeviceSupport";
+}
+
+llvm::StringRef PlatformMacOSX::GetPlatformName() { return "MacOSX.platform"; }
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h
@@ -24,6 +24,11 @@
   ~PlatformDarwinDevice() override;
 
 protected:
+  virtual Status GetSharedModuleWithLocalCache(
+  const ModuleSpec _spec, lldb::ModuleSP _sp,
+  const FileSpecList *module_search_paths_ptr,
+  llvm::SmallVectorImpl *old_modules, bool *did_create_ptr);
+
   struct SDKDirectoryInfo {
 SDKDirectoryInfo(const FileSpec _dir_spec);
 FileSpec directory;
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -7,6 +7,9 @@
 //===--===//
 
 #include "PlatformDarwinDevice.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -263,3 +266,168 @@
 return m_device_support_directory_for_os_version.c_str();
   return nullptr;
 }
+
+static lldb_private::Status
+MakeCacheFolderForFile(const FileSpec _cache_spec) {
+  FileSpec module_cache_folder =
+  module_cache_spec.CopyByRemovingLastPathComponent();
+  return llvm::sys::fs::create_directory(module_cache_folder.GetPath());
+}
+
+static lldb_private::Status
+BringInRemoteFile(Platform *platform,
+  const lldb_private::ModuleSpec _spec,
+  const FileSpec _cache_spec) {
+  MakeCacheFolderForFile(module_cache_spec);
+  Status err = platform->GetFile(module_spec.GetFileSpec(), module_cache_spec);
+  return err;
+}
+
+lldb_private::Status PlatformDarwinDevice::GetSharedModuleWithLocalCache(
+const lldb_private::ModuleSpec _spec, lldb::ModuleSP _sp,
+const lldb_private::FileSpecList *module_search_paths_ptr,
+llvm::SmallVectorImpl *old_modules, bool *did_create_ptr) {
+
+  Log *log = GetLog(LLDBLog::Platform);
+  LLDB_LOGF(log,
+"[%s] Trying to find module %s/%s - platform path %s/%s symbol "
+"path %s/%s",

[Lldb-commits] [PATCH] D124799: [lldb] Hoist device support out of PlatformRemoteDarwinDevice (NFC)

2022-05-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG41c0ff1e740b: [lldb] Hoist device support out of 
PlatformRemoteDarwinDevice (NFC) (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D124799?vs=426500=426549#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124799

Files:
  lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h

Index: lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
@@ -1,5 +1,4 @@
-//===-- PlatformRemoteDarwinDevice.h -*- C++
-//-*-===//
+//===-- PlatformRemoteDarwinDevice.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.
@@ -10,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEDARWINDEVICE_H
 #define LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEDARWINDEVICE_H
 
-#include "PlatformDarwin.h"
+#include "PlatformDarwinDevice.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
@@ -34,7 +33,7 @@
 class Target;
 class UUID;
 
-class PlatformRemoteDarwinDevice : public PlatformDarwin {
+class PlatformRemoteDarwinDevice : public PlatformDarwinDevice {
 public:
   PlatformRemoteDarwinDevice();
 
@@ -64,39 +63,10 @@
   }
 
 protected:
-  struct SDKDirectoryInfo {
-SDKDirectoryInfo(const FileSpec _dir_spec);
-FileSpec directory;
-ConstString build;
-llvm::VersionTuple version;
-bool user_cached;
-  };
-
-  typedef std::vector SDKDirectoryInfoCollection;
-
-  std::mutex m_sdk_dir_mutex;
-  SDKDirectoryInfoCollection m_sdk_directory_infos;
-  std::string m_device_support_directory;
-  std::string m_device_support_directory_for_os_version;
   std::string m_build_update;
   uint32_t m_last_module_sdk_idx = UINT32_MAX;
   uint32_t m_connected_module_sdk_idx = UINT32_MAX;
 
-  bool UpdateSDKDirectoryInfosIfNeeded();
-
-  const char *GetDeviceSupportDirectory();
-
-  const char *GetDeviceSupportDirectoryForOSVersion();
-
-  const SDKDirectoryInfo *GetSDKDirectoryForLatestOSVersion();
-
-  const SDKDirectoryInfo *GetSDKDirectoryForCurrentOSVersion();
-
-  static FileSystem::EnumerateDirectoryResult
-  GetContainedFilesIntoVectorOfStringsCallback(void *baton,
-   llvm::sys::fs::file_type ft,
-   llvm::StringRef path);
-
   bool GetFileInSDK(const char *platform_file_path, uint32_t sdk_idx,
 FileSpec _file);
 
@@ -106,9 +76,6 @@
   // UINT32_MAX if that SDK not found.
   uint32_t GetSDKIndexBySDKDirectoryInfo(const SDKDirectoryInfo *sdk_info);
 
-  virtual llvm::StringRef GetDeviceSupportDirectoryName() = 0;
-  virtual llvm::StringRef GetPlatformName() = 0;
-
 private:
   PlatformRemoteDarwinDevice(const PlatformRemoteDarwinDevice &) = delete;
   const PlatformRemoteDarwinDevice &
Index: lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
@@ -38,9 +38,7 @@
 
 /// Default Constructor
 PlatformRemoteDarwinDevice::PlatformRemoteDarwinDevice()
-: PlatformDarwin(false), // This is a remote platform
-  m_sdk_directory_infos(), m_device_support_directory(),
-  m_device_support_directory_for_os_version(), m_build_update() {}
+: PlatformDarwinDevice(false) {} // This is a remote platform
 
 /// Destructor.
 ///
@@ -129,259 +127,6 @@
   return error;
 }
 
-FileSystem::EnumerateDirectoryResult
-PlatformRemoteDarwinDevice::GetContainedFilesIntoVectorOfStringsCallback(
-void *baton, llvm::sys::fs::file_type ft, llvm::StringRef path) {
-  ((PlatformRemoteDarwinDevice::SDKDirectoryInfoCollection *)baton)
-  ->push_back(PlatformRemoteDarwinDevice::SDKDirectoryInfo(FileSpec(path)));
-  return FileSystem::eEnumerateDirectoryResultNext;
-}
-
-bool PlatformRemoteDarwinDevice::UpdateSDKDirectoryInfosIfNeeded() {
-  Log *log = GetLog(LLDBLog::Host);
-  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) {
-

[Lldb-commits] [lldb] 322b413 - [lldb] Move GetSharedModuleWithLocalCache to PlatformDarwinDevice (NFC)

2022-05-02 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-05-02T17:34:40-07:00
New Revision: 322b4130415ac51ba48c993dfebc9c0b38d5bc32

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

LOG: [lldb] Move GetSharedModuleWithLocalCache to PlatformDarwinDevice (NFC)

Refactoring in preparation of D124801.

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

Added: 


Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index d1696766166cd..cac9c672afb4c 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -200,167 +200,6 @@ Status PlatformDarwin::ResolveSymbolFile(Target ,
   return {};
 }
 
-static lldb_private::Status
-MakeCacheFolderForFile(const FileSpec _cache_spec) {
-  FileSpec module_cache_folder =
-  module_cache_spec.CopyByRemovingLastPathComponent();
-  return llvm::sys::fs::create_directory(module_cache_folder.GetPath());
-}
-
-static lldb_private::Status
-BringInRemoteFile(Platform *platform,
-  const lldb_private::ModuleSpec _spec,
-  const FileSpec _cache_spec) {
-  MakeCacheFolderForFile(module_cache_spec);
-  Status err = platform->GetFile(module_spec.GetFileSpec(), module_cache_spec);
-  return err;
-}
-
-lldb_private::Status PlatformDarwin::GetSharedModuleWithLocalCache(
-const lldb_private::ModuleSpec _spec, lldb::ModuleSP _sp,
-const lldb_private::FileSpecList *module_search_paths_ptr,
-llvm::SmallVectorImpl *old_modules, bool *did_create_ptr) {
-
-  Log *log = GetLog(LLDBLog::Platform);
-  LLDB_LOGF(log,
-"[%s] Trying to find module %s/%s - platform path %s/%s symbol "
-"path %s/%s",
-(IsHost() ? "host" : "remote"),
-module_spec.GetFileSpec().GetDirectory().AsCString(),
-module_spec.GetFileSpec().GetFilename().AsCString(),
-module_spec.GetPlatformFileSpec().GetDirectory().AsCString(),
-module_spec.GetPlatformFileSpec().GetFilename().AsCString(),
-module_spec.GetSymbolFileSpec().GetDirectory().AsCString(),
-module_spec.GetSymbolFileSpec().GetFilename().AsCString());
-
-  Status err;
-
-  if (CheckLocalSharedCache()) {
-// When debugging on the host, we are most likely using the same shared
-// cache as our inferior. The dylibs from the shared cache might not
-// exist on the filesystem, so let's use the images in our own memory
-// to create the modules.
-
-// Check if the requested image is in our shared cache.
-SharedCacheImageInfo image_info =
-HostInfo::GetSharedCacheImageInfo(module_spec.GetFileSpec().GetPath());
-
-// If we found it and it has the correct UUID, let's proceed with
-// creating a module from the memory contents.
-if (image_info.uuid &&
-(!module_spec.GetUUID() || module_spec.GetUUID() == image_info.uuid)) {
-  ModuleSpec shared_cache_spec(module_spec.GetFileSpec(), image_info.uuid,
-   image_info.data_sp);
-  err = ModuleList::GetSharedModule(shared_cache_spec, module_sp,
-module_search_paths_ptr, old_modules,
-did_create_ptr);
-  if (module_sp)
-return err;
-}
-  }
-
-  err = ModuleList::GetSharedModule(module_spec, module_sp,
-module_search_paths_ptr, old_modules,
-did_create_ptr);
-  if (module_sp)
-return err;
-
-  if (!IsHost()) {
-std::string cache_path(GetLocalCacheDirectory());
-// Only search for a locally cached file if we have a valid cache path
-if (!cache_path.empty()) {
-  std::string module_path(module_spec.GetFileSpec().GetPath());
-  cache_path.append(module_path);
-  FileSpec module_cache_spec(cache_path);
-
-  // if rsync is supported, always bring in the file - rsync will be very
-  // efficient when files are the same on the local and remote end of the
-  // connection
-  if (this->GetSupportsRSync()) {
-err = BringInRemoteFile(this, module_spec, module_cache_spec);
-if (err.Fail())
-  return err;
-if (FileSystem::Instance().Exists(module_cache_spec)) {
-  Log *log = GetLog(LLDBLog::Platform);
-  LLDB_LOGF(log,

[Lldb-commits] [lldb] 41c0ff1 - [lldb] Hoist device support out of PlatformRemoteDarwinDevice (NFC)

2022-05-02 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-05-02T17:34:40-07:00
New Revision: 41c0ff1e740bc0962104a2619b908f8c24e28741

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

LOG: [lldb] Hoist device support out of PlatformRemoteDarwinDevice (NFC)

Refactoring in preparation of D124801.

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

Added: 
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h

Modified: 
lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt 
b/lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
index bd9343773b3c2..3b43598741ed4 100644
--- a/lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
+++ b/lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
@@ -8,6 +8,7 @@ lldb_tablegen(PlatformMacOSXPropertiesEnum.inc 
-gen-lldb-property-enum-defs
 
 list(APPEND PLUGIN_PLATFORM_MACOSX_SOURCES
   PlatformDarwin.cpp
+  PlatformDarwinDevice.cpp
   PlatformDarwinKernel.cpp
   PlatformMacOSX.cpp
   PlatformRemoteAppleBridge.cpp

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
new file mode 100644
index 0..90aacc3b4ca64
--- /dev/null
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -0,0 +1,265 @@
+//===-- PlatformDarwinDevice.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 "PlatformDarwinDevice.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+PlatformDarwinDevice::~PlatformDarwinDevice() = default;
+
+FileSystem::EnumerateDirectoryResult
+PlatformDarwinDevice::GetContainedFilesIntoVectorOfStringsCallback(
+void *baton, llvm::sys::fs::file_type ft, llvm::StringRef path) {
+  ((PlatformDarwinDevice::SDKDirectoryInfoCollection *)baton)
+  ->push_back(PlatformDarwinDevice::SDKDirectoryInfo(FileSpec(path)));
+  return FileSystem::eEnumerateDirectoryResultNext;
+}
+
+bool PlatformDarwinDevice::UpdateSDKDirectoryInfosIfNeeded() {
+  Log *log = GetLog(LLDBLog::Host);
+  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());
+  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);
+  if (log) {
+LLDB_LOGF(log,
+  "PlatformDarwinDevice::UpdateSDKDirectoryInfosIfNeeded added 
"
+  "--sysroot SDK directory %s",
+  m_sdk_sysroot.GetCString());
+  }
+  return true;
+}
+const char *device_support_dir = GetDeviceSupportDirectory();
+if (log) {
+  LLDB_LOGF(log,
+"PlatformDarwinDevice::UpdateSDKDirectoryInfosIfNeeded Got "
+"DeviceSupport directory %s",
+device_support_dir);
+}
+if (device_support_dir) {
+  const bool find_directories = true;
+  const bool find_files = false;
+  const bool find_other = false;
+
+  SDKDirectoryInfoCollection builtin_sdk_directory_infos;
+  FileSystem::Instance().EnumerateDirectory(
+  m_device_support_directory, find_directories, find_files, find_other,
+  GetContainedFilesIntoVectorOfStringsCallback,
+  _sdk_directory_infos);
+
+  // Only add SDK directories that have symbols in them, some SDKs only
+  // contain developer disk images and no symbols, so they aren't useful to
+  // us.
+  FileSpec sdk_symbols_symlink_fspec;
+  for (const auto _directory_info : builtin_sdk_directory_infos) {
+sdk_symbols_symlink_fspec = sdk_directory_info.directory;
+sdk_symbols_symlink_fspec.AppendPathComponent("Symbols");
+if (FileSystem::Instance().Exists(sdk_symbols_symlink_fspec)) {
+  m_sdk_directory_infos.push_back(sdk_directory_info);
+  if (log) {
+LLDB_LOGF(log,
+

[Lldb-commits] [lldb] d006773 - [lldb] Remove unused PlatformRemoteDarwinDevice::FindFileInAllSDKs

2022-05-02 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-05-02T17:34:39-07:00
New Revision: d0067738e0cf764a92e1cc5ee0c8b2af8659bada

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

LOG: [lldb] Remove unused PlatformRemoteDarwinDevice::FindFileInAllSDKs

As far as I can tell this function is unused both upstream and
downstream.

Added: 


Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
index be03cdbd5666d..83e98ff4e06a2 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
@@ -382,25 +382,6 @@ const char 
*PlatformRemoteDarwinDevice::GetDeviceSupportDirectoryForOSVersion()
   return nullptr;
 }
 
-uint32_t PlatformRemoteDarwinDevice::FindFileInAllSDKs(const char 
*platform_file_path,
-  FileSpecList _list) {
-  Log *log = GetLog(LLDBLog::Host);
-  if (platform_file_path && platform_file_path[0] &&
-  UpdateSDKDirectoryInfosIfNeeded()) {
-const uint32_t num_sdk_infos = m_sdk_directory_infos.size();
-lldb_private::FileSpec local_file;
-// First try for an exact match of major, minor and update
-for (uint32_t sdk_idx = 0; sdk_idx < num_sdk_infos; ++sdk_idx) {
-  LLDB_LOGV(log, "Searching for {0} in sdk path {1}", platform_file_path,
-m_sdk_directory_infos[sdk_idx].directory);
-  if (GetFileInSDK(platform_file_path, sdk_idx, local_file)) {
-file_list.Append(local_file);
-  }
-}
-  }
-  return file_list.GetSize();
-}
-
 bool PlatformRemoteDarwinDevice::GetFileInSDK(const char *platform_file_path,
  uint32_t sdk_idx,
  lldb_private::FileSpec _file) {

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h 
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
index 02cb09df7f77f..6623a94f5745c 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
@@ -97,15 +97,9 @@ class PlatformRemoteDarwinDevice : public PlatformDarwin {
llvm::sys::fs::file_type ft,
llvm::StringRef path);
 
-  uint32_t FindFileInAllSDKs(const char *platform_file_path,
- FileSpecList _list);
-
   bool GetFileInSDK(const char *platform_file_path, uint32_t sdk_idx,
 FileSpec _file);
 
-  uint32_t FindFileInAllSDKs(const FileSpec _file,
- FileSpecList _list);
-
   uint32_t GetConnectedSDKIndex();
 
   // Get index of SDK in SDKDirectoryInfoCollection by its pointer and return



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

[Lldb-commits] [lldb] d75cc08 - [lldb] Remove PlatformRemoteMacOSX::GetFileWithUUID overload (NFC)

2022-05-02 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-05-02T17:34:36-07:00
New Revision: d75cc0859391e90a79bb0de6300d5d69db015421

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

LOG: [lldb] Remove PlatformRemoteMacOSX::GetFileWithUUID overload (NFC)

There's no reason PlatformRemoteMacOSX has to override GetFileWithUUID.

Added: 


Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.cpp
index 99643791ad328..460f6d8bd6170 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.cpp
@@ -70,8 +70,8 @@ PlatformSP PlatformRemoteMacOSX::CreateInstance(bool force,
 const char *triple_cstr =
 arch ? arch->GetTriple().getTriple().c_str() : "";
 
-LLDB_LOGF(log, "PlatformMacOSX::%s(force=%s, arch={%s,%s})", __FUNCTION__,
-  force ? "true" : "false", arch_name, triple_cstr);
+LLDB_LOGF(log, "PlatformRemoteMacOSX::%s(force=%s, arch={%s,%s})",
+  __FUNCTION__, force ? "true" : "false", arch_name, triple_cstr);
   }
 
   bool create = force;
@@ -141,52 +141,6 @@ PlatformRemoteMacOSX::GetSupportedArchitectures(const 
ArchSpec _info) {
   return result;
 }
 
-lldb_private::Status PlatformRemoteMacOSX::GetFileWithUUID(
-const lldb_private::FileSpec _file,
-const lldb_private::UUID *uuid_ptr, lldb_private::FileSpec _file) {
-  if (m_remote_platform_sp) {
-std::string local_os_build;
-#if !defined(__linux__)
-local_os_build = HostInfo::GetOSBuildString().getValueOr("");
-#endif
-llvm::Optional remote_os_build =
-m_remote_platform_sp->GetOSBuildString();
-if (local_os_build == remote_os_build) {
-  // same OS version: the local file is good enough
-  local_file = platform_file;
-  return Status();
-} else {
-  // try to find the file in the cache
-  std::string cache_path(GetLocalCacheDirectory());
-  std::string module_path(platform_file.GetPath());
-  cache_path.append(module_path);
-  FileSpec module_cache_spec(cache_path);
-  if (FileSystem::Instance().Exists(module_cache_spec)) {
-local_file = module_cache_spec;
-return Status();
-  }
-  // bring in the remote module file
-  FileSpec module_cache_folder =
-  module_cache_spec.CopyByRemovingLastPathComponent();
-  // try to make the local directory first
-  Status err(
-  llvm::sys::fs::create_directory(module_cache_folder.GetPath()));
-  if (err.Fail())
-return err;
-  err = GetFile(platform_file, module_cache_spec);
-  if (err.Fail())
-return err;
-  if (FileSystem::Instance().Exists(module_cache_spec)) {
-local_file = module_cache_spec;
-return Status();
-  } else
-return Status("unable to obtain valid module file");
-}
-  }
-  local_file = platform_file;
-  return Status();
-}
-
 llvm::StringRef PlatformRemoteMacOSX::GetDescriptionStatic() {
   return "Remote Mac OS X user platform plug-in.";
 }

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h 
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h
index d84b5c03c5873..17ae372e3616b 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h
@@ -41,9 +41,6 @@ class PlatformRemoteMacOSX : public virtual 
PlatformRemoteDarwinDevice {
 
   llvm::StringRef GetDescription() override { return GetDescriptionStatic(); }
 
-  Status GetFileWithUUID(const FileSpec _file, const UUID *uuid_ptr,
- FileSpec _file) override;
-
   std::vector
   GetSupportedArchitectures(const ArchSpec _host_arch) override;
 



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

[Lldb-commits] [PATCH] D124648: [trace][intelpt] Support system-wide tracing [3] - Refactor IntelPTThreadTrace

2022-05-02 Thread walter erquinigo via Phabricator via lldb-commits

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

update test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124648

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Perf.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  lldb/unittests/Process/Linux/CMakeLists.txt
  lldb/unittests/Process/Linux/IntelPTCollectorTests.cpp
  lldb/unittests/Process/Linux/PerfTests.cpp

Index: lldb/unittests/Process/Linux/PerfTests.cpp
===
--- lldb/unittests/Process/Linux/PerfTests.cpp
+++ lldb/unittests/Process/Linux/PerfTests.cpp
@@ -86,4 +86,135 @@
 (SLEEP_NANOS + acceptable_overhead).count());
 }
 
+size_t ReadCylicBufferWrapper(void *buf, size_t buf_size, void *cyc_buf,
+  size_t cyc_buf_size, size_t cyc_start,
+  size_t offset) {
+  llvm::MutableArrayRef dst(reinterpret_cast(buf),
+ buf_size);
+  llvm::ArrayRef src(reinterpret_cast(cyc_buf),
+  cyc_buf_size);
+  ReadCyclicBuffer(dst, src, cyc_start, offset);
+  return dst.size();
+}
+
+TEST(CyclicBuffer, EdgeCases) {
+  size_t bytes_read;
+  uint8_t cyclic_buffer[6] = {'l', 'i', 'c', 'c', 'y', 'c'};
+
+  // We will always leave the last bytes untouched
+  // so that string comparisons work.
+  char smaller_buffer[4] = {};
+
+  // empty buffer to read into
+  bytes_read = ReadCylicBufferWrapper(smaller_buffer, 0, cyclic_buffer,
+  sizeof(cyclic_buffer), 3, 0);
+  ASSERT_EQ(0u, bytes_read);
+
+  // empty cyclic buffer
+  bytes_read = ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+  cyclic_buffer, 0, 3, 0);
+  ASSERT_EQ(0u, bytes_read);
+
+  // bigger offset
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 6);
+  ASSERT_EQ(0u, bytes_read);
+
+  // wrong offset
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 7);
+  ASSERT_EQ(0u, bytes_read);
+
+  // wrong start
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 7);
+  ASSERT_EQ(0u, bytes_read);
+}
+
+TEST(CyclicBuffer, EqualSizeBuffer) {
+  size_t bytes_read = 0;
+  uint8_t cyclic_buffer[6] = {'l', 'i', 'c', 'c', 'y', 'c'};
+
+  char cyclic[] = "cyclic";
+  for (size_t i = 0; i < sizeof(cyclic); i++) {
+// We will always leave the last bytes untouched
+// so that string comparisons work.
+char equal_size_buffer[7] = {};
+bytes_read =
+ReadCylicBufferWrapper(equal_size_buffer, sizeof(cyclic_buffer),
+   cyclic_buffer, sizeof(cyclic_buffer), 3, i);
+ASSERT_EQ((sizeof(cyclic) - i - 1), bytes_read);
+ASSERT_STREQ(equal_size_buffer, (cyclic + i));
+  }
+}
+
+TEST(CyclicBuffer, SmallerSizeBuffer) {
+  size_t bytes_read;
+  uint8_t cyclic_buffer[6] = {'l', 'i', 'c', 'c', 'y', 'c'};
+
+  // We will always leave the last bytes untouched
+  // so that string comparisons work.
+  char smaller_buffer[4] = {};
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 0);
+  ASSERT_EQ(3u, bytes_read);
+  ASSERT_STREQ(smaller_buffer, "cyc");
+
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 1);
+  ASSERT_EQ(3u, bytes_read);
+  ASSERT_STREQ(smaller_buffer, "ycl");
+
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 2);
+  ASSERT_EQ(3u, bytes_read);
+  ASSERT_STREQ(smaller_buffer, "cli");
+
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 3);
+  ASSERT_EQ(3u, bytes_read);
+  ASSERT_STREQ(smaller_buffer, "lic");
+
+  {
+char

[Lldb-commits] [PATCH] D124814: Fix debugserver translation check

2022-05-02 Thread Alexandre Perez via Phabricator via lldb-commits

aperez created this revision.
Herald added subscribers: pengfei, kristof.beyls.
Herald added a project: All.
aperez edited the summary of this revision.
aperez added a project: LLDB.
Herald added a subscriber: JDevlieghere.
aperez added reviewers: clayborg, jasonmolenda.
aperez published this revision for review.
aperez added a comment.
Herald added a subscriber: lldb-commits.

I did not add unit tests because I would need to debug an x86_64 binary using 
an x86_64 debugserver on arm64, and was not sure if the test infrastructure 
allowed for that.

I did run a few manual tests which I'm including in this comment. I compiled 
and used a `Darwin-x86_64` debugserver with the patch from this diff applied. 
`x86_64` processes are now handed off to the translated debugserver:

  $ lldb main_x86_64
  (lldb) target create "main_x86_64"
  Current executable set to '/Users/alexandreperez/temp/main_x86_64' (x86_64).
  (lldb) b main
  Breakpoint 1: where = main_x86_64`main + 11 at main.cpp:2:3, address = 
0x00013fab
  (lldb) r
  Process 80635 launched: '/Users/alexandreperez/temp/main_x86_64' (x86_64)
  Process 80635 stopped
  * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  frame #0: 0x00013fab main_x86_64`main at main.cpp:2:3
 1  int main() {
  -> 2return 0;
 3  }
  (lldb)

Verified that the `arm64` binary fail to attach using the patched 
`Darwin-x86_64` debugserver, as expected:

  $ lldb main_arm64
  (lldb) target create "main_arm64"
  Current executable set to '/Users/alexandreperez/temp/main_arm64' (arm64).
  (lldb) r
  error: process exited with status -1 (debugserver is x86_64 binary running in 
translation, attach failed.)
  (lldb)

Using the `LLDB_DEBUGSERVER_PATH` environment variable prevents from handing 
off to the translated debugserver, as expected:

  $ LLDB_DEBUGSERVER_PATH="/path/to/patched/debugserver" lldb main_x86_64
  (lldb) target create "main_x86_64"
  Current executable set to '/Users/alexandreperez/temp/main_x86_64' (x86_64).
  (lldb) r
  error: process exited with status -1 (debugserver is x86_64 binary running in 
translation, attach failed.)
  (lldb)


Currently, debugserver has a test to check if it was launched in
translation. The intent was to cover the case where an x86_64
debugserver attempts to control an arm64/arm64e process, returning
an error. However, this check also covers the case where users
are attaching to an x86_64 process, exiting out before attempting
to hand off control to the translated debugserver at
`/Library/Apple/usr/libexec/oah/debugserver`.

This diff delays the debugserver translation check until after
determining whether to hand off control to 
`/Library/Apple/usr/libexec/oah/debugserver`. Only when the 
process is not translated and thus has not been handed off do we
check if the debugserver is translated, erroring out in that case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124814

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNBDefs.h
  lldb/tools/debugserver/source/RNBRemote.cpp


Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3753,17 +3753,6 @@
 char err_str[1024] = {'\0'};
 std::string attach_name;
 
-if (DNBDebugserverIsTranslated()) {
-  DNBLogError("debugserver is x86_64 binary running in translation, attach 
"
-  "failed.");
-  std::string return_message = "E96;";
-  return_message +=
-  cstring_to_asciihex_string("debugserver is x86_64 binary running in "
- "translation, attached failed.");
-  SendPacket(return_message);
-  return rnb_err;
-}
-
 if (strstr(p, "vAttachWait;") == p) {
   p += strlen("vAttachWait;");
   if (!GetProcessNameFrom_vAttach(p, attach_name)) {
@@ -3823,6 +3812,17 @@
   return HandlePacket_UNIMPLEMENTED(p);
 }
 
+if (attach_pid == INVALID_NUB_PROCESS_ARCH) {
+  DNBLogError("debugserver is x86_64 binary running in translation, attach 
"
+  "failed.");
+  std::string return_message = "E96;";
+  return_message +=
+  cstring_to_asciihex_string("debugserver is x86_64 binary running in "
+ "translation, attach failed.");
+  SendPacket(return_message.c_str());
+  return rnb_err;
+}
+
 if (attach_pid != INVALID_NUB_PROCESS) {
   if (m_ctx.ProcessID() != attach_pid)
 m_ctx.SetProcessID(attach_pid);
Index: lldb/tools/debugserver/source/DNBDefs.h
===
--- lldb/tools/debugserver/source/DNBDefs.h
+++ lldb/tools/debugserver/source/DNBDefs.h
@@ -54,6 +54,7 @@
 typedef uint32_t nub_bool_t;
 
 #define INVALID_NUB_PROCESS ((nub_process_t)0)
+#define

[Lldb-commits] [PATCH] D124579: Make partial function name matching match on context boundaries

2022-05-02 Thread Greg Clayton via Phabricator via lldb-commits

clayborg accepted this revision.
clayborg added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/include/lldb/Target/Language.h:225
+  // symbol was really B::A::my_function.  We want that to be
+  // a match.  But we wouldn't want this to match AnotherB::A::my_function.  
The
+  // user is specifying a truncated path, not a truncated set of characters.

jingham wrote:
> clayborg wrote:
> > The comment should be adding something extra to "A" right? See suggested 
> > code change 
> Yes, the comment was wrong.  I had a fancier example that was taking too many 
> characters and I didn't delete enough when simplifying.  But I'm not sure 
> what code change you are referring to?  I don't see how this is related to 
> GetFunctionName.
It isn't related to GetFunctionName, I have a code change that you can see on 
the web page that suggests the edit to make to fix the comment. I see you fixed 
the comment, so nothing more needed for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124579

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

[Lldb-commits] [PATCH] D124801: [lldb] Make GetSharedModuleWithLocalCache consider the device support directory

2022-05-02 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

LGTM!


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

https://reviews.llvm.org/D124801

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

[Lldb-commits] [PATCH] D124800: [lldb] Move GetSharedModuleWithLocalCache to PlatformDarwinDevice (NFC)

2022-05-02 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

LGTM!


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

https://reviews.llvm.org/D124800

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

[Lldb-commits] [PATCH] D124799: [lldb] Hoist device support out of PlatformRemoteDarwinDevice (NFC)

2022-05-02 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

LGTM


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

https://reviews.llvm.org/D124799

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

[Lldb-commits] [PATCH] D124801: [lldb] Make GetSharedModuleWithLocalCache consider the device support directory

2022-05-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Make GetSharedModuleWithLocalCache consider the device support directory. In 
the past we only needed the device support directory to debug remote processes. 
Since the introduction of Apple Silicon and Rosetta this stopped being true. 
When debugging a Rosetta process on macOS we need to consider the Rosetta 
expanded shared cache. This patch and it dependencies move that logic out of 
PlatfromRemoteDarwinDevice into a new abstract class called 
PlatfromDarwinDevice. The new platform sit in between PlatformDarwin and 
PlatformMacOSX and PlatformRemoteDarwinDevice and has all the necessary logic 
to deal with the device support directory.

Technically I could have moved everything in PlatfromDarwinDevice into 
PlatfromDarwin but decided that this logic is sufficiently self contained that 
it warrants its own abstraction.

rdar://91966349


https://reviews.llvm.org/D124801

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -328,6 +328,29 @@
 return err;
   }
 }
+
+// We failed to find the module in our shared cache. Let's see if we have a
+// copy in our device support directory.
+FileSpec device_support_spec(GetDeviceSupportDirectoryForOSVersion());
+device_support_spec.AppendPathComponent("Symbols");
+device_support_spec.AppendPathComponent(
+module_spec.GetFileSpec().GetPath());
+FileSystem::Instance().Resolve(device_support_spec);
+if (FileSystem::Instance().Exists(device_support_spec)) {
+  ModuleSpec local_spec(device_support_spec, module_spec.GetUUID());
+  err = ModuleList::GetSharedModule(local_spec, module_sp,
+module_search_paths_ptr, old_modules,
+did_create_ptr);
+  if (module_sp) {
+LLDB_LOGF(log,
+  "[%s] module %s was found in Device Support "
+  "directory: %s",
+  (IsHost() ? "host" : "remote"),
+  module_spec.GetFileSpec().GetPath().c_str(),
+  local_spec.GetFileSpec().GetPath().c_str());
+return err;
+  }
+}
   }
 
   err = ModuleList::GetSharedModule(module_spec, module_sp,


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -328,6 +328,29 @@
 return err;
   }
 }
+
+// We failed to find the module in our shared cache. Let's see if we have a
+// copy in our device support directory.
+FileSpec device_support_spec(GetDeviceSupportDirectoryForOSVersion());
+device_support_spec.AppendPathComponent("Symbols");
+device_support_spec.AppendPathComponent(
+module_spec.GetFileSpec().GetPath());
+FileSystem::Instance().Resolve(device_support_spec);
+if (FileSystem::Instance().Exists(device_support_spec)) {
+  ModuleSpec local_spec(device_support_spec, module_spec.GetUUID());
+  err = ModuleList::GetSharedModule(local_spec, module_sp,
+module_search_paths_ptr, old_modules,
+did_create_ptr);
+  if (module_sp) {
+LLDB_LOGF(log,
+  "[%s] module %s was found in Device Support "
+  "directory: %s",
+  (IsHost() ? "host" : "remote"),
+  module_spec.GetFileSpec().GetPath().c_str(),
+  local_spec.GetFileSpec().GetPath().c_str());
+return err;
+  }
+}
   }
 
   err = ModuleList::GetSharedModule(module_spec, module_sp,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D124800: [lldb] Move GetSharedModuleWithLocalCache to PlatformDarwinDevice (NFC)

2022-05-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

https://reviews.llvm.org/D124800

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h

Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
@@ -9,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMMACOSX_H
 #define LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMMACOSX_H
 
-#include "PlatformDarwin.h"
+#include "PlatformDarwinDevice.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
@@ -28,7 +28,7 @@
 class Process;
 class Target;
 
-class PlatformMacOSX : public PlatformDarwin {
+class PlatformMacOSX : public PlatformDarwinDevice {
 public:
   PlatformMacOSX();
 
@@ -69,6 +69,10 @@
 return PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
 target, options, XcodeSDK::Type::MacOSX);
   }
+
+protected:
+  llvm::StringRef GetDeviceSupportDirectoryName() override;
+  llvm::StringRef GetPlatformName() override;
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -96,7 +96,7 @@
 }
 
 /// Default Constructor
-PlatformMacOSX::PlatformMacOSX() : PlatformDarwin(true) {}
+PlatformMacOSX::PlatformMacOSX() : PlatformDarwinDevice(true) {}
 
 ConstString PlatformMacOSX::GetSDKDirectory(lldb_private::Target ) {
   ModuleSP exe_module_sp(target.GetExecutableModule());
@@ -211,3 +211,9 @@
   }
   return error;
 }
+
+llvm::StringRef PlatformMacOSX::GetDeviceSupportDirectoryName() {
+  return "macOS DeviceSupport";
+}
+
+llvm::StringRef PlatformMacOSX::GetPlatformName() { return "MacOSX.platform"; }
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h
@@ -24,6 +24,11 @@
   ~PlatformDarwinDevice() override;
 
 protected:
+  virtual Status GetSharedModuleWithLocalCache(
+  const ModuleSpec _spec, lldb::ModuleSP _sp,
+  const FileSpecList *module_search_paths_ptr,
+  llvm::SmallVectorImpl *old_modules, bool *did_create_ptr);
+
   struct SDKDirectoryInfo {
 SDKDirectoryInfo(const FileSpec _dir_spec);
 FileSpec directory;
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -7,6 +7,9 @@
 //===--===//
 
 #include "PlatformDarwinDevice.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -263,3 +266,168 @@
 return m_device_support_directory_for_os_version.c_str();
   return nullptr;
 }
+
+static lldb_private::Status
+MakeCacheFolderForFile(const FileSpec _cache_spec) {
+  FileSpec module_cache_folder =
+  module_cache_spec.CopyByRemovingLastPathComponent();
+  return llvm::sys::fs::create_directory(module_cache_folder.GetPath());
+}
+
+static lldb_private::Status
+BringInRemoteFile(Platform *platform,
+  const lldb_private::ModuleSpec _spec,
+  const FileSpec _cache_spec) {
+  MakeCacheFolderForFile(module_cache_spec);
+  Status err = platform->GetFile(module_spec.GetFileSpec(), module_cache_spec);
+  return err;
+}
+
+lldb_private::Status PlatformDarwinDevice::GetSharedModuleWithLocalCache(
+const lldb_private::ModuleSpec _spec, lldb::ModuleSP _sp,
+const lldb_private::FileSpecList *module_search_paths_ptr,
+llvm::SmallVectorImpl *old_modules, bool *did_create_ptr) {
+
+  Log *log = GetLog(LLDBLog::Platform);
+  LLDB_LOGF(log,
+"[%s] Trying to find module %s/%s - platform path %s/%s symbol "
+"path %s/%s",
+(IsHost() ? "host" : "remote"),
+module_spec.GetFileSpec().GetDirectory().AsCString(),
+module_spec.GetFileSpec().GetFilename().AsCString(),
+

[Lldb-commits] [PATCH] D124799: [lldb] Hoist device support out of PlatformRemoteDarwinDevice (NFC)

2022-05-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

https://reviews.llvm.org/D124799

Files:
  lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.h
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h

Index: lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
@@ -1,5 +1,4 @@
-//===-- PlatformRemoteDarwinDevice.h -*- C++
-//-*-===//
+//===-- PlatformRemoteDarwinDevice.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.
@@ -10,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEDARWINDEVICE_H
 #define LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEDARWINDEVICE_H
 
-#include "PlatformDarwin.h"
+#include "PlatformDarwinDevice.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
@@ -34,7 +33,7 @@
 class Target;
 class UUID;
 
-class PlatformRemoteDarwinDevice : public PlatformDarwin {
+class PlatformRemoteDarwinDevice : public PlatformDarwinDevice {
 public:
   PlatformRemoteDarwinDevice();
 
@@ -64,39 +63,12 @@
   }
 
 protected:
-  struct SDKDirectoryInfo {
-SDKDirectoryInfo(const FileSpec _dir_spec);
-FileSpec directory;
-ConstString build;
-llvm::VersionTuple version;
-bool user_cached;
-  };
-
-  typedef std::vector SDKDirectoryInfoCollection;
-
   std::mutex m_sdk_dir_mutex;
   SDKDirectoryInfoCollection m_sdk_directory_infos;
-  std::string m_device_support_directory;
-  std::string m_device_support_directory_for_os_version;
   std::string m_build_update;
   uint32_t m_last_module_sdk_idx = UINT32_MAX;
   uint32_t m_connected_module_sdk_idx = UINT32_MAX;
 
-  bool UpdateSDKDirectoryInfosIfNeeded();
-
-  const char *GetDeviceSupportDirectory();
-
-  const char *GetDeviceSupportDirectoryForOSVersion();
-
-  const SDKDirectoryInfo *GetSDKDirectoryForLatestOSVersion();
-
-  const SDKDirectoryInfo *GetSDKDirectoryForCurrentOSVersion();
-
-  static FileSystem::EnumerateDirectoryResult
-  GetContainedFilesIntoVectorOfStringsCallback(void *baton,
-   llvm::sys::fs::file_type ft,
-   llvm::StringRef path);
-
   uint32_t FindFileInAllSDKs(const char *platform_file_path,
  FileSpecList _list);
 
@@ -112,9 +84,6 @@
   // UINT32_MAX if that SDK not found.
   uint32_t GetSDKIndexBySDKDirectoryInfo(const SDKDirectoryInfo *sdk_info);
 
-  virtual llvm::StringRef GetDeviceSupportDirectoryName() = 0;
-  virtual llvm::StringRef GetPlatformName() = 0;
-
 private:
   PlatformRemoteDarwinDevice(const PlatformRemoteDarwinDevice &) = delete;
   const PlatformRemoteDarwinDevice &
Index: lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
@@ -38,9 +38,8 @@
 
 /// Default Constructor
 PlatformRemoteDarwinDevice::PlatformRemoteDarwinDevice()
-: PlatformDarwin(false), // This is a remote platform
-  m_sdk_directory_infos(), m_device_support_directory(),
-  m_device_support_directory_for_os_version(), m_build_update() {}
+: PlatformDarwinDevice(false), // This is a remote platform
+  m_sdk_directory_infos(), m_build_update() {}
 
 /// Destructor.
 ///
@@ -129,259 +128,6 @@
   return error;
 }
 
-FileSystem::EnumerateDirectoryResult
-PlatformRemoteDarwinDevice::GetContainedFilesIntoVectorOfStringsCallback(
-void *baton, llvm::sys::fs::file_type ft, llvm::StringRef path) {
-  ((PlatformRemoteDarwinDevice::SDKDirectoryInfoCollection *)baton)
-  ->push_back(PlatformRemoteDarwinDevice::SDKDirectoryInfo(FileSpec(path)));
-  return FileSystem::eEnumerateDirectoryResultNext;
-}
-
-bool PlatformRemoteDarwinDevice::UpdateSDKDirectoryInfosIfNeeded() {
-  Log *log = GetLog(LLDBLog::Host);
-  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());
-  FileSystem::Instance().Resolve(sdk_sysroot_fspec);
-  const SDKDirectoryInfo

[Lldb-commits] [PATCH] D124785: [lldb/Core] Fix "sticky" long progress messages

2022-05-02 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added inline comments.



Comment at: lldb/source/Core/Debugger.cpp:1873-1875
+  if (message.size() + prefix_width + suffix_width >= term_width)
+message.erase(message.begin() + term_width - prefix_width - suffix_width,
+  message.end());

JDevlieghere wrote:
> Should this account for the control characters? Do they cause a line wrap?
In my understanding, if the control character is complete, it shouldn't print 
anything to the screen so they shouldn't cause a line wrap


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124785

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

[Lldb-commits] [PATCH] D124785: [lldb/Core] Fix "sticky" long progress messages

2022-05-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Can we add a pexpect test for this?




Comment at: lldb/source/Core/Debugger.cpp:1873-1875
+  if (message.size() + prefix_width + suffix_width >= term_width)
+message.erase(message.begin() + term_width - prefix_width - suffix_width,
+  message.end());

Should this account for the control characters? Do they cause a line wrap?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124785

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

[Lldb-commits] [PATCH] D124409: Filter non-external static members from SBType::GetFieldAtIndex.

2022-05-02 Thread Sigurður Ásgeirsson via Phabricator via lldb-commits

siggi-alpheus added a comment.

This is passing lldb-checks locally. Is there anything else I need to add or 
tweak, or is this ready for review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124409

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

[Lldb-commits] [PATCH] D124409: Filter non-external static members from SBType::GetFieldAtIndex.

2022-05-02 Thread Sigurður Ásgeirsson via Phabricator via lldb-commits

siggi-alpheus updated this revision to Diff 426452.
siggi-alpheus added a comment.

Fix awkwardly named variable in the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124409

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s
@@ -0,0 +1,321 @@
+# RUN: llvm-mc --triple=x86_64-pc-linux --filetype=obj %s -o %t
+# RUN: %lldb -o "target variable ug" -b %t | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = (m = 0)
+
+# This tests that a static member in a class declared in the anonymous namespace
+# does not appear as a field of the class. There is a difference between the
+# debug info generated by gcc and clang, where clang flags the static member
+# with DW_AT_external, but gcc does not.
+#
+# This is the code used to generate the assembly:
+#
+# namespace {
+# struct U {
+#   static const int s = 0;
+#   int m;
+# };
+# }
+#
+# U ug;
+#
+# int main() {
+#   return ug.m;
+# }
+#
+# Compiled with:
+#   gcc -S -g test.cpp
+#
+
+	.file	"test.cpp"
+	.text
+.Ltext0:
+	.local	ug
+	.comm	ug,4,4
+	.globl	main
+	.type	main, @function
+main:
+.LFB0:
+	.file 1 "test.cpp"
+	.loc 1 10 12
+	.cfi_startproc
+	endbr64
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset 6, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register 6
+	.loc 1 11 13
+	movl	ug(%rip), %eax
+	.loc 1 12 1
+	popq	%rbp
+	.cfi_def_cfa 7, 8
+	ret
+	.cfi_endproc
+.LFE0:
+	.size	main, .-main
+.Letext0:
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.long	0x95
+	.value	0x4
+	.long	.Ldebug_abbrev0
+	.byte	0x8
+	.uleb128 0x1
+	.long	.LASF0
+	.byte	0x4
+	.long	.LASF1
+	.long	.LASF2
+	.quad	.Ltext0
+	.quad	.Letext0-.Ltext0
+	.long	.Ldebug_line0
+	.uleb128 0x2
+	.long	0x51
+	.uleb128 0x3
+	.string	"U"
+	.byte	0x4
+	.byte	0x1
+	.byte	0x2
+	.byte	0x8
+	.uleb128 0x4
+	.string	"s"
+	.byte	0x1
+	.byte	0x3
+	.byte	0x14
+	.long	0x60
+	.byte	0
+	.uleb128 0x5
+	.string	"m"
+	.byte	0x1
+	.byte	0x4
+	.byte	0x6
+	.long	0x59
+	.byte	0
+	.byte	0
+	.byte	0
+	.uleb128 0x6
+	.byte	0x1
+	.byte	0x1
+	.byte	0x1
+	.long	0x2d
+	.uleb128 0x7
+	.byte	0x4
+	.byte	0x5
+	.string	"int"
+	.uleb128 0x8
+	.long	0x59
+	.uleb128 0x9
+	.string	"ug"
+	.byte	0x1
+	.byte	0x8
+	.byte	0x3
+	.long	0x32
+	.uleb128 0x9
+	.byte	0x3
+	.quad	ug
+	.uleb128 0xa
+	.long	.LASF3
+	.byte	0x1
+	.byte	0xa
+	.byte	0x5
+	.long	0x59
+	.quad	.LFB0
+	.quad	.LFE0-.LFB0
+	.uleb128 0x1
+	.byte	0x9c
+	.byte	0
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1
+	.uleb128 0x11
+	.byte	0x1
+	.uleb128 0x25
+	.uleb128 0xe
+	.uleb128 0x13
+	.uleb128 0xb
+	.uleb128 0x3
+	.uleb128 0xe
+	.uleb128 0x1b
+	.uleb128 0xe
+	.uleb128 0x11
+	.uleb128 0x1
+	.uleb128 0x12
+	.uleb128 0x7
+	.uleb128 0x10
+	.uleb128 0x17
+	.byte	0
+	.byte	0
+	.uleb128 0x2
+	.uleb128 0x39
+	.byte	0x1
+	.uleb128 0x1
+	.uleb128 0x13
+	.byte	0
+	.byte	0
+	.uleb128 0x3
+	.uleb128 0x13
+	.byte	0x1
+	.uleb128 0x3
+	.uleb128 0x8
+	.uleb128 0xb
+	.uleb128 0xb
+	.uleb128 0x3a
+	.uleb128 0xb
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x39
+	.uleb128 0xb
+	.byte	0
+	.byte	0
+	.uleb128 0x4
+	.uleb128 0xd
+	.byte	0
+	.uleb128 0x3
+	.uleb128 0x8
+	.uleb128 0x3a
+	.uleb128 0xb
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x39
+	.uleb128 0xb
+	.uleb128 0x49
+	.uleb128 0x13
+	.uleb128 0x3c
+	.uleb128 0x19
+	.uleb128 0x1c
+	.uleb128 0xb
+	.byte	0
+	.byte	0
+	.uleb128 0x5
+	.uleb128 0xd
+	.byte	0
+	.uleb128 0x3
+	.uleb128 0x8
+	.uleb128 0x3a
+	.uleb128 0xb
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x39
+	.uleb128 0xb
+	.uleb128 0x49
+	.uleb128 0x13
+	.uleb128 0x38
+	.uleb128 0xb
+	.byte	0
+	.byte	0
+	.uleb128 0x6
+	.uleb128 0x3a
+	.byte	0
+	.uleb128 0x3a
+	.uleb128 0xb
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x39
+	.uleb128 0xb
+	.uleb128 0x18
+	.uleb128 0x13
+	.byte	0
+	.byte	0
+	.uleb128 0x7
+	.uleb128 0x24
+	.byte	0
+	.uleb128 0xb
+	.uleb128 0xb
+	.uleb128 0x3e
+	.uleb128 0xb
+	.uleb128 0x3
+	.uleb128 0x8
+	.byte	0
+	.byte	0
+	.uleb128 0x8
+	.uleb128 0x26
+	.byte	0
+	.uleb128 0x49
+	.uleb128 0x13
+	.byte	0
+	.byte	0
+	.uleb128 0x9
+	.uleb128 0x34
+	.byte	0
+	.uleb128 0x3
+	.uleb128 0x8
+	.uleb128 0x3a
+	.uleb128 0xb
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x39
+	.uleb128 0xb
+	.uleb128 0x49
+	.uleb128 0x13
+	.uleb128 0x2
+	.uleb128 0x18
+	.byte	0
+	.byte	0
+	.uleb128 0xa
+	.uleb128 0x2e
+	.byte	0
+	.uleb128 0x3f
+	.uleb128 0x19
+	.uleb128 0x3
+	.uleb128 0xe
+	.uleb128 0x3a
+	.uleb128 0xb
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x39
+	.uleb128 0xb
+	.uleb128 0x49
+	.uleb128 0x13
+	.uleb128 0x11
+	.uleb128 0x1
+	.uleb128 0x12
+	.uleb128 0x7
+	.uleb128 0x40
+	.uleb128 0x18
+	.uleb128 0x2117
+	.uleb128 0x19

[Lldb-commits] [PATCH] D124785: [lldb/Core] Fix "sticky" long progress messages

2022-05-02 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

When the terminal window is too small, lldb would wrap progress messages
accross multiple lines which would break the progress event handling
code that is supposed to clear the message once the progress is completed.

This causes the progress message to stay, sometimes partially which can
be confusing for the user.

To fix this issue, this patch trims the progress message to the terminal
width taking into account the progress counter leading the message for
finite progress events and also the trailing `...`.

rdar://91993836

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124785

Files:
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1857,11 +1857,26 @@
 output->Printf(
 "%s", ansi::FormatAnsiTerminalCodes(ansi_prefix, use_color).c_str());
 
-  // Print the progress message.
+  // Trim the progress message if it exceeds the window's width and print it.
   std::string message = data->GetMessage();
-  if (data->GetTotal() != UINT64_MAX) {
+  uint64_t progress_total = data->GetTotal();
+  uint32_t term_width = GetTerminalWidth();
+
+  size_t prefix_width = 0;
+  if (data->IsFinite()) {
+prefix_width += 4; // '[%PRIu64/%PRIu64] %s'
+prefix_width += std::to_string(progress_total).size() * 2;
+  }
+
+  const size_t suffix_width = 3; // %s...
+
+  if (message.size() + prefix_width + suffix_width >= term_width)
+message.erase(message.begin() + term_width - prefix_width - suffix_width,
+  message.end());
+
+  if (data->IsFinite()) {
 output->Printf("[%" PRIu64 "/%" PRIu64 "] %s...", data->GetCompleted(),
-   data->GetTotal(), message.c_str());
+   progress_total, message.c_str());
   } else {
 output->Printf("%s...", message.c_str());
   }
Index: lldb/include/lldb/Core/DebuggerEvents.h
===
--- lldb/include/lldb/Core/DebuggerEvents.h
+++ lldb/include/lldb/Core/DebuggerEvents.h
@@ -32,6 +32,7 @@
 
   static const ProgressEventData *GetEventDataFromEvent(const Event 
*event_ptr);
   uint64_t GetID() const { return m_id; }
+  bool IsFinite() const { return m_total != UINT64_MAX; }
   uint64_t GetCompleted() const { return m_completed; }
   uint64_t GetTotal() const { return m_total; }
   const std::string () const { return m_message; }


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1857,11 +1857,26 @@
 output->Printf(
 "%s", ansi::FormatAnsiTerminalCodes(ansi_prefix, use_color).c_str());
 
-  // Print the progress message.
+  // Trim the progress message if it exceeds the window's width and print it.
   std::string message = data->GetMessage();
-  if (data->GetTotal() != UINT64_MAX) {
+  uint64_t progress_total = data->GetTotal();
+  uint32_t term_width = GetTerminalWidth();
+
+  size_t prefix_width = 0;
+  if (data->IsFinite()) {
+prefix_width += 4; // '[%PRIu64/%PRIu64] %s'
+prefix_width += std::to_string(progress_total).size() * 2;
+  }
+
+  const size_t suffix_width = 3; // %s...
+
+  if (message.size() + prefix_width + suffix_width >= term_width)
+message.erase(message.begin() + term_width - prefix_width - suffix_width,
+  message.end());
+
+  if (data->IsFinite()) {
 output->Printf("[%" PRIu64 "/%" PRIu64 "] %s...", data->GetCompleted(),
-   data->GetTotal(), message.c_str());
+   progress_total, message.c_str());
   } else {
 output->Printf("%s...", message.c_str());
   }
Index: lldb/include/lldb/Core/DebuggerEvents.h
===
--- lldb/include/lldb/Core/DebuggerEvents.h
+++ lldb/include/lldb/Core/DebuggerEvents.h
@@ -32,6 +32,7 @@
 
   static const ProgressEventData *GetEventDataFromEvent(const Event *event_ptr);
   uint64_t GetID() const { return m_id; }
+  bool IsFinite() const { return m_total != UINT64_MAX; }
   uint64_t GetCompleted() const { return m_completed; }
   uint64_t GetTotal() const { return m_total; }
   const std::string () const { return m_message; }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D124648: [trace][intelpt] Support system-wide tracing [3] - Refactor IntelPTThreadTrace

2022-05-02 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.



Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.h:136
-
 /// Manages a list of thread traces.
 class IntelPTThreadTraceCollection {

wallace wrote:
> jj10306 wrote:
> > update doc since this is no longer tied to thread's traces
> good catch!
actually the comment is right because this collection is in fact for threads


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124648

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

[Lldb-commits] [PATCH] D124760: [lldb] Fix ppc64 detection in lldb

2022-05-02 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:315
+  uint32_t endian = header.e_ident[EI_DATA];
+  if(endian == ELFDATA2LSB)
+return ArchSpec::eCore_ppc64le_generic;





Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:326
 return riscvVariantFromElfFlags(header);
+  else if (header.e_machine == llvm::ELF::EM_PPC64)
+return ppc64VariantFromElfFlags(header);

Move before EM_RISCV for an alphabetical order.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124760

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

[Lldb-commits] [PATCH] D124648: [trace][intelpt] Support system-wide tracing [3] - Refactor IntelPTThreadTrace

2022-05-02 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.



Comment at: lldb/docs/lldb-gdb-remote.txt:498
 //  Binary data kinds:
-//- threadTraceBuffer: trace buffer for a thread.
+//- traceBuffer: trace buffer for a thread.
 //- procFsCpuInfo: contents of the /proc/cpuinfo file.

jj10306 wrote:
> If perCore tracing is enabled, how will this packet work since currently it 
> requires a tid, but in perCore mode the trace data will contain all activity 
> on that core, not just the specified thread?
the jLLDBGetBinaryData packet has, for example, an optional tid argument that 
indicates the data correspond to a specific tid. I'll add a new optional 
parameter called core_id, so that we can associate a data chunk with a core_id



Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.h:136
-
 /// Manages a list of thread traces.
 class IntelPTThreadTraceCollection {

jj10306 wrote:
> update doc since this is no longer tied to thread's traces
good catch!



Comment at: 
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:292-298
+  if (Expected perf_event = PerfEvent::Init(*attr, tid)) {
+if (Error mmap_err = perf_event->MmapMetadataAndBuffers(buffer_numpages,
+buffer_numpages)) {
+  return std::move(mmap_err);
+}
+return IntelPTSingleBufferTraceUP(
+new IntelPTSingleBufferTrace(std::move(*perf_event), tid));

jj10306 wrote:
> The PerfEvent logic will need to be updated to support per CPU or per thread 
> as it currently only supports per thread
yes, definitely



Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h:1
+//===-- IntelPTSingleBufferTrace.h  -*- C++ 
-*-===//
+//

jj10306 wrote:
> nit: thoughts on the name  `IntelPTTraceBuffer` instead of 
> `IntelPTSingleBufferTrace`? The current name is a bit wordy imo
this is intentional because the next file will be IntelPTMultiCoreTrace.h. 

I also don't want to call this IntelPTTraceBuffer because it'll eventually have 
itrace data so the object won't be just a trace buffer.

But I'm open to any suggestions because I also don't like super verbose names



Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h:41-48
+  /// \param[in] tid
+  /// The tid of the thread to be traced.
+  ///
+  /// \return
+  ///   A \a IntelPTSingleBufferTrace instance if tracing was successful, or
+  ///   an \a llvm::Error otherwise.
+  static llvm::Expected

jj10306 wrote:
> Shouldn't this structure be general and have no notion of a tid since it 
> could represent the buffer of a single thread or the buffer of a single CPU?
> The way I see it, this structure simply wraps the buffer of a specific 
> perf_event but has no notion of if it's for a specific tid or cpu.
> Then you could have two subclasses, one for thread one for cpu, that inherit 
> from this and have the additional context about the buffer. The inheritance 
> may be overkill, but point I'm trying to make is that this structure should 
> be agnostic to what "unit's" (thread or cpu) trace data it contains
what I was thinking is to change this in a later diff to be like 
  Start(const TraceIntelPTStartRequest , Optional tid, 
Optional core_id);

which seems to me generic enough for all possible use cases. LLDB couldn't 
support cgroups, so we are limited to tids and core_ids. With this, it seems to 
me that having two child classes it's a little bit too much because the only 
difference are just two lines of code where we specify the perf_event 
configuration for tid and core_id.

But if you insist in the clarify of the subclasses, I for sure can do it that 
way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124648

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

[Lldb-commits] [PATCH] D124640: [trace][intelpt] Support system-wide tracing [2] - Add a dummy --per-core-tracing option

2022-05-02 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.



Comment at: 
lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp:66
 OptionParsingStarting(ExecutionContext *execution_context) {
-  m_thread_buffer_size = kDefaultThreadBufferSize;
+  m_trace_buffer_size = kDefaultTraceBufferSize;
   m_enable_tsc = kDefaultEnableTscValue;

jj10306 wrote:
> So now m_trace_buffer_size is the size of each trace buffer, regardless of 
> the buffer is for a single thread or a single core?
yep


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124640

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

[Lldb-commits] [PATCH] D124760: [lldb] Fix ppc64 detection in lldb

2022-05-02 Thread Pavel Labath via Phabricator via lldb-commits

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: JDevlieghere.

There were probably easier ways to test this, but it's nice to have the core 
file nonetheless. LGTM, assuming the core is of a reasonable size.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124760

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

[Lldb-commits] [PATCH] D124573: [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-05-02 Thread walter erquinigo via Phabricator via lldb-commits

wallace added a comment.

thanks for your review :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124573

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

[Lldb-commits] [PATCH] D124640: [trace][intelpt] Support system-wide tracing [2] - Add a dummy --per-core-tracing option

2022-05-02 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.



Comment at: lldb/docs/lldb-gdb-remote.txt:369
+//
+// /* process tracing only */
 // "processBufferSizeLimit": ,

jj10306 wrote:
> Why do we only want this option for process tracing?
> Per cpu tracing collects all trace data agnostic to a user specified 
> process/thread, so why should this only be exposed for process wide? I think 
> it makes more sense to decouple the `perCoreTracing` option from 
> process/threads specific options entirely so it is its own option all 
> together and cannot be used in conjunction with process/thread options.
> If there is reason to not go down that route, we then should also add support 
> for `perCoreTracing` with the  thread tracing option, not just the process 
> tracing option as I feel it doesn't make since to only expose this for 
> process tracing since it's doing the same thing behind the scenes.
The reason is that if you want to trace a specific thread, it's actually much 
better to use single-buffer thread tracing than per cpu. You get full fidelity 
and no data loss in this mode. On the other hand, per cpu tracing is really 
useful in a thread-unbounded case where you are okay with having no data for 
unfrequent threads.
Per-core single-tracing is doable, but it's very impractical in the context of 
LLDB. However, if we ever need to implement it eventually, we for sure can 
refactor the code for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124640

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

[Lldb-commits] [PATCH] D124573: [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-05-02 Thread Jakob Johnson via Phabricator via lldb-commits

jj10306 added a comment.

Much cleaner now, thanks for separating out the procfs logic 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124573

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

[Lldb-commits] [PATCH] D124573: [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-05-02 Thread Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5de0a3e9da72: [trace][intelpt] Support system-wide tracing 
[1] - Add a method for accessing… (authored by Walter Erquinigo 
wall...@fb.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124573

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Procfs.cpp
  lldb/source/Plugins/Process/Linux/Procfs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  lldb/unittests/Process/Linux/CMakeLists.txt
  lldb/unittests/Process/Linux/PerfTests.cpp
  lldb/unittests/Process/Linux/ProcfsTests.cpp

Index: lldb/unittests/Process/Linux/ProcfsTests.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Linux/ProcfsTests.cpp
@@ -0,0 +1,104 @@
+//===-- ProcfsTests.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 "Procfs.h"
+
+#include "lldb/Host/linux/Support.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace process_linux;
+using namespace llvm;
+
+TEST(Perf, HardcodedLogicalCoreIDs) {
+  Expected> core_ids =
+  GetAvailableLogicalCoreIDs(R"(processor   : 13
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 2886.370
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 19
+cpu cores   : 20
+apicid  : 103
+initial apicid  : 103
+fpu : yes
+fpu_exception   : yes
+cpuid level : 22
+power management:
+
+processor   : 24
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 2768.494
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 20
+cpu cores   : 20
+apicid  : 105
+power management:
+
+processor   : 35
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 2884.703
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 24
+cpu cores   : 20
+apicid  : 113
+
+processor   : 79
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 3073.955
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 28
+cpu cores   : 20
+apicid  : 121
+power management:
+)");
+
+  ASSERT_TRUE((bool)core_ids);
+  ASSERT_THAT(*core_ids, ::testing::ElementsAre(13, 24, 35, 79));
+}
+
+TEST(Perf, RealLogicalCoreIDs) {
+  // We first check we can read /proc/cpuinfo
+  auto buffer_or_error = errorOrToExpected(getProcFile("cpuinfo"));
+  if (!buffer_or_error)
+GTEST_SKIP() << toString(buffer_or_error.takeError());
+
+  // At this point we shouldn't fail parsing the core ids
+  Expected> core_ids = GetAvailableLogicalCoreIDs();
+  ASSERT_TRUE((bool)core_ids);
+  ASSERT_GT((int)core_ids->size(), 0) << "We must see at least one core";
+}
Index: lldb/unittests/Process/Linux/PerfTests.cpp
===
--- lldb/unittests/Process/Linux/PerfTests.cpp
+++ lldb/unittests/Process/Linux/PerfTests.cpp
@@ -13,6 +13,7 @@
 #include "llvm/Support/Error.h"
 
 #include "gtest/gtest.h"
+
 #include 
 #include 
 
Index: lldb/unittests/Process/Linux/CMakeLists.txt
===
--- lldb/unittests/Process/Linux/CMakeLists.txt
+++ lldb/unittests/Process/Linux/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(ProcessLinuxTests
   IntelPTCollectorTests.cpp
   PerfTests.cpp
+  ProcfsTests.cpp
 
   LINK_LIBS
 lldbPluginProcessLinux
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp

[Lldb-commits] [lldb] 5de0a3e - [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-05-02 Thread Walter Erquinigo via lldb-commits


Author: Walter Erquinigo
Date: 2022-05-02T08:48:49-07:00
New Revision: 5de0a3e9da721189dd648a90e87829d375c5d17f

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

LOG: [trace][intelpt] Support system-wide tracing [1] - Add a method for 
accessing the list of logical core ids

In order to open perf events per core, we need to first get the list of
core ids available in the system. So I'm adding a function that does
that by parsing /proc/cpuinfo. That seems to be the simplest and most
portable way to do that.

Besides that, I made a few refactors and renames to reflect better that
the cpu info that we use in lldb-server comes from procfs.

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

Added: 
lldb/source/Plugins/Process/Linux/Procfs.cpp
lldb/unittests/Process/Linux/ProcfsTests.cpp

Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Process/Linux/CMakeLists.txt
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
lldb/source/Plugins/Process/Linux/Perf.cpp
lldb/source/Plugins/Process/Linux/Procfs.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
lldb/unittests/Process/Linux/CMakeLists.txt
lldb/unittests/Process/Linux/PerfTests.cpp

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 2ddc3183b80e7..545fbb46ae242 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -467,7 +467,7 @@ read packet: OK/E;A
 //
 //  Binary data kinds:
 //- threadTraceBuffer: trace buffer for a thread.
-//- cpuInfo: contents of the /proc/cpuinfo file.
+//- procfsCpuInfo: contents of the /proc/cpuinfo file.
 //
 //  Counter info kinds:
 //tsc-perf-zero-conversion:
@@ -522,7 +522,7 @@ read packet: {...object}/E;A
 //
 //  Binary data kinds:
 //- threadTraceBuffer: trace buffer for a thread.
-//- cpuInfo: contents of the /proc/cpuinfo file.
+//- procfsCpuInfo: contents of the /proc/cpuinfo file.
 //--
 
 send packet: 
jLLDBTraceGetBinaryData:{"type":,"kind":,"tid":,"offset":,"size":}]

diff  --git a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h 
b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
index 8960949f2039f..294b0205af452 100644
--- a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
@@ -18,6 +18,12 @@
 /// See docs/lldb-gdb-remote.txt for more information.
 namespace lldb_private {
 
+// List of data kinds used by jLLDBGetState and jLLDBGetBinaryData.
+struct IntelPTDataKinds {
+  static const char *kProcFsCpuInfo;
+  static const char *kThreadTraceBuffer;
+};
+
 /// jLLDBTraceStart gdb-remote packet
 /// \{
 struct TraceIntelPTStartRequest : TraceStartRequest {

diff  --git a/lldb/source/Plugins/Process/Linux/CMakeLists.txt 
b/lldb/source/Plugins/Process/Linux/CMakeLists.txt
index cc70edba3483e..125cc0e38ca21 100644
--- a/lldb/source/Plugins/Process/Linux/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/Linux/CMakeLists.txt
@@ -9,6 +9,7 @@ add_lldb_library(lldbPluginProcessLinux
   NativeRegisterContextLinux_x86_64.cpp
   NativeThreadLinux.cpp
   Perf.cpp
+  Procfs.cpp
   SingleStepCheck.cpp
 
   LINK_LIBS

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
index 8c10828627971..e2303b37eb883 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -9,6 +9,7 @@
 #include "IntelPTCollector.h"
 
 #include "Perf.h"
+#include "Procfs.h"
 
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #include "lldb/Host/linux/Support.h"
@@ -57,21 +58,6 @@ enum IntelPTConfigFileType {
   BitOffset
 };
 
-/// Get the content of /proc/cpuinfo that can be later used to decode traces.
-static Expected> GetCPUInfo() {
-  static llvm::Optional> cpu_info;
-  if (!cpu_info) {
-auto buffer_or_error = errorOrToExpected(getProcFile("cpuinfo"));
-if (!buffer_or_error)
-  return buffer_or_error.takeError();
-MemoryBuffer  = **buffer_or_error;
-cpu_info = std::vector(
-reinterpret_cast(buffer.getBufferStart()),
-reinterpret_cast(buffer.getBufferEnd()));
-  }
-  return *cpu_info;
-}
-
 static Expected ReadIntelPTConfigFile(const char *file,
 IntelPTConfigFileType type) {
   ErrorOr> stream =
@@ -430,7 +416,7 @@ void 
IntelPTThreadTrace::ReadCyclicBuffer(llvm::MutableArrayRef ,

[Lldb-commits] [PATCH] D124573: [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-05-02 Thread walter erquinigo via Phabricator via lldb-commits

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

final nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124573

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Procfs.cpp
  lldb/source/Plugins/Process/Linux/Procfs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  lldb/unittests/Process/Linux/CMakeLists.txt
  lldb/unittests/Process/Linux/PerfTests.cpp
  lldb/unittests/Process/Linux/ProcfsTests.cpp

Index: lldb/unittests/Process/Linux/ProcfsTests.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Linux/ProcfsTests.cpp
@@ -0,0 +1,104 @@
+//===-- ProcfsTests.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 "Procfs.h"
+
+#include "lldb/Host/linux/Support.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace process_linux;
+using namespace llvm;
+
+TEST(Perf, HardcodedLogicalCoreIDs) {
+  Expected> core_ids =
+  GetAvailableLogicalCoreIDs(R"(processor   : 13
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 2886.370
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 19
+cpu cores   : 20
+apicid  : 103
+initial apicid  : 103
+fpu : yes
+fpu_exception   : yes
+cpuid level : 22
+power management:
+
+processor   : 24
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 2768.494
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 20
+cpu cores   : 20
+apicid  : 105
+power management:
+
+processor   : 35
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 2884.703
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 24
+cpu cores   : 20
+apicid  : 113
+
+processor   : 79
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 3073.955
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 28
+cpu cores   : 20
+apicid  : 121
+power management:
+)");
+
+  ASSERT_TRUE((bool)core_ids);
+  ASSERT_THAT(*core_ids, ::testing::ElementsAre(13, 24, 35, 79));
+}
+
+TEST(Perf, RealLogicalCoreIDs) {
+  // We first check we can read /proc/cpuinfo
+  auto buffer_or_error = errorOrToExpected(getProcFile("cpuinfo"));
+  if (!buffer_or_error)
+GTEST_SKIP() << toString(buffer_or_error.takeError());
+
+  // At this point we shouldn't fail parsing the core ids
+  Expected> core_ids = GetAvailableLogicalCoreIDs();
+  ASSERT_TRUE((bool)core_ids);
+  ASSERT_GT((int)core_ids->size(), 0) << "We must see at least one core";
+}
Index: lldb/unittests/Process/Linux/PerfTests.cpp
===
--- lldb/unittests/Process/Linux/PerfTests.cpp
+++ lldb/unittests/Process/Linux/PerfTests.cpp
@@ -13,6 +13,7 @@
 #include "llvm/Support/Error.h"
 
 #include "gtest/gtest.h"
+
 #include 
 #include 
 
Index: lldb/unittests/Process/Linux/CMakeLists.txt
===
--- lldb/unittests/Process/Linux/CMakeLists.txt
+++ lldb/unittests/Process/Linux/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(ProcessLinuxTests
   IntelPTCollectorTests.cpp
   PerfTests.cpp
+  ProcfsTests.cpp
 
   LINK_LIBS
 lldbPluginProcessLinux
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -13,6 +13,9 @@
 
 namespace lldb_private {
 
+const char

[Lldb-commits] [PATCH] D124648: [trace][intelpt] Support system-wide tracing [3] - Refactor IntelPTThreadTrace

2022-05-02 Thread Jakob Johnson via Phabricator via lldb-commits

jj10306 requested changes to this revision.
jj10306 added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/docs/lldb-gdb-remote.txt:498
 //  Binary data kinds:
-//- threadTraceBuffer: trace buffer for a thread.
+//- traceBuffer: trace buffer for a thread.
 //- procFsCpuInfo: contents of the /proc/cpuinfo file.

If perCore tracing is enabled, how will this packet work since currently it 
requires a tid, but in perCore mode the trace data will contain all activity on 
that core, not just the specified thread?



Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.h:136
-
 /// Manages a list of thread traces.
 class IntelPTThreadTraceCollection {

update doc since this is no longer tied to thread's traces



Comment at: 
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:292-298
+  if (Expected perf_event = PerfEvent::Init(*attr, tid)) {
+if (Error mmap_err = perf_event->MmapMetadataAndBuffers(buffer_numpages,
+buffer_numpages)) {
+  return std::move(mmap_err);
+}
+return IntelPTSingleBufferTraceUP(
+new IntelPTSingleBufferTrace(std::move(*perf_event), tid));

The PerfEvent logic will need to be updated to support per CPU or per thread as 
it currently only supports per thread



Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h:1
+//===-- IntelPTSingleBufferTrace.h  -*- C++ 
-*-===//
+//

nit: thoughts on the name  `IntelPTTraceBuffer` instead of 
`IntelPTSingleBufferTrace`? The current name is a bit wordy imo



Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h:41-48
+  /// \param[in] tid
+  /// The tid of the thread to be traced.
+  ///
+  /// \return
+  ///   A \a IntelPTSingleBufferTrace instance if tracing was successful, or
+  ///   an \a llvm::Error otherwise.
+  static llvm::Expected

Shouldn't this structure be general and have no notion of a tid since it could 
represent the buffer of a single thread or the buffer of a single CPU?
The way I see it, this structure simply wraps the buffer of a specific 
perf_event but has no notion of if it's for a specific tid or cpu.
Then you could have two subclasses, one for thread one for cpu, that inherit 
from this and have the additional context about the buffer. The inheritance may 
be overkill, but point I'm trying to make is that this structure should be 
agnostic to what "unit's" (thread or cpu) trace data it contains


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124648

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

[Lldb-commits] [PATCH] D123415: [lldb] Fix crash when a type has no definition

2022-05-02 Thread Sigurður Ásgeirsson via Phabricator via lldb-commits

siggi-alpheus abandoned this revision.
siggi-alpheus added a comment.

This is not a good fix for the crash. A better fix is in 
https://reviews.llvm.org/D124370.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123415

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

[Lldb-commits] [PATCH] D124573: [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-05-02 Thread walter erquinigo via Phabricator via lldb-commits

wallace updated this revision to Diff 426422.
wallace added a comment.
Herald added a subscriber: mgorny.

move procfs functions to Procfs.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124573

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Procfs.cpp
  lldb/source/Plugins/Process/Linux/Procfs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  lldb/unittests/Process/Linux/CMakeLists.txt
  lldb/unittests/Process/Linux/PerfTests.cpp
  lldb/unittests/Process/Linux/ProcfsTests.cpp

Index: lldb/unittests/Process/Linux/ProcfsTests.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Linux/ProcfsTests.cpp
@@ -0,0 +1,104 @@
+//===-- ProcfsTests.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 "Procfs.h"
+
+#include "lldb/Host/linux/Support.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace process_linux;
+using namespace llvm;
+
+TEST(Perf, HardcodedLogicalCoreIDs) {
+  Expected> core_ids =
+  GetAvailableLogicalCoreIDs(R"(processor   : 13
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 2886.370
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 19
+cpu cores   : 20
+apicid  : 103
+initial apicid  : 103
+fpu : yes
+fpu_exception   : yes
+cpuid level : 22
+power management:
+
+processor   : 24
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 2768.494
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 20
+cpu cores   : 20
+apicid  : 105
+power management:
+
+processor   : 35
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 2884.703
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 24
+cpu cores   : 20
+apicid  : 113
+
+processor   : 79
+vendor_id   : GenuineIntel
+cpu family  : 6
+model   : 85
+model name  : Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz
+stepping: 4
+microcode   : 0x265
+cpu MHz : 3073.955
+cache size  : 28160 KB
+physical id : 1
+siblings: 40
+core id : 28
+cpu cores   : 20
+apicid  : 121
+power management:
+)");
+
+  ASSERT_TRUE((bool)core_ids);
+  ASSERT_THAT(*core_ids, ::testing::ElementsAre(13, 24, 35, 79));
+}
+
+TEST(Perf, RealLogicalCoreIDs) {
+  // We first check we can read /proc/cpuinfo
+  auto buffer_or_error = errorOrToExpected(getProcFile("cpuinfo"));
+  if (!buffer_or_error)
+GTEST_SKIP() << toString(buffer_or_error.takeError());
+
+  // At this point we shouldn't fail parsing the core ids
+  Expected> core_ids = GetAvailableLogicalCoreIDs();
+  ASSERT_TRUE((bool)core_ids);
+  ASSERT_GT((int)core_ids->size(), 0) << "We must see at least one core";
+}
Index: lldb/unittests/Process/Linux/PerfTests.cpp
===
--- lldb/unittests/Process/Linux/PerfTests.cpp
+++ lldb/unittests/Process/Linux/PerfTests.cpp
@@ -10,9 +10,13 @@
 
 #include "Perf.h"
 
+#include "lldb/Host/linux/Support.h"
+
 #include "llvm/Support/Error.h"
 
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
+
 #include 
 #include 
 
Index: lldb/unittests/Process/Linux/CMakeLists.txt
===
--- lldb/unittests/Process/Linux/CMakeLists.txt
+++ lldb/unittests/Process/Linux/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(ProcessLinuxTests
   IntelPTCollectorTests.cpp
   PerfTests.cpp
+  ProcfsTests.cpp
 
   LINK_LIBS
 lldbPluginProcessLinux
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
---

[Lldb-commits] [PATCH] D124640: [trace][intelpt] Support system-wide tracing [2] - Add a dummy --per-core-tracing option

2022-05-02 Thread Jakob Johnson via Phabricator via lldb-commits

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

Before doing a complete review can you provide clarity on the decision to only 
support perCore for process wide (see my inline comment with my 
thoughts/questions)?  Understanding this will potentially provide the answers 
to many other questions I was going to leave, so figured I'd ask it ahead of 
time (:




Comment at: lldb/docs/lldb-gdb-remote.txt:369
+//
+// /* process tracing only */
 // "processBufferSizeLimit": ,

Why do we only want this option for process tracing?
Per cpu tracing collects all trace data agnostic to a user specified 
process/thread, so why should this only be exposed for process wide? I think it 
makes more sense to decouple the `perCoreTracing` option from process/threads 
specific options entirely so it is its own option all together and cannot be 
used in conjunction with process/thread options.
If there is reason to not go down that route, we then should also add support 
for `perCoreTracing` with the  thread tracing option, not just the process 
tracing option as I feel it doesn't make since to only expose this for process 
tracing since it's doing the same thing behind the scenes.



Comment at: 
lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp:66
 OptionParsingStarting(ExecutionContext *execution_context) {
-  m_thread_buffer_size = kDefaultThreadBufferSize;
+  m_trace_buffer_size = kDefaultTraceBufferSize;
   m_enable_tsc = kDefaultEnableTscValue;

So now m_trace_buffer_size is the size of each trace buffer, regardless of the 
buffer is for a single thread or a single core?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124640

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

[Lldb-commits] [PATCH] D124673: [llvm][lldb] use FindLibEdit.cmake everywhere

2022-05-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D124673#3484157 , @MaskRay wrote:

> I vaguely recall that some lldb bots use a stand-alone build.

Yup, that's correct. The bot in question is 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-standalone/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124673

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

[Lldb-commits] [PATCH] D124573: [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-05-02 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.



Comment at: lldb/unittests/Process/Linux/PerfTests.cpp:53
 
+TEST(Perf, HardcodedLogicalCoreIDs) {
+  Expected> core_ids =

jj10306 wrote:
> Not directly related to this diff, but I just realized the proc fs parsing 
> logic doesn't belong in the Perf files since it has nothing to do with perf. 
> Feel free to move those functions and these associated tests to a more 
> appropriate location or we can do it in a future diff since this isn't the 
> diff that made that mistake.
that's a valid point. I'll create a new file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124573

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

[Lldb-commits] [PATCH] D124573: [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-05-02 Thread Jakob Johnson via Phabricator via lldb-commits

jj10306 accepted this revision.
jj10306 added inline comments.



Comment at: lldb/unittests/Process/Linux/PerfTests.cpp:53
 
+TEST(Perf, HardcodedLogicalCoreIDs) {
+  Expected> core_ids =

Not directly related to this diff, but I just realized the proc fs parsing 
logic doesn't belong in the Perf files since it has nothing to do with perf. 
Feel free to move those functions and these associated tests to a more 
appropriate location or we can do it in a future diff since this isn't the diff 
that made that mistake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124573

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

[Lldb-commits] [PATCH] D124760: [lldb] Fix ppc64 detection in lldb

2022-05-02 Thread serge via Phabricator via lldb-commits

serge-sans-paille created this revision.
serge-sans-paille added reviewers: MaskRay, jasonmolenda, labath.
Herald added subscribers: StephenFan, shchenz, kbarton, nemanjai, emaste.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Currently, ppc64le and ppc64 (defaulting to big endian) have the same
descriptor, thus the linear scan always return ppc64le. Handle that through
subtype.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124760

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-ppc64.core
  lldb/test/API/functionalities/postmortem/elf-core/linux-ppc64.out


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -24,12 +24,14 @@
 _i386_pid = 32306
 _x86_64_pid = 32259
 _s390x_pid = 1045
+_ppc64_pid = 28146
 _ppc64le_pid = 28147
 
 _aarch64_regions = 4
 _i386_regions = 4
 _x86_64_regions = 5
 _s390x_regions = 2
+_ppc64_regions = 2
 _ppc64le_regions = 2
 
 @skipIfLLVMTargetMissing("AArch64")
@@ -49,6 +51,12 @@
 self.do_test("linux-ppc64le", self._ppc64le_pid, self._ppc64le_regions,
  "linux-ppc64le.ou")
 
+@skipIfLLVMTargetMissing("PowerPC")
+def test_ppc64(self):
+"""Test that lldb can read the process information from an ppc64 linux 
core file."""
+self.do_test("linux-ppc64", self._ppc64_pid, self._ppc64_regions,
+ "linux-ppc64.ou")
+
 @skipIfLLVMTargetMissing("X86")
 def test_x86_64(self):
 """Test that lldb can read the process information from an x86_64 
linux core file."""
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -358,9 +358,9 @@
  0xu, 0xu}, // Intel MCU // FIXME: is this correct?
 {ArchSpec::eCore_ppc_generic, llvm::ELF::EM_PPC, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // PowerPC
-{ArchSpec::eCore_ppc64le_generic, llvm::ELF::EM_PPC64, 
LLDB_INVALID_CPUTYPE,
+{ArchSpec::eCore_ppc64le_generic, llvm::ELF::EM_PPC64, 
ArchSpec::eCore_ppc64le_generic,
  0xu, 0xu}, // PowerPC64le
-{ArchSpec::eCore_ppc64_generic, llvm::ELF::EM_PPC64, LLDB_INVALID_CPUTYPE,
+{ArchSpec::eCore_ppc64_generic, llvm::ELF::EM_PPC64, 
ArchSpec::eCore_ppc64_generic,
  0xu, 0xu}, // PowerPC64
 {ArchSpec::eCore_arm_generic, llvm::ELF::EM_ARM, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARM
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -310,11 +310,21 @@
   }
 }
 
+static uint32_t ppc64VariantFromElfFlags(const elf::ELFHeader ) {
+  uint32_t endian = header.e_ident[EI_DATA];
+  if(endian == ELFDATA2LSB)
+return ArchSpec::eCore_ppc64le_generic;
+  else
+return ArchSpec::eCore_ppc64_generic;
+}
+
 static uint32_t subTypeFromElfHeader(const elf::ELFHeader ) {
   if (header.e_machine == llvm::ELF::EM_MIPS)
 return mipsVariantFromElfFlags(header);
   else if (header.e_machine == llvm::ELF::EM_RISCV)
 return riscvVariantFromElfFlags(header);
+  else if (header.e_machine == llvm::ELF::EM_PPC64)
+return ppc64VariantFromElfFlags(header);
 
   return LLDB_INVALID_CPUTYPE;
 }


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -24,12 +24,14 @@
 _i386_pid = 32306
 _x86_64_pid = 32259
 _s390x_pid = 1045
+_ppc64_pid = 28146
 _ppc64le_pid = 28147
 
 _aarch64_regions = 4
 _i386_regions = 4
 _x86_64_regions = 5
 _s390x_regions = 2
+_ppc64_regions = 2
 _ppc64le_regions = 2
 
 @skipIfLLVMTargetMissing("AArch64")
@@ -49,6 +51,12 @@
 self.do_test("linux-ppc64le", self._ppc64le_pid, self._ppc64le_regions,
  "linux-ppc64le.ou")
 
+@skipIfLLVMTargetMissing("PowerPC")
+def test_ppc64(self):
+"""Test that lldb can read the process information from an ppc64 linux core file."""
+self.do_test("linux-ppc64", self._ppc64_pid, self._ppc64_regions,
+ "linux-ppc64.ou")
+

44 matches

Mail list logo