[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

2021-04-16 Thread Jason Molenda via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe9fe788d3260: Target::ReadMemory read from read-only binary 
file Section, not memory (authored by jasonmolenda).

Changed prior to commit:
  https://reviews.llvm.org/D100338?vs=337735&id=338257#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Symbol/Function.h
  lldb/include/lldb/Target/Target.h
  lldb/source/API/SBFunction.cpp
  lldb/source/API/SBSymbol.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Core/Address.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/Value.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Expression/IRMemoryMap.cpp
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/ThreadPlanStepRange.cpp
  lldb/source/Target/Trace.cpp

Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -200,7 +200,7 @@
   // Now we try using the current function's disassembler
   if (sc.function) {
 DisassemblerSP disassembler =
-sc.function->GetInstructions(exe_ctx, nullptr, true);
+sc.function->GetInstructions(exe_ctx, nullptr);
 if (TryDumpInstructionInfo(s, disassembler, exe_ctx, address))
   return disassembler;
   }
@@ -209,9 +209,9 @@
   Target &target = exe_ctx.GetTargetRef();
   const ArchSpec &arch = target.GetArchitecture();
   AddressRange range(address, arch.GetMaximumOpcodeByteSize() * 1);
-  DisassemblerSP disassembler = Disassembler::DisassembleRange(
-  arch, /*plugin_name*/ nullptr,
-  /*flavor*/ nullptr, target, range, /*prefer_file_cache*/ true);
+  DisassemblerSP disassembler =
+  Disassembler::DisassembleRange(arch, /*plugin_name*/ nullptr,
+ /*flavor*/ nullptr, target, range);
   if (TryDumpInstructionInfo(s, disassembler, exe_ctx, address))
 return disassembler;
   return nullptr;
Index: lldb/source/Target/ThreadPlanStepRange.cpp
===
--- lldb/source/Target/ThreadPlanStepRange.cpp
+++ lldb/source/Target/ThreadPlanStepRange.cpp
@@ -264,10 +264,9 @@
 // Disassemble the address range given:
 const char *plugin_name = nullptr;
 const char *flavor = nullptr;
-const bool prefer_file_cache = true;
 m_instruction_ranges[i] = Disassembler::DisassembleRange(
 GetTarget().GetArchitecture(), plugin_name, flavor, GetTarget(),
-m_address_ranges[i], prefer_file_cache);
+m_address_ranges[i]);
   }
   if (!m_instruction_ranges[i])
 return nullptr;
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1717,8 +1717,8 @@
   return 0;
 }
 
