[Lldb-commits] [PATCH] D122373: [NFC] Be more lazy about looking up simulator SDKs on macs

[Jason Molenda via Phabricator via lldb-commits]

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

This is a perf fix for when lldb on a mac is iterating through all of the 
Platforms looking for a good one, and the iOS/watchOS/tvOS simulators all call 
xcrun to find an SDK.  If those SDKs are not installed, the xcruns can be quite 
expensive.  The code today determines the SDK path before it knows if the 
Platform will create itself or not.   When the SDKs are present, these xcrun 
calls are very inexpensive, but some people have different setups.

This patch passes the names of the SDKs into the central method that decides 
whether to create the Platform, and only searches for the SDK path via xcrun if 
this Platform is a match.

Adrian, I think you might have written this?  If you'd like to review.  Else I 
can ask around if anyone wants to take a look.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122373

Files:
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h

Index: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
@@ -59,7 +59,8 @@
  llvm::Triple::OSType preferred_os,
  llvm::SmallVector supported_os,
  llvm::SmallVector supported_triples,
- llvm::StringRef sdk, XcodeSDK::Type sdk_type,
+ std::string sdk_name_preferred, std::string sdk_name_secondary,
+ XcodeSDK::Type sdk_type,
  CoreSimulatorSupport::DeviceType::ProductFamilyID kind,
  bool force, const ArchSpec *arch);
 
Index: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -274,13 +274,23 @@
   return result;
 }
 
+static llvm::StringRef GetXcodeSDKDir(std::string preferred,
+  std::string secondary) {
+  llvm::StringRef sdk;
+  sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(preferred)));
+  if (sdk.empty())
+sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(secondary)));
+  return sdk;
+}
+
 PlatformSP PlatformAppleSimulator::CreateInstance(
 const char *class_name, const char *description, ConstString plugin_name,
 llvm::SmallVector supported_arch,
 llvm::Triple::OSType preferred_os,
 llvm::SmallVector supported_os,
 llvm::SmallVector supported_triples,
-llvm::StringRef sdk, lldb_private::XcodeSDK::Type sdk_type,
+std::string sdk_name_preferred, std::string sdk_name_secondary,
+lldb_private::XcodeSDK::Type sdk_type,
 CoreSimulatorSupport::DeviceType::ProductFamilyID kind, bool force,
 const ArchSpec *arch) {
   Log *log = GetLog(LLDBLog::Platform);
@@ -338,6 +348,8 @@
   if (create) {
 LLDB_LOGF(log, "%s::%s() creating platform", class_name, __FUNCTION__);
 
+llvm::StringRef sdk =
+GetXcodeSDKDir(sdk_name_preferred, sdk_name_secondary);
 return PlatformSP(new PlatformAppleSimulator(
 class_name, description, plugin_name, preferred_os, supported_triples,
 sdk, sdk_type, kind));
@@ -514,15 +526,6 @@
  !arch->TripleEnvironmentWasSpecified();
 }
 
-static llvm::StringRef GetXcodeSDKDir(std::string preferred,
-  std::string secondary) {
-  llvm::StringRef sdk;
-  sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(preferred)));
-  if (sdk.empty())
-sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(secondary)));
-  return sdk;
-}
-
 static const char *g_ios_plugin_name = "ios-simulator";
 static const char *g_ios_description = "iPhone simulator platform plug-in.";
 
