date:20200428

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

2020-04-28 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D78801#2004501 , @paolosev wrote:

> I am adding all the pieces to this patch to make the whole picture clearer; I 
> thought to add a piece at the time to simplify reviews, but probably it ended 
> up making things more obscure. I can always split this patch later and I need 
> to refactor everything anyway.
>
> So, the idea is to use DWARF as debug info for Wasm, as it is already 
> supported by LLVM and Emscripten. For this we introduced some time ago the 
> plugin classes ObjectFileWasm, SymbolVendorWasm and DynamicLoaderWasmDYLD. 
> However, WebAssembly is peculiarly different from the native targets. When 
> source code is compiled to Wasm, Clang produces a module that contains Wasm 
> bytecode (a bit like it happens with Java and C#) and the DWARF info refers 
> to this bytecode.
>  The Wasm module then runs in a Wasm runtime. (It is also possible to 
> AoT-compile Wasm to native, but this is outside the scope of this patch).
>
> Therefore, LLDB cannot debug Wasm by just controlling the inferior process, 
> but it needs to talk with the Wasm engine to query the Wasm engine state. For 
> example, for backtrace, only the runtime knows what is the current call 
> stack. Hence the idea of using the gdb-remote protocol: if a Wasm engine has 
> a GDB stub LLDB can connect to it to start a debugging session and access its 
> state.
>
> Wasm execution is defined in terms of a stack machine. There are no registers 
> (besides the implicit IP) and most Wasm instructions push/pop values 
> into/from a virtual stack. Besides the stack the other possible stores are a 
> set of parameters and locals defined in the function, a set of global 
> variables defined in the module and the module memory, which is separated 
> from the code address space.
>
> The DWARF debug info to evaluate the value of variables is defined in terms 
> of these constructs. For example, we can have something like this in DWARF:
>
>   0x5a88:  DW_TAG_variable
> DW_AT_location(0x06f3: 
>[0x0840, 0x0850): DW_OP_WASM_location 
> 0x0 +8, DW_OP_stack_value)
> DW_AT_name("xx")
> DW_AT_type(0x2b17 "float")
> […]
>
>
> Which says that on that address range the value of ‘xx’ can be evaluated as 
> the content of the 8th local. Here DW_OP_WASM_location is a Wasm-specific 
> opcode, with two args, the first defines the store (0: Local, 1: Global, 2: 
> the operand stack) and the index in that store. In most cases the value of 
> the variable could be retrieved from the Wasm memory instead.

So is there memory to be read from the WASM runtime? Couldn't 
DW_OP_WASM_location 0x0 +8 be turned into an address that can be used to read 
the variable? It is also unclear what DW_OP_stack_value is used for here. The 
DWARF expression has no idea how many bytes to read for this value unless each 
virtual stack location knows how big it is? What happens if you have an array 
of a million items? That will not fit on the DWARF expression stack and each 
member would need to be read from memory?

It seems like the DW_OP_WASM_location + args should result in the address of 
the variable being pushed into the stack and the DW_OP_stack_value should be 
removed. This would mean at the end of the expression the address of the 
variable is on the stack and LLDB will just read it using the normal memory 
read? Am I missing something? Are there multiple memory regions? Are variables 
not considered to be in memory?

> So, when LLDB wants to evaluate this variable, in 
> `DWARFExpression::Evaluate()`, it needs to know what is the current the value 
> of the Wasm locals, or to access the memory, and for this it needs to query 
> the Wasm engine.

> This is why there are changes to DWARFExpression::Evaluate(), to support the 
> DW_OP_WASM_location case, and this is also why I created a class that derives 
> from ProcessGDBRemote and overrides ReadMemory() in order to query the wasm 
> engine. Also Value::GetValueAsData() needs to be modified when the value is 
> retrieved from Wasm memory.

It would be fine to ask the lldb_private::Process class to evaluate any unknown 
DWARF expression opcodes like DW_OP_WASM_location and return the result.

Why do we need to override read memory? Is there more than one address space? 
Can't the DWARF expression DW_OP_WASM_location + args turn into an address that 
normal read memory can access? Or are the virtual stacks separate and not 
actually in the address space? If the virtual stack slot for locals/globals and 
stack values always know their sizes and can provide the contents, the 
DW_OP_WASM_location opcode should end up creating a buffer just like 
DW_OP_piece does and the value will be contained in there in the DWARF 
expression and there is no need for the DW_OP_stack_value

[Lldb-commits] [PATCH] D78839: [lldb-vscode] Add an option for loading core files

2020-04-28 Thread Pavel Labath via Phabricator via lldb-commits

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

Looks good. It may be interesting to tack a couple of resume/step-over/etc 
commands at the end of the test to make just the whole thing doesn't go berserk 
when those operations fail.

In D78839#2006698 , @wallace wrote:

> I also removed that bad use of auto. It seems that all StringRefs gotten from 
> the JSON object are null-terminated.

Ok, json::parse is documented to return "self-contained" json object which owns 
its strings. It's not a big stretch to assume that those owned strings will 
also be null terminated, though it still seems moderately dangerous...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78839

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

[Lldb-commits] [lldb] f07f2ce - [lldb/unittest] Adjust CheckIPSupport function to avoid double-consume of llvm::Error

2020-04-28 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2020-04-28T13:35:07+02:00
New Revision: f07f2cee9b4e19df78a19c9b2b552e2e5ba9d478

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

LOG: [lldb/unittest] Adjust CheckIPSupport function to avoid double-consume of 
llvm::Error

The problem caught by clang-tidy and reported by Tobias Bosch.

Added: 


Modified: 
lldb/unittests/Host/SocketTestUtilities.cpp

Removed: 




diff  --git a/lldb/unittests/Host/SocketTestUtilities.cpp 
b/lldb/unittests/Host/SocketTestUtilities.cpp
index c56b5f7a7809..e2006b85115d 100644
--- a/lldb/unittests/Host/SocketTestUtilities.cpp
+++ b/lldb/unittests/Host/SocketTestUtilities.cpp
@@ -101,16 +101,19 @@ static bool CheckIPSupport(llvm::StringRef Proto, 
llvm::StringRef Addr) {
  "Creating a canary {0} TCP socket failed: {1}.",
  Proto, Err)
  .str();
-  if (Err.isA() &&
-  errorToErrorCode(std::move(Err)) ==
-  std::make_error_code(std::errc::address_not_available)) {
+  bool HasAddrNotAvail = false;
+  handleAllErrors(std::move(Err), [&](std::unique_ptr ECErr) {
+if (ECErr->convertToErrorCode() ==
+std::make_error_code(std::errc::address_not_available))
+  HasAddrNotAvail = true;
+  });
+  if (HasAddrNotAvail) {
 GTEST_LOG_(WARNING)
 << llvm::formatv(
"Assuming the host does not support {0}. Skipping test.", Proto)
.str();
 return false;
   }
-  consumeError(std::move(Err));
   GTEST_LOG_(WARNING) << "Continuing anyway. The test will probably fail.";
   return true;
 }



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

[Lldb-commits] [lldb] 5cee8dd - [lldb-vscode] A couple of small style fixes

2020-04-28 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2020-04-28T13:35:07+02:00
New Revision: 5cee8ddcc7535c95e4dd00fc428d2a52630eaa31

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

LOG: [lldb-vscode] A couple of small style fixes

to make the code conform to llvm style better:
- avoid use of auto where the type is not obivous
- avoid StringRef::data where it is not needed

No functional change intended.

Added: 


Modified: 
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/JSONUtils.cpp 
b/lldb/tools/lldb-vscode/JSONUtils.cpp
index 2e7a57ee397e..8fcf179b29aa 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -9,9 +9,9 @@
 #include 
 
 #include "llvm/ADT/Optional.h"
-
 #include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
 
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBBreakpointLocation.h"
@@ -41,8 +41,8 @@ llvm::StringRef GetAsString(const llvm::json::Value &value) {
 
 // Gets a string from a JSON object using the key, or returns an empty string.
 llvm::StringRef GetString(const llvm::json::Object &obj, llvm::StringRef key) {
-  if (auto value = obj.getString(key))
-return GetAsString(*value);
+  if (llvm::Optional value = obj.getString(key))
+return *value;
   return llvm::StringRef();
 }
 
@@ -114,13 +114,9 @@ std::vector GetStrings(const 
llvm::json::Object *obj,
   strs.push_back(value.getAsString()->str());
   break;
 case llvm::json::Value::Number:
-case llvm::json::Value::Boolean: {
-  std::string s;
-  llvm::raw_string_ostream strm(s);
-  strm << value;
-  strs.push_back(strm.str());
+case llvm::json::Value::Boolean:
+  strs.push_back(llvm::to_string(value));
   break;
-}
 case llvm::json::Value::Null:
 case llvm::json::Value::Object:
 case llvm::json::Value::Array:

diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 04a2125e3456..b267ea088b1c 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -531,15 +531,14 @@ void request_attach(const llvm::json::Object &request) {
   g_vsc.exit_commands = GetStrings(arguments, "exitCommands");
   auto attachCommands = GetStrings(arguments, "attachCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
-  const auto debuggerRoot = GetString(arguments, "debuggerRoot");
+  const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
   // in the debug map of the main executable have relative paths which require
   // the lldb-vscode binary to have its working directory set to that relative
   // root for the .o files in order to be able to load debug info.
-  if (!debuggerRoot.empty()) {
-llvm::sys::fs::set_current_path(debuggerRoot.data());
-  }
+  if (!debuggerRoot.empty())
+llvm::sys::fs::set_current_path(debuggerRoot);
 
   // Run any initialize LLDB commands the user specified in the launch.json
   g_vsc.RunInitCommands();
@@ -1363,15 +1362,14 @@ void request_launch(const llvm::json::Object &request) {
   g_vsc.exit_commands = GetStrings(arguments, "exitCommands");
   auto launchCommands = GetStrings(arguments, "launchCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
-  const auto debuggerRoot = GetString(arguments, "debuggerRoot");
+  const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
   // in the debug map of the main executable have relative paths which require
   // the lldb-vscode binary to have its working directory set to that relative
   // root for the .o files in order to be able to load debug info.
-  if (!debuggerRoot.empty()) {
-llvm::sys::fs::set_current_path(debuggerRoot.data());
-  }
+  if (!debuggerRoot.empty())
+llvm::sys::fs::set_current_path(debuggerRoot);
 
   // Run any initialize LLDB commands the user specified in the launch.json.
   // This is run before target is created, so commands can't do anything with



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

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

2020-04-28 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

> I don't have a way to write a C-based test for this as I don't know how to 
> craft a user expression in C that will make such a thing. I asked around a 
> bit and nobody had an easy way to do this.

Grepping clang's tests for `weak_odr` was very educational. The most portable 
way to produce this linkage seems to be:

  template void f() {}
  template void f();

As for whether this patch is correct -- I don't know. The weak_odr functions 
are allowed to be replaced (that's the `weak` part), but the replacement is 
supposed to be equivalent (the `odr` part). So, strictly speaking, it may be ok 
to just export this function, but if we wanted to be closer to what happens for 
realz, we should check first if there isn't a more definitive version available 
elsewhere...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78972



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

[Lldb-commits] [PATCH] D74892: [lldb][cmake] Also use local submodule visibility on Darwin

2020-04-28 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8baa0b9439b5: [lldb][cmake] Also use local submodule 
visibility on Darwin (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74892

Files:
  llvm/CMakeLists.txt


Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -401,11 +401,10 @@
 option(LLVM_ENABLE_MODULES "Compile with C++ modules enabled." OFF)
 if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
   option(LLVM_ENABLE_MODULE_DEBUGGING "Compile with -gmodules." ON)
-  option(LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY "Compile with 
-fmodules-local-submodule-visibility." OFF)
 else()
   option(LLVM_ENABLE_MODULE_DEBUGGING "Compile with -gmodules." OFF)
-  option(LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY "Compile with 
-fmodules-local-submodule-visibility." ON)
 endif()
+option(LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY "Compile with 
-fmodules-local-submodule-visibility." ON)
 option(LLVM_ENABLE_LIBCXX "Use libc++ if available." OFF)
 option(LLVM_STATIC_LINK_CXX_STDLIB "Statically link the standard library." OFF)
 option(LLVM_ENABLE_LLD "Use lld as C and C++ linker." OFF)


Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -401,11 +401,10 @@
 option(LLVM_ENABLE_MODULES "Compile with C++ modules enabled." OFF)
 if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
   option(LLVM_ENABLE_MODULE_DEBUGGING "Compile with -gmodules." ON)
-  option(LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY "Compile with -fmodules-local-submodule-visibility." OFF)
 else()
   option(LLVM_ENABLE_MODULE_DEBUGGING "Compile with -gmodules." OFF)
-  option(LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY "Compile with -fmodules-local-submodule-visibility." ON)
 endif()
+option(LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY "Compile with -fmodules-local-submodule-visibility." ON)
 option(LLVM_ENABLE_LIBCXX "Use libc++ if available." OFF)
 option(LLVM_STATIC_LINK_CXX_STDLIB "Statically link the standard library." OFF)
 option(LLVM_ENABLE_LLD "Use lld as C and C++ linker." OFF)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

2020-04-28 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D78801#2007083 , @clayborg wrote:

> It would be fine to ask the lldb_private::Process class to evaluate any 
> unknown DWARF expression opcodes like DW_OP_WASM_location and return the 
> result.


While that idea has occurred to me too, I am not convinced it is a good one:

- it replaces one odd dependency with another one. Why should a Process need to 
know how to evaluate a DWARF expression? Or even that DWARF exists for that 
matter? This seems totally unrelated to what other Process functions are doing 
currently...
- I am not sure it even completely removes wasm knowledge from e.g. 
DWARFExpression -- the class would presumably still need to know how to parse 
this opcode.
- the interface could get very complicated if we wanted to implement typed 
stacks present in DWARF5 -- presumably the API would need to return the type of 
the result, in addition to its value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78882: [IR] Replace all uses of CallBase::getCalledValue() with getCalledOperand().

2020-04-28 Thread Craig Topper via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa58b62b4a2b9: [IR] Replace all uses of 
CallBase::getCalledValue() with getCalledOperand(). (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78882

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  lldb/source/Expression/IRInterpreter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/CodeGen/FastISel.h
  llvm/include/llvm/IR/AbstractCallSite.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/lib/Analysis/AliasAnalysisEvaluator.cpp
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/lib/Analysis/Lint.cpp
  llvm/lib/Analysis/MemorySSA.cpp
  llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
  llvm/lib/Analysis/StackSafetyAnalysis.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp
  llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/CodeGen/WasmEHPrepare.cpp
  llvm/lib/CodeGen/WinEHPrepare.cpp
  llvm/lib/ExecutionEngine/Interpreter/Execution.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AArch64/AArch64PromoteConstant.cpp
  llvm/lib/Target/AMDGPU/AMDGPUAnnotateKernelFeatures.cpp
  llvm/lib/Target/AMDGPU/AMDGPUFixFunctionBitcasts.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/ARM/ARMFastISel.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
  llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86WinEHState.cpp
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/IPO/CalledValuePropagation.cpp
  llvm/lib/Transforms/IPO/GlobalOpt.cpp
  llvm/lib/Transforms/IPO/PruneEH.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/lib/Transforms/Instrumentation/ValueProfilePlugins.inc
  llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
  llvm/lib/Transforms/Utils/Evaluator.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Utils/LowerInvoke.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/LTO/X86/type-mapping-bug3.ll
  llvm/tools/llvm-diff/DiffConsumer.cpp
  llvm/tools/llvm-diff/DifferenceEngine.cpp
  mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

Index: mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
===
--- mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -720,7 +720,7 @@
   op = b.create(loc, tys, b.getSymbolRefAttr(callee->getName()),
 ops);
 } else {
-  Value calledValue = processValue(ci->getCalledValue());
+  Value calledValue = processValue(ci->getCalledOperand());
   if (!calledValue)
 return failure();
   ops.insert(ops.begin(), calledValue);
@@ -766,7 +766,7 @@
   ops, blocks[ii->getNormalDest()], normalArgs,
   blocks[ii->getUnwindDest()], unwindArgs);
 } else {
-  ops.insert(ops.begin(), processValue(ii->getCalledValue()));
+  ops.insert(ops.begin(), processValue(ii->getCalledOperand()));
   op = b.create(loc, tys, ops, blocks[ii->getNormalDest()],
   normalArgs, blocks[ii->getUnwindDest()],
   unwindArgs);
Index: llvm/tools/llvm-diff/DifferenceEngine.cpp
===
--- llvm/tools/llvm-diff/DifferenceEngine.cpp
+++ llvm/tools/llvm-diff/DifferenceEngine.cpp
@@ -224,7 +224,7 @@
 
   bool diffCallSites(CallBase &L, CallBase &R, bool Compl

[Lldb-commits] [PATCH] D78882: [IR] Replace all uses of CallBase::getCalledValue() with getCalledOperand().

2020-04-28 Thread David Blaikie via Phabricator via lldb-commits

dblaikie accepted this revision.
dblaikie added a comment.

Looks great - thanks for the pointer type fixup(s) along the way too!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78882



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

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

2020-04-28 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

In D78972#2007493 , @labath wrote:

> > I don't have a way to write a C-based test for this as I don't know how to 
> > craft a user expression in C that will make such a thing. I asked around a 
> > bit and nobody had an easy way to do this.
>
> Grepping clang's tests for `weak_odr` was very educational. The most portable 
> way to produce this linkage seems to be:
>
>   template void f() {}
>   template void f();
>
>
> As for whether this patch is correct -- I don't know. The weak_odr functions 
> are allowed to be replaced (that's the `weak` part), but the replacement is 
> supposed to be equivalent (the `odr` part). So, strictly speaking, it may be 
> ok to just export this function, but if we wanted to be closer to what 
> happens for realz, we should check first if there isn't a more definitive 
> version available elsewhere...

I can't get the expression parser to successfully run this code and then call 
the function in a later evaluation.  Apparently we're not all the way up to 
creating & instantiating templates.  I tried it with and without this patch and 
it didn't make any difference.

In the context of the REPL & top-level, I'm not sure how we would find the 
"equivalent version".  But if it is supposed to be equivalent, is that 
important for functions the REPL & top-level are generating?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78972

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

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

2020-04-28 Thread Vedant Kumar via Phabricator via lldb-commits

vsk added a comment.

Friendly ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77843



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

[Lldb-commits] [PATCH] D78839: [lldb-vscode] Add an option for loading core files

2020-04-28 Thread walter erquinigo via Phabricator via lldb-commits

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

added the test suggested by @labath


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78839

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
  lldb/test/API/tools/lldb-vscode/coreFile/linux-x86_64.core
  lldb/test/API/tools/lldb-vscode/coreFile/linux-x86_64.out
  lldb/tools/lldb-vscode/README.md
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -218,8 +218,12 @@
 			},
 			"exitCommands": {
 "type": "array",
-	"description": "Commands executed at the end of debugging session.",
-	"default": []
+"description": "Commands executed at the end of debugging session.",
+"default": []
+			},
+			"coreFile": {
+"type": "string",
+"description": "Path to the core file to debug."
 			}
 		}
 	}
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -530,7 +530,9 @@
   g_vsc.stop_commands = GetStrings(arguments, "stopCommands");
   g_vsc.exit_commands = GetStrings(arguments, "exitCommands");
   auto attachCommands = GetStrings(arguments, "attachCommands");
-  g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
+  llvm::StringRef core_file = GetString(arguments, "coreFile");
+  g_vsc.stop_at_entry =
+  core_file.empty() ? GetBoolean(arguments, "stopOnEntry", false) : true;
   const auto debuggerRoot = GetString(arguments, "debuggerRoot");
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
@@ -569,7 +571,10 @@
 // Disable async events so the attach will be successful when we return from
 // the launch call and the launch will happen synchronously
 g_vsc.debugger.SetAsync(false);
-g_vsc.target.Attach(attach_info, error);
+if (core_file.empty())
+  g_vsc.target.Attach(attach_info, error);
+else
+  g_vsc.target.LoadCore(core_file.data(), error);
 // Reenable async events
 g_vsc.debugger.SetAsync(true);
   } else {
@@ -584,7 +589,7 @@
 
   SetSourceMapFromArguments(*arguments);
 
-  if (error.Success()) {
+  if (error.Success() && core_file.empty()) {
 auto attached_pid = g_vsc.target.GetProcess().GetProcessID();
 if (attached_pid == LLDB_INVALID_PROCESS_ID) {
   if (attachCommands.empty())
Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -181,15 +181,15 @@
 
 ### Loading a Core File
 
-Loading a core file can use the `"attach"` request along with the
-`"attachCommands"` to implement a custom attach:
+This loads the coredump file `/cores/123.core` associated with the program
+`/tmp/a.out`:
 
 ```javascript
 {
-  "name": "Attach to Name (wait)",
+  "name": "Load coredump",
   "type": "lldb-vscode",
   "request": "attach",
-  "attachCommands": ["target create -c /path/to/123.core /path/to/executable"],
-  "stopOnEntry": false
+  "coreFile": "/cores/123.core",
+  "program": "/tmp/a.out"
 }
 ```
Index: lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
@@ -0,0 +1,43 @@
+"""
+Test lldb-vscode coreFile attaching
+"""
+
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+import os
+
+
+class TestVSCode_coreFile(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfWindows
+@skipIfRemote
+@skipIfLLVMTargetMissing("X86")
+def test_core_file(self):
+current_dir = os.path.dirname(os.path.realpath(__file__))
+exe_file = os.path.join(current_dir, "linux-x86_64.out")
+core_file = os.path.join(current_dir, "linux-x86_64.core")
+
+self.create_debug_adaptor()
+self.attach(exe_file, coreFile=core_file)
+
+expected_frames = [
+{'column': 0, 'id': 524288, 'line': 4, 'name': 'bar', 'source': {'name': 'main.c', 'path': '/home/labath/test/main.c'}},
+{'column': 0, 'id': 524289, 'line': 10, 'name': 'foo', 'source': {'name': 'main.c', 'path': '/home/labath/test/main.c'}},
+{'column': 0, 'id': 524290, 'line': 16, 'name': '_s

[Lldb-commits] [lldb] 8372582 - [lldb-vscode] Add an option for loading core files

2020-04-28 Thread Walter Erquinigo via lldb-commits


Author: Walter Erquinigo
Date: 2020-04-28T13:03:02-07:00
New Revision: 83725822c52535c239b1a7991023f96bfbc95568

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

LOG: [lldb-vscode] Add an option for loading core files

Summary:
Currently loading core files on lldb-vscode is broken because there's a check 
in the attach workflow that asserts that the PID is valid, which of course 
fails for this case.
Hence, I'm adding a "coreFile" argument for the attach request, which does the 
work correctly.

I don't know how to test it effectively so that it runs on the buildbots and 
the debugger can in fact makes sense of it. Anyway, the change has been 
relatively simple.

Reviewers: labath, clayborg

Subscribers: lldb-commits

Tags: #lldb

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

Added: 
lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
lldb/test/API/tools/lldb-vscode/coreFile/linux-x86_64.core
lldb/test/API/tools/lldb-vscode/coreFile/linux-x86_64.out

Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/tools/lldb-vscode/README.md
lldb/tools/lldb-vscode/lldb-vscode.cpp
lldb/tools/lldb-vscode/package.json

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index 3b06fa07d048..790628d2b0fd 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -239,7 +239,7 @@ def continue_to_exit(self, exitCode=0):
 
 def attach(self, program=None, pid=None, waitFor=None, trace=None,
initCommands=None, preRunCommands=None, stopCommands=None,
-   exitCommands=None, attachCommands=None):
+   exitCommands=None, attachCommands=None, coreFile=None):
 '''Build the default Makefile target, create the VSCode debug adaptor,
and attach to the process.
 '''
@@ -257,7 +257,7 @@ def cleanup():
 program=program, pid=pid, waitFor=waitFor, trace=trace,
 initCommands=initCommands, preRunCommands=preRunCommands,
 stopCommands=stopCommands, exitCommands=exitCommands,
-attachCommands=attachCommands)
+attachCommands=attachCommands, coreFile=coreFile)
 if not (response and response['success']):
 self.assertTrue(response['success'],
 'attach failed (%s)' % (response['message']))

diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 805de43f25f3..643a313dca1c 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -450,7 +450,7 @@ def replay_packets(self, replay_file_path):
 def request_attach(self, program=None, pid=None, waitFor=None, trace=None,
initCommands=None, preRunCommands=None,
stopCommands=None, exitCommands=None,
-   attachCommands=None):
+   attachCommands=None, coreFile=None):
 args_dict = {}
 if pid is not None:
 args_dict['pid'] = pid
@@ -471,6 +471,8 @@ def request_attach(self, program=None, pid=None, 
waitFor=None, trace=None,
 args_dict['exitCommands'] = exitCommands
 if attachCommands:
 args_dict['attachCommands'] = attachCommands
+if coreFile:
+args_dict['coreFile'] = coreFile
 command_dict = {
 'command': 'attach',
 'type': 'request',

diff  --git a/lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py 
b/lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
new file mode 100644
index ..55efd91d827a
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
@@ -0,0 +1,43 @@
+"""
+Test lldb-vscode coreFile attaching
+"""
+
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+import os
+
+
+class TestVSCode_coreFile(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfWindows
+@skipIfRemote
+@skipIfLLVMTargetMissing("X86")
+def test_core_file(self):
+current_dir = os.path.dirname(os.path.realpath(__file__))
+exe_file = os.path.join(current_dir, "linux-x86_64.out")
+core

[Lldb-commits] [PATCH] D78839: [lldb-vscode] Add an option for loading core files

2020-04-28 Thread Phabricator via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG83725822c525: [lldb-vscode] Add an option for loading core 
files (authored by Walter Erquinigo ).

Changed prior to commit:
  https://reviews.llvm.org/D78839?vs=260731&id=260748#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78839

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
  lldb/test/API/tools/lldb-vscode/coreFile/linux-x86_64.core
  lldb/test/API/tools/lldb-vscode/coreFile/linux-x86_64.out
  lldb/tools/lldb-vscode/README.md
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -218,8 +218,12 @@
 			},
 			"exitCommands": {
 "type": "array",
-	"description": "Commands executed at the end of debugging session.",
-	"default": []
+"description": "Commands executed at the end of debugging session.",
+"default": []
+			},
+			"coreFile": {
+"type": "string",
+"description": "Path to the core file to debug."
 			}
 		}
 	}
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -530,7 +530,9 @@
   g_vsc.stop_commands = GetStrings(arguments, "stopCommands");
   g_vsc.exit_commands = GetStrings(arguments, "exitCommands");
   auto attachCommands = GetStrings(arguments, "attachCommands");
-  g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
+  llvm::StringRef core_file = GetString(arguments, "coreFile");
+  g_vsc.stop_at_entry =
+  core_file.empty() ? GetBoolean(arguments, "stopOnEntry", false) : true;
   const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
@@ -568,7 +570,10 @@
 // Disable async events so the attach will be successful when we return from
 // the launch call and the launch will happen synchronously
 g_vsc.debugger.SetAsync(false);
-g_vsc.target.Attach(attach_info, error);
+if (core_file.empty())
+  g_vsc.target.Attach(attach_info, error);
+else
+  g_vsc.target.LoadCore(core_file.data(), error);
 // Reenable async events
 g_vsc.debugger.SetAsync(true);
   } else {
@@ -583,7 +588,7 @@
 
   SetSourceMapFromArguments(*arguments);
 
-  if (error.Success()) {
+  if (error.Success() && core_file.empty()) {
 auto attached_pid = g_vsc.target.GetProcess().GetProcessID();
 if (attached_pid == LLDB_INVALID_PROCESS_ID) {
   if (attachCommands.empty())
Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -181,15 +181,15 @@
 
 ### Loading a Core File
 
-Loading a core file can use the `"attach"` request along with the
-`"attachCommands"` to implement a custom attach:
+This loads the coredump file `/cores/123.core` associated with the program
+`/tmp/a.out`:
 
 ```javascript
 {
-  "name": "Attach to Name (wait)",
+  "name": "Load coredump",
   "type": "lldb-vscode",
   "request": "attach",
-  "attachCommands": ["target create -c /path/to/123.core /path/to/executable"],
-  "stopOnEntry": false
+  "coreFile": "/cores/123.core",
+  "program": "/tmp/a.out"
 }
 ```
Index: lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
@@ -0,0 +1,43 @@
+"""
+Test lldb-vscode coreFile attaching
+"""
+
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+import os
+
+
+class TestVSCode_coreFile(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfWindows
+@skipIfRemote
+@skipIfLLVMTargetMissing("X86")
+def test_core_file(self):
+current_dir = os.path.dirname(os.path.realpath(__file__))
+exe_file = os.path.join(current_dir, "linux-x86_64.out")
+core_file = os.path.join(current_dir, "linux-x86_64.core")
+
+self.create_debug_adaptor()
+self.attach(exe_file, coreFile=core_file)
+
+expected_frames = [
+{'column': 0, 'id': 524288, 'line': 4, 'name': 'bar', 'source': {'name': 'main.c', 'path': '/home/labath/test/main.c'}},
+

[Lldb-commits] [PATCH] D78712: [lldb/Host] Improve error messages on unowned read files

2020-04-28 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 260751.
mib edited the summary of this revision.
mib added a comment.

Removed the introduced `FileSystem::ReadableByCurrentUser` to rely only 
`FileSystem::Open` as Pavel suggested.
Updated the tests to reflect the changes introduced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78712

Files:
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/test/API/commands/target/basic/TestTargetCommand.py

Index: lldb/test/API/commands/target/basic/TestTargetCommand.py
===
--- lldb/test/API/commands/target/basic/TestTargetCommand.py
+++ lldb/test/API/commands/target/basic/TestTargetCommand.py
@@ -326,7 +326,7 @@
 @no_debug_info_test
 def test_target_create_nonexistent_core_file(self):
 self.expect("target create -c doesntexist", error=True,
-substrs=["core file 'doesntexist' doesn't exist"])
+substrs=["Cannot open 'doesntexist': No such file or directory"])
 
 # Write only files don't seem to be supported on Windows.
 @skipIfWindows
@@ -335,12 +335,20 @@
 tf = tempfile.NamedTemporaryFile()
 os.chmod(tf.name, stat.S_IWRITE)
 self.expect("target create -c '" + tf.name + "'", error=True,
-substrs=["core file '", "' is not readable"])
+substrs=["Cannot open '", "': Permission denied"])
+
+@skipIfWindows
+@no_debug_info_test
+def test_target_create_unowned_core_file(self):
+  tf = tempfile.NamedTemporaryFile()
+  os.chmod(tf.name, 0)
+  self.expect("target create -c '" + tf.name + "'", error=True,
+  substrs=["Cannot open '", "': Permission denied"])
 
 @no_debug_info_test
 def test_target_create_nonexistent_sym_file(self):
 self.expect("target create -s doesntexist doesntexisteither", error=True,
-substrs=["invalid symbol file path 'doesntexist'"])
+substrs=["Cannot open '", "': No such file or directory"])
 
 @skipIfWindows
 @no_debug_info_test
@@ -357,7 +365,7 @@
 tf = tempfile.NamedTemporaryFile()
 os.chmod(tf.name, stat.S_IWRITE)
 self.expect("target create -s '" + tf.name + "' no_exe", error=True,
-substrs=["symbol file '", "' is not readable"])
+substrs=["Cannot open '", "': Permission denied"])
 
 @no_debug_info_test
 def test_target_delete_all(self):
Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -91,20 +91,16 @@
 // Resolve any executable within a bundle on MacOSX
 Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
 
-if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
+auto file =
+FileSystem::Instance().Open(resolved_module_spec.GetFileSpec(),
+lldb_private::File::eOpenOptionRead);
+
+if (!file) {
+  error.SetErrorStringWithFormatv("Cannot open '{0}': {1}.",
+  resolved_module_spec.GetFileSpec(),
+  llvm::toString(file.takeError()));
+} else
   error.Clear();
-else {
-  const uint32_t permissions = FileSystem::Instance().GetPermissions(
-  resolved_module_spec.GetFileSpec());
-  if (permissions && (permissions & eFilePermissionsEveryoneR) == 0)
-error.SetErrorStringWithFormat(
-"executable '%s' is not readable",
-resolved_module_spec.GetFileSpec().GetPath().c_str());
-  else
-error.SetErrorStringWithFormat(
-"unable to find executable for '%s'",
-resolved_module_spec.GetFileSpec().GetPath().c_str());
-}
   } else {
 if (m_remote_platform_sp) {
   error =
@@ -188,15 +184,15 @@
   }
 
   if (error.Fail() || !exe_module_sp) {
-if (FileSystem::Instance().Readable(
-resolved_module_spec.GetFileSpec())) {
+if (FileSystem::Instance().Open(resolved_module_spec.GetFileSpec(),
+lldb_private::File::eOpenOptionRead)) {
   error.SetErrorStringWithFormat(
   "'%s' doesn't contain any '%s' platform architectures: %s",
   resolved_module_spec.GetFileSpec().GetPath().c_str(),
   GetPluginName().GetCString(), arch_names.GetData());
 } else {
   error.SetErrorStringWithFormat(
-  "'%s' is not readable",
+  "'%s' is not readable by the current user",
   resolved_module_spec.GetFileSpec().GetPath().c_str());
 }
   }
Index: lldb/source/

[Lldb-commits] [PATCH] D78712: [lldb/Host] Improve error messages on unowned read files

2020-04-28 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

I also prefer this approach, seems less fragile from a testing perspective and 
reduces the different code paths.




Comment at: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:102
+  llvm::toString(file.takeError()));
+} else
   error.Clear();

We should be consistent with the braces, both clauses are single-line. 
Personally I'd refactor the code and make this an early return instead.



Comment at: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:187
   if (error.Fail() || !exe_module_sp) {
-if (FileSystem::Instance().Readable(
-resolved_module_spec.GetFileSpec())) {
+if (FileSystem::Instance().Open(resolved_module_spec.GetFileSpec(),
+lldb_private::File::eOpenOptionRead)) {

Not your fault, but it seems weird that we're doing this check after using the 
`resolved_module_spec` and only if it failed.. Shouldn't we try to read it 
first and stop early if that fails? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78712



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

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

2020-04-28 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added inline comments.



Comment at: lldb/source/DataFormatters/StringPrinter.cpp:175
+  constexpr unsigned max_buffer_size = 7;
+  uint8_t *data = new uint8_t[max_buffer_size];
+  switch (escape_style) {

I really wish we could get ride of the naked `new`. It seems possible.

- We know the buffer size
- We know the "expected" escaped_len
- We could write something like a `make_StringPrinterBufferPointer` in the same 
spirit as `make_unique`

The problem we want to avoid w/ the naked `new` is that the code becomes more 
complicated over time and somehow a later change disconnects the `new` w/ the 
creation of the `StringPrinterBufferPointer` which manages the lifetime. 

This comment applies to the code later on as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77843



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

2020-04-28 Thread Paolo Severini via Phabricator via lldb-commits

paolosev added a comment.

In D78801#2007083 , @clayborg wrote:

> So is there memory to be read from the WASM runtime? Couldn't 
> DW_OP_WASM_location 0x0 +8 be turned into an address that can be used to read 
> the variable? It is also unclear what DW_OP_stack_value is used for here. The 
> DWARF expression has no idea how many bytes to read for this value unless 
> each virtual stack location knows how big it is? What happens if you have an 
> array of a million items? That will not fit on the DWARF expression stack and 
> each member would need to be read from memory?
>
> It seems like the DW_OP_WASM_location + args should result in the address of 
> the variable being pushed into the stack and the DW_OP_stack_value should be 
> removed. This would mean at the end of the expression the address of the 
> variable is on the stack and LLDB will just read it using the normal memory 
> read? Am I missing something? Are there multiple memory regions? Are 
> variables not considered to be in memory?

`DW_OP_WASM_location 0x0 +8` is not really in memory, or more precisely, its 
runtime representation is an internal detail of the Wasm runtime.
WebAssembly code has a peculiar structure, see for example 
https://developer.mozilla.org/en-US/docs/WebAssembly/Understanding_the_text_format
 for more details.
Ignoring memory for a moment, there are no registers in Wasm and instead Wasm 
instructions read/write from/to function locals, module globals and stack 
operands, which can only have one of these types:

- i32: 32-bit integer
- i64: 64-bit integer
- f32: 32-bit floating point
- f64: 64-bit floating point

There is still is ongoing work in LLVM 
(https://reviews.llvm.org/D77353/new/#change-OJue38RNV2Gz) to define the 
perfect representation of these Wasm constructs in DWARF, but currently what is 
generated by LLVM has this format:

  DW_OP_WASM_location wasm-op index

Where:

  DW_OP_WASM_location := 0xED
  wasm-op := wasm-local | wasm-global | wasm-operand-stack

  wasm-local := 0x00 i:uleb128(The value is located in the 
currently executing function’s index-th local)
  wasm-global := 0x01 i:uleb128   (The value is located in the index-th 
global)
  wasm-operand-stack := 0x02 i:uleb128(The value is located in the indexth 
entry on the operand stack)

https://yurydelendik.github.io/webassembly-dwarf/ describes the rationale 
behind the addition of DW_OP_WASM_location to DWARF.

For example a function like:

  int add(int a, int b) { return a + b; }

Could be compiled to:

  (func $add (param $lhs i32) (param $rhs i32) (result i32)
local.get $lhs
local.get $rhs
i32.add)

and the corresponding DWARF would describe that:

- the value of `a` can be retrieved as DW_OP_WASM_location 0 0 (first local in 
the function)
- the value of `b` can be retrieved as DW_OP_WASM_location 0 1 (second local in 
the function)

Of course DW_OP_WASM_location cannot represent the values of complex types. For 
a complex type like a C++ array with 1M items:

  uint8_t* p = new uint8_t[100];

DWARF would describe the location of the pointer `p` (for example it could be 
in a local) and then the debugger would find DWARF info that describes its 
type, it would then send a request like `qWasmLocal` to get the value from the 
Wasm runtime, and receive the value of p, let’s say 0x8000c000.
From there LLDB might query to read chunks of memory starting from 0x8000c000, 
if the user asks to explore the content of the array.

Note that not all Wasm code requires the new location description 
`DW_OP_WASM_location`. In many cases locations are encoded using preexisting 
codes. For example when compiling without optimizations, -O0, almost all 
variables are encoded as a delta from the frame pointer register. But the frame 
pointer register itself is often defined as a DW_OP_WASM_location:

  0x0112:   DW_TAG_subprogram
  DW_AT_low_pc  (0x0761)
  DW_AT_high_pc (0x07db)
  DW_AT_frame_base  (DW_OP_WASM_location 0x0 +4, 
DW_OP_stack_value)
  DW_AT_linkage_name
("_Z10quick_sortI4NodeIyE4lessIS1_EEvPT_xT0_")
  DW_AT_name("quick_sort, 
less > >")
  DW_AT_decl_file   
("C:\dev\test\emscripten_tests\sort\.\sort.h")
  DW_AT_decl_line   (45)
  DW_AT_external(true)

  0x012a: DW_TAG_formal_parameter
DW_AT_location  (DW_OP_fbreg +20)
DW_AT_name  ("array")
DW_AT_type  (0x03bb "Node*")
…

This would also work because LLDB would send a qWasmLocal to calculate the 
value of the frame register.

> Why do we need to override read memory? Is there more than one address space? 
> Can't the DWARF expression DW_OP_WASM_location + args turn into an address 
> that normal read memory can access? Or are the virtual s

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

2020-04-28 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D78801#2007795 , @labath wrote:

> In D78801#2007083 , @clayborg wrote:
>
> > It would be fine to ask the lldb_private::Process class to evaluate any 
> > unknown DWARF expression opcodes like DW_OP_WASM_location and return the 
> > result.
>
>
> While that idea has occurred to me too, I am not convinced it is a good one:
>
> - it replaces one odd dependency with another one. Why should a Process need 
> to know how to evaluate a DWARF expression? Or even that DWARF exists for 
> that matter? This seems totally unrelated to what other Process functions are 
> doing currently...

But it is what people do in reality. DW_OP_low_user and DW_OP_high_user are 
ranges that are made available to people to customize their DWARF opcodes. If 
you don't handle it, you are hosed and can't show a variable location. And to 
make things worse, two different compilers could both use the same value in 
that range. So they made DWARF expressions customizable with no real attempt to 
make them function for different architectures. that is unless you standardize 
it and make a real opcode that gets accepted into DWARF. The kind of DWARF 
location opcode that is being used here could easily be generalized into a 
DW_OP_get_stack_variable with a bunch of args, but at some point you have to 
talk to someone that is in communication with the runtime of the thing you are 
debugging to get the answer. So I do believe asking the process for this is not 
out of scope.

> - I am not sure it even completely removes wasm knowledge from e.g. 
> DWARFExpression -- the class would presumably still need to know how to parse 
> this opcode.

It is true and this is another hole in the "people can extend DWARF easily" 
scenario. We need to know what opcode arguments are and that would need to be 
hard coded for now. But it wouldn't have to rely on anything except virtual 
function on the generic lldb_private::Process/Thread APIs. In this case as soon 
as we get an unknown opcode we would need to pass the DataExtractor and the 
offset into it so the process could extract the arguments. Not clean, but 
better than making DWARFExpression depend on process plug-ins IMHO.

> - the interface could get very complicated if we wanted to implement typed 
> stacks present in DWARF5 -- presumably the API would need to return the type 
> of the result, in addition to its value.

DWARF5 just further clarifies what each value on the opcode stack is (file 
address, load address, the value itself, etc). Right now DWARF expression just 
infer what a value is based on the opcodes. So I don't see a huge problem here 
as anything we do will need to work with DWARF5.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801

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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

2020-04-28 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

Thanks for the explanations if everything WASM related. I now understand much 
better what you have.

> Read Wasm memory.
>  IN : $qWasmMem:frame_index;addr;len
>  OUT: $xx..xx

frame index seems weird in a memory read packet. Seems like the module ID 
should be passed instead of the frame index.

Reading memory could be handled with memory identifiers, or segments. Currently 
the packets for m and M only have an address, but someone out there must 
support reading from CODE and DATA address segments in the DSP world. I have an 
email out to Ted Woodward to see what they do for Qualcomm's hexagon DSPs. I'll 
let you know what I find. Maybe each WASM module can identify N segments it 
needs and each module would have its own unique segments. Are module ID's just 
1 based indexes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

2020-04-28 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

Could we just always use memory reading and have the address contain more info? 
Right now you have the top 32 bits for the module ID. Could it be something 
like:

  struct WasmAddress {
uint64_t module_id:16;
uint64_t space:4; // 0 == code, 1 == data, 2 == global, 3==local, 4 == stack
uint64_t frame_id:??;
uint64_t addr: ??;
  }

This would be a bitfield that would all fit into a 64 bit value and could then 
be easily sent to the GDB server with the standard m and M packets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

2020-04-28 Thread Paolo Severini via Phabricator via lldb-commits

paolosev added a comment.

In D78801#2009103 , @clayborg wrote:

> Thanks for the explanations if everything WASM related. I now understand much 
> better what you have.
>
> > Read Wasm memory.
> >  IN : $qWasmMem:frame_index;addr;len
> >  OUT: $xx..xx
>
> frame index seems weird in a memory read packet. Seems like the module ID 
> should be passed instead of the frame index.
>
> Reading memory could be handled with memory identifiers, or segments. 
> Currently the packets for m and M only have an address, but someone out there 
> must support reading from CODE and DATA address segments in the DSP world. I 
> have an email out to Ted Woodward to see what they do for Qualcomm's hexagon 
> DSPs. I'll let you know what I find. Maybe each WASM module can identify N 
> segments it needs and each module would have its own unique segments. Are 
> module ID's just 1 based indexes?

True, for qWasmMem (and actually also for qWasmGlobal) it would be sufficient 
to pass the moduleId, but qWasmLocal and qWasmStackValue really need a frame 
index, and it is very easy for the runtime to find the module from a frame 
index, that's why I was passing only frame indices for uniformity. Easy to 
change :)

For reading memory, yes, I came up with ad hoc mechanism that works for Wasm, 
but if I could reuse existing mechanisms already used by other architectures 
and supported by Wasm, obviously it would be better.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801

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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

2020-04-28 Thread Paolo Severini via Phabricator via lldb-commits

paolosev added a comment.

In D78801#2009128 , @clayborg wrote:

> Could we just always use memory reading and have the address contain more 
> info? Right now you have the top 32 bits for the module ID. Could it be 
> something like:
>
>   struct WasmAddress {
> uint64_t module_id:16;
> uint64_t space:4; // 0 == code, 1 == data, 2 == global, 3==local, 4 == 
> stack
> uint64_t frame_id:??;
> uint64_t addr: ??;
>   }
>
>
> This would be a bitfield that would all fit into a 64 bit value and could 
> then be easily sent to the GDB server with the standard m and M packets.

This is interesting. We could certainly use a few bits to specify the space `0 
== code, 1 == data, 2 == global, 3==local, 4 == stack` and then have either the 
module_id or the frame_index, according to the space, and just send "m" packets.

  struct WasmAddress {
uint64_t scope:3;
uint64_t module_id_or_frame_index:29;
uint64_t address: 32;
  }

But then these "m" packets would have to be interpreted according to these 
rules by a Wasm-aware GDB-remote stub, so I am not sure we would gain much 
besides avoiding the introduction of four new custom query commands.
In a function like `Value::GetValueAsData` we would still have to have 
Wasm-specific code to generate the memory address in this format, and actually 
there it is easier to pass the current frame_index, which is readily available, 
rather than calculating the corresponding module_index.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801

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

[Lldb-commits] [lldb] 6e69338 - [lldb/Host] Pass a StringRef to the FileSpec ctor

2020-04-28 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2020-04-28T19:21:58-07:00
New Revision: 6e693386afedab5237b42679d94ca6c72684621d

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

LOG: [lldb/Host] Pass a StringRef to the FileSpec ctor

The FileSpec constructor takes a StringRef so there's no point in going
through a C string.

Added: 


Modified: 
lldb/source/Host/common/Host.cpp

Removed: 




diff  --git a/lldb/source/Host/common/Host.cpp 
b/lldb/source/Host/common/Host.cpp
index 8a6af3881a0f..a5705c92afec 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -521,7 +521,7 @@ Status Host::RunShellCommand(const Args &args, const 
FileSpec &working_dir,
 }
   }
 
-  FileSpec output_file_spec(output_file_path.c_str());
+  FileSpec output_file_spec(output_file_path.str());
   // Set up file descriptors.
   launch_info.AppendSuppressFileAction(STDIN_FILENO, true, false);
   if (output_file_spec)



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

[Lldb-commits] [lldb] 75c3d6f - [lldb/Platform] Synchronize access to SDK String Map.

2020-04-28 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2020-04-28T19:21:58-07:00
New Revision: 75c3d6f49c450a8b522a6731c317e75b73a2f5a3

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

LOG: [lldb/Platform] Synchronize access to SDK String Map.

The SwiftASTContext queries this function in parallel and requires
synchronization.

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 5252d37a01c5..6a00afba68ca 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1762,6 +1762,7 @@ 
PlatformDarwin::FindXcodeContentsDirectoryInPath(llvm::StringRef path) {
 }
 
 std::string PlatformDarwin::GetSDKPath(XcodeSDK sdk) {
+  std::lock_guard guard(m_sdk_path_mutex);
   std::string &path = m_sdk_path[sdk.GetString()];
   if (!path.empty())
 return path;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
index d3b4181aafa0..e4f717380e8b 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -171,6 +171,7 @@ class PlatformDarwin : public PlatformPOSIX {
 
   std::string m_developer_directory;
   llvm::StringMap m_sdk_path;
+  std::mutex m_sdk_path_mutex;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformDarwin);



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

[Lldb-commits] [lldb] b14c37a - [lldb/Platform] Return a std::string from GetSDKPath

2020-04-28 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2020-04-28T19:21:58-07:00
New Revision: b14c37a29a5455853419f5fe0605f6023c51de89

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

LOG: [lldb/Platform] Return a std::string from GetSDKPath

Nothing guarantees that the objects in the StringMap remains at the same
address when the StringMap grows. Therefore we shouldn't return a
reference into the StringMap but return a copy of the string instead.

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index 872e5301d984..640261033c4b 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -435,7 +435,7 @@ class Platform : public PluginInterface {
 return lldb_private::ConstString();
   }
 
-  virtual llvm::StringRef GetSDKPath(lldb_private::XcodeSDK sdk) {
+  virtual std::string GetSDKPath(lldb_private::XcodeSDK sdk) {
 return {};
   }
 

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 0777c78aa22d..5252d37a01c5 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1761,11 +1761,11 @@ 
PlatformDarwin::FindXcodeContentsDirectoryInPath(llvm::StringRef path) {
   return {};
 }
 
-llvm::StringRef PlatformDarwin::GetSDKPath(XcodeSDK sdk) {
+std::string PlatformDarwin::GetSDKPath(XcodeSDK sdk) {
   std::string &path = m_sdk_path[sdk.GetString()];
-  if (path.empty())
-path = HostInfo::GetXcodeSDK(sdk);
-  return path;
+  if (!path.empty())
+return path;
+  return HostInfo::GetXcodeSDK(sdk);
 }
 
 FileSpec PlatformDarwin::GetXcodeContentsDirectory() {

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
index 7d205be59689..d3b4181aafa0 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -89,7 +89,7 @@ class PlatformDarwin : public PlatformPOSIX {
   llvm::Expected
   FetchExtendedCrashInformation(lldb_private::Process &process) override;
 
-  llvm::StringRef GetSDKPath(lldb_private::XcodeSDK sdk) override;
+  std::string GetSDKPath(lldb_private::XcodeSDK sdk) override;
 
   static lldb_private::FileSpec GetXcodeContentsDirectory();
   static lldb_private::FileSpec GetXcodeDeveloperDirectory();



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

[Lldb-commits] [lldb] 9f8b447 - Extend max register size to accommodate AArch64 SVE vector regs

2020-04-28 Thread Muhammad Omair Javaid via lldb-commits


Author: Muhammad Omair Javaid
Date: 2020-04-29T07:51:30+05:00
New Revision: 9f8b4472fb601fcee84613a558f8d734314d98b5

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

LOG: Extend max register size to accommodate AArch64 SVE vector regs

Summary: This patch increases maximum register size to 256 bytes to accommodate 
AArch64 SVE registers maximum possible size of 256 bytes.

Reviewers: labath, jankratochvil, rengolin

Reviewed By: labath

Subscribers: tschuett, kristof.beyls, danielkiss, lldb-commits

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

Added: 


Modified: 
lldb/include/lldb/Utility/RegisterValue.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Utility/RegisterValue.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/RegisterValue.h 
b/lldb/include/lldb/Utility/RegisterValue.h
index eeb3ce52a82b..494f8be5391c 100644
--- a/lldb/include/lldb/Utility/RegisterValue.h
+++ b/lldb/include/lldb/Utility/RegisterValue.h
@@ -26,7 +26,8 @@ struct RegisterInfo;
 
 class RegisterValue {
 public:
-  enum { kMaxRegisterByteSize = 64u };
+  // big enough to support up to 256 byte AArch64 SVE
+  enum { kMaxRegisterByteSize = 256u };
 
   enum Type {
 eTypeInvalid,
@@ -261,7 +262,7 @@ class RegisterValue {
   struct {
 uint8_t bytes[kMaxRegisterByteSize]; // This must be big enough to hold any
  // register for any supported target.
-uint8_t length;
+uint16_t length;
 lldb::ByteOrder byte_order;
   } buffer;
 };

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 7d6cb2a3484b..ae2f4bd041c9 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2040,7 +2040,7 @@ 
GDBRemoteCommunicationServerLLGS::Handle_P(StringExtractorGDBRemote &packet) {
 packet, "P packet missing '=' char after register number");
 
   // Parse out the value.
-  uint8_t reg_bytes[32]; // big enough to support up to 256 bit ymmN register
+  uint8_t reg_bytes[RegisterValue::kMaxRegisterByteSize];
   size_t reg_size = packet.GetHexBytesAvail(reg_bytes);
 
   // Get the thread to use.

diff  --git a/lldb/source/Utility/RegisterValue.cpp 
b/lldb/source/Utility/RegisterValue.cpp
index bb56ade71663..91f4025c923c 100644
--- a/lldb/source/Utility/RegisterValue.cpp
+++ b/lldb/source/Utility/RegisterValue.cpp
@@ -810,7 +810,7 @@ bool RegisterValue::operator==(const RegisterValue &rhs) 
const {
   if (buffer.length != rhs.buffer.length)
 return false;
   else {
-uint8_t length = buffer.length;
+uint16_t length = buffer.length;
 if (length > kMaxRegisterByteSize)
   length = kMaxRegisterByteSize;
 return memcmp(buffer.bytes, rhs.buffer.bytes, length) == 0;



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

[Lldb-commits] [PATCH] D77044: Extend max register size to accommodate AArch64 SVE vector regs

2020-04-28 Thread Muhammad Omair Javaid via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9f8b4472fb60: Extend max register size to accommodate 
AArch64 SVE vector regs (authored by omjavaid).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77044

Files:
  lldb/include/lldb/Utility/RegisterValue.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Utility/RegisterValue.cpp


Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -810,7 +810,7 @@
   if (buffer.length != rhs.buffer.length)
 return false;
   else {
-uint8_t length = buffer.length;
+uint16_t length = buffer.length;
 if (length > kMaxRegisterByteSize)
   length = kMaxRegisterByteSize;
 return memcmp(buffer.bytes, rhs.buffer.bytes, length) == 0;
Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2040,7 +2040,7 @@
 packet, "P packet missing '=' char after register number");
 
   // Parse out the value.
-  uint8_t reg_bytes[32]; // big enough to support up to 256 bit ymmN register
+  uint8_t reg_bytes[RegisterValue::kMaxRegisterByteSize];
   size_t reg_size = packet.GetHexBytesAvail(reg_bytes);
 
   // Get the thread to use.
Index: lldb/include/lldb/Utility/RegisterValue.h
===
--- lldb/include/lldb/Utility/RegisterValue.h
+++ lldb/include/lldb/Utility/RegisterValue.h
@@ -26,7 +26,8 @@
 
 class RegisterValue {
 public:
-  enum { kMaxRegisterByteSize = 64u };
+  // big enough to support up to 256 byte AArch64 SVE
+  enum { kMaxRegisterByteSize = 256u };
 
   enum Type {
 eTypeInvalid,
@@ -261,7 +262,7 @@
   struct {
 uint8_t bytes[kMaxRegisterByteSize]; // This must be big enough to hold any
  // register for any supported target.
-uint8_t length;
+uint16_t length;
 lldb::ByteOrder byte_order;
   } buffer;
 };


Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -810,7 +810,7 @@
   if (buffer.length != rhs.buffer.length)
 return false;
   else {
-uint8_t length = buffer.length;
+uint16_t length = buffer.length;
 if (length > kMaxRegisterByteSize)
   length = kMaxRegisterByteSize;
 return memcmp(buffer.bytes, rhs.buffer.bytes, length) == 0;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2040,7 +2040,7 @@
 packet, "P packet missing '=' char after register number");
 
   // Parse out the value.
-  uint8_t reg_bytes[32]; // big enough to support up to 256 bit ymmN register
+  uint8_t reg_bytes[RegisterValue::kMaxRegisterByteSize];
   size_t reg_size = packet.GetHexBytesAvail(reg_bytes);
 
   // Get the thread to use.
Index: lldb/include/lldb/Utility/RegisterValue.h
===
--- lldb/include/lldb/Utility/RegisterValue.h
+++ lldb/include/lldb/Utility/RegisterValue.h
@@ -26,7 +26,8 @@
 
 class RegisterValue {
 public:
-  enum { kMaxRegisterByteSize = 64u };
+  // big enough to support up to 256 byte AArch64 SVE
+  enum { kMaxRegisterByteSize = 256u };
 
   enum Type {
 eTypeInvalid,
@@ -261,7 +262,7 @@
   struct {
 uint8_t bytes[kMaxRegisterByteSize]; // This must be big enough to hold any
  // register for any supported target.
-uint8_t length;
+uint16_t length;
 lldb::ByteOrder byte_order;
   } buffer;
 };
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] b14c37a - [lldb/Platform] Return a std::string from GetSDKPath

2020-04-28 Thread Jonas Devlieghere via lldb-commits

Actually the statement in the commit message is incorrect, it looks like
StringMap does guarantee both key and value to be stable. The patch is
still correct though, because GetXcodeSDK does return a std::string, so we
were returning a reference to a temporary.

On Tue, Apr 28, 2020 at 7:22 PM Jonas Devlieghere via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

>
> Author: Jonas Devlieghere
> Date: 2020-04-28T19:21:58-07:00
> New Revision: b14c37a29a5455853419f5fe0605f6023c51de89
>
> URL:
> https://github.com/llvm/llvm-project/commit/b14c37a29a5455853419f5fe0605f6023c51de89
> DIFF:
> https://github.com/llvm/llvm-project/commit/b14c37a29a5455853419f5fe0605f6023c51de89.diff
>
> LOG: [lldb/Platform] Return a std::string from GetSDKPath
>
> Nothing guarantees that the objects in the StringMap remains at the same
> address when the StringMap grows. Therefore we shouldn't return a
> reference into the StringMap but return a copy of the string instead.
>
> Added:
>
>
> Modified:
> lldb/include/lldb/Target/Platform.h
> lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
> lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
>
> Removed:
>
>
>
>
> 
> diff  --git a/lldb/include/lldb/Target/Platform.h
> b/lldb/include/lldb/Target/Platform.h
> index 872e5301d984..640261033c4b 100644
> --- a/lldb/include/lldb/Target/Platform.h
> +++ b/lldb/include/lldb/Target/Platform.h
> @@ -435,7 +435,7 @@ class Platform : public PluginInterface {
>  return lldb_private::ConstString();
>}
>
> -  virtual llvm::StringRef GetSDKPath(lldb_private::XcodeSDK sdk) {
> +  virtual std::string GetSDKPath(lldb_private::XcodeSDK sdk) {
>  return {};
>}
>
>
> diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
> b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
> index 0777c78aa22d..5252d37a01c5 100644
> --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
> +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
> @@ -1761,11 +1761,11 @@
> PlatformDarwin::FindXcodeContentsDirectoryInPath(llvm::StringRef path) {
>return {};
>  }
>
> -llvm::StringRef PlatformDarwin::GetSDKPath(XcodeSDK sdk) {
> +std::string PlatformDarwin::GetSDKPath(XcodeSDK sdk) {
>std::string &path = m_sdk_path[sdk.GetString()];
> -  if (path.empty())
> -path = HostInfo::GetXcodeSDK(sdk);
> -  return path;
> +  if (!path.empty())
> +return path;
> +  return HostInfo::GetXcodeSDK(sdk);
>  }
>
>  FileSpec PlatformDarwin::GetXcodeContentsDirectory() {
>
> diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
> b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
> index 7d205be59689..d3b4181aafa0 100644
> --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
> +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
> @@ -89,7 +89,7 @@ class PlatformDarwin : public PlatformPOSIX {
>llvm::Expected
>FetchExtendedCrashInformation(lldb_private::Process &process) override;
>
> -  llvm::StringRef GetSDKPath(lldb_private::XcodeSDK sdk) override;
> +  std::string GetSDKPath(lldb_private::XcodeSDK sdk) override;
>
>static lldb_private::FileSpec GetXcodeContentsDirectory();
>static lldb_private::FileSpec GetXcodeDeveloperDirectory();
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] b14c37a - [lldb/Platform] Return a std::string from GetSDKPath

2020-04-28 Thread Jonas Devlieghere via lldb-commits

My previous message is also bogus, I was looking at the updated code
instead of the original code. I think it's time to call it a day before I
say more nonsense. I'll take a fresh look tomorrow :-)

On Tue, Apr 28, 2020 at 8:01 PM Jonas Devlieghere 
wrote:

> Actually the statement in the commit message is incorrect, it looks like
> StringMap does guarantee both key and value to be stable. The patch is
> still correct though, because GetXcodeSDK does return a std::string, so we
> were returning a reference to a temporary.
>
> On Tue, Apr 28, 2020 at 7:22 PM Jonas Devlieghere via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>>
>> Author: Jonas Devlieghere
>> Date: 2020-04-28T19:21:58-07:00
>> New Revision: b14c37a29a5455853419f5fe0605f6023c51de89
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/b14c37a29a5455853419f5fe0605f6023c51de89
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/b14c37a29a5455853419f5fe0605f6023c51de89.diff
>>
>> LOG: [lldb/Platform] Return a std::string from GetSDKPath
>>
>> Nothing guarantees that the objects in the StringMap remains at the same
>> address when the StringMap grows. Therefore we shouldn't return a
>> reference into the StringMap but return a copy of the string instead.
>>
>> Added:
>>
>>
>> Modified:
>> lldb/include/lldb/Target/Platform.h
>> lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
>> lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
>>
>> Removed:
>>
>>
>>
>>
>> 
>> diff  --git a/lldb/include/lldb/Target/Platform.h
>> b/lldb/include/lldb/Target/Platform.h
>> index 872e5301d984..640261033c4b 100644
>> --- a/lldb/include/lldb/Target/Platform.h
>> +++ b/lldb/include/lldb/Target/Platform.h
>> @@ -435,7 +435,7 @@ class Platform : public PluginInterface {
>>  return lldb_private::ConstString();
>>}
>>
>> -  virtual llvm::StringRef GetSDKPath(lldb_private::XcodeSDK sdk) {
>> +  virtual std::string GetSDKPath(lldb_private::XcodeSDK sdk) {
>>  return {};
>>}
>>
>>
>> diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
>> b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
>> index 0777c78aa22d..5252d37a01c5 100644
>> --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
>> +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
>> @@ -1761,11 +1761,11 @@
>> PlatformDarwin::FindXcodeContentsDirectoryInPath(llvm::StringRef path) {
>>return {};
>>  }
>>
>> -llvm::StringRef PlatformDarwin::GetSDKPath(XcodeSDK sdk) {
>> +std::string PlatformDarwin::GetSDKPath(XcodeSDK sdk) {
>>std::string &path = m_sdk_path[sdk.GetString()];
>> -  if (path.empty())
>> -path = HostInfo::GetXcodeSDK(sdk);
>> -  return path;
>> +  if (!path.empty())
>> +return path;
>> +  return HostInfo::GetXcodeSDK(sdk);
>>  }
>>
>>  FileSpec PlatformDarwin::GetXcodeContentsDirectory() {
>>
>> diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
>> b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
>> index 7d205be59689..d3b4181aafa0 100644
>> --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
>> +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
>> @@ -89,7 +89,7 @@ class PlatformDarwin : public PlatformPOSIX {
>>llvm::Expected
>>FetchExtendedCrashInformation(lldb_private::Process &process) override;
>>
>> -  llvm::StringRef GetSDKPath(lldb_private::XcodeSDK sdk) override;
>> +  std::string GetSDKPath(lldb_private::XcodeSDK sdk) override;
>>
>>static lldb_private::FileSpec GetXcodeContentsDirectory();
>>static lldb_private::FileSpec GetXcodeDeveloperDirectory();
>>
>>
>>
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 7c8fa95 - lldb: use the newer `find_package` if available

2020-04-28 Thread Saleem Abdulrasool via lldb-commits


Author: Saleem Abdulrasool
Date: 2020-04-29T03:54:33Z
New Revision: 7c8fa95395e719f9a2c211ee0f574ac9ef88a19d

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

LOG: lldb: use the newer `find_package` if available

Now that the rest of LLVM prefers python3 over python2, the LLDB path
should follow suite.  Add a fallback path to python2 for non-Windows
targets.

Added: 


Modified: 
lldb/cmake/modules/FindPythonInterpAndLibs.cmake

Removed: 




diff  --git a/lldb/cmake/modules/FindPythonInterpAndLibs.cmake 
b/lldb/cmake/modules/FindPythonInterpAndLibs.cmake
index daf51ba54ad2..fa7a39185586 100644
--- a/lldb/cmake/modules/FindPythonInterpAndLibs.cmake
+++ b/lldb/cmake/modules/FindPythonInterpAndLibs.cmake
@@ -9,11 +9,11 @@ if(PYTHON_LIBRARIES AND PYTHON_INCLUDE_DIRS AND 
PYTHON_EXECUTABLE AND SWIG_EXECU
 else()
   find_package(SWIG 2.0)
   if (SWIG_FOUND)
-if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Windows")
+if(NOT CMAKE_VERSION VERSION_LESS 3.12)
   # Use PYTHON_HOME as a hint to find Python 3.
   set(Python3_ROOT_DIR "${PYTHON_HOME}")
   find_package(Python3 COMPONENTS Interpreter Development)
-  if (Python3_FOUND AND Python3_Interpreter_FOUND)
+  if(Python3_FOUND AND Python3_Interpreter_FOUND)
 set(PYTHON_LIBRARIES ${Python3_LIBRARIES})
 set(PYTHON_INCLUDE_DIRS ${Python3_INCLUDE_DIRS})
 set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
@@ -22,6 +22,20 @@ else()
   PYTHON_INCLUDE_DIRS
   PYTHON_EXECUTABLE
   SWIG_EXECUTABLE)
+  elseif(NOT CMAKE_SYSTEM_NAME STREQUAL Windows)
+# Use PYTHON_HOME as a hint to find Python 2.
+set(Python2_ROOT_DIR "${PYTHON_HOME}")
+find_package(Python2 COMPONENTS Interpreter Development)
+if(Python2_FOUND AND Python2_Interpreter_FOUND)
+  set(PYTHON_LIBRARIES ${Python2_LIBRARIES})
+  set(PYTHON_INCLUDE_DIRS ${Python2_INCLUDE_DIRS})
+  set(PYTHON_EXECUTABLE ${Python2_EXECUTABLE})
+  mark_as_advanced(
+PYTHON_LIBRARIES
+PYTHON_INCLUDE_DIRS
+PYTHON_EXECUTABLE
+SWIG_EXECUTABLE)
+endif()
   endif()
 else()
   find_package(PythonInterp)



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

[Lldb-commits] [lldb] e35dbb3 - Fix LLDB elf core dump register access for ARM/AArch64

2020-04-28 Thread Muhammad Omair Javaid via lldb-commits


Author: Muhammad Omair Javaid
Date: 2020-04-29T09:24:39+05:00
New Revision: e35dbb3c8878236754c4ec127591d9ef4665bdf8

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

LOG: Fix LLDB elf core dump register access for ARM/AArch64

Summary:
This patch adds support to access AArch64 FP SIMD core dump registers and adds 
a test case to verify registers.

This patches fixes a bug where doing "register read --all" causes lldb to crash.

Reviewers: labath

Reviewed By: labath

Subscribers: kristof.beyls, danielkiss, lldb-commits

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

Added: 
lldb/test/API/functionalities/postmortem/elf-core/aarch64-neon.c
lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.core
lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64.core
lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64.out

Modified: 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
index 310c5e142ef3..b76f26a584c0 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
@@ -44,10 +44,12 @@ bool RegisterContextCorePOSIX_arm::WriteFPR() {
 bool RegisterContextCorePOSIX_arm::ReadRegister(const RegisterInfo *reg_info,
 RegisterValue &value) {
   lldb::offset_t offset = reg_info->byte_offset;
-  uint64_t v = m_gpr.GetMaxU64(&offset, reg_info->byte_size);
-  if (offset == reg_info->byte_offset + reg_info->byte_size) {
-value = v;
-return true;
+  if (offset + reg_info->byte_size <= GetGPRSize()) {
+uint64_t v = m_gpr.GetMaxU64(&offset, reg_info->byte_size);
+if (offset == reg_info->byte_offset + reg_info->byte_size) {
+  value = v;
+  return true;
+}
   }
   return false;
 }

diff  --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
index 3e176b6eeff9..95c419b053fa 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -24,6 +24,9 @@ 
RegisterContextCorePOSIX_arm64::RegisterContextCorePOSIX_arm64(
   gpregset.GetByteSize());
   m_gpr.SetData(m_gpr_buffer);
   m_gpr.SetByteOrder(gpregset.GetByteOrder());
+
+  m_fpregset = getRegset(
+  notes, register_info->GetTargetArchitecture().GetTriple(), FPR_Desc);
 }
 
 RegisterContextCorePOSIX_arm64::~RegisterContextCorePOSIX_arm64() {}
@@ -45,11 +48,26 @@ bool RegisterContextCorePOSIX_arm64::WriteFPR() {
 bool RegisterContextCorePOSIX_arm64::ReadRegister(const RegisterInfo *reg_info,
   RegisterValue &value) {
   lldb::offset_t offset = reg_info->byte_offset;
-  uint64_t v = m_gpr.GetMaxU64(&offset, reg_info->byte_size);
-  if (offset == reg_info->byte_offset + reg_info->byte_size) {
-value = v;
-return true;
+  if (offset + reg_info->byte_size <= GetGPRSize()) {
+uint64_t v = m_gpr.GetMaxU64(&offset, reg_info->byte_size);
+if (offset == reg_info->byte_offset + reg_info->byte_size) {
+  value = v;
+  return true;
+}
   }
+
+  const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
+  if (reg == LLDB_INVALID_REGNUM)
+return false;
+
+  offset -= GetGPRSize();
+  if (IsFPR(reg) && offset + reg_info->byte_size <= sizeof(FPU)) {
+Status error;
+value.SetFromMemoryData(reg_info, m_fpregset.GetDataStart() + offset,
+reg_info->byte_size, lldb::eByteOrderLittle, 
error);
+return error.Success();
+  }
+
   return false;
 }
 

diff  --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
index 7c35d89c4f13..5bbcdf5677f6 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -48,6 +48,7 @@ class RegisterContextCorePOSIX_arm64 : public 
RegisterContextPOSIX_arm64 {
 private:
   lldb::DataBufferSP m_gpr_buffer;
   lldb_private::DataExtractor m_gpr;
+  lldb_private::DataExtractor m_fpregset;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_PROCESS_ELF_CORE_

[Lldb-commits] [PATCH] D77793: Fix LLDB elf core dump register access for ARM/AArch64

2020-04-28 Thread Muhammad Omair Javaid via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
omjavaid marked an inline comment as done.
Closed by commit rGe35dbb3c8878: Fix LLDB elf core dump register access for 
ARM/AArch64 (authored by omjavaid).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D77793?vs=259182&id=260835#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77793

Files:
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/aarch64-neon.c
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.core
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64.core
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64.out

Index: lldb/test/API/functionalities/postmortem/elf-core/aarch64-neon.c
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/elf-core/aarch64-neon.c
@@ -0,0 +1,28 @@
+// compile with -march=armv8-a+sve on compatible aarch64 compiler
+// linux-aarch64-sve.core was generated by: aarch64-linux-gnu-gcc-8
+// commandline: -march=armv8-a+sve -nostdlib -static -g linux-aarch64-sve.c
+static void bar(char *boom) {
+  char F = 'b';
+  asm volatile("fmov d0,  #0.5\n\t");
+  asm volatile("fmov d1,  #1.5\n\t");
+  asm volatile("fmov d2,  #2.5\n\t");
+  asm volatile("fmov d3,  #3.5\n\t");
+  asm volatile("fmov s4,  #4.5\n\t");
+  asm volatile("fmov s5,  #5.5\n\t");
+  asm volatile("fmov s6,  #6.5\n\t");
+  asm volatile("fmov s7,  #7.5\n\t");
+  asm volatile("movi v8.16b, #0x11\n\t");
+  asm volatile("movi v31.16b, #0x30\n\t");
+
+  *boom = 47; // Frame bar
+}
+
+static void foo(char *boom, void (*boomer)(char *)) {
+  char F = 'f';
+  boomer(boom); // Frame foo
+}
+
+void _start(void) {
+  char F = '_';
+  foo(0, bar); // Frame _start
+}
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -19,6 +19,7 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
+_aarch64_pid = 37688
 _i386_pid = 32306
 _x86_64_pid = 32259
 _s390x_pid = 1045
@@ -27,12 +28,20 @@
 _mips_o32_pid = 3532
 _ppc64le_pid = 28147
 
+_aarch64_regions = 4
 _i386_regions = 4
 _x86_64_regions = 5
 _s390x_regions = 2
 _mips_regions = 5
 _ppc64le_regions = 2
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64(self):
+"""Test that lldb can read the process information from an aarch64 linux core file."""
+self.do_test("linux-aarch64", self._aarch64_pid, self._aarch64_regions, "a.out")
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("X86")
 def test_i386(self):
@@ -248,6 +257,61 @@
 self.dbg.DeleteTarget(target)
 
 @skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64_regs(self):
+# check 64 bit ARM core files
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-aarch64-neon.core")
+
+values = {}
+values["x1"] = "0x002f"
+values["w1"] = "0x002f"
+values["fp"] = "0x007fc5dd7f20"
+values["lr"] = "0x00400180"
+values["sp"] = "0x007fc5dd7f00"
+values["pc"] = "0x0040014c"
+values["v0"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0xe0 0x3f 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["v1"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0xf8 0x3f 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["v2"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x04 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["v3"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x0c 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["v4"] = "{0x00 0x00 0x90 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["v5"] = "{0x00 0x00 0xb0 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["v6"] = "{0x00 0x00 0xd0 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["v7"] = "{0x00 0x00 0xf0 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["v8"] = "{0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11}"
+values["v27"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Lldb-commits] [PATCH] D78839: [lldb-vscode] Add an option for loading core files

[Lldb-commits] [lldb] f07f2ce - [lldb/unittest] Adjust CheckIPSupport function to avoid double-consume of llvm::Error

[Lldb-commits] [lldb] 5cee8dd - [lldb-vscode] A couple of small style fixes

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

[Lldb-commits] [PATCH] D74892: [lldb][cmake] Also use local submodule visibility on Darwin

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Lldb-commits] [PATCH] D78882: [IR] Replace all uses of CallBase::getCalledValue() with getCalledOperand().

[Lldb-commits] [PATCH] D78882: [IR] Replace all uses of CallBase::getCalledValue() with getCalledOperand().

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

[Lldb-commits] [PATCH] D78839: [lldb-vscode] Add an option for loading core files

[Lldb-commits] [lldb] 8372582 - [lldb-vscode] Add an option for loading core files

[Lldb-commits] [PATCH] D78839: [lldb-vscode] Add an option for loading core files

[Lldb-commits] [PATCH] D78712: [lldb/Host] Improve error messages on unowned read files

[Lldb-commits] [PATCH] D78712: [lldb/Host] Improve error messages on unowned read files

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Lldb-commits] [lldb] 6e69338 - [lldb/Host] Pass a StringRef to the FileSpec ctor

[Lldb-commits] [lldb] 75c3d6f - [lldb/Platform] Synchronize access to SDK String Map.

[Lldb-commits] [lldb] b14c37a - [lldb/Platform] Return a std::string from GetSDKPath

[Lldb-commits] [lldb] 9f8b447 - Extend max register size to accommodate AArch64 SVE vector regs

[Lldb-commits] [PATCH] D77044: Extend max register size to accommodate AArch64 SVE vector regs

Re: [Lldb-commits] [lldb] b14c37a - [lldb/Platform] Return a std::string from GetSDKPath

Re: [Lldb-commits] [lldb] b14c37a - [lldb/Platform] Return a std::string from GetSDKPath

[Lldb-commits] [lldb] 7c8fa95 - lldb: use the newer `find_package` if available

[Lldb-commits] [lldb] e35dbb3 - Fix LLDB elf core dump register access for ARM/AArch64

[Lldb-commits] [PATCH] D77793: Fix LLDB elf core dump register access for ARM/AArch64

33 matches

Site Navigation

Mail list logo

Footer information