-size_t Target::ReadMemory(const Address &addr, bool prefer_file_cache,
-  void *dst, size_t dst_len, Status &error,
+size_t Target::ReadMemory(const Address &addr, void *dst, size_t dst_len,
+  Status &error, bool force_live_memory,
   lldb::addr_t *load_addr_ptr) {
   error.Clear();
 
@@ -1753,10 +1753,20 @@
   if (!resolved_addr.IsValid())
 resolved_addr = addr;
 
-  if (prefer_file_cache) {
-bytes_read = ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
-if (bytes_read > 0)
-  return bytes_read;
+  bool is_readonly = false;
+  // Read from file cache if read-only section.
+  if (!force_live_memory && resolved_addr.IsSectionOffset()) {
+SectionSP section_sp(addr.GetSection());
+if (section_sp) {
+  auto permissions = Flags(section_sp->GetPermissions());
+  is_readonly = !permissions.Test(ePermissionsWritable) &&
+permissions.Test(ePermissionsReadable);
+}
+if (is_readonly) {
+  bytes_read = ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
+  if (b

[Lldb-commits] [lldb] e9fe788 - Target::ReadMemory read from read-only binary file Section, not memory

2021-04-16 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2021-04-16T16:13:07-07:00
New Revision: e9fe788d326090cb6155c0dec90b44c932273dd3

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

LOG: Target::ReadMemory read from read-only binary file Section, not memory

Commiting this patch for Augusto Noronha who is getting set
up still.

This patch changes Target::ReadMemory so the default behavior
when a read is in a Section that is read-only is to fetch the
data from the local binary image, instead of reading it from
memory.  Update all callers to use their old preferences
(the old prefer_file_cache bool) using the new API; we should
revisit these calls and see if they really intend to read
live memory, or if reading from a read-only Section would be
equivalent and important for performance-sensitive cases.

rdar://30634422

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

Added: 


Modified: 
lldb/include/lldb/Core/Disassembler.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Target/Target.h
lldb/source/API/SBFunction.cpp
lldb/source/API/SBSymbol.cpp
lldb/source/API/SBTarget.cpp
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Core/Address.cpp
lldb/source/Core/Disassembler.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Core/Value.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Expression/IRMemoryMap.cpp
lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Symbol/Symbol.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/ThreadPlanStepRange.cpp
lldb/source/Target/Trace.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Disassembler.h 
b/lldb/include/lldb/Core/Disassembler.h
index 9a694de0f60a0..0a76f0a12b9d2 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -394,10 +394,12 @@ class Disassembler : public 
std::enable_shared_from_this,
 lldb::addr_t value;
   };
 
-  static lldb::DisassemblerSP
-  DisassembleRange(const ArchSpec &arch, const char *plugin_name,
-   const char *flavor, Target &target,
-   const AddressRange &disasm_range, bool prefer_file_cache);
+  static lldb::DisassemblerSP DisassembleRange(const ArchSpec &arch,
+   const char *plugin_name,
+   const char *flavor,
+   Target &target,
+   const AddressRange 
&disasm_range,
+   bool force_live_memory = false);
 
   static lldb::DisassemblerSP
   DisassembleBytes(const ArchSpec &arch, const char *plugin_name,
@@ -426,7 +428,8 @@ class Disassembler : public 
std::enable_shared_from_this,
  Stream &strm);
 
   size_t ParseInstructions(Target &target, Address address, Limit limit,
-   Stream *error_strm_ptr, bool prefer_file_cache);
+   Stream *error_strm_ptr,
+   bool force_live_memory = false);
 
   virtual size_t DecodeInstructions(const Address &base_addr,
 const DataExtractor &data,

diff  --git a/lldb/include/lldb/Symbol/Function.h 
b/lldb/include/lldb/Symbol/Function.h
index 300d829219d4d..aae5b4a496c2c 100644
--- a/lldb/include/lldb/Symbol/Function.h
+++ b/lldb/include/lldb/Symbol/Function.h
@@ -631,10 +631,10 @@ class Function : public UserID, public SymbolContextScope 
{
 
   lldb::DisassemblerSP GetInstructions(const ExecutionContext &exe_ctx,
const char *flavor,
-   bool prefer_file_cache);
+   bool force_live_memory = false);
 
   bool GetDisassembly(const ExecutionContext &exe_ctx, const char *flavor,
-  bool prefer_file_cache, Stream &strm);
+  Stream &strm, bool force_live_memory = false);
 
 protected:
   enum {

diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 0c2131a60b4bb..2192

[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-16 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added a comment.

In D100515#2695559 , @JDevlieghere 
wrote:

> In D100515#2695395 , @justincohen 
> wrote:
>
>> Out of curiosity: Typically should one be able to set 
>> target.process.virtual-addressable-bits after the target has been created?  
>> Or is it expected that users will need to run in the following order only:
>>
>>   settings set target.process.virtual-addressable-bits ...
>>   target create -c 
>>
>> Setting virtual-addressable-bits won't do anythin after the target has been 
>> created (and perhaps that is working as intended?)
>
> Yep, that was exactly why I'm always reading the setting if the mask isn't 
> set yet (as opposed to setting it once in the constructor).

And it's an open question of what's the correct behavior if there's a 
dynamically-set value AND a user specified setting.  The patch currently 
prefers the dynamically-set value and I think that's the right choice, but 
it'll be interesting to see if there's a platform/system where that doesn't 
work for some users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100515

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

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

2021-04-16 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 338232.
mgorny added a comment.

Added a unittest covering the whole server life cycle, including attaching a 
(mocked) process, forking, detaching a child, detaching the parent, forking 
from the child and detaching all processes.


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

https://reviews.llvm.org/D100191

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/unittests/Process/gdb-remote/CMakeLists.txt
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerLLGSTest.cpp
  lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h

Index: lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
===
--- lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
+++ lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
@@ -25,6 +25,9 @@
   MOCK_METHOD2(ProcessStateChanged,
void(NativeProcessProtocol *Process, StateType State));
   MOCK_METHOD1(DidExec, void(NativeProcessProtocol *Process));
+  MOCK_METHOD2(NewSubprocess,
+   void(NativeProcessProtocol *parent_process,
+std::unique_ptr &child_process));
 };
 
 // NB: This class doesn't use the override keyword to avoid
Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerLLGSTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerLLGSTest.cpp
@@ -0,0 +1,139 @@
+//===-- GDBRemoteCommunicationServerLLGSTest.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "lldb/Host/common/NativeProcessProtocol.h"
+
+#include "GDBRemoteTestUtils.h"
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h"
+#include "TestingSupport/Host/NativeProcessTestUtils.h"
+
+namespace lldb_private {
+namespace process_gdb_remote {
+
+class MockFactory : public NativeProcessProtocol::Factory {
+public:
+  llvm::Expected>
+  Launch(ProcessLaunchInfo &launch_info,
+ NativeProcessProtocol::NativeDelegate &native_delegate,
+ MainLoop &mainloop) const override {
+return llvm::createStringError(llvm::inconvertibleErrorCode(), "Not supported");
+  }
+
+  virtual llvm::Expected>
+  Attach(lldb::pid_t pid,
+ NativeProcessProtocol::NativeDelegate &native_delegate,
+ MainLoop &mainloop) const override {
+NativeProcessProtocol *process = new MockProcess(native_delegate, ArchSpec("x86_64-pc-linux"), pid);
+return std::unique_ptr(process);
+  }
+};
+
+class TestServer : public GDBRemoteCommunicationServerLLGS {
+  MainLoop m_main_loop;
+  MockFactory m_process_factory;
+
+public:
+  TestServer() : GDBRemoteCommunicationServerLLGS(m_main_loop, m_process_factory) {}
+
+  using GDBRemoteCommunicationServerLLGS::m_debugged_processes;
+  using GDBRemoteCommunicationServerLLGS::m_current_process;
+  using GDBRemoteCommunicationServerLLGS::m_continue_process;
+
+  using GDBRemoteCommunicationServerLLGS::Handle_D;
+};
+
+TEST(GDBRemoteCommunicationServerLLGSTest, NewSubprocess) {
+  TestServer server;
+  Status error;
+
+  // Initial state: no process.
+  EXPECT_EQ(server.m_debugged_processes.size(), 0u);
+  EXPECT_EQ(server.m_current_process, nullptr);
+  EXPECT_EQ(server.m_continue_process, nullptr);
+
+  // Attach the main process.
+  error = server.AttachToProcess(1234);
+  ASSERT_TRUE(error.Success());
+  EXPECT_EQ(server.m_debugged_processes.size(), 1u);
+  auto it = server.m_debugged_processes.find(1234);
+  ASSERT_NE(it, server.m_debugged_processes.end());
+  NativeProcessProtocol *parent = it->second.get();
+  EXPECT_EQ(parent->GetID(), 1234u);
+  EXPECT_EQ(server.m_current_process, parent);
+  EXPECT_EQ(server.m_continue_process, parent);
+
+  // Simulate a fork.
+  std::unique_ptr child_up{new MockProcess(server, ArchSpec("x86_64-pc-linux"), 1235)};
+  server.NewSubprocess(parent, child_up);
+  EXPECT_EQ(child_up, nullptr);
+  EXPECT_EQ(server.m_debugged_processes.size(), 2u);
+  it = server.m_debugged_processes.find(1235);
+  ASSERT_NE(it, server.m_debugged_processes.end());
+  NativeProcessProtocol *child = it->second.get();
+  EXPECT_EQ(child->GetID(), 1235u);
+  EXPECT_EQ(server.m_current_process, parent);
+  EXPECT_EQ(server.m_continue_process, parent);
+
+  // Detach the child.
+  StringExtractorGDBRemote child_detach{"D;04D3"};
+  EXPECT_CALL(*static_cast *>(child), Detach()

[Lldb-commits] [PATCH] D100520: [lldb] Set code and data address mask from qHostInfo

2021-04-16 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdb2da0c8f907: [lldb] Set addressable bits from qHostInfo 
(authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100520

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp


Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1042,6 +1042,12 @@
  process_arch.GetTriple().getTriple());
   }
 
+  if (int addresssable_bits = m_gdb_comm.GetAddressingBits()) {
+lldb::addr_t address_mask = ~((1ULL << addresssable_bits) - 1);
+SetCodeAddressMask(address_mask);
+SetDataAddressMask(address_mask);
+  }
+
   if (process_arch.IsValid()) {
 const ArchSpec &target_arch = GetTarget().GetArchitecture();
 if (target_arch.IsValid()) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -275,6 +275,8 @@
 
   ArchSpec GetSystemArchitecture();
 
+  uint32_t GetAddressingBits();
+
   bool GetHostname(std::string &s);
 
   lldb::addr_t GetShlibInfoAddr();
@@ -573,6 +575,7 @@
   // continue, step, etc
 
   uint32_t m_num_supported_hardware_watchpoints;
+  uint32_t m_addressing_bits;
 
   ArchSpec m_host_arch;
   ArchSpec m_process_arch;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -100,11 +100,11 @@
   m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
   m_curr_pid(LLDB_INVALID_PROCESS_ID), m_curr_tid(LLDB_INVALID_THREAD_ID),
   m_curr_tid_run(LLDB_INVALID_THREAD_ID),
-  m_num_supported_hardware_watchpoints(0), m_host_arch(), m_process_arch(),
-  m_os_build(), m_os_kernel(), m_hostname(), m_gdb_server_name(),
-  m_gdb_server_version(UINT32_MAX), m_default_packet_timeout(0),
-  m_max_packet_size(0), m_qSupported_response(),
-  m_supported_async_json_packets_is_valid(false),
+  m_num_supported_hardware_watchpoints(0), m_addressing_bits(0),
+  m_host_arch(), m_process_arch(), m_os_build(), m_os_kernel(),
+  m_hostname(), m_gdb_server_name(), m_gdb_server_version(UINT32_MAX),
+  m_default_packet_timeout(0), m_max_packet_size(0),
+  m_qSupported_response(), m_supported_async_json_packets_is_valid(false),
   m_supported_async_json_packets_sp(), m_qXfer_memory_map(),
   m_qXfer_memory_map_loaded(false) {}
 
@@ -1202,11 +1202,13 @@
   } else if (name.equals("ptrsize")) {
 if (!value.getAsInteger(0, pointer_byte_size))
   ++num_keys_decoded;
+  } else if (name.equals("addressing_bits")) {
+if (!value.getAsInteger(0, m_addressing_bits))
+  ++num_keys_decoded;
   } else if (name.equals("os_version") ||
- name.equals(
- "version")) // Older debugserver binaries used the
- // "version" key instead of
- // "os_version"...
+ name.equals("version")) // Older debugserver binaries used
+ // the "version" key instead of
+ // "os_version"...
   {
 if (!m_os_version.tryParse(value))
   ++num_keys_decoded;
@@ -1357,6 +1359,11 @@
   return m_host_arch;
 }
 
+uint32_t GDBRemoteCommunicationClient::GetAddressingBits() {
+  if (m_qHostInfo_is_valid == eLazyBoolCalculate)
+GetHostInfo();
+  return m_addressing_bits;
+}
 seconds GDBRemoteCommunicationClient::GetHostDefaultPacketTimeout() {
   if (m_qHostInfo_is_valid == eLazyBoolCalculate)
 GetHostInfo();


Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1042,6 +1042,12 @@
  process_arch.GetTriple().getTriple());
   }
 
+  if (int addresssable_bits = m_gdb_comm.GetAddressingBits(

[Lldb-commits] [PATCH] D100521: [lldb] Fix up code addresses in RegisterContextUnwind

2021-04-16 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8770b4ecca55: [lldb] Implement ABI::Fix{Code,Data}Address 
for AArch64 (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D100521?vs=338129&id=338227#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100521

Files:
  lldb/include/lldb/Target/ABI.h
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
  lldb/source/Target/RegisterContextUnwind.cpp

Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -1730,6 +1730,10 @@
   RegisterValue reg_value;
   if (ReadRegisterValueFromRegisterLocation(regloc, reg_info, reg_value)) {
 old_caller_pc_value = reg_value.GetAsUInt64();
+if (ProcessSP process_sp = m_thread.GetProcess()) {
+  if (ABISP abi = process_sp->GetABI())
+old_caller_pc_value = abi->FixCodeAddress(old_caller_pc_value);
+}
   }
 }
   }
@@ -1785,6 +1789,10 @@
 if (ReadRegisterValueFromRegisterLocation(regloc, reg_info,
   reg_value)) {
   new_caller_pc_value = reg_value.GetAsUInt64();
+  if (ProcessSP process_sp = m_thread.GetProcess()) {
+if (ABISP abi = process_sp->GetABI())
+  new_caller_pc_value = abi->FixCodeAddress(new_caller_pc_value);
+  }
 }
   }
 }
@@ -2121,6 +2129,12 @@
   }
   if (ReadRegisterValueFromRegisterLocation(regloc, reg_info, reg_value)) {
 value = reg_value.GetAsUInt64();
+if (pc_register) {
+  if (ProcessSP process_sp = m_thread.GetProcess()) {
+if (ABISP abi = process_sp->GetABI())
+  value = abi->FixCodeAddress(value);
+  }
+}
 return true;
   }
   return false;
@@ -2162,7 +2176,19 @@
   lldb_regnum, regloc, m_frame_number - 1, is_pc_regnum))
 return false;
 
-  return ReadRegisterValueFromRegisterLocation(regloc, reg_info, value);
+  bool result = ReadRegisterValueFromRegisterLocation(regloc, reg_info, value);
+  if (result) {
+if (is_pc_regnum && value.GetType() == RegisterValue::eTypeUInt64) {
+  addr_t reg_value = value.GetAsUInt64(LLDB_INVALID_ADDRESS);
+  if (reg_value != LLDB_INVALID_ADDRESS) {
+if(ProcessSP process_sp = m_thread.GetProcess()) {
+  if (ABISP abi = process_sp->GetABI())
+value = abi->FixCodeAddress(reg_value);
+}
+  }
+}
+  }
+  return result;
 }
 
 bool RegisterContextUnwind::WriteRegister(const RegisterInfo *reg_info,
Index: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
===
--- lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
+++ lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
@@ -67,6 +67,8 @@
 
   bool GetPointerReturnRegister(const char *&name) override;
 
+  lldb::addr_t FixAddress(lldb::addr_t pc, lldb::addr_t mask) override;
+
   // Static Functions
 
   static void Initialize();
Index: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
===
--- lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -782,6 +782,11 @@
   return return_valobj_sp;
 }
 
+lldb::addr_t ABISysV_arm64::FixAddress(addr_t pc, addr_t mask) {
+  lldb::addr_t pac_sign_extension = 0x0080ULL;
+  return (pc & pac_sign_extension) ? pc | mask : pc & (~mask);
+}
+
 void ABISysV_arm64::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
 "SysV ABI for AArch64 targets", CreateInstance);