@@ -540,10 +543,6 @@
   static PlatformSP CreateInstance(bool force, const ArchSpec *arch) {
 if (shouldSkipSimulatorPlatform(force, arch))
   return nullptr;
-llvm::StringRef sdk;
-sdk = HostInfo::GetXcodeSDKPath(XcodeSDK("iPhoneSimulator.Internal.sdk"));
-if (sdk.empty())
-  sdk = HostInfo::GetXcodeSDKPath(XcodeSDK("iPhoneSimulator.sdk"));
 
 return PlatformAppleSimulator::CreateInstance(
 "PlatformiOSSimulator", g_ios_description,
@@ -566,7 +565,7 @@
 #endif
 #endif
 },
-GetXcodeSDKDir("iPhoneSimulator.Internal.sdk", "iPhoneSimulator.sdk"),
+"iPhoneSimulator.Internal.sdk", "iPhoneSimulator.sdk",
 XcodeSDK::Type::iPhoneSimulator,
 CoreSimulatorSupport::DeviceType::ProductFamilyID::

[Lldb-commits] [PATCH] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I did something along these lines for lldb-server memory tagging tests in 
`lldb/test/API/tools/lldb-server/memory-tagging/main.c`. In that case you've 
got 3 points where you we could skip the test, and you could check the auxv but 
that was overkill for this example.

Does FreeBSD have an equivalent to the auxv feature bits? (we could always 
sprinkle some #defines around if not).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120485

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

[Lldb-commits] [lldb] 00fb050 - [lldb] Remove unused Module argument (NFC)

[Nikita Popov via lldb-commits]

Author: Nikita Popov
Date: 2022-03-24T12:53:02+01:00
New Revision: 00fb0504082ea0b4b0c25d1ae773b39874d88e95

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

LOG: [lldb] Remove unused Module argument (NFC)

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
index f5c3c139a3ee2..f7f1982f4059a 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
@@ -24,10 +24,9 @@
 
 using namespace lldb_private;
 
-static bool isRSAPICall(llvm::Module &module, llvm::CallInst *call_inst) {
+static bool isRSAPICall(llvm::CallInst *call_inst) {
   // TODO get the list of renderscript modules from lldb and check if
   // this llvm::Module calls into any of them.
-  (void)module;
   const auto func_name = call_inst->getCalledFunction()->getName();
   if (func_name.startswith("llvm") || func_name.startswith("lldb"))
 return false;
@@ -38,8 +37,7 @@ static bool isRSAPICall(llvm::Module &module, llvm::CallInst 
*call_inst) {
   return true;
 }
 
-static bool isRSLargeReturnCall(llvm::Module &module,
-llvm::CallInst *call_inst) {
+static bool isRSLargeReturnCall(llvm::CallInst *call_inst) {
   // i686 and x86_64 returns for large vectors in the RenderScript API are not
   // handled as normal register pairs, but as a hidden sret type. This is not
   // reflected in the debug info or mangled symbol name, and the android ABI
@@ -50,7 +48,6 @@ static bool isRSLargeReturnCall(llvm::Module &module,
   // It is perhaps an unreliable heuristic, and relies on bcc not generating
   // AVX code, so if the android ABI one day provides for AVX, this function
   // may go out of fashion.
-  (void)module;
   if (!call_inst || !call_inst->getCalledFunction())
 return false;
 
@@ -68,9 +65,7 @@ static bool isRSAllocationPtrTy(const llvm::Type *type) {
  ptr_type->getStructName().startswith("struct.rs_allocation");
 }
 
-static bool isRSAllocationTyCallSite(llvm::Module &module,
- llvm::CallInst *call_inst) {
-  (void)module;
+static bool isRSAllocationTyCallSite(llvm::CallInst *call_inst) {
   if (!call_inst->hasByValArgument())
 return false;
   for (const auto *param : call_inst->operand_values())
@@ -125,7 +120,7 @@ static llvm::FunctionType 
*cloneToStructRetFnTy(llvm::CallInst *call_inst) {
 
 static bool
 findRSCallSites(llvm::Module &module, std::set &rs_callsites,
-bool (*predicate)(llvm::Module &, llvm::CallInst *)) {
+bool (*predicate)(llvm::CallInst *)) {
   bool found = false;
 
   for (auto &func : module.getFunctionList())
@@ -136,7 +131,7 @@ findRSCallSites(llvm::Module &module, 
std::set &rs_callsites,
 if (!call_inst || !call_inst->getCalledFunction())
   // This is not the call-site you are looking for...
   continue;
-if (isRSAPICall(module, call_inst) && predicate(module, call_inst)) {
+if (isRSAPICall(call_inst) && predicate(call_inst)) {
   rs_callsites.insert(call_inst);
   found = true;
 }



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

[Lldb-commits] [lldb] b3fbbab - [lldb] Use byval type

[Nikita Popov via lldb-commits]

Author: Nikita Popov
Date: 2022-03-24T12:55:42+01:00
New Revision: b3fbbabdc1f7a52c0cc2756036a132b9aaab5742

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

LOG: [lldb] Use byval type

Query byval type instead of pointer element type.

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
index f7f1982f4059a..ec8f8d83c4b37 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
@@ -56,21 +56,19 @@ static bool isRSLargeReturnCall(llvm::CallInst *call_inst) {
  ->getPrimitiveSizeInBits() > 128;
 }
 
-static bool isRSAllocationPtrTy(const llvm::Type *type) {
-  if (!type->isPointerTy())
-return false;
-  auto ptr_type = type->getPointerElementType();
-
-  return ptr_type->isStructTy() &&
- ptr_type->getStructName().startswith("struct.rs_allocation");
+static bool isRSAllocationTy(const llvm::Type *type) {
+  return type->isStructTy() &&
+ type->getStructName().startswith("struct.rs_allocation");
 }
 
 static bool isRSAllocationTyCallSite(llvm::CallInst *call_inst) {
   if (!call_inst->hasByValArgument())
 return false;
-  for (const auto *param : call_inst->operand_values())
-if (isRSAllocationPtrTy(param->getType()))
-  return true;
+  for (unsigned i = 0; i < call_inst->arg_size(); ++i) {
+if (llvm::Type *ByValTy = call_inst->getParamByValType(i))
+  if (isRSAllocationTy(ByValTy))
+return true;
+  }
   return false;
 }
 



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

[Lldb-commits] [lldb] 840bb72 - [lldb] Avoid pointer element type accesses

[Nikita Popov via lldb-commits]

Author: Nikita Popov
Date: 2022-03-24T13:09:23+01:00
New Revision: 840bb725435c729f47458c48723d434ce45d57ee

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

LOG: [lldb] Avoid pointer element type accesses

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
llvm/docs/OpaquePointers.rst

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 10d1e99d756f8..60fe84035a5b7 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -1757,20 +1757,19 @@ bool IRForTarget::ReplaceVariables(Function 
&llvm_function) {
 llvm::Instruction *entry_instruction = llvm::cast(
 m_entry_instruction_finder.GetValue(function));
 
+Type *int8Ty = Type::getInt8Ty(function->getContext());
 ConstantInt *offset_int(
 ConstantInt::get(offset_type, offset, true));
 GetElementPtrInst *get_element_ptr = GetElementPtrInst::Create(
-argument->getType()->getPointerElementType(), argument,
-offset_int, "", entry_instruction);
+int8Ty, argument, offset_int, "", entry_instruction);
 
 if (name == m_result_name && !m_result_is_pointer) {
   BitCastInst *bit_cast = new BitCastInst(
   get_element_ptr, value->getType()->getPointerTo(), "",
   entry_instruction);
 
-  LoadInst *load =
-  new LoadInst(bit_cast->getType()->getPointerElementType(),
-   bit_cast, "", entry_instruction);
+  LoadInst *load = new LoadInst(value->getType(), bit_cast, "",
+entry_instruction);
 
   return load;
 } else {

diff  --git a/llvm/docs/OpaquePointers.rst b/llvm/docs/OpaquePointers.rst
index b13a82895edb3..2e15ce4f11946 100644
--- a/llvm/docs/OpaquePointers.rst
+++ b/llvm/docs/OpaquePointers.rst
@@ -204,8 +204,6 @@ open problems:
 
 * Some pointer element type uses remain in LLVM.
 
-* Some pointer element type uses remain in LLDB.
-
 * Some pointer element type uses remain in MLIR.
 
 * Some pointer element type uses remain in Polly.



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

[Lldb-commits] [lldb] 9b79187 - [trace][intelpt] Server side changes for TSC to wall time conversion

[Jakob Johnson via lldb-commits]

Author: Jakob Johnson
Date: 2022-03-24T05:36:21-07:00
New Revision: 9b79187c96a3bc2c245ab54d49accc12336f0cee

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

LOG: [trace][intelpt] Server side changes for TSC to wall time conversion

Update the response schema of the TraceGetState packet and add
Intel PT specific response structure that contains the TSC conversion,
if it exists. The IntelPTCollector loads the TSC conversion and caches
it to prevent unnecessary calls to perf_event_open. Move the TSC conversion
calculation from Perf.h to TraceIntelPTGDBRemotePackets.h to remove
dependency on Linux specific headers.

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

Added: 
lldb/unittests/Utility/TraceGDBRemotePacketsTest.cpp

Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Utility/TraceGDBRemotePackets.h
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
lldb/source/Plugins/Process/Linux/IntelPTCollector.h
lldb/source/Plugins/Process/Linux/Perf.cpp
lldb/source/Plugins/Process/Linux/Perf.h
lldb/source/Plugins/Process/POSIX/ProcessPOSIXLog.h
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
lldb/unittests/Process/Linux/PerfTests.cpp
lldb/unittests/Utility/CMakeLists.txt

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 980dc77c86f53..2ddc3183b80e7 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -451,7 +451,12 @@ read packet: OK/E;A
 //"size": ,
 //Size in bytes of this thread data.
 //  },
-//}]
+//],
+//"counters"?: {
+//  "info_kind": {...parameters specific to the provided counter info 
kind},
+//  Each entry includes information related to counters associated 
with the trace.
+//  They are described below.
+//}
 //  }
 //
 // NOTES
@@ -463,6 +468,26 @@ read packet: OK/E;A
 //  Binary data kinds:
 //- threadTraceBuffer: trace buffer for a thread.
 //- cpuInfo: contents of the /proc/cpuinfo file.
+//
+//  Counter info kinds:
+//tsc-perf-zero-conversion:
+//
+//This field allows converting Intel processor's TSC values to a wall time.
+//It is available through the Linux perf_event API when cap_user_time and 
cap_user_time_zero
+//are set.
+//See the documentation of time_zero in
+//https://man7.org/linux/man-pages/man2/perf_event_open.2.html for more 
information about
+//the calculation and the meaning of the values in the schema below.
+///
+//Sub-schema for this field:
+//
+//{
+//  "tsc-perf-zero-conversion": {
+//"time_mult": ,
+//"time_shift": ,
+//"time_zero": ,
+//  }
+//}
 //--
 
 send packet: jLLDBTraceGetState:{"type":}]

diff  --git a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h 
b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
index 1d2448b05f2a4..b2669ee3d813d 100644
--- a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
@@ -10,6 +10,7 @@
 #define LLDB_UTILITY_TRACEGDBREMOTEPACKETS_H
 
 #include "llvm/Support/JSON.h"
+#include 
 
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
@@ -116,6 +117,31 @@ bool fromJSON(const llvm::json::Value &value, 
TraceThreadState &packet,
 
 llvm::json::Value toJSON(const TraceThreadState &packet);
 
+/// Interface for 
diff erent algorithms used to convert trace
+/// counters into 
diff erent units.
+template  class TraceCounterConversion {
+public:
+  virtual ~TraceCounterConversion() = default;
+
+  /// Convert from raw counter value to the target type.
+  ///
+  /// \param[in] raw_counter_value
+  ///   The raw counter value to be converted.
+  ///
+  /// \return
+  ///   The converted counter value.
+  virtual ToType Convert(uint64_t raw_counter_value) = 0;
+
+  /// Serialize trace counter conversion values to JSON.
+  ///
+  /// \return
+  ///   \a llvm::json::Value representing the trace counter conversion object.
+  virtual llvm::json::Value toJSON() = 0;
+};
+
+using TraceTscConversionUP =
+std::unique_ptr>;
+
 struct TraceGetStateResponse {
   std::vector tracedThreads;
   std::vector processBinaryData;

diff  --git a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h 
b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
index 8f4947b1f189c..8960949f2039f 100644
--- a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
@@ -11,6 +11,10 @@
 
 #include "lldb/Utility/TraceGDBRemotePackets.h"
 
+#include "llvm/Support/JSON.h"
+
+#include 
+
 /// 

[Lldb-commits] [PATCH] D122246: [trace][intelpt] Server side changes for TSC to wall time conversion

[Jakob Johnson via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9b79187c96a3: [trace][intelpt] Server side changes for TSC 
to wall time conversion (authored by jj10306).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122246

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/TraceGDBRemotePackets.h
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Perf.h
  lldb/source/Plugins/Process/POSIX/ProcessPOSIXLog.h
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  lldb/unittests/Process/Linux/PerfTests.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/TraceGDBRemotePacketsTest.cpp

Index: lldb/unittests/Utility/TraceGDBRemotePacketsTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/TraceGDBRemotePacketsTest.cpp
@@ -0,0 +1,101 @@
+//===-- TraceGDBRemotePacketsTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Utility/TraceIntelPTGDBRemotePackets.h"
+
+#include "gtest/gtest.h"
+
+#include 
+
+using namespace lldb_private;
+using namespace llvm;
+
+// Test serialization and deserialization of a non-empty
+// TraceIntelPTGetStateResponse.
+TEST(TraceGDBRemotePacketsTest, IntelPTGetStateResponse) {
+  // This test works as follows:
+  //  1. Create a non-empty TraceIntelPTGetStateResponse
+  //  2. Serialize to JSON
+  //  3. Deserialize the serialized JSON value
+  //  4. Ensure the original value and the deserialized value are equivalent
+  //
+  //  Notes:
+  //- We intentionally set an integer value out of its signed range
+  //  to ensure the serialization/deserialization isn't lossy since JSON
+  //  operates on signed values
+
+  // Choose arbitrary values for time_mult and time_shift
+  uint32_t test_time_mult = 1076264588;
+  uint16_t test_time_shift = 31;
+  // Intentionally set time_zero value out of the signed type's range.
+  uint64_t test_time_zero =
+  static_cast(std::numeric_limits::max()) + 1;
+
+  // Create TraceIntelPTGetStateResponse.
+  TraceIntelPTGetStateResponse response;
+  response.tsc_conversion = std::make_unique(
+  test_time_mult, test_time_shift, test_time_zero);
+
+  // Serialize then deserialize.
+  Expected deserialized_response =
+  json::parse(
+  llvm::formatv("{0}", toJSON(response)).str(),
+  "TraceIntelPTGetStateResponse");
+  if (!deserialized_response)
+FAIL() << toString(deserialized_response.takeError());
+
+  // Choose arbitrary TSC value to test the Convert function.
+  const uint64_t TSC = std::numeric_limits::max();
+  // Expected nanosecond value pre calculated using the TSC to wall time
+  // conversion formula located in the time_zero section of
+  // https://man7.org/linux/man-pages/man2/perf_event_open.2.html
+  const uint64_t EXPECTED_NANOS = 9223372039007304983u;
+
+  uint64_t pre_serialization_conversion =
+  response.tsc_conversion->Convert(TSC).count();
+  uint64_t post_serialization_conversion =
+  deserialized_response->tsc_conversion->Convert(TSC).count();
+
+  // Check equality:
+  // Ensure that both the TraceGetStateResponse and TraceIntelPTGetStateResponse
+  // portions of the JSON representation are unchanged.
+  ASSERT_EQ(toJSON(response), toJSON(*deserialized_response));
+  // Ensure the result of the Convert function is unchanged.
+  ASSERT_EQ(EXPECTED_NANOS, pre_serialization_conversion);
+  ASSERT_EQ(EXPECTED_NANOS, post_serialization_conversion);
+}
+
+// Test serialization and deserialization of an empty
+// TraceIntelPTGetStateResponse.
+TEST(TraceGDBRemotePacketsTest, IntelPTGetStateResponseEmpty) {
+  // This test works as follows:
+  //  1. Create an empty TraceIntelPTGetStateResponse
+  //  2. Serialize to JSON
+  //  3. Deserialize the serialized JSON value
+  //  4. Ensure the original value and the deserialized value are equivalent
+
+  // Create TraceIntelPTGetStateResponse.
+  TraceIntelPTGetStateResponse response;
+
+  // Serialize then deserialize.
+  Expected deserialized_response =
+  json::parse(
+  llvm::formatv("{0}", toJSON(response)).str(),
+  "TraceIntelPTGetStateResponse");
+  if (!deserialized_response)
+FAIL() << toString(deserialized_response.takeError());
+
+  // Check equality:
+  // Ensure that both the TraceGetStateResponse and TraceIntelPTGetState

[Lldb-commits] [PATCH] D122389: [lldb] Fix interpreting absolute Windows paths with forward slashes

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

In practice, Windows paths can use either backslashes or forward slashes.

This fixes an issue reported downstream at
https://github.com/mstorsjo/llvm-mingw/issues/266.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122389

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Utility/FileSpec.cpp
  lldb/unittests/Utility/FileSpecTest.cpp


Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -196,9 +196,12 @@
   EXPECT_EQ(FileSpec::Style::posix, FileSpec::GuessPathStyle("//net/bar.txt"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(C:\foo.txt)"));
+  EXPECT_EQ(FileSpec::Style::windows,
+FileSpec::GuessPathStyle(R"(C:/foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(\\net\foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:\)"));
+  EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:/)"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo/bar.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("Z:"));
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -311,7 +311,8 @@
   if (absolute_path.startswith(R"(\\)"))
 return Style::windows;
   if (absolute_path.size() >= 3 && llvm::isAlpha(absolute_path[0]) &&
-  absolute_path.substr(1, 2) == R"(:\)")
+  (absolute_path.substr(1, 2) == R"(:\)" ||
+   absolute_path.substr(1, 2) == R"(:/)"))
 return Style::windows;
   return llvm::None;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -772,7 +772,8 @@
 
   // check whether we have a windows path, and so the first character is a
   // drive-letter not a hostname.
-  if (host.size() == 1 && llvm::isAlpha(host[0]) && path.startswith("\\"))
+  if (host.size() == 1 && llvm::isAlpha(host[0]) &&
+  (path.startswith("\\") || path.startswith("/")))
 return path_from_dwarf;
 
   return path;


Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -196,9 +196,12 @@
   EXPECT_EQ(FileSpec::Style::posix, FileSpec::GuessPathStyle("//net/bar.txt"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(C:\foo.txt)"));
+  EXPECT_EQ(FileSpec::Style::windows,
+FileSpec::GuessPathStyle(R"(C:/foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(\\net\foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:\)"));
+  EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:/)"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo/bar.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("Z:"));
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -311,7 +311,8 @@
   if (absolute_path.startswith(R"(\\)"))
 return Style::windows;
   if (absolute_path.size() >= 3 && llvm::isAlpha(absolute_path[0]) &&
-  absolute_path.substr(1, 2) == R"(:\)")
+  (absolute_path.substr(1, 2) == R"(:\)" ||
+   absolute_path.substr(1, 2) == R"(:/)"))
 return Style::windows;
   return llvm::None;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -772,7 +772,8 @@
 
   // check whether we have a windows path, and so the first character is a
   // drive-letter not a hostname.
-  if (host.size() == 1 && llvm::isAlpha(host[0]) && path.startswith("\\"))
+  if (host.size() == 1 && llvm::isAlpha(host[0]) &&
+  (path.startswith("\\") || path.startswith("/")))
 return path_from_dwarf;
 
   return path;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:103-105
+// llvm::Error DecodedThread::GetError(uint64_t idx) const {
+//   return m_errors.at(idx);
+// }

Errors can only be copied, that's why we need to create a new instance of the 
error that is a copy of the original one. We can draw inspiration from 
IntelPTInstruction::ToError(), which can now be deleted



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:148-151
+  /// Get the error at some instruction index from the decoded trace.
+  ///
+  /// \return
+  ///   The error of the trace.

we can make the documentation clearer



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:155
+  /// Append a successfully decoded instruction.
+  void AppendInstruction(IntelPTInstruction ins);
+

this has to use the new parameter pack semantics, so that you can pass either 
`{pt_insn}` or `{pt_insn, timestamp}` without having to create copies of the 
IntelPTInstruction class




Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:157
+
+  /// Append an error of instruction decoding.
+  void AppendError(llvm::Error err);





Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:110
+const size_t raw_trace_size) {
+  DecodedThread decoded_thread = DecodedThread(
+  thread_sp, std::vector(), raw_trace_size);

make this a shared pointer since the beginning. Use make_shared here



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:148
+decoded_thread.AppendError(make_error(time_error, 
insn.ip));
+decoded_thread.AppendInstruction(IntelPTInstruction(insn));
 break;

ideally you should be able to do `decoded_thread.AppendInstruction({insn})` here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122293

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

[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:227
+
+static Expected DecodeLiveThread(const ThreadSP &thread_sp,
+  TraceIntelPT &trace) {

don't use expected. The DecodedThread object already can store errors. Just 
return a DecodedThreadSP and assign it a single error and return it. If you 
need to return the failed DecodedThread before you know the size of the buffer, 
just pass 0 as size



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:241
+DecodedThreadSP LiveThreadDecoder::DoDecode() {
+  return *(DecodeLiveThread(m_thread_sp, m_trace));
 }

remove the *, because we will make DecodeLiveThread not return an Expected


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122293

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

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 417960.
DavidSpickett added a comment.
Herald added a project: All.

Switch to removing non-address bits in lldb instead of lldb-server.

The breakpoint issues I mention only really happen if you try to break on a 
tagged
function pointer. Which is pretty niche, but I hope to address it later anyway.

On the issue of whether to use FixData vs FixCode there's 2 maybe 3 ways to go:

- Assume that they're the same, which does work for Linux, for now.
- Add a method that does both fixes, on the assumption that the virtual address 
size for code and data is the same so no harm done and all bits will be removed 
either way.
- Extensively track whether addresses refer to code or data. In some situations 
this is possible (looking at the exec bits of a memory mapping for example) but 
I don't have a great idea what that looks like at this time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118794

Files:
  lldb/source/Target/Process.cpp
  lldb/source/Target/ProcessTrace.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/Makefile
  
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c

Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
@@ -0,0 +1,25 @@
+#include 
+#include 
+#include 
+
+int main(int argc, char const *argv[]) {
+  size_t page_size = sysconf(_SC_PAGESIZE);
+  // Note that we allocate memory here because if we used
+  // stack or globals lldb might read it in the course of
+  // running to the breakpoint. Before the test can look
+  // for those reads.
+  char *buf = mmap(0, page_size, PROT_READ | PROT_WRITE,
+   MAP_ANONYMOUS | MAP_SHARED, -1, 0);
+  if (buf == MAP_FAILED)
+return 1;
+
+#define sign_ptr(ptr) __asm__ __volatile__("pacdza %0" : "=r"(ptr) : "r"(ptr))
+
+  // Set top byte to something.
+  char *buf_with_non_address = (char *)((size_t)buf | (size_t)0xff << 56);
+  sign_ptr(buf_with_non_address);
+  // Address is now:
+  // <8 bit top byte tag>
+
+  return 0; // Set break point at this line.
+}
Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
@@ -0,0 +1,177 @@
+"""
+Test that lldb removes non-address bits in situations where they would cause
+failures if not removed. Like when reading memory. Tests are done at command
+and API level because commands may remove non-address bits for display
+reasons which can make it seem like the operation as a whole works but at the
+API level it won't if we don't remove them there also.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxNonAddressBitMemoryAccessTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def setup_test(self):
+if not self.isAArch64PAuth():
+self.skipTest('Target must support pointer authentication.')
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(self, "main.c",
+line_number('main.c', '// Set break point at this line.'),
+num_expected_locations=1)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+def check_cmd_read_write(self, write_to, read_from, data):
+self.runCmd("memory write {} {}".format(write_to, data))
+self.expect("memory read {}".format(read_from),
+substrs=[data])
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_non_address_bit_memory_read_write_cmds(self):
+self.setup_test()
+
+# Writes should be visible through either pointer
+self.check_cmd_read_write("buf", "buf", "01 02 03 04")
+self.check_cmd_read_write("buf_with_non_address", "buf_with_non_address", "02 03 04 05")
+self.check_cmd_read_write("buf", "buf_with_non_address", "03 04 05 06")
+self.check_cmd_read_write("buf_with_non_address", "buf", "04 05 06 07")
+
+def get_ptr_values(self):
+frame  = self.process().GetThreadAtIndex(

[Lldb-commits] [PATCH] D122411: [lldb][AArch64] Fix corefile memory reads when there are non-address bits

[David Spickett via Phabricator via lldb-commits]

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

Previously if you read a code/data mask before there was a valid thread
you would get the top byte mask. This meant the value was "valid" as in,
don't read it again.

When using a corefile we ask for the data mask very early on and this
meant that later once you did have a thread it wouldn't read the
register to get the rest of the mask.

This fixes that and adds a corefile test generated from the same program
as in my previous change on this theme.

Depends on D118794 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122411

Files:
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/a.out.corefile
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/corefile
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c


Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
===
--- lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
@@ -13,6 +13,13 @@
   if (buf == MAP_FAILED)
 return 1;
 
+  // Some known values to go in the corefile, since we cannot
+  // write to corefile memory.
+  buf[0] = 'L';
+  buf[1] = 'L';
+  buf[2] = 'D';
+  buf[3] = 'B';
+
 #define sign_ptr(ptr) __asm__ __volatile__("pacdza %0" : "=r"(ptr) : "r"(ptr))
 
   // Set top byte to something.
@@ -21,5 +28,8 @@
   // Address is now:
   // <8 bit top byte tag>
 
+  // Uncomment this line to crash and generate a corefile.
+  //*(char*)0 = 0;
+
   return 0; // Set break point at this line.
 }
Index: 
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
===
--- 
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
+++ 
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
@@ -175,3 +175,21 @@
 
 if not found_read_buf:
 self.fail("Did not find any reads of buf.")
+
+@skipIfLLVMTargetMissing("AArch64")
+def test_non_address_bit_memory_corefile(self):
+if not self.isAArch64PAuth():
+self.skipTest('Target must support pointer authentication.')
+
+self.runCmd("target create a.out.corefile --core corefile")
+
+self.expect("thread list", substrs=['stopped',
+'stop reason = signal SIGSEGV'])
+
+# No caching (the program/corefile are the cache) and no writing
+# to memory. So just check that tagged/untagged addresses read
+# the same location.
+
+expected = ["4c 4c 44 42", "LLDB"]
+self.expect("memory read buf", substrs=expected)
+self.expect("memory read buf_with_non_address", substrs=expected)
Index: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
===
--- lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -794,14 +794,20 @@
 // Reads code or data address mask for the current Linux process.
 static lldb::addr_t ReadLinuxProcessAddressMask(lldb::ProcessSP process_sp,
 llvm::StringRef reg_name) {
-  // Linux configures user-space virtual addresses with top byte ignored.
-  // We set default value of mask such that top byte is masked out.
-  uint64_t address_mask = ~((1ULL << 56) - 1);
-  // If Pointer Authentication feature is enabled then Linux exposes
-  // PAC data and code mask register. Try reading relevant register
-  // below and merge it with default address mask calculated above.
+  // 0 means there isn't a mask or it has not been read yet.
+  // We do not return the top byte mask unless thread_sp is valid.
+  // This prevents calls to this function before the thread is setup locking
+  // in the value to just the top byte mask, in cases where pointer 
authentication
+  // might also be active.
+  uint64_t address_mask = 0;
   lldb::ThreadSP thread_sp = process_sp->GetThreadList().GetSelectedThread();
   if (thread_sp) {
+// Linux configures user-space virtual addresses with top byte ignored.
+// We set default value of mask such that top byte is masked out.
+address_mask = ~((1ULL << 56) - 1);
+// If Pointer Authentication feature is enabled then Linux exposes
+// PAC data and code mask register. Try reading relevant register
+// below and merge it with default ad

[Lldb-commits] [PATCH] D122411: [lldb][AArch64] Fix corefile memory reads when there are non-address bits

[David Spickett via Phabricator via lldb-commits]

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

This smells a little bit like a hack but in some ways it makes sense that an 
address mask value couldn't possibly be valid until we have a thread setup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122411

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

[Lldb-commits] [lldb] aca9648 - [LLDB] Cleanup for Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

[Shafik Yaghmour via lldb-commits]

Author: Shafik Yaghmour
Date: 2022-03-24T10:00:26-07:00
New Revision: aca96480784b5373ad7229816b00297690354208

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

LOG: [LLDB] Cleanup for Fixing DWARFExpression handling of 
ValueType::FileAddress case for DW_OP_deref_size

Late review on https://reviews.llvm.org/D121408 spotted some nice quick 
clean-ups on this code.

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 5ee62fb4376b8..717cbe76450eb 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -960,7 +960,7 @@ void UpdateValueTypeFromLocationDescription(Log *log, const 
DWARFUnit *dwarf_cu,
 ///  check_sectionoffset is true we consider LLDB_INVALID_ADDRESS a
 ///  success if so_addr.IsSectionOffset() is true.
 static llvm::Optional
-ResolveAndLoadFileAddress(ExecutionContext *exe_ctx, lldb::ModuleSP module_sp,
+ResolveLoadAddress(ExecutionContext *exe_ctx, lldb::ModuleSP &module_sp,
   Status *error_ptr, const char *dw_op_type,
   lldb::addr_t file_addr, Address &so_addr,
   bool check_sectionoffset = false) {
@@ -1003,18 +1003,10 @@ static Scalar DerefSizeExtractDataHelper(uint8_t 
*addr_bytes,
   DataExtractor addr_data(addr_bytes, size_addr_bytes, byte_order, size);
 
   lldb::offset_t addr_data_offset = 0;
-  switch (size) {
-  case 1:
-return addr_data.GetU8(&addr_data_offset);
-  case 2:
-return addr_data.GetU16(&addr_data_offset);
-  case 4:
-return addr_data.GetU32(&addr_data_offset);
-  case 8:
-return addr_data.GetU64(&addr_data_offset);
-  default:
+  if (size <= 8)
+return addr_data.GetMaxU64(&addr_data_offset, size);
+  else
 return addr_data.GetAddress(&addr_data_offset);
-  }
 }
 
 bool DWARFExpression::Evaluate(
@@ -1099,7 +1091,6 @@ bool DWARFExpression::Evaluate(
   if (frame)
 stack.back().ConvertToLoadAddress(module_sp.get(),
   frame->CalculateTarget().get());
-
   break;
 
 // The DW_OP_addr_sect_offset4 is used for any location expressions in
@@ -1165,7 +1156,7 @@ bool DWARFExpression::Evaluate(
 LLDB_INVALID_ADDRESS);
 
 Address so_addr;
-auto maybe_load_addr = ResolveAndLoadFileAddress(
+auto maybe_load_addr = ResolveLoadAddress(
 exe_ctx, module_sp, error_ptr, "DW_OP_deref", file_addr, so_addr);
 
 if (!maybe_load_addr)
@@ -1287,7 +1278,7 @@ bool DWARFExpression::Evaluate(
 stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
 Address so_addr;
 auto maybe_load_addr =
-ResolveAndLoadFileAddress(exe_ctx, module_sp, error_ptr,
+ResolveLoadAddress(exe_ctx, module_sp, error_ptr,
   "DW_OP_deref_size", file_addr, so_addr,
   /*check_sectionoffset=*/true);
 



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

[Lldb-commits] [PATCH] D122041: [llvm][utils] Fix llvm::Optional summary provider

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

> There's some CMake trickery

Can you shed some more light on this? Am I understanding right: these 
formatters are llvm, but the test would be in lldb? It seems weird that 
something in llvm/ would only have tests in lldb/. Would it be bad to move this 
file into lldb, and then we can test there? With llvm being a monorepo, a 
source checkout should have access at any path. Maybe the issue would be llvm 
installations and packages that don't include lldb too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122041

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

[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

[Alisamar Husain via Phabricator via lldb-commits]

zrthxn updated this revision to Diff 417969.
zrthxn added a comment.

Error gettting method


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122293

Files:
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp

Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -78,7 +78,7 @@
 }
 
 Error TraceCursorIntelPT::GetError() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].ToError();
+  return m_decoded_thread_sp->GetError(m_pos);
 }
 
 lldb::addr_t TraceCursorIntelPT::GetLoadAddress() {
Index: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
+++ lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
@@ -104,9 +104,11 @@
 ///
 /// \return
 ///   The decoded instructions.
-static std::vector
-DecodeInstructions(pt_insn_decoder &decoder) {
-  std::vector instructions;
+static DecodedThreadSP DecodeInstructions(pt_insn_decoder &decoder,
+  const ThreadSP &thread_sp,
+  const size_t raw_trace_size) {
+  DecodedThread decoded_thread = DecodedThread(
+  thread_sp, std::vector(), raw_trace_size);
 
   while (true) {
 int errcode = FindNextSynchronizationPoint(decoder);
@@ -114,7 +116,7 @@
   break;
 
 if (errcode < 0) {
-  instructions.emplace_back(make_error(errcode));
+  decoded_thread.AppendError(IntelPTError(errcode));
   break;
 }
 
@@ -123,17 +125,17 @@
 while (true) {
   errcode = ProcessPTEvents(decoder, errcode);
   if (errcode < 0) {
-instructions.emplace_back(make_error(errcode));
+decoded_thread.AppendError(IntelPTError(errcode));
 break;
   }
-  pt_insn insn;
 
+  pt_insn insn;
   errcode = pt_insn_next(&decoder, &insn, sizeof(insn));
   if (errcode == -pte_eos)
 break;
 
   if (errcode < 0) {
-instructions.emplace_back(make_error(errcode, insn.ip));
+decoded_thread.AppendError(IntelPTError(errcode, insn.ip));
 break;
   }
 
@@ -142,22 +144,22 @@
   if (time_error == -pte_invalid) {
 // This happens if we invoke the pt_insn_time method incorrectly,
 // but the instruction is good though.
-instructions.emplace_back(
-make_error(time_error, insn.ip));
-instructions.emplace_back(insn);
+decoded_thread.AppendError(IntelPTError(time_error, insn.ip));
+decoded_thread.AppendInstruction(IntelPTInstruction(insn));
 break;
   }
+
   if (time_error == -pte_no_time) {
 // We simply don't have time information, i.e. None of TSC, MTC or CYC
 // was enabled.
-instructions.emplace_back(insn);
+decoded_thread.AppendInstruction(IntelPTInstruction(insn));
   } else {
-instructions.emplace_back(insn, time);
+decoded_thread.AppendInstruction(IntelPTInstruction(insn, time));
   }
 }
   }
 
-  return instructions;
+  return decoded_thread.shared_from_this();
 }
 
 /// Callback used by libipt for reading the process memory.
@@ -176,8 +178,9 @@
   return bytes_read;
 }
 
-static Expected>
-DecodeInMemoryTrace(Process &process, TraceIntelPT &trace_intel_pt,
+static Expected
+DecodeInMemoryTrace(const ThreadSP &thread_sp, Process &process,
+TraceIntelPT &trace_intel_pt,
 MutableArrayRef buffer) {
   Expected cpu_info = trace_intel_pt.GetCPUInfo();
   if (!cpu_info)
@@ -203,77 +206,67 @@
   assert(errcode == 0);
   (void)errcode;
 
-  std::vector instructions = DecodeInstructions(*decoder);
+  DecodedThreadSP decoded_thread =
+  DecodeInstructions(*decoder, thread_sp, buffer.size());
 
   pt_insn_free_decoder(decoder);
-  return instructions;
+  return decoded_thread;
 }
+// ---
 
-static Expected>
-DecodeTraceFile(Process &process, TraceIntelPT &trace_intel_pt,
-const FileSpec &trace_file, size_t &raw_trace_size) {
-  ErrorOr> trace_or_error =
-  MemoryBuffer::getFile(trace_file.GetPath());
-  if (std::error_code err = trace_or_error.getError())
-return errorCodeToError(err);
-
-  MemoryBuffer &trace = **trace_or_error;
-  MutableArrayRef trace_data(
-  // The libipt library does not modify the trace buffer, hence the
-  // following cast is safe.
-  reinterpret_cast(const_cast(trace.getBufferStart())),
-  trace.getBufferSize());
-  raw_trace_size = trace_data.size();
-  return DecodeInMemoryTra

[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

[walter erquinigo via Phabricator via lldb-commits]

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

there are many comments from the previous versions of this diff that you didn't 
apply. Go through all of them first :)




Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:88-91
+  // /// \return
+  // /// An \a llvm::Error object if this class corresponds to an Error, 
or an
+  // /// \a llvm::Error::success otherwise.
+  // llvm::Error ToError() const;

delete it. We don't want to leave old code as comments



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:122
   llvm::Optional m_timestamp;
-  std::unique_ptr m_error;
+  bool is_error;
 };

now that you changed this, could show share in the description of this diff the 
difference in byte size between the old and new code when tracing the same 
number of instructions?



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:152
+  ///   The error of the trace.
+  llvm::Error GetError(uint64_t ins_idx);
+

or GetErrorByInstructionIndex



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:148-151
+  /// Get the error at some instruction index from the decoded trace.
+  ///
+  /// \return
+  ///   The error of the trace.

wallace wrote:
> we can make the documentation clearer
you didn't apply these changes



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:157
+
+  /// Append an error of instruction decoding.
+  void AppendError(llvm::Error err);

wallace wrote:
> 
same here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122293

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

[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

[Alisamar Husain via Phabricator via lldb-commits]

zrthxn updated this revision to Diff 418000.
zrthxn added a comment.

Incorporate other comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122293

Files:
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp

Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -78,7 +78,7 @@
 }
 
 Error TraceCursorIntelPT::GetError() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].ToError();
+  return m_decoded_thread_sp->GetErrorByInstructionIndex(m_pos);
 }
 
 lldb::addr_t TraceCursorIntelPT::GetLoadAddress() {
Index: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
+++ lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
@@ -104,9 +104,12 @@
 ///
 /// \return
 ///   The decoded instructions.
-static std::vector
-DecodeInstructions(pt_insn_decoder &decoder) {
-  std::vector instructions;
+static DecodedThreadSP DecodeInstructions(pt_insn_decoder &decoder,
+  const ThreadSP &thread_sp,
+  const size_t raw_trace_size) {
+  DecodedThreadSP decoded_thread =
+  std::make_shared(DecodedThread(
+  thread_sp, std::vector(), raw_trace_size));
 
   while (true) {
 int errcode = FindNextSynchronizationPoint(decoder);
@@ -114,7 +117,7 @@
   break;
 
 if (errcode < 0) {
-  instructions.emplace_back(make_error(errcode));
+  decoded_thread->AppendError(IntelPTError(errcode));
   break;
 }
 
@@ -123,17 +126,17 @@
 while (true) {
   errcode = ProcessPTEvents(decoder, errcode);
   if (errcode < 0) {
-instructions.emplace_back(make_error(errcode));
+decoded_thread->AppendError(IntelPTError(errcode));
 break;
   }
-  pt_insn insn;
 
+  pt_insn insn;
   errcode = pt_insn_next(&decoder, &insn, sizeof(insn));
   if (errcode == -pte_eos)
 break;
 
   if (errcode < 0) {
-instructions.emplace_back(make_error(errcode, insn.ip));
+decoded_thread->AppendError(IntelPTError(errcode, insn.ip));
 break;
   }
 
@@ -142,22 +145,22 @@
   if (time_error == -pte_invalid) {
 // This happens if we invoke the pt_insn_time method incorrectly,
 // but the instruction is good though.
-instructions.emplace_back(
-make_error(time_error, insn.ip));
-instructions.emplace_back(insn);
+decoded_thread->AppendError(IntelPTError(time_error, insn.ip));
+decoded_thread->AppendInstruction(IntelPTInstruction(insn));
 break;
   }
+
   if (time_error == -pte_no_time) {
 // We simply don't have time information, i.e. None of TSC, MTC or CYC
 // was enabled.
-instructions.emplace_back(insn);
+decoded_thread->AppendInstruction(IntelPTInstruction(insn));
   } else {
-instructions.emplace_back(insn, time);
+decoded_thread->AppendInstruction(IntelPTInstruction(insn, time));
   }
 }
   }
 
-  return instructions;
+  return decoded_thread;
 }
 
 /// Callback used by libipt for reading the process memory.
@@ -176,8 +179,9 @@
   return bytes_read;
 }
 
-static Expected>
-DecodeInMemoryTrace(Process &process, TraceIntelPT &trace_intel_pt,
+static Expected
+DecodeInMemoryTrace(const ThreadSP &thread_sp, Process &process,
+TraceIntelPT &trace_intel_pt,
 MutableArrayRef buffer) {
   Expected cpu_info = trace_intel_pt.GetCPUInfo();
   if (!cpu_info)
@@ -203,77 +207,64 @@
   assert(errcode == 0);
   (void)errcode;
 
-  std::vector instructions = DecodeInstructions(*decoder);
+  DecodedThreadSP decoded_thread =
+  DecodeInstructions(*decoder, thread_sp, buffer.size());
 
   pt_insn_free_decoder(decoder);
-  return instructions;
+  return decoded_thread;
 }
+// ---
 
-static Expected>
-DecodeTraceFile(Process &process, TraceIntelPT &trace_intel_pt,
-const FileSpec &trace_file, size_t &raw_trace_size) {
-  ErrorOr> trace_or_error =
-  MemoryBuffer::getFile(trace_file.GetPath());
-  if (std::error_code err = trace_or_error.getError())
-return errorCodeToError(err);
-
-  MemoryBuffer &trace = **trace_or_error;
-  MutableArrayRef trace_data(
-  // The libipt library does not modify the trace buffer, hence the
-  // following cast is safe.
-  reinterpret_cast(const_cast(trace.getBufferStart())),
-  trace.getBufferSize());
-  raw_trace_size = trac

[Lldb-commits] [PATCH] D122422: [lldb/crashlog] Parse more thread fields and pass it to crashlog scripted process

[Med Ismail Bennani via Phabricator via lldb-commits]

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

Previously, the ScriptedThread used the thread index as the thread id.

This patch parses the crashlog json to extract the actual thread "id" value,
and passes this information to the Crashlog ScriptedProcess blueprint,
to create a higher fidelity ScriptedThreaad.

It also updates the blueprint to return the thread queue, and
removes the thread placeholder name.

Finally, this patch updates the interactive crashlog test to reflect
these changes.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122422

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
@@ -4,17 +4,17 @@
 
 # RUN: cp %S/Inputs/scripted_crashlog.ips %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":160, "bar":20, "foo":24}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -a -i %t.crash' 2>&1 -o "bt all" | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -a -i %t.crash' 2>&1 -o "thread list" -o "bt all" | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
 # CHECK: (lldb) process status
 # CHECK-NEXT: Process 24991 stopped
-# CHECK-NEXT: * thread #3, name = 'CrashLogScriptedThread.thread-2', stop reason = EXC_BAD_ACCESS
+# CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS
 # CHECK-NEXT: frame #0: 0x0001047f5970 scripted_crashlog_json.test.tmp.out`bar
 
 # CHECK: (lldb) thread backtrace
-# CHECK-NEXT: * thread #3, name = 'CrashLogScriptedThread.thread-2', stop reason = EXC_BAD_ACCESS
+# CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS
 # CHECK-NEXT:   * frame #0: 0x0001047f5970 scripted_crashlog_json.test.tmp.out`bar
 # CHECK-NEXT: frame #1: 0x0001047f5998 scripted_crashlog_json.test.tmp.out`foo
 # CHECK-NEXT: frame #2: 0x0001047f5b04 scripted_crashlog_json.test.tmp.out`compute_pow
@@ -24,14 +24,21 @@
 # CHECK-NEXT: frame #6: 0x00018bf5326c libsystem_pthread.dylib`_pthread_start
 # CHECK-NEXT: frame #7: 0x00018bf4e08c libsystem_pthread.dylib`thread_start
 
+# CHECK: (lldb) thread list
+# CHECK-NEXT: Process 24991 stopped
+# CHECK-NEXT:  thread #1: tid = 0x4ea840, 0x00018bf17854 libsystem_kernel.dylib`__ulock_wait{{.*}}, queue = 'com.apple.main-thread'
+# CHECK-NEXT:  thread #2: tid = 0x4ea850, 0x0001047f59e8 scripted_crashlog_json.test.tmp.out`call_and_wait
+# CHECK-NEXT: * thread #3: tid = 0x4ea851, 0x0001047f5970 scripted_crashlog_json.test.tmp.out`bar{{.*}}, stop reason = EXC_BAD_ACCESS
+
+
 # CHECK: (lldb) bt all
-# CHECK-NEXT:   thread #1, name = 'CrashLogScriptedThread.thread-0'
+# CHECK-NEXT:   thread #1
 # CHECK-NEXT: frame #0: 0x00018bf17854 libsystem_kernel.dylib`__ulock_wait
 # CHECK-NEXT: frame #1: 0x00018bf555a0 libsystem_pthread.dylib`_pthread_join
 # CHECK-NEXT: frame #2: 0x00018beae9c0 libc++.1.dylib`std::__1::thread::join
 # CHECK-NEXT: frame #3: 0x0001047f5bb8 scripted_crashlog_json.test.tmp.out`main
 # CHECK-NEXT: frame #4: 0x000104ae5088 dyld`start
-# CHECK-NEXT:   thread #2, name = 'CrashLogScriptedThread.thread-1'
+# CHECK-NEXT:   thread #2
 # CHECK-NEXT: frame #0: 0x0001047f59e8 scripted_crashlog_json.test.tmp.out`call_and_wait
 # CHECK-NEXT: frame #1: 0x0001047f59d4 scripted_crashlog_json.test.tmp.out`call_and_wait
 # CHECK-NEXT: frame #2: 0x0001047f7690 scripted_crashlog_json.test.tmp.out`decltype
@@ -39,7 +46,7 @@
 # CHECK-NEXT: frame #4: 0x0001047f6d58 scripted_crashlog_json.test.tmp.out`void* std::__1::__thread_proxy
 # CHECK-NEXT: frame #5: 0x00018bf5326c libsystem_pthread.dylib`_pthread_start
 # CHECK-NEXT: frame #6: 0x00018bf4e08c libsystem_pthread.dylib`thread_start
-# CHECK-NEXT: * thread #3, name = 'CrashLogScriptedThread.thread-2', stop reason = EXC_BAD_ACCESS
+# CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS
 # CHECK-NEXT:   * frame #0: 0x0001047f5970 scripted_crashlog_json.test.tmp.out`bar
 # CHECK-NEXT: frame #1: 0x0001047f5998 scripted_crashlog_json.test.tmp.out`foo
 # CHECK-NEXT: frame #2: 0x0001047f5b04 scripted_crashlog_json.test.tmp.out`compute_pow
Index: lldb/exam

[Lldb-commits] [PATCH] D122422: [lldb/crashlog] Parse more thread fields and pass it to crashlog scripted process

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122422

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

[Lldb-commits] [PATCH] D122426: [lldb/Utility] Make `StructuredData::Dictionary::GetKeys1 return an `Array`

[Med Ismail Bennani via Phabricator via lldb-commits]

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

This patch changes `StructuredData::Dictionary::GetKeys` return type
from an `StructuredData::ObjectSP` to a `StructuredData::ArraySP`.

The function already stored the keys in an array but implicitely upcasted
it to an `ObjectSP`, which required the user to convert it again to a
Array object to access each element.

Since we know the keys should be held by an iterable container, it makes
more sense to return the allocated ArraySP as-is.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122426

Files:
  lldb/include/lldb/Utility/StructuredData.h


Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -375,15 +375,15 @@
   }
 }
 
-ObjectSP GetKeys() const {
-  auto object_sp = std::make_shared();
+ArraySP GetKeys() const {
+  auto array_sp = std::make_shared();
   collection::const_iterator iter;
   for (iter = m_dict.begin(); iter != m_dict.end(); ++iter) {
 auto key_object_sp = std::make_shared();
 key_object_sp->SetValue(iter->first.AsCString());
-object_sp->Push(key_object_sp);
+array_sp->Push(key_object_sp);
   }
-  return object_sp;
+  return array_sp;
 }
 
 ObjectSP GetValueForKey(llvm::StringRef key) const {


Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -375,15 +375,15 @@
   }
 }
 
-ObjectSP GetKeys() const {
-  auto object_sp = std::make_shared();
+ArraySP GetKeys() const {
+  auto array_sp = std::make_shared();
   collection::const_iterator iter;
   for (iter = m_dict.begin(); iter != m_dict.end(); ++iter) {
 auto key_object_sp = std::make_shared();
 key_object_sp->SetValue(iter->first.AsCString());
-object_sp->Push(key_object_sp);
+array_sp->Push(key_object_sp);
   }
-  return object_sp;
+  return array_sp;
 }
 
 ObjectSP GetValueForKey(llvm::StringRef key) const {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/Target/TraceCursor.h:162-166
+  /// - In terms of efficiency, moving the cursor to a given id should be as
+  /// fast
+  ///   as possible, but not necessarily O(1). That's why the recommended way 
to
+  ///   traverse sequential instructions is to use the \a TraceCursor::Next()
+  ///   method and only use \a TraceCursor::GoToId(id) sparingly.

fix wrap here. Might have been a clang format change



Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:50
+  /// Additional options for configuring the dumping.
+  TraceInstructionDumper(lldb::TraceCursorUP &&cursor_up, Stream &s,
+ const TraceInstructionDumperOptions &options);

Do we want the stream in the options?



Comment at: lldb/source/Commands/Options.td:1126-1128
+Desc<"Continue dumping instructions right where the previous invocation of 
"
+"this command was left, or from the beginning if this is the first "
+"invocation. The --skip and --id arguments are discared if provided.">;

this option might not be needed. Each LLDB command line command can save 
information for how to continue a subsequent command. For example:

```(lldb) memory read $pc
0x12ecf: 48 8d 7d f0 48 8d 35 a6 ff ff ff e8 51 00 00 00  H.}.H.5.Q...
0x12edf: c7 45 ec 01 00 00 00 8b 75 ec 48 8d 3d 1e 10 00  .E..u.H.=...
(lldb) 
0x12eef: 00 31 c0 e8 f5 0e 00 00 e9 00 00 00 00 e9 00 00  .1..
0x12eff: 00 00 8b 45 ec 83 c0 01 89 45 ec e9 d7 ff ff ff  ...E.E..
(lldb) 
0x12f0f: 48 89 c1 89 d0 48 89 4d e0 89 45 dc 48 8d 7d f0  HH.M..E.H.}.
0x12f1f: e8 aa 0e 00 00 48 8b 7d e0 e8 7d 0e 00 00 0f 1f  .H.}..}.
```
Note that I just hit enter after the first read memory. Can we just take 
advantage of this feature instead of addind the "--continue" option?



Comment at: lldb/source/Target/TraceInstructionDumper.cpp:28
+if (!m_cursor_up->GoToId(*m_options.id)) {
+  s.Printf("invalid instruction id\n");
+  SetNoMoreData();

When there is no formatter, you can just use Stream::PutCString(...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122254

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

[Lldb-commits] [PATCH] D122429: [lldb/Plugin] Sort the ScriptedProcess' thread list before creating threads

[Med Ismail Bennani via Phabricator via lldb-commits]

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

With Scripted Processes, in order to create scripted threads, the blueprint
provides a dictionary that have each thread index as the key with the respective
thread instance as the pair value.

In Python, this is fine because a dictionary key can be of any type including
integer types:

  >>> {1: "one", 2: "two", 10: "ten"}
  {1: 'one', 2: 'two', 10: 'ten'}

However, when the python dictionary gets bridged to C++ with convert it to a
`StructuredData::Dictionary` that uses a `std::map`
for storage.

Because `std::map` is an ordered container and ours uses the `ConstString`
type for keys, the thread indices gets converted to strings which makes the
dictionary sorted alphabetically, instead of numerically.

If the ScriptedProcesse has 10 threads or more, it causes thread “10”
(and higher) to be after thread “1”, but before thread “2”.

In order to solve this, this sorts the thread info dictionary keys
numerically, before iterating over them to create ScriptedThreads.

rdar://90327854

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122429

Files:
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -304,33 +304,53 @@
 
   StructuredData::DictionarySP thread_info_sp = 
GetInterface().GetThreadsInfo();
 
-  // FIXME: Need to sort the dictionary otherwise the thread ids won't match 
the
-  // thread indices.
-
   if (!thread_info_sp)
 return ScriptedInterface::ErrorWithMessage(
 LLVM_PRETTY_FUNCTION,
 "Couldn't fetch thread list from Scripted Process.", error);
 
-  auto create_scripted_thread =
-  [this, &old_thread_list, &error,
-   &new_thread_list](ConstString key, StructuredData::Object *val) -> bool 
{
+  // Because `StructuredData::Dictionary` uses a `std::map` for storage, each item is sorted based on the key alphabetical
+  // order. Since `GetThreadsInfo` provides thread indices as the key element,
+  // thread info comes ordered alphabetically, instead of numerically, so we
+  // need to sort the thread indices before creating thread.
+
+  StructuredData::ArraySP keys = thread_info_sp->GetKeys();
+
+  std::vector sorted_keys(keys->GetSize());
+  auto sort_keys = [&sorted_keys](StructuredData::Object *item) -> bool {
+if (!item)
+  return false;
+
+llvm::StringRef value = item->GetStringValue();
+size_t idx = 0;
+
+// Make sure the provided index is actually an integer
+if (!llvm::to_integer(value, idx))
+  return false;
+
+sorted_keys[idx] = value;
+return true;
+  };
+
+  size_t thread_count = thread_info_sp->GetSize();
+
+  if (!keys->ForEach(sort_keys) || sorted_keys.size() != thread_count)
+// Might be worth showing the unsorted thread list instead of return early.
+return ScriptedInterface::ErrorWithMessage(
+LLVM_PRETTY_FUNCTION, "Couldn't sort thread list.", error);
+
+  for (size_t idx = 0; idx < thread_count; idx++) {
+llvm::StringRef key = sorted_keys[idx];
+StructuredData::ObjectSP val = thread_info_sp->GetValueForKey(key);
 if (!val)
   return ScriptedInterface::ErrorWithMessage(
   LLVM_PRETTY_FUNCTION, "Invalid thread info object", error);
 
-lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
-if (!llvm::to_integer(key.AsCString(), tid))
+uint32_t thread_idx = UINT32_MAX;
+if (!llvm::to_integer(key, thread_idx))
   return ScriptedInterface::ErrorWithMessage(
-  LLVM_PRETTY_FUNCTION, "Invalid thread id", error);
-
-if (ThreadSP thread_sp =
-old_thread_list.FindThreadByID(tid, false /*=can_update*/)) {
-  // If the thread was already in the old_thread_list,
-  // just add it back to the new_thread_list.
-  new_thread_list.AddThread(thread_sp);
-  return true;
-}
+  LLVM_PRETTY_FUNCTION, "Invalid thread index", error);
 
 auto thread_or_error = ScriptedThread::Create(*this, val->GetAsGeneric());
 
@@ -346,16 +366,12 @@
   return ScriptedInterface::ErrorWithMessage(
   LLVM_PRETTY_FUNCTION,
   llvm::Twine("Invalid Register Context for thread " +
-  llvm::Twine(key.AsCString()))
+  llvm::Twine(key.data()))
   .str(),
   error);
 
 new_thread_list.AddThread(thread_sp);
-
-return true;
-  };
-
-  thread_info_sp->ForEach(create_scripted_thread);
+  }
 
   return new_thread_list.GetSize(false) > 0;
 }


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
=

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

[walter erquinigo via Phabricator via lldb-commits]

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

fix some comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122254

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  lldb/source/Target/TraceInstructionDumper.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceStartStop.py
  lldb/test/API/commands/trace/TestTraceTimestampCounters.py

Index: lldb/test/API/commands/trace/TestTraceTimestampCounters.py
===
--- lldb/test/API/commands/trace/TestTraceTimestampCounters.py
+++ lldb/test/API/commands/trace/TestTraceTimestampCounters.py
@@ -19,7 +19,7 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["\[0\] \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
+patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -32,7 +32,7 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["\[0\] \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
+patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -45,7 +45,7 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["\[0\] \[tsc=unavailable\] 0x00400511movl"])
+patterns=["0: \[tsc=unavailable\] 0x00400511movl"])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
Index: lldb/test/API/commands/trace/TestTraceStartStop.py
===
--- lldb/test/API/commands/trace/TestTraceStartStop.py
+++ lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -114,29 +114,29 @@
 self.expect("thread trace dump instructions -f",
 patterns=[f'''thread #1: tid = .*
   a.out`main \+ 4 at main.cpp:2
-\[ 0\] {ADDRESS_REGEX}movl'''])
+0: {ADDRESS_REGEX}movl'''])
 
 # We can reconstruct the instructions up to the second line
 self.expect("n")
 self.expect("thread trace dump instructions -f",
 patterns=[f'''thread #1: tid = .*
   a.out`main \+ 4 at main.cpp:2
-\[ 0\] {ADDRESS_REGEX}movl .*
+0: {ADDRESS_REGEX}movl .*
   a.out`main \+ 11 at main.cpp:4
-\[ 1\] {ADDRESS_REGEX}movl .*
-\[ 2\] {ADDRESS_REGEX}jmp  .* ; <\+28> at main.cpp:4
-\[ 3\] {ADDRESS_REGEX}cmpl .*
-\[ 4\] {ADDRESS_REGEX}jle  .* ; <\+20> at main.cpp:5'''])
+1: {ADDRESS_REGEX}movl .*
+2: {ADDRESS_REGEX}jmp  .* ; <\+28> at main.cpp:4
+3: {ADDRESS_REGEX}cmpl .*
+4: {ADDRESS_REGEX}jle  .* ; <\+20> at main.cpp:5'''])
 
 self.expect("thread trace dump instructions",
 patterns=[f'''thread #1: tid = .*
   a.out`main \+ 32 at main.cpp:4
-\[  0\] {ADDRESS_REGEX}jle  .* ; <\+20> at main.cpp:5
-\[ -1\] {ADDRESS_REGEX}cmpl .*
-\[ -2\] {ADDRESS_REGEX}jmp  .* ; <\+28> at main.cpp:4
-\[ -3\] {ADDRESS_REGEX}movl .*
+4: {ADDRESS_REGEX}jle  .* ; <\+20> at main.cpp:5
+3: {ADDRESS_REGEX}cmpl .*
+2: {ADDRESS_REGEX}jmp  .* ; <\+28> at main.cpp:4
+1: {ADDRESS_REGEX}movl .*
   a.out`main \+ 4 at main.cpp:2
-\[ -4\] {ADDRESS_REGEX}movl .* '''])
+0: {ADDRESS_REGEX}movl .* '''])
 
 # We stop tracing
 self.expect("thread trace stop")
@@ -152,12 +152,12 @@
 self.expect("thread trace dump instructions -f",
 patterns=[f'''thread #1: tid = .*
   a.out`main \+ 20 at main.cpp:5
-\[ 0\] {ADDRESS_REGEX}xorl'''])
+0: {ADDRESS_REGEX}xorl'''])
 
 self.expect("thread trace dump instructions",
 patterns=[f'''thread #1: tid = .*
   a.out`main \+ 20 at main.cpp:5
-\[  0\] {ADDRESS_REGEX}xorl'''])
+0: {ADDRESS_REGEX}xorl'''])
 
 self.expect("c")
 # Now the process has finished, so the commands should fail
Index: lldb/test/API/commands/trace/TestTraceDumpInstructions.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInstructions.py
+++ lldb/test/API/commands/trace/TestTraceDumpInstruct

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:50
+  /// Additional options for configuring the dumping.
+  TraceInstructionDumper(lldb::TraceCursorUP &&cursor_up, Stream &s,
+ const TraceInstructionDumperOptions &options);

clayborg wrote:
> Do we want the stream in the options?
we can't do it nicely, because we are adding a TraceInstructionDumper variable 
in CommandObjectTraceDumpInstructions::CommandOptions, where we don't have a 
stream available. I imagine this being the only variable that we can't put in 
the options, so it should be okay


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122254

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

[Lldb-commits] [PATCH] D122041: [llvm][utils] Fix llvm::Optional summary provider

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In D122041#3405821 , @kastiglione 
wrote:

>> There's some CMake trickery
>
> Can you shed some more light on this? Am I understanding right: these 
> formatters are llvm, but the test would be in lldb? It seems weird that 
> something in llvm/ would only have tests in lldb/.

You need an LLDB in order to test the dataformatter, that's why I thought the 
test makes most sense there. The other natural place would be 
`cross-project-tests/`.

> Would it be bad to move this file into lldb, and then we can test there?

It's nice & consistent to have the LLDB data formatters next to the GDB 
dataformatters, but I don't very strong feelings about this.

> With llvm being a monorepo, a source checkout should have access at any path. 
> Maybe the issue would be llvm installations and packages that don't include 
> lldb too?

The test would need to build a debuginfo-enabled binary that links against 
Support (which, I now realize, does not need debug info), So maybe this is not 
going to be much more complicated than any unit tests we are building.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122041

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

[Lldb-commits] [PATCH] D122373: [NFC] Be more lazy about looking up simulator SDKs on macs

[Adrian Prantl via Phabricator via lldb-commits]

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

Fair word of warning — i've broken more than one LLDB build in the past while 
messing with this code :-)

So you're basically sinking the calls to GetXcodeSDKPath into 
PlatformAppleSimulator. That looks safe & good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122373

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

[Lldb-commits] [lldb] 9951578 - Don't search for sim SDK path until we know we need it

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2022-03-24T15:44:57-07:00
New Revision: 99515783a63cd5018fa9231872dc5c8b13d64947

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

LOG: Don't search for sim SDK path until we know we need it

When iterating over all Platforms looking for the best one, on a Mac the
Simulator platforms (iOS, tvOS, watchOS) will first find their SDK
directory by calling xcrun, then decide if they should activate or not.
When that SDK is absent, the call to xcrun to find it can be very slow.
This patch delays that directory search until we know we're activating
this platform, so non-simulator environments don't pay a perf cost ever
time they go through the list of platforms.

Differential Revision: https://reviews.llvm.org/D122373
rdar://87960090

Added: 


Modified: 
lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h

Removed: 




diff  --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm 
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index 426e044e7ab72..3d68fcdb653b9 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -379,6 +379,13 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
 args.AppendArgument("--sdk");
 args.AppendArgument(sdk);
 
+Log *log = GetLog(LLDBLog::Host);
+if (log) {
+  std::string cmdstr;
+  args.GetCommandString(cmdstr);
+  log->Printf("GetXcodeSDK() running shell cmd '%s'", cmdstr.c_str());
+}
+
 int status = 0;
 int signo = 0;
 std::string output_str;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
index 4659ad6d6e13b..1728d3855b1a2 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -274,13 +274,23 @@ std::vector 
PlatformAppleSimulator::GetSupportedArchitectures(
   return result;
 }
 
+static llvm::StringRef GetXcodeSDKDir(std::string preferred,
+  std::string secondary) {
+  llvm::StringRef sdk;
+  sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(preferred)));
+  if (sdk.empty())
+sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(secondary)));
+  return sdk;
+}
+
 PlatformSP PlatformAppleSimulator::CreateInstance(
 const char *class_name, const char *description, ConstString plugin_name,
 llvm::SmallVector supported_arch,
 llvm::Triple::OSType preferred_os,
 llvm::SmallVector supported_os,
 llvm::SmallVector supported_triples,
-llvm::StringRef sdk, lldb_private::XcodeSDK::Type sdk_type,
+std::string sdk_name_preferred, std::string sdk_name_secondary,
+lldb_private::XcodeSDK::Type sdk_type,
 CoreSimulatorSupport::DeviceType::ProductFamilyID kind, bool force,
 const ArchSpec *arch) {
   Log *log = GetLog(LLDBLog::Platform);
@@ -338,6 +348,8 @@ PlatformSP PlatformAppleSimulator::CreateInstance(
   if (create) {
 LLDB_LOGF(log, "%s::%s() creating platform", class_name, __FUNCTION__);
 
+llvm::StringRef sdk =
+GetXcodeSDKDir(sdk_name_preferred, sdk_name_secondary);
 return PlatformSP(new PlatformAppleSimulator(
 class_name, description, plugin_name, preferred_os, supported_triples,
 sdk, sdk_type, kind));
@@ -514,15 +526,6 @@ static bool shouldSkipSimulatorPlatform(bool force, const 
ArchSpec *arch) {
  !arch->TripleEnvironmentWasSpecified();
 }
 
-static llvm::StringRef GetXcodeSDKDir(std::string preferred,
-  std::string secondary) {
-  llvm::StringRef sdk;
-  sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(preferred)));
-  if (sdk.empty())
-sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(secondary)));
-  return sdk;
-}
-
 static const char *g_ios_plugin_name = "ios-simulator";
 static const char *g_ios_description = "iPhone simulator platform plug-in.";
 
@@ -540,10 +543,6 @@ struct PlatformiOSSimulator {
   static PlatformSP CreateInstance(bool force, const ArchSpec *arch) {
 if (shouldSkipSimulatorPlatform(force, arch))
   return nullptr;
-llvm::StringRef sdk;
-sdk = HostInfo::GetXcodeSDKPath(XcodeSDK("iPhoneSimulator.Internal.sdk"));
-if (sdk.empty())
-  sdk = HostInfo::GetXcodeSDKPath(XcodeSDK("iPhoneSimulator.sdk"));
 
 return PlatformAppleSimulator::CreateInstance(
 "PlatformiOSSimulator", g_ios_description,
@@ -566,7 +565,7 @@ struct PlatformiOSSimulator {
 #endif
 #endif
 },
-GetXcodeSDKDir("iPhoneSimulator.Internal.sdk", "iPhoneSimulator.sdk"),
+ 

[Lldb-commits] [PATCH] D122373: [NFC] Be more lazy about looking up simulator SDKs on macs

[Jason Molenda via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG99515783a63c: Don't search for sim SDK path until we 
know we need it (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122373

Files:
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h

Index: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
@@ -59,7 +59,8 @@
  llvm::Triple::OSType preferred_os,
  llvm::SmallVector supported_os,
  llvm::SmallVector supported_triples,
- llvm::StringRef sdk, XcodeSDK::Type sdk_type,
+ std::string sdk_name_preferred, std::string sdk_name_secondary,
+ XcodeSDK::Type sdk_type,
  CoreSimulatorSupport::DeviceType::ProductFamilyID kind,
  bool force, const ArchSpec *arch);
 
Index: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -274,13 +274,23 @@
   return result;
 }
 
+static llvm::StringRef GetXcodeSDKDir(std::string preferred,
+  std::string secondary) {
+  llvm::StringRef sdk;
+  sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(preferred)));
+  if (sdk.empty())
+sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(secondary)));
+  return sdk;
+}
+
 PlatformSP PlatformAppleSimulator::CreateInstance(
 const char *class_name, const char *description, ConstString plugin_name,
 llvm::SmallVector supported_arch,
 llvm::Triple::OSType preferred_os,
 llvm::SmallVector supported_os,
 llvm::SmallVector supported_triples,
-llvm::StringRef sdk, lldb_private::XcodeSDK::Type sdk_type,
+std::string sdk_name_preferred, std::string sdk_name_secondary,
+lldb_private::XcodeSDK::Type sdk_type,
 CoreSimulatorSupport::DeviceType::ProductFamilyID kind, bool force,
 const ArchSpec *arch) {
   Log *log = GetLog(LLDBLog::Platform);
@@ -338,6 +348,8 @@
   if (create) {
 LLDB_LOGF(log, "%s::%s() creating platform", class_name, __FUNCTION__);
 
+llvm::StringRef sdk =
+GetXcodeSDKDir(sdk_name_preferred, sdk_name_secondary);
 return PlatformSP(new PlatformAppleSimulator(
 class_name, description, plugin_name, preferred_os, supported_triples,
 sdk, sdk_type, kind));
@@ -514,15 +526,6 @@
  !arch->TripleEnvironmentWasSpecified();
 }
 
-static llvm::StringRef GetXcodeSDKDir(std::string preferred,
-  std::string secondary) {
-  llvm::StringRef sdk;
-  sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(preferred)));
-  if (sdk.empty())
-sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(secondary)));
-  return sdk;
-}
-
 static const char *g_ios_plugin_name = "ios-simulator";
 static const char *g_ios_description = "iPhone simulator platform plug-in.";
 
@@ -540,10 +543,6 @@
   static PlatformSP CreateInstance(bool force, const ArchSpec *arch) {
 if (shouldSkipSimulatorPlatform(force, arch))
   return nullptr;
-llvm::StringRef sdk;
-sdk = HostInfo::GetXcodeSDKPath(XcodeSDK("iPhoneSimulator.Internal.sdk"));
-if (sdk.empty())
-  sdk = HostInfo::GetXcodeSDKPath(XcodeSDK("iPhoneSimulator.sdk"));
 
 return PlatformAppleSimulator::CreateInstance(
 "PlatformiOSSimulator", g_ios_description,
@@ -566,7 +565,7 @@
 #endif
 #endif
 },
-GetXcodeSDKDir("iPhoneSimulator.Internal.sdk", "iPhoneSimulator.sdk"),
+"iPhoneSimulator.Internal.sdk", "iPhoneSimulator.sdk",
 XcodeSDK::Type::iPhoneSimulator,
 CoreSimulatorSupport::DeviceType::ProductFamilyID::iPhone, force, arch);
   }
@@ -604,7 +603,7 @@
 #endif
 #endif
 },
-GetXcodeSDKDir("AppleTVSimulator.Internal.sdk", "AppleTVSimulator.sdk"),
+"AppleTVSimulator.Internal.sdk", "AppleTVSimulator.sdk",
 XcodeSDK::Type::AppleTVSimulator,
 CoreSimulatorSupport::DeviceType::ProductFamilyID::appleTV, force,
 arch);
@@ -646,7 +645,7 @@
 #endif
 #endif
 },
-GetXcodeSDKDir("WatchSimulator.Internal.sdk", "WatchSimulator.sdk"),
+"WatchSimulator.Internal.sdk", "WatchSimulator.sdk",
 XcodeSDK::Type::WatchSimulator,
 CoreSimulatorSupport::DeviceType::ProductFamilyID::appleWatch, force,
 arch);
Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===

[Lldb-commits] [PATCH] D122422: [lldb/crashlog] Parse thread fields and pass it to crashlog scripted process

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 418093.
mib retitled this revision from "[lldb/crashlog] Parse more thread fields and 
pass it to crashlog scripted process" to "[lldb/crashlog] Parse thread fields 
and pass it to crashlog scripted process".
mib edited the summary of this revision.
mib added a comment.

Add thread name to the crashlog scripted process blueprint.


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

https://reviews.llvm.org/D122422

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
@@ -4,17 +4,17 @@
 
 # RUN: cp %S/Inputs/scripted_crashlog.ips %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":160, "bar":20, "foo":24}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -a -i %t.crash' 2>&1 -o "bt all" | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -a -i %t.crash' 2>&1 -o "thread list" -o "bt all" | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
 # CHECK: (lldb) process status
 # CHECK-NEXT: Process 24991 stopped
-# CHECK-NEXT: * thread #3, name = 'CrashLogScriptedThread.thread-2', stop reason = EXC_BAD_ACCESS
+# CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS
 # CHECK-NEXT: frame #0: 0x0001047f5970 scripted_crashlog_json.test.tmp.out`bar
 
 # CHECK: (lldb) thread backtrace
-# CHECK-NEXT: * thread #3, name = 'CrashLogScriptedThread.thread-2', stop reason = EXC_BAD_ACCESS
+# CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS
 # CHECK-NEXT:   * frame #0: 0x0001047f5970 scripted_crashlog_json.test.tmp.out`bar
 # CHECK-NEXT: frame #1: 0x0001047f5998 scripted_crashlog_json.test.tmp.out`foo
 # CHECK-NEXT: frame #2: 0x0001047f5b04 scripted_crashlog_json.test.tmp.out`compute_pow
@@ -24,14 +24,21 @@
 # CHECK-NEXT: frame #6: 0x00018bf5326c libsystem_pthread.dylib`_pthread_start
 # CHECK-NEXT: frame #7: 0x00018bf4e08c libsystem_pthread.dylib`thread_start
 
+# CHECK: (lldb) thread list
+# CHECK-NEXT: Process 24991 stopped
+# CHECK-NEXT:  thread #1: tid = 0x4ea840, 0x00018bf17854 libsystem_kernel.dylib`__ulock_wait{{.*}}, queue = 'com.apple.main-thread'
+# CHECK-NEXT:  thread #2: tid = 0x4ea850, 0x0001047f59e8 scripted_crashlog_json.test.tmp.out`call_and_wait
+# CHECK-NEXT: * thread #3: tid = 0x4ea851, 0x0001047f5970 scripted_crashlog_json.test.tmp.out`bar{{.*}}, stop reason = EXC_BAD_ACCESS
+
+
 # CHECK: (lldb) bt all
-# CHECK-NEXT:   thread #1, name = 'CrashLogScriptedThread.thread-0'
+# CHECK-NEXT:   thread #1
 # CHECK-NEXT: frame #0: 0x00018bf17854 libsystem_kernel.dylib`__ulock_wait
 # CHECK-NEXT: frame #1: 0x00018bf555a0 libsystem_pthread.dylib`_pthread_join
 # CHECK-NEXT: frame #2: 0x00018beae9c0 libc++.1.dylib`std::__1::thread::join
 # CHECK-NEXT: frame #3: 0x0001047f5bb8 scripted_crashlog_json.test.tmp.out`main
 # CHECK-NEXT: frame #4: 0x000104ae5088 dyld`start
-# CHECK-NEXT:   thread #2, name = 'CrashLogScriptedThread.thread-1'
+# CHECK-NEXT:   thread #2
 # CHECK-NEXT: frame #0: 0x0001047f59e8 scripted_crashlog_json.test.tmp.out`call_and_wait
 # CHECK-NEXT: frame #1: 0x0001047f59d4 scripted_crashlog_json.test.tmp.out`call_and_wait
 # CHECK-NEXT: frame #2: 0x0001047f7690 scripted_crashlog_json.test.tmp.out`decltype
@@ -39,7 +46,7 @@
 # CHECK-NEXT: frame #4: 0x0001047f6d58 scripted_crashlog_json.test.tmp.out`void* std::__1::__thread_proxy
 # CHECK-NEXT: frame #5: 0x00018bf5326c libsystem_pthread.dylib`_pthread_start
 # CHECK-NEXT: frame #6: 0x00018bf4e08c libsystem_pthread.dylib`thread_start
-# CHECK-NEXT: * thread #3, name = 'CrashLogScriptedThread.thread-2', stop reason = EXC_BAD_ACCESS
+# CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS
 # CHECK-NEXT:   * frame #0: 0x0001047f5970 scripted_crashlog_json.test.tmp.out`bar
 # CHECK-NEXT: frame #1: 0x0001047f5998 scripted_crashlog_json.test.tmp.out`foo
 # CHECK-NEXT: frame #2: 0x0001047f5b04 scripted_crashlog_json.test.tmp.out`compute_pow
Index: lldb/examples/python/scripted_process/scripted_process.py
===
--- lldb/examples/python/scripted_process/scripted_process.py
+++ lldb/examples/python/scripted_process/scripted_process.py
@@ -219,8 +219,8 @@
 self.scripted_process = N

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

[Zequan Wu via Phabricator via lldb-commits]

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

- change live debugging test to cpp file.
- mirror fix on previous change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121967

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/inline_sites.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/inline_sites.s
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/inline_sites_live.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/inline_sites.s
  lldb/test/Shell/SymbolFile/NativePDB/inline_sites.test
  lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
  lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp
@@ -157,6 +157,7 @@
 // CHECK-NEXT: | |-ParmVarDecl {{.*}} argc 'int'
 // CHECK-NEXT: | `-ParmVarDecl {{.*}} argv 'char **'
 // CHECK-NEXT: |-FunctionDecl {{.*}} __scrt_common_main_seh 'int ()' static 
+// CHECK-NEXT: |-FunctionDecl {{.*}} invoke_main 'int ()' inline
 // CHECK-NEXT: `-FunctionDecl {{.*}} Function 'int (int, char)'
 // CHECK-NEXT:   |-ParmVarDecl {{.*}} Param1 'int'
 // CHECK-NEXT:   `-ParmVarDecl {{.*}} Param2 'char'
Index: lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
@@ -0,0 +1,111 @@
+// clang-format off
+// REQUIRES: system-windows
+
+// RUN: %build --opt basic -o %t.exe -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+// RUN: %p/Inputs/inline_sites_live.lldbinit 2>&1 | FileCheck %s
+
+inline int bar(int bar_param) {
+volatile int bar_local = bar_param * 3;
+return bar_local;
+}
+
+inline int foo(int foo_param) {
+volatile int foo_local = foo_param + 1;
+++foo_local;
+return bar(foo_local);
+}
+
+int main(int argc, char** argv) {
+volatile int main_local = foo(argc);
+return main_local;
+}
+
+// CHECK:  (lldb) b main
+// CHECK-NEXT: Breakpoint 1: where = {{.*}}`main + 4 [inlined] foo at {{.*}}:14
+// CHECK-NEXT: (lldb) run
+// CHECK-NEXT: Process {{.*}} stopped
+// CHECK-NEXT: * thread #1, stop reason = breakpoint 1.1
+// CHECK-NEXT: frame #0: {{.*}}`main [inlined] foo(foo_param=1) at {{.*}}:14
+// CHECK-NEXT:11   }
+// CHECK-NEXT:12
+// CHECK-NEXT:13   inline int foo(int foo_param) {
+// CHECK-NEXT: -> 14   volatile int foo_local = foo_param + 1;
+// CHECK-NEXT:15   ++foo_local;
+// CHECK-NEXT:16   return bar(foo_local);
+// CHECK-NEXT:17   }
+// CHECK-NEXT: Process {{.*}} launched
+// CHECK-NEXT: (lldb) step
+// CHECK-NEXT: Process {{.*}} stopped
+// CHECK-NEXT: * thread #1, stop reason = step in
+// CHECK-NEXT: frame #0: {{.*}}`main [inlined] foo(foo_param=) at {{.*}}:15
+// CHECK-NEXT:12
+// CHECK-NEXT:13   inline int foo(int foo_param) {
+// CHECK-NEXT:14   volatile int foo_local = foo_param + 1;
+// CHECK-NEXT: -> 15   ++foo_local;
+// CHECK-NEXT:16   return bar(foo_local);
+// CHECK-NEXT:17   }
+// CHECK-NEXT:18
+// CHECK-NEXT: (lldb) p foo_local
+// CHECK-NEXT: (volatile int) $0 = 2
+// CHECK-NEXT: (lldb) step
+// CHECK-NEXT: Process {{.*}} stopped
+// CHECK-NEXT: * thread #1, stop reason = step in
+// CHECK-NEXT: frame #0: {{.*}}`main [inlined] foo(foo_param=) at {{.*}}:16
+// CHECK-NEXT:13   inline int foo(int foo_param) {
+// CHECK-NEXT:14   volatile int foo_local = foo_param + 1;
+// CHECK-NEXT:15   ++foo_local;
+// CHECK-NEXT: -> 16   return bar(foo_local);
+// CHECK-NEXT:17   }
+// CHECK-NEXT:18
+// CHECK-NEXT:19   int main(int argc, char** argv) {
+// CHECK-NEXT: (lldb) p foo_local
+// CHECK-NEXT: (volatile int) $1 = 3
+// CHECK-NEXT: (lldb) step
+// CHECK-NEXT: Process {{.*}} stopped
+// CHECK-NEXT: * thread #1, stop reason = step in
+// CHECK-NEXT: frame #0: {{.*}}`main [inlined] bar(bar_param=3) at {{.*}}:9
+// CHECK-NEXT:6
+// CHECK-NEXT:7
+// CHECK-NEXT:8inline int bar(int bar_param) {
+// CHECK-NEXT: -> 9volatile int bar_local = bar_param * 3;
+// CHECK-NEXT:10   return bar_local;
+// CHECK-NEXT:11   }
+// CHECK-NEXT:12
+// CHECK-NEXT: (lldb) step
+// CHECK-NEXT: Process {{.*}} stopped
+// CHECK-NEXT: * thread #1, stop reason = step in
+// CHECK-NEXT: frame #0: {{.*}}`main [inlined] bar(bar_param=) at {{.*}}:10
+// CHECK-NEXT:7
+// CHECK-NEXT:8inline int bar(int bar_param) 

[Lldb-commits] [PATCH] D122429: [lldb/Plugin] Sort the ScriptedProcess' thread list before creating threads

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:320
+
+  std::vector sorted_keys(keys->GetSize());
+  auto sort_keys = [&sorted_keys](StructuredData::Object *item) -> bool {

Won't this overflow when you return the example from the summary?

```
{1: "one", 2: "two", 10: "ten"}
```

Would it be easier to populate our own `map` and iterate over 
that below? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122429

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

[Lldb-commits] [PATCH] D122461: [lldb] Add a fuzzer for target create

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added a subscriber: mgorny.
Herald added a project: All.
JDevlieghere requested review of this revision.

This patch adds a fuzzer that interprets inputs as object files and makes lldb 
create targets from them. It is very similar to the llvm-dwarfdump fuzzer which 
found a bunch of issues in libObject. I let it run in the background for an 
hour or so and it identified 15  or so inputs that cause lldb to crash.


https://reviews.llvm.org/D122461

Files:
  lldb/tools/CMakeLists.txt
  lldb/tools/lldb-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-fuzzer-target.cpp
  lldb/tools/lldb-fuzzer/utils/CMakeLists.txt
  lldb/tools/lldb-fuzzer/utils/TempFile.cpp
  lldb/tools/lldb-fuzzer/utils/TempFile.h

Index: lldb/tools/lldb-fuzzer/utils/TempFile.h
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/utils/TempFile.h
@@ -0,0 +1,27 @@
+//===-- Utils.h ---===//
+//
+// 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 "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+namespace lldb_fuzzer {
+
+class TempFile {
+public:
+  TempFile() = default;
+  ~TempFile();
+
+  static std::unique_ptr Create(uint8_t *data, size_t size);
+  llvm::StringRef GetPath() { return m_path.str(); }
+
+private:
+  llvm::SmallString<128> m_path;
+};
+
+} // namespace lldb_fuzzer
Index: lldb/tools/lldb-fuzzer/utils/TempFile.cpp
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/utils/TempFile.cpp
@@ -0,0 +1,33 @@
+//===-- Utils.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 "llvm/Support/FileSystem.h"
+#include 
+
+using namespace lldb_fuzzer;
+using namespace llvm;
+
+TempFile::~TempFile() {
+  if (!m_path.empty())
+sys::fs::remove(m_path.str(), true);
+}
+
+std::unique_ptr TempFile::Create(uint8_t *data, size_t size) {
+  int fd;
+  std::unique_ptr temp_file = std::make_unique();
+  std::error_code ec = sys::fs::createTemporaryFile("lldb-fuzzer", "input", fd,
+temp_file->m_path);
+  if (ec)
+return nullptr;
+
+  raw_fd_ostream os(fd, true);
+  os.write(reinterpret_cast(data), size);
+  os.close();
+
+  return temp_file;
+}
Index: lldb/tools/lldb-fuzzer/utils/CMakeLists.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/utils/CMakeLists.txt
@@ -0,0 +1,6 @@
+add_lldb_library(lldbFuzzerUtils
+  TempFile.cpp
+
+  LINK_COMPONENTS
+Support
+  )
Index: lldb/tools/lldb-fuzzer/lldb-fuzzer-target.cpp
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-fuzzer-target.cpp
@@ -0,0 +1,34 @@
+//===-- lldb-fuzzer-target.cpp - Fuzz LLDB ===//
+//
+// 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 
+
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+
+using namespace lldb;
+using namespace lldb_fuzzer;
+using namespace llvm;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+  return 0;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  auto file = TempFile::Create(data, size);
+  if (!file)
+return 1;
+
+  SBDebugger debugger = SBDebugger::Create(false);
+  SBTarget target = debugger.CreateTarget(file->GetPath().data());
+  debugger.DeleteTarget(target);
+  SBDebugger::Destroy(debugger);
+
+  return 0;
+}
Index: lldb/tools/lldb-fuzzer/CMakeLists.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/CMakeLists.txt
@@ -0,0 +1,17 @@
+add_subdirectory(utils)
+
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_llvm_fuzzer(lldb-fuzzer-target
+  EXCLUDE_FROM_ALL
+  lldb-fuzzer-target.cpp
+  )
+
+target_link_libraries(lldb-fuzzer-target
+  PRIVATE
+  liblldb
+  lldbFuzzerUtils
+  )
+
Index: lldb/tools/CMakeLists.txt
===
--- lldb/tools/CMak

[Lldb-commits] [PATCH] D122461: [lldb] Add a fuzzer for target create

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Fix ASCII art


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

https://reviews.llvm.org/D122461

Files:
  lldb/tools/CMakeLists.txt
  lldb/tools/lldb-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-fuzzer-target.cpp
  lldb/tools/lldb-fuzzer/utils/CMakeLists.txt
  lldb/tools/lldb-fuzzer/utils/TempFile.cpp
  lldb/tools/lldb-fuzzer/utils/TempFile.h

Index: lldb/tools/lldb-fuzzer/utils/TempFile.h
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/utils/TempFile.h
@@ -0,0 +1,27 @@
+//===-- TempFile.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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+namespace lldb_fuzzer {
+
+class TempFile {
+public:
+  TempFile() = default;
+  ~TempFile();
+
+  static std::unique_ptr Create(uint8_t *data, size_t size);
+  llvm::StringRef GetPath() { return m_path.str(); }
+
+private:
+  llvm::SmallString<128> m_path;
+};
+
+} // namespace lldb_fuzzer
Index: lldb/tools/lldb-fuzzer/utils/TempFile.cpp
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/utils/TempFile.cpp
@@ -0,0 +1,33 @@
+//===-- TempFile.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 "llvm/Support/FileSystem.h"
+#include 
+
+using namespace lldb_fuzzer;
+using namespace llvm;
+
+TempFile::~TempFile() {
+  if (!m_path.empty())
+sys::fs::remove(m_path.str(), true);
+}
+
+std::unique_ptr TempFile::Create(uint8_t *data, size_t size) {
+  int fd;
+  std::unique_ptr temp_file = std::make_unique();
+  std::error_code ec = sys::fs::createTemporaryFile("lldb-fuzzer", "input", fd,
+temp_file->m_path);
+  if (ec)
+return nullptr;
+
+  raw_fd_ostream os(fd, true);
+  os.write(reinterpret_cast(data), size);
+  os.close();
+
+  return temp_file;
+}
Index: lldb/tools/lldb-fuzzer/utils/CMakeLists.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/utils/CMakeLists.txt
@@ -0,0 +1,6 @@
+add_lldb_library(lldbFuzzerUtils
+  TempFile.cpp
+
+  LINK_COMPONENTS
+Support
+  )
Index: lldb/tools/lldb-fuzzer/lldb-fuzzer-target.cpp
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-fuzzer-target.cpp
@@ -0,0 +1,34 @@
+//===-- lldb-fuzzer-target.cpp - Fuzz target creation -===//
+//
+// 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 
+
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+
+using namespace lldb;
+using namespace lldb_fuzzer;
+using namespace llvm;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+  return 0;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  auto file = TempFile::Create(data, size);
+  if (!file)
+return 1;
+
+  SBDebugger debugger = SBDebugger::Create(false);
+  SBTarget target = debugger.CreateTarget(file->GetPath().data());
+  debugger.DeleteTarget(target);
+  SBDebugger::Destroy(debugger);
+
+  return 0;
+}
Index: lldb/tools/lldb-fuzzer/CMakeLists.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/CMakeLists.txt
@@ -0,0 +1,17 @@
+add_subdirectory(utils)
+
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_llvm_fuzzer(lldb-fuzzer-target
+  EXCLUDE_FROM_ALL
+  lldb-fuzzer-target.cpp
+  )
+
+target_link_libraries(lldb-fuzzer-target
+  PRIVATE
+  liblldb
+  lldbFuzzerUtils
+  )
+
Index: lldb/tools/CMakeLists.txt
===
--- lldb/tools/CMakeLists.txt
+++ lldb/tools/CMakeLists.txt
@@ -6,6 +6,7 @@
 # i.e. if a target requires it as dependency. The typical
 # example is `check-lldb`. So, we pass EXCLUDE_FROM_ALL here.
 add_subdirectory(lldb-test EXCLUDE_FROM_ALL)
+add_subdirectory(lldb-fuzzer EXCLUDE_FROM_ALL)
 
 add_lldb_tool_subdirectory(lldb-instr)
 add_lldb_tool_sub