Index: lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
===
--- lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
+++ lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
@@ -62,6 +62,8 @@
 return true;
   }
 
+  lldb::addr_t FixAddress(lldb::addr_t pc, lldb::addr_t mask) override;
+
   // Static Functions
 
   static void Initialize();
Index: lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
===
--- lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
@@ -815,6 +815,11 @@
   return return_valobj_sp;
 }
 
+lldb::addr_t ABIMacOSX_arm64::FixAddress(addr_t pc, addr_t mask) {
+  lldb::addr_t pac_sign_e

[Lldb-commits] [lldb] db2da0c - [lldb] Set addressable bits from qHostInfo

2021-04-16 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2021-04-16T13:49:38-07:00
New Revision: db2da0c8f907c444e0bf8b788d8186c4c67db0b2

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

LOG: [lldb] Set addressable bits from qHostInfo

Read the number of addressable bits from the qHostInfo packet and use it
to set the code and data address mask in the process. The data
(addressing_bits) is already present in the packet.

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

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index ef6441289fc50..d709716d8e459 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -100,11 +100,11 @@ 
GDBRemoteCommunicationClient::GDBRemoteCommunicationClient()
   m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
   m_curr_pid(LLDB_INVALID_PROCESS_ID), m_curr_tid(LLDB_INVALID_THREAD_ID),
   m_curr_tid_run(LLDB_INVALID_THREAD_ID),
-  m_num_supported_hardware_watchpoints(0), m_host_arch(), m_process_arch(),
-  m_os_build(), m_os_kernel(), m_hostname(), m_gdb_server_name(),
-  m_gdb_server_version(UINT32_MAX), m_default_packet_timeout(0),
-  m_max_packet_size(0), m_qSupported_response(),
-  m_supported_async_json_packets_is_valid(false),
+  m_num_supported_hardware_watchpoints(0), m_addressing_bits(0),
+  m_host_arch(), m_process_arch(), m_os_build(), m_os_kernel(),
+  m_hostname(), m_gdb_server_name(), m_gdb_server_version(UINT32_MAX),
+  m_default_packet_timeout(0), m_max_packet_size(0),
+  m_qSupported_response(), m_supported_async_json_packets_is_valid(false),
   m_supported_async_json_packets_sp(), m_qXfer_memory_map(),
   m_qXfer_memory_map_loaded(false) {}
 
@@ -1202,11 +1202,13 @@ bool GDBRemoteCommunicationClient::GetHostInfo(bool 
force) {
   } else if (name.equals("ptrsize")) {
 if (!value.getAsInteger(0, pointer_byte_size))
   ++num_keys_decoded;
+  } else if (name.equals("addressing_bits")) {
+if (!value.getAsInteger(0, m_addressing_bits))
+  ++num_keys_decoded;
   } else if (name.equals("os_version") ||
- name.equals(
- "version")) // Older debugserver binaries used the
- // "version" key instead of
- // "os_version"...
+ name.equals("version")) // Older debugserver binaries used
+ // the "version" key instead of
+ // "os_version"...
   {
 if (!m_os_version.tryParse(value))
   ++num_keys_decoded;
@@ -1357,6 +1359,11 @@ GDBRemoteCommunicationClient::GetHostArchitecture() {
   return m_host_arch;
 }
 
+uint32_t GDBRemoteCommunicationClient::GetAddressingBits() {
+  if (m_qHostInfo_is_valid == eLazyBoolCalculate)
+GetHostInfo();
+  return m_addressing_bits;
+}
 seconds GDBRemoteCommunicationClient::GetHostDefaultPacketTimeout() {
   if (m_qHostInfo_is_valid == eLazyBoolCalculate)
 GetHostInfo();

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index d559882ed7dc6..1891842a69444 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -275,6 +275,8 @@ class GDBRemoteCommunicationClient : public 
GDBRemoteClientBase {
 
   ArchSpec GetSystemArchitecture();
 
+  uint32_t GetAddressingBits();
+
   bool GetHostname(std::string &s);
 
   lldb::addr_t GetShlibInfoAddr();
@@ -573,6 +575,7 @@ class GDBRemoteCommunicationClient : public 
GDBRemoteClientBase {
   // continue, step, etc
 
   uint32_t m_num_supported_hardware_watchpoints;
+  uint32_t m_addressing_bits;
 
   ArchSpec m_host_arch;
   ArchSpec m_process_arch;

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 97368d5b0e31e..5ff00f8566990 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1042,6 +1042,12 @@ void ProcessGDBRemote::D

[Lldb-commits] [lldb] 8770b4e - [lldb] Implement ABI::Fix{Code, Data}Address for AArch64

2021-04-16 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2021-04-16T13:49:38-07:00
New Revision: 8770b4ecca557b02d37188ae2fa5479e6136b2fb

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

LOG: [lldb] Implement ABI::Fix{Code,Data}Address for AArch64

Implement FixCodeAddress and FixDataAddress for ABIMacOSX_arm64 and
ABISysV_arm64 and add missing calls to RegisterContextUnwind. We need
this to unwind on Apple Silicon where libraries like libSystem are
arm64e even when the program being debugged is arm64.

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

Added: 


Modified: 
lldb/include/lldb/Target/ABI.h
lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
lldb/source/Target/RegisterContextUnwind.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/ABI.h b/lldb/include/lldb/Target/ABI.h
index 131b2eaff765c..8fbb6aae68c4c 100644
--- a/lldb/include/lldb/Target/ABI.h
+++ b/lldb/include/lldb/Target/ABI.h
@@ -117,12 +117,13 @@ class ABI : public PluginInterface {
   // "pc".
   virtual bool CodeAddressIsValid(lldb::addr_t pc) = 0;
 
-  virtual lldb::addr_t FixCodeAddress(lldb::addr_t pc) {
-// Some targets might use bits in a code address to indicate a mode switch.
-// ARM uses bit zero to signify a code address is thumb, so any ARM ABI
-// plug-ins would strip those bits.
-return pc;
-  }
+  /// Some targets might use bits in a code address to indicate a mode switch.
+  /// ARM uses bit zero to signify a code address is thumb, so any ARM ABI
+  /// plug-ins would strip those bits.
+  /// @{
+  virtual lldb::addr_t FixCodeAddress(lldb::addr_t pc) { return pc; }
+  virtual lldb::addr_t FixDataAddress(lldb::addr_t pc) { return pc; }
+  /// @}
 
   llvm::MCRegisterInfo &GetMCRegisterInfo() { return *m_mc_register_info_up; }
 
@@ -147,6 +148,10 @@ class ABI : public PluginInterface {
   lldb::ProcessWP m_process_wp;
   std::unique_ptr m_mc_register_info_up;
 
+  virtual lldb::addr_t FixCodeAddress(lldb::addr_t pc, lldb::addr_t mask) {
+return pc;
+  }
+
 private:
   ABI(const ABI &) = delete;
   const ABI &operator=(const ABI &) = delete;

diff  --git a/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp 
b/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
index 7cae4cc427501..42d73ce39ed6f 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
@@ -11,6 +11,7 @@
 #include "ABISysV_arm64.h"
 #include "Utility/ARM64_DWARF_Registers.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/Process.h"
 
 LLDB_PLUGIN_DEFINE(ABIAArch64)
 
@@ -24,6 +25,18 @@ void ABIAArch64::Terminate() {
   ABIMacOSX_arm64::Terminate();
 }
 
+lldb::addr_t ABIAArch64::FixCodeAddress(lldb::addr_t pc) {
+  if (lldb::ProcessSP process_sp = GetProcessSP())
+return FixAddress(pc, process_sp->GetCodeAddressMask());
+  return pc;
+}
+
+lldb::addr_t ABIAArch64::FixDataAddress(lldb::addr_t pc) {
+  if (lldb::ProcessSP process_sp = GetProcessSP())
+return FixAddress(pc, process_sp->GetDataAddressMask());
+  return pc;
+}
+
 std::pair
 ABIAArch64::GetEHAndDWARFNums(llvm::StringRef name) {
   if (name == "pc")

diff  --git a/lldb/source/Plugins/ABI/AArch64/ABIAArch64.h 
b/lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
index bdff648f1b522..41bbf5cfdeb96 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
+++ b/lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
@@ -16,7 +16,14 @@ class ABIAArch64: public lldb_private::MCBasedABI {
   static void Initialize();
   static void Terminate();
 
+  virtual lldb::addr_t FixCodeAddress(lldb::addr_t pc) override;
+  virtual lldb::addr_t FixDataAddress(lldb::addr_t pc) override;
+
 protected:
+  virtual lldb::addr_t FixAddress(lldb::addr_t pc, lldb::addr_t mask) {
+return pc;
+  }
+
   std::pair
   GetEHAndDWARFNums(llvm::StringRef name) override;
 

diff  --git a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp 
b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
index 861310e3ea0c1..c7ae128d874f5 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
@@ -815,6 +815,11 @@ ValueObjectSP ABIMacOSX_arm64::GetReturnValueObjectImpl(
   return return_valobj_sp;
 }
 
+lldb::addr_t ABIMacOSX_arm64::FixAddress(addr_t pc, addr_t mask) {
+  lldb::addr_t pac_sign_extension = 0x0080ULL;
+  return (pc & pac_sign_extension) ? pc | mask : pc & (~mask);
+}
+
 void ABIMacOSX_arm64::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(), pluginDesc,

[Lldb-commits] [PATCH] D100520: [lldb] Set code and data address mask from qHostInfo

2021-04-16 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

Looks good!


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

https://reviews.llvm.org/D100520

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

[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-16 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfdbb5a7a91b0: [lldb] Add code and data address mask to 
Process (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100515

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

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -233,6 +233,9 @@
   def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">,
 DefaultFalse,
 Desc<"If true, stepping operations will run all threads.  This is equivalent to setting the run-mode option to 'all-threads'.">;
+  def VirtualAddressableBits: Property<"virtual-addressable-bits", "UInt64">,
+DefaultUnsignedValue<0>,
+Desc<"The number of bits used for addressing. If the value is 39, then bits 0..38 are used for addressing. The default value of 0 means unspecified.">;
 }
 
 let Definition = "platform" in {
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -200,6 +200,16 @@
   return m_collection_sp->GetPropertyAtIndexAsFileSpec(nullptr, idx);
 }
 
+uint32_t ProcessProperties::GetVirtualAddressableBits() const {
+  const uint32_t idx = ePropertyVirtualAddressableBits;
+  return m_collection_sp->GetPropertyAtIndexAsUInt64(
+  nullptr, idx, g_process_properties[idx].default_uint_value);
+}
+
+void ProcessProperties::SetVirtualAddressableBits(uint32_t bits) {
+  const uint32_t idx = ePropertyVirtualAddressableBits;
+  m_collection_sp->SetPropertyAtIndexAsUInt64(nullptr, idx, bits);
+}
 void ProcessProperties::SetPythonOSPluginPath(const FileSpec &file) {
   const uint32_t idx = ePropertyPythonOSPluginPath;
   m_collection_sp->SetPropertyAtIndexAsFileSpec(nullptr, idx, file);
@@ -5547,6 +5557,26 @@
   m_queue_list_stop_id = 0;
 }
 
+lldb::addr_t Process::GetCodeAddressMask() {
+  if (m_code_address_mask == 0) {
+if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
+  lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
+  SetCodeAddressMask(address_mask);
+}
+  }
+  return m_code_address_mask;
+}
+
+lldb::addr_t Process::GetDataAddressMask() {
+  if (m_data_address_mask == 0) {
+if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
+  lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
+  SetDataAddressMask(address_mask);
+}
+  }
+  return m_data_address_mask;
+}
+
 void Process::DidExec() {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
   LLDB_LOGF(log, "Process::%s()", __FUNCTION__);
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -77,6 +77,8 @@
   Args GetExtraStartupCommands() const;
   void SetExtraStartupCommands(const Args &args);
   FileSpec GetPythonOSPluginPath() const;
+  uint32_t GetVirtualAddressableBits() const;
+  void SetVirtualAddressableBits(uint32_t bits);
   void SetPythonOSPluginPath(const FileSpec &file);
   bool GetIgnoreBreakpointsInExpressions() const;
   void SetIgnoreBreakpointsInExpressions(bool ignore);
@@ -1330,6 +1332,17 @@
 
   virtual void DidExit() {}
 
+  lldb::addr_t GetCodeAddressMask();
+  lldb::addr_t GetDataAddressMask();
+
+  void SetCodeAddressMask(lldb::addr_t code_address_mask) {
+m_code_address_mask = code_address_mask;
+  }
+
+  void SetDataAddressMask(lldb::addr_t data_address_mask) {
+m_data_address_mask = data_address_mask;
+  }
+
   /// Get the Modification ID of the process.
   ///
   /// \return
@@ -2878,6 +2891,13 @@
   /// from looking up or creating things during or after a finalize call.
   std::atomic m_finalizing;
 
+  /// Mask for code an data addresses. The default value (0) means no mask is
+  /// set.
+  /// @{
+  lldb::addr_t m_code_address_mask = 0;
+  lldb::addr_t m_data_address_mask = 0;
+  /// @}
+
   bool m_clear_thread_plans_on_stop;
   bool m_force_next_event_delivery;
   lldb::StateType m_last_broadcast_state; /// This helps with the Public event
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] fdbb5a7 - [lldb] Add code and data address mask to Process

2021-04-16 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2021-04-16T12:30:54-07:00
New Revision: fdbb5a7a91b00d1e4a9a16fee96763917a411fff

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

LOG: [lldb] Add code and data address mask to Process

Add a code and data address mask to Process with respective getters and
setters and a setting that allows the user to specify the mast as a
number of addressable bits. The masks will be used by FixCodeAddress and
FixDataAddress respectively in the ABI classes.

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

Added: 


Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/Target/Process.cpp
lldb/source/Target/TargetProperties.td

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index abd0d39fe9699..0a2a6f6dfb5b8 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -77,6 +77,8 @@ class ProcessProperties : public Properties {
   Args GetExtraStartupCommands() const;
   void SetExtraStartupCommands(const Args &args);
   FileSpec GetPythonOSPluginPath() const;
+  uint32_t GetVirtualAddressableBits() const;
+  void SetVirtualAddressableBits(uint32_t bits);
   void SetPythonOSPluginPath(const FileSpec &file);
   bool GetIgnoreBreakpointsInExpressions() const;
   void SetIgnoreBreakpointsInExpressions(bool ignore);
@@ -1330,6 +1332,17 @@ class Process : public 
std::enable_shared_from_this,
 
   virtual void DidExit() {}
 
+  lldb::addr_t GetCodeAddressMask();
+  lldb::addr_t GetDataAddressMask();
+
+  void SetCodeAddressMask(lldb::addr_t code_address_mask) {
+m_code_address_mask = code_address_mask;
+  }
+
+  void SetDataAddressMask(lldb::addr_t data_address_mask) {
+m_data_address_mask = data_address_mask;
+  }
+
   /// Get the Modification ID of the process.
   ///
   /// \return
@@ -2878,6 +2891,13 @@ void PruneThreadPlans();
   /// from looking up or creating things during or after a finalize call.
   std::atomic m_finalizing;
 
+  /// Mask for code an data addresses. The default value (0) means no mask is
+  /// set.
+  /// @{
+  lldb::addr_t m_code_address_mask = 0;
+  lldb::addr_t m_data_address_mask = 0;
+  /// @}
+
   bool m_clear_thread_plans_on_stop;
   bool m_force_next_event_delivery;
   lldb::StateType m_last_broadcast_state; /// This helps with the Public event

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 986df94e73beb..9f39e78e5d726 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -200,6 +200,16 @@ FileSpec ProcessProperties::GetPythonOSPluginPath() const {
   return m_collection_sp->GetPropertyAtIndexAsFileSpec(nullptr, idx);
 }
 
+uint32_t ProcessProperties::GetVirtualAddressableBits() const {
+  const uint32_t idx = ePropertyVirtualAddressableBits;
+  return m_collection_sp->GetPropertyAtIndexAsUInt64(
+  nullptr, idx, g_process_properties[idx].default_uint_value);
+}
+
+void ProcessProperties::SetVirtualAddressableBits(uint32_t bits) {
+  const uint32_t idx = ePropertyVirtualAddressableBits;
+  m_collection_sp->SetPropertyAtIndexAsUInt64(nullptr, idx, bits);
+}
 void ProcessProperties::SetPythonOSPluginPath(const FileSpec &file) {
   const uint32_t idx = ePropertyPythonOSPluginPath;
   m_collection_sp->SetPropertyAtIndexAsFileSpec(nullptr, idx, file);
@@ -5547,6 +5557,26 @@ void Process::Flush() {
   m_queue_list_stop_id = 0;
 }
 
+lldb::addr_t Process::GetCodeAddressMask() {
+  if (m_code_address_mask == 0) {
+if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
+  lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
+  SetCodeAddressMask(address_mask);
+}
+  }
+  return m_code_address_mask;
+}
+
+lldb::addr_t Process::GetDataAddressMask() {
+  if (m_data_address_mask == 0) {
+if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
+  lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
+  SetDataAddressMask(address_mask);
+}
+  }
+  return m_data_address_mask;
+}
+
 void Process::DidExec() {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
   LLDB_LOGF(log, "Process::%s()", __FUNCTION__);

diff  --git a/lldb/source/Target/TargetProperties.td 
b/lldb/source/Target/TargetProperties.td
index e22f94069131a..a3634a0bd54f1 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -233,6 +233,9 @@ let Definition = "process" in {
   def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">,
 DefaultFalse,
 Desc<"If true, stepping operations will run all threads.  This is 
equivalent to setting the run-mode option to 'all-threads'.">;
+  def VirtualAddressableBits: Property

[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-16 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D100515#2695395 , @justincohen 
wrote:

> Out of curiosity: Typically should one be able to set 
> target.process.virtual-addressable-bits after the target has been created?  
> Or is it expected that users will need to run in the following order only:
>
>   settings set target.process.virtual-addressable-bits ...
>   target create -c 
>
> Setting virtual-addressable-bits won't do anythin after the target has been 
> created (and perhaps that is working as intended?)

Yep, that was exactly why I'm always reading the setting if the mask isn't set 
yet (as opposed to setting it once in the constructor).


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

https://reviews.llvm.org/D100515

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

[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-16 Thread Justin Cohen via Phabricator via lldb-commits 
justincohen added a comment.

Out of curiosity: Typically should one be able to set 
target.process.virtual-addressable-bits after the target has been created?  Or 
is it expected that users will need to run in the following order only:

  settings set target.process.virtual-addressable-bits ...
  target create -c 

Setting virtual-addressable-bits won't do anythin after the target has been 
created (and perhaps that is working as intended?)


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

https://reviews.llvm.org/D100515

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

Re: [Lldb-commits] [lldb] 3dc24bc - [LLDB] Re-land: Use path relative to binary for finding .dwo files.

2021-04-16 Thread David Blaikie via lldb-commits 
For future reference it'd be good to include some details in a
recommit including the original commit hash, the revert hash, the
reason for the revert and what's changed in this commit to address
that problem.

For others reading along, according to the phab review this test hit a
bug in lldb and the test has been tweaked to avoid the bug - Pavel
Labath's going to look into the bug in the future.

On Fri, Apr 16, 2021 at 11:13 AM Caroline Tice via lldb-commits
 wrote:
>
>
> Author: Caroline Tice
> Date: 2021-04-16T11:12:39-07:00
> New Revision: 3dc24bc31edbc01dea085b24a6a6b024d7ae531c
>
> URL: 
> https://github.com/llvm/llvm-project/commit/3dc24bc31edbc01dea085b24a6a6b024d7ae531c
> DIFF: 
> https://github.com/llvm/llvm-project/commit/3dc24bc31edbc01dea085b24a6a6b024d7ae531c.diff
>
> LOG: [LLDB] Re-land: Use path relative to binary for finding .dwo files.
>
> DWARF allows .dwo file paths to be relative rather than absolute. When
> they are relative, DWARF uses DW_AT_comp_dir to find the .dwo
> file. DW_AT_comp_dir can also be relative, making the entire search
> patch for the .dwo file relative. In this case, LLDB currently
> searches relative to its current working directory, i.e. the directory
> from which the debugger was launched. This is not right, as the
> compiler, which generated the relative paths, can have no idea where
> the debugger will be launched. The correct thing is to search relative
> to the location of the executable binary. That is what this patch
> does.
>
> Differential Revision: https://reviews.llvm.org/D97786
>
> Added:
> lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
>
> Modified:
> lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>
> Removed:
>
>
>
> 
> diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
> b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
> index 3a04f429c7c75..da170383405fb 100644
> --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
> +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
> @@ -1651,6 +1651,13 @@ SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
>return nullptr;
>
>  dwo_file.SetFile(comp_dir, FileSpec::Style::native);
> +if (dwo_file.IsRelative()) {
> +  // if DW_AT_comp_dir is relative, it should be relative to the location
> +  // of the executable, not to the location from which the debugger was
> +  // launched.
> +  dwo_file.PrependPathComponent(
> +  m_objfile_sp->GetFileSpec().GetDirectory().GetStringRef());
> +}
>  FileSystem::Instance().Resolve(dwo_file);
>  dwo_file.AppendPathComponent(dwo_name);
>}
>
> diff  --git a/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s 
> b/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
> new file mode 100644
> index 0..1916461e3ce8b
> --- /dev/null
> +++ b/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
> @@ -0,0 +1,173 @@
> +# Test to verify LLDB searches for dwos with relative paths relative to the
> +# binary location, not relative to LLDB's launch location.
> +
> +# RUN: llvm-mc --filetype=obj --triple x86_64-pc-linux %s -o %t.o
> +# RUN: llvm-objcopy --split-dwo=%T/dwo-relative-path.dwo %t.o
> +
> +# RUN: cd ../..
> +
> +# RUN: %lldb %t.o -o "target var x" -b 2>&1 | FileCheck %s
> +
> +# CHECK: x = 10
> +
> +   .file   "dwo-relative-path.cpp"
> +   .file   0 "." "dwo-relative-path.cpp" md5 
> 0xadc61d242247514c5d402d62db34b825
> +   .type   x,@object   # @x
> +   .data
> +   .globl  x
> +   .p2align2
> +x:
> +   .long   10  # 0xa
> +   .size   x, 4
> +
> +   .section.debug_abbrev,"",@progbits
> +   .byte   1   # Abbreviation Code
> +   .byte   74  # DW_TAG_skeleton_unit
> +   .byte   0   # DW_CHILDREN_no
> +   .byte   16  # DW_AT_stmt_list
> +   .byte   23  # DW_FORM_sec_offset
> +   .byte   114 # DW_AT_str_offsets_base
> +   .byte   23  # DW_FORM_sec_offset
> +   .byte   27  # DW_AT_comp_dir
> +   .byte   37  # DW_FORM_strx1
> +   .ascii  "\264B" # DW_AT_GNU_pubnames
> +   .byte   25  # DW_FORM_flag_present
> +   .byte   118 # DW_AT_dwo_name
> +   .byte   37  # DW_FORM_strx1
> +   .byte   115 # DW_AT_addr_base
> +   .byte   23  # DW_FORM_sec_offset
> +   .byte   0   # EOM(1)
> +   .byte   0   # EOM(2)
> +   .byte   0

[Lldb-commits] [lldb] 3dc24bc - [LLDB] Re-land: Use path relative to binary for finding .dwo files.

2021-04-16 Thread Caroline Tice via lldb-commits 

Author: Caroline Tice
Date: 2021-04-16T11:12:39-07:00
New Revision: 3dc24bc31edbc01dea085b24a6a6b024d7ae531c

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

LOG: [LLDB] Re-land: Use path relative to binary for finding .dwo files.

DWARF allows .dwo file paths to be relative rather than absolute. When
they are relative, DWARF uses DW_AT_comp_dir to find the .dwo
file. DW_AT_comp_dir can also be relative, making the entire search
patch for the .dwo file relative. In this case, LLDB currently
searches relative to its current working directory, i.e. the directory
from which the debugger was launched. This is not right, as the
compiler, which generated the relative paths, can have no idea where
the debugger will be launched. The correct thing is to search relative
to the location of the executable binary. That is what this patch
does.

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 3a04f429c7c75..da170383405fb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1651,6 +1651,13 @@ SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
   return nullptr;
 
 dwo_file.SetFile(comp_dir, FileSpec::Style::native);
+if (dwo_file.IsRelative()) {
+  // if DW_AT_comp_dir is relative, it should be relative to the location
+  // of the executable, not to the location from which the debugger was
+  // launched.
+  dwo_file.PrependPathComponent(
+  m_objfile_sp->GetFileSpec().GetDirectory().GetStringRef());
+}
 FileSystem::Instance().Resolve(dwo_file);
 dwo_file.AppendPathComponent(dwo_name);
   }

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s 
b/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
new file mode 100644
index 0..1916461e3ce8b
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
@@ -0,0 +1,173 @@
+# Test to verify LLDB searches for dwos with relative paths relative to the
+# binary location, not relative to LLDB's launch location.
+
+# RUN: llvm-mc --filetype=obj --triple x86_64-pc-linux %s -o %t.o
+# RUN: llvm-objcopy --split-dwo=%T/dwo-relative-path.dwo %t.o
+
+# RUN: cd ../..
+
+# RUN: %lldb %t.o -o "target var x" -b 2>&1 | FileCheck %s
+
+# CHECK: x = 10
+
+   .file   "dwo-relative-path.cpp"
+   .file   0 "." "dwo-relative-path.cpp" md5 
0xadc61d242247514c5d402d62db34b825
+   .type   x,@object   # @x
+   .data
+   .globl  x
+   .p2align2
+x:
+   .long   10  # 0xa
+   .size   x, 4
+
+   .section.debug_abbrev,"",@progbits
+   .byte   1   # Abbreviation Code
+   .byte   74  # DW_TAG_skeleton_unit
+   .byte   0   # DW_CHILDREN_no
+   .byte   16  # DW_AT_stmt_list
+   .byte   23  # DW_FORM_sec_offset
+   .byte   114 # DW_AT_str_offsets_base
+   .byte   23  # DW_FORM_sec_offset
+   .byte   27  # DW_AT_comp_dir
+   .byte   37  # DW_FORM_strx1
+   .ascii  "\264B" # DW_AT_GNU_pubnames
+   .byte   25  # DW_FORM_flag_present
+   .byte   118 # DW_AT_dwo_name
+   .byte   37  # DW_FORM_strx1
+   .byte   115 # DW_AT_addr_base
+   .byte   23  # DW_FORM_sec_offset
+   .byte   0   # EOM(1)
+   .byte   0   # EOM(2)
+   .byte   0   # EOM(3)
+   .section.debug_info,"",@progbits
+.Lcu_begin0:
+   .long   .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+   .short  5   # DWARF version number
+   .byte   4   # DWARF Unit Type
+   .byte   8   # Address Size (in bytes)
+   .long   .debug_abbrev   # Offset Into Abbrev. Section
+   .quad   3752513468363206953
+   .byte   1   # Abbrev [1] 0x14:0x14 
DW_TAG_skeleton_unit
+   .long   .Lline_table_start0 # DW_AT_stmt_list
+

[Lldb-commits] [PATCH] D100418: [lldb] [MainLoop] Support multiple callbacks per signal

2021-04-16 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 338170.
mgorny marked an inline comment as done.
mgorny added a comment.

Fixed the iterator, formatting and added a test for adding, running and 
removing multiple handlers.


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

https://reviews.llvm.org/D100418

Files:
  lldb/include/lldb/Host/MainLoop.h
  lldb/source/Host/common/MainLoop.cpp
  lldb/unittests/Host/MainLoopTest.cpp

Index: lldb/unittests/Host/MainLoopTest.cpp
===
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -152,4 +152,49 @@
   killer.join();
   ASSERT_EQ(1u, callback_count);
 }
+
+// Test that two callbacks can be registered for the same signal
+// and unregistered independently.
+TEST_F(MainLoopTest, TwoSignalCallbacks) {
+  MainLoop loop;
+  Status error;
+  int callback2_count = 0;
+  int callback3_count = 0;
+
+  auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
+  ASSERT_TRUE(error.Success());
+
+  {
+// Run a single iteration with two callbacks enabled.
+auto handle2 = loop.RegisterSignal(
+SIGUSR1, [&](MainLoopBase &loop) { ++callback2_count; }, error);
+ASSERT_TRUE(error.Success());
+
+kill(getpid(), SIGUSR1);
+ASSERT_TRUE(loop.Run().Success());
+ASSERT_EQ(1u, callback_count);
+ASSERT_EQ(1u, callback2_count);
+ASSERT_EQ(0u, callback3_count);
+  }
+
+  {
+// Make sure that remove + add new works.
+auto handle3 = loop.RegisterSignal(
+SIGUSR1, [&](MainLoopBase &loop) { ++callback3_count; }, error);
+ASSERT_TRUE(error.Success());
+
+kill(getpid(), SIGUSR1);
+ASSERT_TRUE(loop.Run().Success());
+ASSERT_EQ(2u, callback_count);
+ASSERT_EQ(1u, callback2_count);
+ASSERT_EQ(1u, callback3_count);
+  }
+
+  // Both extra callbacks should be unregistered now.
+  kill(getpid(), SIGUSR1);
+  ASSERT_TRUE(loop.Run().Success());
+  ASSERT_EQ(3u, callback_count);
+  ASSERT_EQ(1u, callback2_count);
+  ASSERT_EQ(1u, callback3_count);
+}
 #endif
Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -302,13 +302,15 @@
   error.SetErrorString("Signal polling is not supported on this platform.");
   return nullptr;
 #else
-  if (m_signals.find(signo) != m_signals.end()) {
-error.SetErrorStringWithFormat("Signal %d already monitored.", signo);
-return nullptr;
+  auto signal_it = m_signals.find(signo);
+  if (signal_it != m_signals.end()) {
+auto callback_it = signal_it->second.callbacks.insert(
+signal_it->second.callbacks.end(), callback);
+return SignalHandleUP(new SignalHandle(*this, signo, callback_it));
   }
 
   SignalInfo info;
-  info.callback = callback;
+  info.callbacks.push_back(callback);
   struct sigaction new_action;
   new_action.sa_sigaction = &SignalHandler;
   new_action.sa_flags = SA_SIGINFO;
@@ -338,9 +340,10 @@
 &new_action.sa_mask, &old_set);
   assert(ret == 0 && "pthread_sigmask failed");
   info.was_blocked = sigismember(&old_set, signo);
-  m_signals.insert({signo, info});
+  auto insert_ret = m_signals.insert({signo, info});
 
-  return SignalHandleUP(new SignalHandle(*this, signo));
+  return SignalHandleUP(new SignalHandle(
+  *this, signo, insert_ret.first->second.callbacks.begin()));
 #endif
 }
 
@@ -350,13 +353,19 @@
   assert(erased);
 }
 
-void MainLoop::UnregisterSignal(int signo) {
+void MainLoop::UnregisterSignal(int signo,
+std::list::iterator callback_it) {
 #if SIGNAL_POLLING_UNSUPPORTED
   Status("Signal polling is not supported on this platform.");
 #else
   auto it = m_signals.find(signo);
   assert(it != m_signals.end());
 
+  it->second.callbacks.erase(callback_it);
+  // Do not remove the signal handler unless all callbacks have been erased.
+  if (!it->second.callbacks.empty())
+return;
+
   sigaction(signo, &it->second.old_action, nullptr);
 
   sigset_t set;
@@ -398,8 +407,14 @@
 
 void MainLoop::ProcessSignal(int signo) {
   auto it = m_signals.find(signo);
-  if (it != m_signals.end())
-it->second.callback(*this); // Do the work
+  if (it != m_signals.end()) {
+// The callback may actually register/unregister signal handlers,
+// so we need to create a copy first.
+llvm::SmallVector callbacks_to_run{
+it->second.callbacks.begin(), it->second.callbacks.end()};
+for (auto &x : callbacks_to_run)
+  x(*this); // Do the work
+  }
 }
 
 void MainLoop::ProcessReadObject(IOObject::WaitableHandle handle) {
Index: lldb/include/lldb/Host/MainLoop.h
===
--- lldb/include/lldb/Host/MainLoop.h
+++ lldb/include/lldb/Host/MainLoop.h
@@ -13,6 +13,7 @@
 #include "lldb/Host/MainLoopBase.h"
 #include "llvm/ADT/DenseMap.h"
 #include 
+#include 
 
 #if !HAV

[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

2021-04-16 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

This looks good, thanks for doing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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

[Lldb-commits] [PATCH] D100521: [lldb] Fix up code addresses in RegisterContextUnwind

2021-04-16 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 338129.

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

https://reviews.llvm.org/D100521

Files:
  lldb/include/lldb/Target/ABI.h
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
  lldb/source/Target/RegisterContextUnwind.cpp

Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -383,7 +383,7 @@
   // symbol/function information - just stick in some reasonable defaults and
   // hope we can unwind past this frame.  If we're above a trap handler,
   // we may be at a bogus address because we jumped through a bogus function
-  // pointer and trapped, so don't force the arch default unwind plan in that 
+  // pointer and trapped, so don't force the arch default unwind plan in that
   // case.
   ModuleSP pc_module_sp(m_current_pc.GetModule());
   if ((!m_current_pc.IsValid() || !pc_module_sp) &&
@@ -1277,7 +1277,7 @@
 // arch default unwind plan is used as the Fast Unwind Plan, we
 // need to recognize this & switch over to the Full Unwind Plan
 // to see what unwind rule that (more knoweldgeable, probably)
-// UnwindPlan has.  If the full UnwindPlan says the register 
+// UnwindPlan has.  If the full UnwindPlan says the register
 // location is Undefined, then it really is.
 if (active_row->GetRegisterInfo(regnum.GetAsKind(unwindplan_registerkind),
 unwindplan_regloc) &&
@@ -1325,14 +1325,14 @@
   m_full_unwind_plan_sp->GetReturnAddressRegister() !=
   LLDB_INVALID_REGNUM) {
 // If this is a trap handler frame, we should have access to
-// the complete register context when the interrupt/async 
+// the complete register context when the interrupt/async
 // signal was received, we should fetch the actual saved $pc
 // value instead of the Return Address register.
 // If $pc is not available, fall back to the RA reg.
 UnwindPlan::Row::RegisterLocation scratch;
 if (m_frame_type == eTrapHandlerFrame &&
-active_row->GetRegisterInfo 
-  (pc_regnum.GetAsKind (unwindplan_registerkind), scratch)) {
+active_row->GetRegisterInfo(
+pc_regnum.GetAsKind(unwindplan_registerkind), scratch)) {
   UnwindLogMsg("Providing pc register instead of rewriting to "
"RA reg because this is a trap handler and there is "
"a location for the saved pc register value.");
@@ -1730,6 +1730,12 @@
   RegisterValue reg_value;
   if (ReadRegisterValueFromRegisterLocation(regloc, reg_info, reg_value)) {
 old_caller_pc_value = reg_value.GetAsUInt64();
+ProcessSP process_sp(m_thread.GetProcess());
+if (process_sp) {
+  ABI *abi = process_sp->GetABI().get();
+  if (abi)
+old_caller_pc_value = abi->FixCodeAddress(old_caller_pc_value);
+}
   }
 }
   }
@@ -1785,6 +1791,12 @@
 if (ReadRegisterValueFromRegisterLocation(regloc, reg_info,
   reg_value)) {
   new_caller_pc_value = reg_value.GetAsUInt64();
+  ProcessSP process_sp(m_thread.GetProcess());
+  if (process_sp) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  new_caller_pc_value = abi->FixCodeAddress(new_caller_pc_value);
+  }
 }
   }
 }
@@ -2121,6 +2133,14 @@
   }
   if (ReadRegisterValueFromRegisterLocation(regloc, reg_info, reg_value)) {
 value = reg_value.GetAsUInt64();
+if (pc_register) {
+  ProcessSP process_sp(m_thread.GetProcess());
+  if (process_sp) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  value = abi->FixCodeAddress(value);
+  }
+}
 return true;
   }
   return false;
@@ -2162,7 +2182,19 @@
   lldb_regnum, regloc, m_frame_number - 1, is_pc_regnum))
 return false;
 
-  return ReadRegisterValueFromRegisterLocation(regloc, reg_info, value);
+  bool result = ReadRegisterValueFromRegisterLocation(regloc, reg_info, value);
+  if (result) {
+if (is_pc_regnum && value.GetType() == RegisterValue::eTypeUInt64) {
+  ProcessSP process_sp(m_thread.GetProcess());
+  addr_t reg_value = value.GetAsUInt64(LLDB_INVALID_ADDRESS);
+  if (process_sp && reg_value != LLDB_INVALID_ADDRESS) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  value = abi->FixCodeAddress(reg_value);
+  }
+}
+  }
+  return result;
 }
 
 bool RegisterContextU

[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-16 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 338128.

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

https://reviews.llvm.org/D100515

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

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -233,6 +233,9 @@
   def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">,
 DefaultFalse,
 Desc<"If true, stepping operations will run all threads.  This is equivalent to setting the run-mode option to 'all-threads'.">;
+  def VirtualAddressableBits: Property<"virtual-addressable-bits", "UInt64">,
+DefaultUnsignedValue<0>,
+Desc<"The number of bits used for addressing. If the value is 39, then bits 0..38 are used for addressing. The default value of 0 means unspecified.">;
 }
 
 let Definition = "platform" in {
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -200,6 +200,16 @@
   return m_collection_sp->GetPropertyAtIndexAsFileSpec(nullptr, idx);
 }
 
+uint32_t ProcessProperties::GetVirtualAddressableBits() const {
+  const uint32_t idx = ePropertyVirtualAddressableBits;
+  return m_collection_sp->GetPropertyAtIndexAsUInt64(
+  nullptr, idx, g_process_properties[idx].default_uint_value);
+}
+
+void ProcessProperties::SetVirtualAddressableBits(uint32_t bits) {
+  const uint32_t idx = ePropertyVirtualAddressableBits;
+  m_collection_sp->SetPropertyAtIndexAsUInt64(nullptr, idx, bits);
+}
 void ProcessProperties::SetPythonOSPluginPath(const FileSpec &file) {
   const uint32_t idx = ePropertyPythonOSPluginPath;
   m_collection_sp->SetPropertyAtIndexAsFileSpec(nullptr, idx, file);
@@ -5547,6 +5557,26 @@
   m_queue_list_stop_id = 0;
 }
 
+lldb::addr_t Process::GetCodeAddressMask() {
+  if (m_code_address_mask == 0) {
+if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
+  lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
+  SetCodeAddressMask(address_mask);
+}
+  }
+  return m_code_address_mask;
+}
+
+lldb::addr_t Process::GetDataAddressMask() {
+  if (m_data_address_mask == 0) {
+if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
+  lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
+  SetDataAddressMask(address_mask);
+}
+  }
+  return m_data_address_mask;
+}
+
 void Process::DidExec() {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
   LLDB_LOGF(log, "Process::%s()", __FUNCTION__);
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -77,6 +77,8 @@
   Args GetExtraStartupCommands() const;
   void SetExtraStartupCommands(const Args &args);
   FileSpec GetPythonOSPluginPath() const;
+  uint32_t GetVirtualAddressableBits() const;
+  void SetVirtualAddressableBits(uint32_t bits);
   void SetPythonOSPluginPath(const FileSpec &file);
   bool GetIgnoreBreakpointsInExpressions() const;
   void SetIgnoreBreakpointsInExpressions(bool ignore);
@@ -1330,6 +1332,17 @@
 
   virtual void DidExit() {}
 
+  lldb::addr_t GetCodeAddressMask();
+  lldb::addr_t GetDataAddressMask();
+
+  void SetCodeAddressMask(lldb::addr_t code_address_mask) {
+m_code_address_mask = code_address_mask;
+  }
+
+  void SetDataAddressMask(lldb::addr_t data_address_mask) {
+m_data_address_mask = data_address_mask;
+  }
+
   /// Get the Modification ID of the process.
   ///
   /// \return
@@ -2878,6 +2891,13 @@
   /// from looking up or creating things during or after a finalize call.
   std::atomic m_finalizing;
 
+  /// Mask for code an data addresses. The default value (0) means no mask is
+  /// set.
+  /// @{
+  lldb::addr_t m_code_address_mask = 0;
+  lldb::addr_t m_data_address_mask = 0;
+  /// @}
+
   bool m_clear_thread_plans_on_stop;
   bool m_force_next_event_delivery;
   lldb::StateType m_last_broadcast_state; /// This helps with the Public event
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-16 Thread Peter Collingbourne via Phabricator via lldb-commits 
pcc added inline comments.



Comment at: lldb/source/Target/Process.cpp:5569-5570
+
+  if (m_code_address_mask == 0)
+return -1; // All bits are used for addressing.
+

jasonmolenda wrote:
> pcc wrote:
> > JDevlieghere wrote:
> > > jasonmolenda wrote:
> > > > pcc wrote:
> > > > > Is this part correct? (Same below.) In D100521 you have
> > > > > ```
> > > > >   if (pc & pac_sign_extension)
> > > > > return pc | mask;
> > > > >   return pc & (~mask);
> > > > > ```
> > > > > So it looks like this would cause the pc to be set to 0 (or -1)?
> > > > I get confused so I like to do this by hand quickly to make sure I 
> > > > understand.
> > > > 
> > > > given mask of 111 and addr of xxx1011 where 'x' are PAC bits,
> > > > 
> > > > b55 == 1: m | a == 011
> > > > b55 == 0: ~m & a == 0001011
> > > > 
> > > > given mask of 111, low address 0001011 and high address 011,
> > > > 
> > > > b55 == 1: m & ha == ha
> > > > b55 == 0: ~m | la == la
> > > > 
> > > > am I  not thinking of something that could unify these?  I can confuse 
> > > > myself so easily with these things.
> > > > 
> > > > We could also detect a mask of -1 and just return the original address 
> > > > in FixCodeAddress/FixDataAddress, right.  That would be very simple.
> > > I've added checking for -1 in D100521
> > With a mask of 111 isn't it
> > 
> > b55 == 1: m | ha == 111
> > b55 == 0: ~m & la == 000
> > 
> > I think you can just remove lines 5569-5570 here as well as lines 819-820 
> > from D100521.
> urk.  my caveat that I often confuse myself with these has been proven true. 
> :) 
> 
> So you're suggesting the default, uninitialized, mask is 0.  If we have that, 
> then
> 
> mask 0, low address 0001011, high address 011
> b55 == 1: m | ha == 011
> b55 == 0: ~m & la == 0001011
> 
> In the mask, any bit set to 0 is passed through as-is.  Any bit set to 1 is 
> going to be cleared or set in these FixAddress methods.  I see.
Yes, exactly.


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

https://reviews.llvm.org/D100515

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

[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-16 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added inline comments.



Comment at: lldb/source/Target/Process.cpp:5569-5570
+
+  if (m_code_address_mask == 0)
+return -1; // All bits are used for addressing.
+

pcc wrote:
> JDevlieghere wrote:
> > jasonmolenda wrote:
> > > pcc wrote:
> > > > Is this part correct? (Same below.) In D100521 you have
> > > > ```
> > > >   if (pc & pac_sign_extension)
> > > > return pc | mask;
> > > >   return pc & (~mask);
> > > > ```
> > > > So it looks like this would cause the pc to be set to 0 (or -1)?
> > > I get confused so I like to do this by hand quickly to make sure I 
> > > understand.
> > > 
> > > given mask of 111 and addr of xxx1011 where 'x' are PAC bits,
> > > 
> > > b55 == 1: m | a == 011
> > > b55 == 0: ~m & a == 0001011
> > > 
> > > given mask of 111, low address 0001011 and high address 011,
> > > 
> > > b55 == 1: m & ha == ha
> > > b55 == 0: ~m | la == la
> > > 
> > > am I  not thinking of something that could unify these?  I can confuse 
> > > myself so easily with these things.
> > > 
> > > We could also detect a mask of -1 and just return the original address in 
> > > FixCodeAddress/FixDataAddress, right.  That would be very simple.
> > I've added checking for -1 in D100521
> With a mask of 111 isn't it
> 
> b55 == 1: m | ha == 111
> b55 == 0: ~m & la == 000
> 
> I think you can just remove lines 5569-5570 here as well as lines 819-820 
> from D100521.
urk.  my caveat that I often confuse myself with these has been proven true. :) 

So you're suggesting the default, uninitialized, mask is 0.  If we have that, 
then

mask 0, low address 0001011, high address 011
b55 == 1: m | ha == 011
b55 == 0: ~m & la == 0001011

In the mask, any bit set to 0 is passed through as-is.  Any bit set to 1 is 
going to be cleared or set in these FixAddress methods.  I see.


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

https://reviews.llvm.org/D100515

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

[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-16 Thread Peter Collingbourne via Phabricator via lldb-commits 
pcc added inline comments.



Comment at: lldb/source/Target/Process.cpp:5569-5570
+
+  if (m_code_address_mask == 0)
+return -1; // All bits are used for addressing.
+

JDevlieghere wrote:
> jasonmolenda wrote:
> > pcc wrote:
> > > Is this part correct? (Same below.) In D100521 you have
> > > ```
> > >   if (pc & pac_sign_extension)
> > > return pc | mask;
> > >   return pc & (~mask);
> > > ```
> > > So it looks like this would cause the pc to be set to 0 (or -1)?
> > I get confused so I like to do this by hand quickly to make sure I 
> > understand.
> > 
> > given mask of 111 and addr of xxx1011 where 'x' are PAC bits,
> > 
> > b55 == 1: m | a == 011
> > b55 == 0: ~m & a == 0001011
> > 
> > given mask of 111, low address 0001011 and high address 011,
> > 
> > b55 == 1: m & ha == ha
> > b55 == 0: ~m | la == la
> > 
> > am I  not thinking of something that could unify these?  I can confuse 
> > myself so easily with these things.
> > 
> > We could also detect a mask of -1 and just return the original address in 
> > FixCodeAddress/FixDataAddress, right.  That would be very simple.
> I've added checking for -1 in D100521
With a mask of 111 isn't it

b55 == 1: m | ha == 111
b55 == 0: ~m & la == 000

I think you can just remove lines 5569-5570 here as well as lines 819-820 from 
D100521.


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

https://reviews.llvm.org/D100515

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