date:20210303

[Lldb-commits] [lldb] 3dcbfa2 - [debugserver] Fix more compiler warnings on arm64

2021-03-03 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-03-03T23:12:12-08:00
New Revision: 3dcbfa27d432e3fddb61c5ad32534ce2bc1836e1

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

LOG: [debugserver] Fix more compiler warnings on arm64

This fixes the following two warnings in code that's only compiled on
arm64:

 - warning: cast from 'const void *' to 'unsigned char *' drops const
   qualifier [-Wcast-qual]
 - warning: embedding a directive within macro arguments has undefined
   behavior [-Wembedded-directive]

Added: 


Modified: 
lldb/tools/debugserver/source/MacOSX/arm/DNBArchImpl.cpp
lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp

Removed: 




diff  --git a/lldb/tools/debugserver/source/MacOSX/arm/DNBArchImpl.cpp 
b/lldb/tools/debugserver/source/MacOSX/arm/DNBArchImpl.cpp
index 82c49ce9b667..6a6f2fd09bd3 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm/DNBArchImpl.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm/DNBArchImpl.cpp
@@ -2118,7 +2118,7 @@ nub_size_t DNBArchMachARM::SetRegisterContext(const void 
*buf,
 
 // Copy each struct individually to avoid any padding that might be between
 // the structs in m_state.context
-uint8_t *p = (uint8_t *)buf;
+uint8_t *p = const_cast(reinterpret_cast(buf));
 ::memcpy(_state.context.gpr, p, sizeof(m_state.context.gpr));
 p += sizeof(m_state.context.gpr);
 ::memcpy(_state.context.vfp, p, sizeof(m_state.context.vfp));
@@ -2126,7 +2126,7 @@ nub_size_t DNBArchMachARM::SetRegisterContext(const void 
*buf,
 ::memcpy(_state.context.exc, p, sizeof(m_state.context.exc));
 p += sizeof(m_state.context.exc);
 
-size_t bytes_written = p - (uint8_t *)buf;
+size_t bytes_written = p - reinterpret_cast(buf);
 UNUSED_IF_ASSERT_DISABLED(bytes_written);
 assert(bytes_written == size);
 

diff  --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp 
b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index 2ea6b78f..bed54f93e29c 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -149,6 +149,18 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
  (thread_state_t)_state.context.gpr, );
   if (DNBLogEnabledForAny(LOG_THREAD)) {
 uint64_t *x = _state.context.gpr.__x[0];
+
+#if defined(__LP64__)
+uint64_t log_fp = arm_thread_state64_get_fp(m_state.context.gpr);
+uint64_t log_lr = arm_thread_state64_get_lr(m_state.context.gpr);
+uint64_t log_sp = arm_thread_state64_get_sp(m_state.context.gpr);
+uint64_t log_pc = arm_thread_state64_get_pc(m_state.context.gpr);
+#else
+uint64_t log_fp = m_state.context.gpr.__fp;
+uint64_t log_fp = m_state.context.gpr.__lr;
+uint64_t log_fp = m_state.context.gpr.__sp;
+uint64_t log_fp = m_state.context.gpr.__pc,
+#endif
 DNBLogThreaded(
 "thread_get_state(0x%4.4x, %u, , %u) => 0x%8.8x (count = %u) regs"
 "\n   x0=%16.16llx"
@@ -189,16 +201,7 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
 x[0], x[1], x[2], x[3], x[4], x[5], x[6], x[7], x[8], x[9], x[0], 
x[11],
 x[12], x[13], x[14], x[15], x[16], x[17], x[18], x[19], x[20], x[21],
 x[22], x[23], x[24], x[25], x[26], x[27], x[28],
-#if defined(__LP64__)
-(uint64_t) arm_thread_state64_get_fp (m_state.context.gpr),
-(uint64_t) arm_thread_state64_get_lr (m_state.context.gpr),
-(uint64_t) arm_thread_state64_get_sp (m_state.context.gpr),
-(uint64_t) arm_thread_state64_get_pc (m_state.context.gpr),
-#else
-m_state.context.gpr.__fp, m_state.context.gpr.__lr,
-m_state.context.gpr.__sp, m_state.context.gpr.__pc,
-#endif
-m_state.context.gpr.__cpsr);
+log_fp, log_lr, log_sp, log_pc, m_state.context.gpr.__cpsr);
   }
   m_state.SetError(set, Read, kret);
   return kret;
@@ -2280,7 +2283,7 @@ nub_size_t DNBArchMachARM64::SetRegisterContext(const 
void *buf,
 
 // Copy each struct individually to avoid any padding that might be between
 // the structs in m_state.context
-uint8_t *p = (uint8_t *)buf;
+uint8_t *p = const_cast(reinterpret_cast(buf));
 ::memcpy(_state.context.gpr, p, sizeof(m_state.context.gpr));
 p += sizeof(m_state.context.gpr);
 ::memcpy(_state.context.vfp, p, sizeof(m_state.context.vfp));
@@ -2288,7 +2291,7 @@ nub_size_t DNBArchMachARM64::SetRegisterContext(const 
void *buf,
 ::memcpy(_state.context.exc, p, sizeof(m_state.context.exc));
 p += sizeof(m_state.context.exc);
 
-size_t bytes_written = p - (uint8_t *)buf;
+size_t bytes_written = p - reinterpret_cast(buf);
 UNUSED_IF_ASSERT_DISABLED(bytes_written);

[Lldb-commits] [PATCH] D97910: [lldb/Interpreter] Make OptionGroupPythonClassWithDict options non-required

2021-03-03 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h:12
 
+#include 
+

We generally include system headers after project headers.



Comment at: lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h:68
   OptionDefinition m_option_definition[4];
+  std::bitset<4> m_required_options;
 };

I don't mind the bitset but we have a `Flags` utility that seems like a good 
fit for this. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97910

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

[Lldb-commits] [PATCH] D95100: [lldb/Commands] Fix short option collision for `process launch`

2021-03-03 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib closed this revision.
mib added a comment.

Landed in 103ad3f90 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95100

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

[Lldb-commits] [PATCH] D97910: [lldb/Interpreter] Make OptionGroupPythonClassWithDict options non-required

2021-03-03 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 328020.
mib added a comment.

Fix header guard length.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97910

Files:
  lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
  lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp

Index: lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp
===
--- lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp
+++ lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp
@@ -13,12 +13,10 @@
 using namespace lldb;
 using namespace lldb_private;
 
-OptionGroupPythonClassWithDict::OptionGroupPythonClassWithDict
-(const char *class_use,
- bool is_class,
- int class_option,
- int key_option, 
- int value_option) : m_is_class(is_class) {
+OptionGroupPythonClassWithDict::OptionGroupPythonClassWithDict(
+const char *class_use, bool is_class, int class_option, int key_option,
+int value_option, uint16_t required_options)
+: m_is_class(is_class), m_required_options(required_options) {
   m_key_usage_text.assign("The key for a key/value pair passed to the "
   "implementation of a ");
   m_key_usage_text.append(class_use);
@@ -36,7 +34,7 @@
   m_class_usage_text.append(".");
   
   m_option_definition[0].usage_mask = LLDB_OPT_SET_1;
-  m_option_definition[0].required = true;
+  m_option_definition[0].required = m_required_options.test(0);
   m_option_definition[0].long_option = "script-class";
   m_option_definition[0].short_option = class_option;
   m_option_definition[0].validator = nullptr;
@@ -47,7 +45,7 @@
   m_option_definition[0].usage_text = m_class_usage_text.data();
 
   m_option_definition[1].usage_mask = LLDB_OPT_SET_2;
-  m_option_definition[1].required = false;
+  m_option_definition[1].required = m_required_options.test(1);
   m_option_definition[1].long_option = "structured-data-key";
   m_option_definition[1].short_option = key_option;
   m_option_definition[1].validator = nullptr;
@@ -58,7 +56,7 @@
   m_option_definition[1].usage_text = m_key_usage_text.data();
 
   m_option_definition[2].usage_mask = LLDB_OPT_SET_2;
-  m_option_definition[2].required = false;
+  m_option_definition[2].required = m_required_options.test(2);
   m_option_definition[2].long_option = "structured-data-value";
   m_option_definition[2].short_option = value_option;
   m_option_definition[2].validator = nullptr;
@@ -69,7 +67,7 @@
   m_option_definition[2].usage_text = m_value_usage_text.data();
   
   m_option_definition[3].usage_mask = LLDB_OPT_SET_3;
-  m_option_definition[3].required = true;
+  m_option_definition[3].required = m_required_options.test(3);
   m_option_definition[3].long_option = "python-function";
   m_option_definition[3].short_option = class_option;
   m_option_definition[3].validator = nullptr;
@@ -78,7 +76,6 @@
   m_option_definition[3].completion_type = 0;
   m_option_definition[3].argument_type = eArgTypePythonFunction;
   m_option_definition[3].usage_text = m_class_usage_text.data();
-
 }
 
 Status OptionGroupPythonClassWithDict::SetOptionValue(
Index: lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
===
--- lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
+++ lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
@@ -1,4 +1,4 @@
-//===-- OptionGroupPythonClassWithDict.h -*- C++ -*-===//
+//===-- OptionGroupPythonClassWithDict.h *- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -9,6 +9,8 @@
 #ifndef LLDB_INTERPRETER_OPTIONGROUPPYTHONCLASSWITHDICT_H
 #define LLDB_INTERPRETER_OPTIONGROUPPYTHONCLASSWITHDICT_H
 
+#include 
+
 #include "lldb/lldb-types.h"
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Utility/StructuredData.h"
@@ -23,12 +25,20 @@
 // StructuredData::Dictionary is constructed with those pairs.
 class OptionGroupPythonClassWithDict : public OptionGroup {
 public:
-  OptionGroupPythonClassWithDict(const char *class_use,
- bool is_class = true,
- int class_option = 'C',
- int key_option = 'k', 
- int value_option = 'v');
-  
+  enum OptionKind {
+eScriptClass= 1 << 0,
+eDictKey= 1 << 1,
+eDictValue  = 1 << 2,
+eScriptFunction = 1 << 3,
+eAllOptions = (eScriptClass | eDictKey | eDictValue | eScriptFunction)
+  };
+
+  OptionGroupPythonClassWithDict(const char *class_use, bool is_class = true,
+ int class_option = 'C', int key_option = 'k',
+ int value_option = 'v',

[Lldb-commits] [PATCH] D97910: [lldb/Interpreter] Make OptionGroupPythonClassWithDict options non-required

2021-03-03 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

When using `OptionGroupPythonClassWithDict` options in an `OptionGroup`
with other `Options`, it can happen that the combinaison of some options
of each group makes the command invalid.

To solve that issue, this patch adds a bitmask argument to the
`OptionGroupPythonClassWithDict` constuctor that is used to mark each
option as required (or not).

If the `required_options` bitmask isn't passed to the constructor, the
class will keep its default behaviour, making the `--script-class` and
`--python-function` options required.

rdar://65508855

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97910

Files:
  lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
  lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp

Index: lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp
===
--- lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp
+++ lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp
@@ -13,12 +13,10 @@
 using namespace lldb;
 using namespace lldb_private;
 
-OptionGroupPythonClassWithDict::OptionGroupPythonClassWithDict
-(const char *class_use,
- bool is_class,
- int class_option,
- int key_option, 
- int value_option) : m_is_class(is_class) {
+OptionGroupPythonClassWithDict::OptionGroupPythonClassWithDict(
+const char *class_use, bool is_class, int class_option, int key_option,
+int value_option, uint16_t required_options)
+: m_is_class(is_class), m_required_options(required_options) {
   m_key_usage_text.assign("The key for a key/value pair passed to the "
   "implementation of a ");
   m_key_usage_text.append(class_use);
@@ -36,7 +34,7 @@
   m_class_usage_text.append(".");
   
   m_option_definition[0].usage_mask = LLDB_OPT_SET_1;
-  m_option_definition[0].required = true;
+  m_option_definition[0].required = m_required_options.test(0);
   m_option_definition[0].long_option = "script-class";
   m_option_definition[0].short_option = class_option;
   m_option_definition[0].validator = nullptr;
@@ -47,7 +45,7 @@
   m_option_definition[0].usage_text = m_class_usage_text.data();
 
   m_option_definition[1].usage_mask = LLDB_OPT_SET_2;
-  m_option_definition[1].required = false;
+  m_option_definition[1].required = m_required_options.test(1);
   m_option_definition[1].long_option = "structured-data-key";
   m_option_definition[1].short_option = key_option;
   m_option_definition[1].validator = nullptr;
@@ -58,7 +56,7 @@
   m_option_definition[1].usage_text = m_key_usage_text.data();
 
   m_option_definition[2].usage_mask = LLDB_OPT_SET_2;
-  m_option_definition[2].required = false;
+  m_option_definition[2].required = m_required_options.test(2);
   m_option_definition[2].long_option = "structured-data-value";
   m_option_definition[2].short_option = value_option;
   m_option_definition[2].validator = nullptr;
@@ -69,7 +67,7 @@
   m_option_definition[2].usage_text = m_value_usage_text.data();
   
   m_option_definition[3].usage_mask = LLDB_OPT_SET_3;
-  m_option_definition[3].required = true;
+  m_option_definition[3].required = m_required_options.test(3);
   m_option_definition[3].long_option = "python-function";
   m_option_definition[3].short_option = class_option;
   m_option_definition[3].validator = nullptr;
@@ -78,7 +76,6 @@
   m_option_definition[3].completion_type = 0;
   m_option_definition[3].argument_type = eArgTypePythonFunction;
   m_option_definition[3].usage_text = m_class_usage_text.data();
-
 }
 
 Status OptionGroupPythonClassWithDict::SetOptionValue(
Index: lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
===
--- lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
+++ lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
@@ -9,6 +9,8 @@
 #ifndef LLDB_INTERPRETER_OPTIONGROUPPYTHONCLASSWITHDICT_H
 #define LLDB_INTERPRETER_OPTIONGROUPPYTHONCLASSWITHDICT_H
 
+#include 
+
 #include "lldb/lldb-types.h"
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Utility/StructuredData.h"
@@ -23,12 +25,20 @@
 // StructuredData::Dictionary is constructed with those pairs.
 class OptionGroupPythonClassWithDict : public OptionGroup {
 public:
-  OptionGroupPythonClassWithDict(const char *class_use,
- bool is_class = true,
- int class_option = 'C',
- int key_option = 'k', 
- int value_option = 'v');
-  
+  enum OptionKind {
+eScriptClass= 1 << 0,
+eDictKey= 1 << 1,
+eDictValue  = 1 << 2,
+eScriptFunction = 1 << 3,
+eAllOptions = (eScriptClass

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

2021-03-03 Thread Jason Molenda via Phabricator via lldb-commits

This revision was not accepted when it landed; it landed in state "Needs 
Revision".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG266bb78f7d13: LanguageRuntime for 0th frame unwind, simplify 
getting pc-for-symbolication (authored by jasonmolenda).

Changed prior to commit:
  https://reviews.llvm.org/D97644?vs=327930=327991#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97644

Files:
  lldb/include/lldb/Target/LanguageRuntime.h
  lldb/include/lldb/Target/RegisterContext.h
  lldb/include/lldb/Target/RegisterContextUnwind.h
  lldb/include/lldb/Target/StackFrame.h
  lldb/source/Target/LanguageRuntime.cpp
  lldb/source/Target/RegisterContext.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/UnwindLLDB.cpp

Index: lldb/source/Target/UnwindLLDB.cpp
===
--- lldb/source/Target/UnwindLLDB.cpp
+++ lldb/source/Target/UnwindLLDB.cpp
@@ -420,6 +420,8 @@
   // too behaves like the zeroth frame (i.e. the pc might not
   // be pointing just past a call in it)
   behaves_like_zeroth_frame = true;
+} else if (m_frames[idx]->reg_ctx_lldb_sp->BehavesLikeZerothFrame()) {
+  behaves_like_zeroth_frame = true;
 } else {
   behaves_like_zeroth_frame = false;
 }
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -522,27 +522,10 @@
 SymbolContext unwind_sc = unwind_frame_sp->GetSymbolContext(
 eSymbolContextBlock | eSymbolContextFunction);
 Block *unwind_block = unwind_sc.block;
+TargetSP target_sp = m_thread.CalculateTarget();
 if (unwind_block) {
-  Address curr_frame_address(unwind_frame_sp->GetFrameCodeAddress());
-  TargetSP target_sp = m_thread.CalculateTarget();
-  // Be sure to adjust the frame address to match the address that was
-  // used to lookup the symbol context above. If we are in the first
-  // concrete frame, then we lookup using the current address, else we
-  // decrement the address by one to get the correct location.
-  if (idx > 0) {
-if (curr_frame_address.GetOffset() == 0) {
-  // If curr_frame_address points to the first address in a section
-  // then after adjustment it will point to an other section. In that
-  // case resolve the address again to the correct section plus
-  // offset form.
-  addr_t load_addr = curr_frame_address.GetOpcodeLoadAddress(
-  target_sp.get(), AddressClass::eCode);
-  curr_frame_address.SetOpcodeLoadAddress(
-  load_addr - 1, target_sp.get(), AddressClass::eCode);
-} else {
-  curr_frame_address.Slide(-1);
-}
-  }
+  Address curr_frame_address(
+  unwind_frame_sp->GetFrameCodeAddressForSymbolication());
 
   SymbolContext next_frame_sc;
   Address next_frame_address;
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -213,6 +213,35 @@
   return m_frame_code_addr;
 }
 
+// This can't be rewritten into a call to
+// RegisterContext::GetPCForSymbolication because this
+// StackFrame may have been constructed with a special pc,
+// e.g. tail-call artificial frames.
+Address StackFrame::GetFrameCodeAddressForSymbolication() {
+  Address lookup_addr(GetFrameCodeAddress());
+  if (!lookup_addr.IsValid())
+return lookup_addr;
+  if (m_behaves_like_zeroth_frame)
+return lookup_addr;
+
+  addr_t offset = lookup_addr.GetOffset();
+  if (offset > 0) {
+lookup_addr.SetOffset(offset - 1);
+  } else {
+// lookup_addr is the start of a section.  We need do the math on the
+// actual load address and re-compute the section.  We're working with
+// a 'noreturn' function at the end of a section.
+TargetSP target_sp = CalculateTarget();
+if (target_sp) {
+  addr_t addr_minus_one = lookup_addr.GetOpcodeLoadAddress(
+  target_sp.get(), AddressClass::eCode) -
+  1;
+  lookup_addr.SetOpcodeLoadAddress(addr_minus_one, target_sp.get());
+}
+  }
+  return lookup_addr;
+}
+
 bool StackFrame::ChangePC(addr_t pc) {
   std::lock_guard guard(m_mutex);
   // We can't change the pc value of a history stack frame - it is immutable.
@@ -288,30 +317,7 @@
 // If this is not frame zero, then we need to subtract 1 from the PC value
 // when doing address lookups since the PC will be on the instruction
 // following the function call instruction...
-
-

[Lldb-commits] [lldb] 266bb78 - LanguageRuntime for 0th frame unwind, simplify getting pc-for-symbolication

2021-03-03 Thread Jason Molenda via lldb-commits


Author: Jason Molenda
Date: 2021-03-03T19:29:40-08:00
New Revision: 266bb78f7d133a344992c5bfca61ef11ee152cb0

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

LOG: LanguageRuntime for 0th frame unwind, simplify getting pc-for-symbolication

Add calls into LanguageRuntime when finding the unwind method to
use out of the 0th (currently executing) stack frame.

Allow for the LanguageRuntimes to indicate if this stack frames
should be treated like a zeroth-frame -- symbolication should be
done based on the saved pc address, not decremented like normal ABI
function calls.

Add methods to RegisterContext and StackFrame to get a pc value
suitable for symbolication, to reduce the number of places in lldb
where we decrement the saved pc values before symbolication.


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

Added: 


Modified: 
lldb/include/lldb/Target/LanguageRuntime.h
lldb/include/lldb/Target/RegisterContext.h
lldb/include/lldb/Target/RegisterContextUnwind.h
lldb/include/lldb/Target/StackFrame.h
lldb/source/Target/LanguageRuntime.cpp
lldb/source/Target/RegisterContext.cpp
lldb/source/Target/RegisterContextUnwind.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/UnwindLLDB.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/LanguageRuntime.h 
b/lldb/include/lldb/Target/LanguageRuntime.h
index a8fc3eed0674..2f95c2643318 100644
--- a/lldb/include/lldb/Target/LanguageRuntime.h
+++ b/lldb/include/lldb/Target/LanguageRuntime.h
@@ -183,9 +183,29 @@ class LanguageRuntime : public Runtime, public 
PluginInterface {
   /// asynchronous calls they made, but are part of a logical backtrace that
   /// we want to show the developer because that's how they think of the
   /// program flow.
+  ///
+  /// \param[in] thread
+  /// The thread that the unwind is happening on.
+  ///
+  /// \param[in] regctx
+  /// The RegisterContext for the frame we need to create an UnwindPlan.
+  /// We don't yet have a StackFrame when we're selecting the UnwindPlan.
+  ///
+  /// \param[out] behaves_like_zeroth_frame
+  /// With normal ABI calls, all stack frames except the zeroth frame need
+  /// to have the return-pc value backed up by 1 for symbolication 
purposes.
+  /// For these LanguageRuntime unwind plans, they may not follow normal 
ABI
+  /// calling conventions and the return pc may need to be symbolicated
+  /// as-is.
+  ///
+  /// \return
+  /// Returns an UnwindPlan to find the caller frame if it should be used,
+  /// instead of the UnwindPlan that would normally be used for this
+  /// function.
   static lldb::UnwindPlanSP
   GetRuntimeUnwindPlan(lldb_private::Thread ,
-   lldb_private::RegisterContext *regctx);
+   lldb_private::RegisterContext *regctx,
+   bool _like_zeroth_frame);
 
 protected:
   // The static GetRuntimeUnwindPlan method above is only implemented in the
@@ -193,7 +213,8 @@ class LanguageRuntime : public Runtime, public 
PluginInterface {
   // provide one of these UnwindPlans.
   virtual lldb::UnwindPlanSP
   GetRuntimeUnwindPlan(lldb::ProcessSP process_sp,
-   lldb_private::RegisterContext *regctx) {
+   lldb_private::RegisterContext *regctx,
+   bool _like_zeroth_frame) {
 return lldb::UnwindPlanSP();
   }
 

diff  --git a/lldb/include/lldb/Target/RegisterContext.h 
b/lldb/include/lldb/Target/RegisterContext.h
index 5e795e59f941..c5068feedd5b 100644
--- a/lldb/include/lldb/Target/RegisterContext.h
+++ b/lldb/include/lldb/Target/RegisterContext.h
@@ -148,6 +148,31 @@ class RegisterContext : public 
std::enable_shared_from_this,
 
   uint64_t GetPC(uint64_t fail_value = LLDB_INVALID_ADDRESS);
 
+  /// Get an address suitable for symbolication.
+  /// When symbolicating -- computing line, block, function --
+  /// for a function in the middle of the stack, using the return
+  /// address can lead to unexpected results for the user.
+  /// A function that ends in a tail-call may have another function
+  /// as the "return" address, but it will never actually return.
+  /// Or a noreturn call in the middle of a function is the end of
+  /// a block of instructions, and a DWARF location list entry for
+  /// the return address may be a very 
diff erent code path with
+  /// incorrect results when printing variables for this frame.
+  ///
+  /// At a source line view, the user expects the current-line indictation
+  /// to point to the function call they're under, not the next source line.
+  ///
+  /// The return address (GetPC()) should always be shown to the user,
+  /// but when computing context, keeping within

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

2021-03-03 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added inline comments.



Comment at: lldb/source/Target/StackFrame.cpp:217-226
+  Address lookup_addr(GetFrameCodeAddress());
+  if (!lookup_addr.IsValid())
+return lookup_addr;
+  if (m_behaves_like_zeroth_frame)
+return lookup_addr;
+
+  addr_t offset = lookup_addr.GetOffset();

jasonmolenda wrote:
> clayborg wrote:
> > Should we be using the "bool RegisterContext::GetPCForSymbolication(Address 
> > )" function here to avoid having more than one place that is 
> > subtracting from the PC value?
> Yeah, I thought about that as I was doing it.  I'll unify these.
Ah, I see why they can't be unified.  A StackFrame may be set up with a pc 
value different than the register context for the concrete frame here, e.g. for 
a tail-call artifical stack frame.  It took me a little while to figure out why 
deferring to the StackFrame's RegisterContext was causing failures in those 
tests.  I added a comment saying why this code needs to decr-pc too, so someone 
doesn't try to clean these up in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97644

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

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

2021-03-03 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added inline comments.



Comment at: lldb/source/Target/RegisterContext.cpp:162
+  if (!BehavesLikeZerothFrame())
+pc--;
+  const bool allow_section_end = true;

jasonmolenda wrote:
> clayborg wrote:
> > Can we decrement the PC by the minimum instruction size? If we can get an 
> > ArchSpec we can call:
> > 
> > ```
> > uint32_t ArchSpec::GetMinimumOpcodeByteSize() const;
> > ```
> > 
> I don't know if it's worth it.  For fixed-width architectures, we'll be 
> symbolicating an instruction boundary, but for others, we're just 
> decrementing within the bounds of the instruction.  We should not ever show 
> this address anywhere, it's only used for setting context and we need to be 
> within the bounds of the instruction.  pc-1 seems fine to me.  I don't feel 
> very strongly about this.
My only reason to vote for doing this is to avoid creating a lookup address 
that looks like a thumb address on 32 bit ARM if we subtract 1 from it and the 
code is available above. That being said, I don't feel strongly about this 
either. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97644

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

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

2021-03-03 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added inline comments.



Comment at: lldb/source/Target/RegisterContext.cpp:162
+  if (!BehavesLikeZerothFrame())
+pc--;
+  const bool allow_section_end = true;

clayborg wrote:
> Can we decrement the PC by the minimum instruction size? If we can get an 
> ArchSpec we can call:
> 
> ```
> uint32_t ArchSpec::GetMinimumOpcodeByteSize() const;
> ```
> 
I don't know if it's worth it.  For fixed-width architectures, we'll be 
symbolicating an instruction boundary, but for others, we're just decrementing 
within the bounds of the instruction.  We should not ever show this address 
anywhere, it's only used for setting context and we need to be within the 
bounds of the instruction.  pc-1 seems fine to me.  I don't feel very strongly 
about this.



Comment at: lldb/source/Target/StackFrame.cpp:217-226
+  Address lookup_addr(GetFrameCodeAddress());
+  if (!lookup_addr.IsValid())
+return lookup_addr;
+  if (m_behaves_like_zeroth_frame)
+return lookup_addr;
+
+  addr_t offset = lookup_addr.GetOffset();

clayborg wrote:
> Should we be using the "bool RegisterContext::GetPCForSymbolication(Address 
> )" function here to avoid having more than one place that is 
> subtracting from the PC value?
Yeah, I thought about that as I was doing it.  I'll unify these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97644

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

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-03 Thread Greg Clayton via Phabricator via lldb-commits

clayborg updated this revision to Diff 327945.
clayborg added a comment.

Fix typo in headerdoc comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/Progress.h
  lldb/include/lldb/lldb-types.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/CMakeLists.txt
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/Progress.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -100,6 +100,12 @@
 
 typedef void (*RequestCallback)(const llvm::json::Object );
 
+static void LLDBProgressCallback(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total,
+ void *baton) {
+  g_vsc.SendProgressEvent(progress_id, message, completed, total);
+}
+
 enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
 SOCKET AcceptConnection(int portno) {
@@ -1352,6 +1358,8 @@
 // }
 void request_initialize(const llvm::json::Object ) {
   g_vsc.debugger = lldb::SBDebugger::Create(true /*source_init_files*/);
+  g_vsc.debugger.SetProgressCallback(LLDBProgressCallback, nullptr);
+
   // Create an empty target right away since we might get breakpoint requests
   // before we are given an executable to launch in a "launch" request, or a
   // executable when attaching to a process by process ID in a "attach"
@@ -1448,6 +1456,8 @@
   body.try_emplace("supportsDelayedStackTraceLoading", true);
   // The debug adapter supports the 'loadedSources' request.
   body.try_emplace("supportsLoadedSourcesRequest", false);
+  // The debug adapter supports sending progress reporting events.
+  body.try_emplace("supportsProgressReporting", true);
 
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
Index: lldb/tools/lldb-vscode/VSCode.h
===
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -132,6 +132,9 @@
 
   void SendOutput(OutputType o, const llvm::StringRef output);
 
+  void SendProgressEvent(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total);
+
   void __attribute__((format(printf, 3, 4)))
   SendFormattedOutput(OutputType o, const char *format, ...);
 
Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -40,7 +40,7 @@
   focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false),
   stop_at_entry(false), is_attach(false),
   reverse_request_seq(0), waiting_for_run_in_terminal(false) {
-  const char *log_file_path = getenv("LLDBVSCODE_LOG");
+  const char *log_file_path = "/tmp/vscode.txt"; // getenv("LLDBVSCODE_LOG");
 #if defined(_WIN32)
   // Windows opens stdout and stdin in text mode which converts \n to 13,10
   // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
@@ -225,6 +225,142 @@
   SendJSON(llvm::json::Value(std::move(event)));
 }
 
+// interface ProgressStartEvent extends Event {
+//   event: 'progressStart';
+//
+//   body: {
+// /**
+//  * An ID that must be used in subsequent 'progressUpdate' and
+//  'progressEnd'
+//  * events to make them refer to the same progress reporting.
+//  * IDs must be unique within a debug session.
+//  */
+// progressId: string;
+//
+// /**
+//  * Mandatory (short) title of the progress reporting. Shown in the UI to
+//  * describe the long running operation.
+//  */
+// title: string;
+//
+// /**
+//  * The request ID that this progress report is related to. If specified a
+//  * debug adapter is expected to emit
+//  * progress events for the long running request until the request has
+//  been
+//  * either completed or cancelled.
+//  * If the request ID is omitted, the progress report is assumed to be
+//  * related to some general activity of the debug adapter.
+//  */
+// requestId?: number;
+//
+// /**
+//  * If true, the request that reports progress may be canceled with a
+//  * 'cancel' request.
+//  * So this property basically controls whether the client should use UX
+//  that
+//  * supports

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

2021-03-03 Thread Greg Clayton via Phabricator via lldb-commits

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



Comment at: lldb/source/Target/RegisterContext.cpp:162
+  if (!BehavesLikeZerothFrame())
+pc--;
+  const bool allow_section_end = true;

Can we decrement the PC by the minimum instruction size? If we can get an 
ArchSpec we can call:

```
uint32_t ArchSpec::GetMinimumOpcodeByteSize() const;
```




Comment at: lldb/source/Target/StackFrame.cpp:217-226
+  Address lookup_addr(GetFrameCodeAddress());
+  if (!lookup_addr.IsValid())
+return lookup_addr;
+  if (m_behaves_like_zeroth_frame)
+return lookup_addr;
+
+  addr_t offset = lookup_addr.GetOffset();

Should we be using the "bool RegisterContext::GetPCForSymbolication(Address 
)" function here to avoid having more than one place that is 
subtracting from the PC value?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97644

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

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

2021-03-03 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda updated this revision to Diff 327930.
jasonmolenda added a comment.

Rewrite patch to expose "get pc for symbolication" APIs instead of "behaves 
like zeroth frame" APIs so that we don't have different parts of lldb 
decrementing the pc for symbolication.  Doing a final round of tests on 
different targets etc but I believe this is done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97644

Files:
  lldb/include/lldb/Target/LanguageRuntime.h
  lldb/include/lldb/Target/RegisterContext.h
  lldb/include/lldb/Target/RegisterContextUnwind.h
  lldb/include/lldb/Target/StackFrame.h
  lldb/source/Target/LanguageRuntime.cpp
  lldb/source/Target/RegisterContext.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/UnwindLLDB.cpp

Index: lldb/source/Target/UnwindLLDB.cpp
===
--- lldb/source/Target/UnwindLLDB.cpp
+++ lldb/source/Target/UnwindLLDB.cpp
@@ -420,6 +420,8 @@
   // too behaves like the zeroth frame (i.e. the pc might not
   // be pointing just past a call in it)
   behaves_like_zeroth_frame = true;
+} else if (m_frames[idx]->reg_ctx_lldb_sp->BehavesLikeZerothFrame()) {
+  behaves_like_zeroth_frame = true;
 } else {
   behaves_like_zeroth_frame = false;
 }
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -522,27 +522,10 @@
 SymbolContext unwind_sc = unwind_frame_sp->GetSymbolContext(
 eSymbolContextBlock | eSymbolContextFunction);
 Block *unwind_block = unwind_sc.block;
+TargetSP target_sp = m_thread.CalculateTarget();
 if (unwind_block) {
-  Address curr_frame_address(unwind_frame_sp->GetFrameCodeAddress());
-  TargetSP target_sp = m_thread.CalculateTarget();
-  // Be sure to adjust the frame address to match the address that was
-  // used to lookup the symbol context above. If we are in the first
-  // concrete frame, then we lookup using the current address, else we
-  // decrement the address by one to get the correct location.
-  if (idx > 0) {
-if (curr_frame_address.GetOffset() == 0) {
-  // If curr_frame_address points to the first address in a section
-  // then after adjustment it will point to an other section. In that
-  // case resolve the address again to the correct section plus
-  // offset form.
-  addr_t load_addr = curr_frame_address.GetOpcodeLoadAddress(
-  target_sp.get(), AddressClass::eCode);
-  curr_frame_address.SetOpcodeLoadAddress(
-  load_addr - 1, target_sp.get(), AddressClass::eCode);
-} else {
-  curr_frame_address.Slide(-1);
-}
-  }
+  Address curr_frame_address(
+  unwind_frame_sp->GetFrameCodeAddressForSymbolication());
 
   SymbolContext next_frame_sc;
   Address next_frame_address;
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -213,6 +213,31 @@
   return m_frame_code_addr;
 }
 
+Address StackFrame::GetFrameCodeAddressForSymbolication() {
+  Address lookup_addr(GetFrameCodeAddress());
+  if (!lookup_addr.IsValid())
+return lookup_addr;
+  if (m_behaves_like_zeroth_frame)
+return lookup_addr;
+
+  addr_t offset = lookup_addr.GetOffset();
+  if (offset > 0) {
+lookup_addr.SetOffset(offset - 1);
+  } else {
+// lookup_addr is the start of a section.  We need do the math on the
+// actual load address and re-compute the section.  We're working with
+// a 'noreturn' function at the end of a section.
+TargetSP target_sp = CalculateTarget();
+if (target_sp) {
+  addr_t addr_minus_one = lookup_addr.GetOpcodeLoadAddress(
+  target_sp.get(), AddressClass::eCode) -
+  1;
+  lookup_addr.SetOpcodeLoadAddress(addr_minus_one, target_sp.get());
+}
+  }
+  return lookup_addr;
+}
+
 bool StackFrame::ChangePC(addr_t pc) {
   std::lock_guard guard(m_mutex);
   // We can't change the pc value of a history stack frame - it is immutable.
@@ -288,30 +313,7 @@
 // If this is not frame zero, then we need to subtract 1 from the PC value
 // when doing address lookups since the PC will be on the instruction
 // following the function call instruction...
-
-Address lookup_addr(GetFrameCodeAddress());
-if (!m_behaves_like_zeroth_frame && lookup_addr.IsValid()) {
-  addr_t offset = lookup_addr.GetOffset();
-  if (offset > 0) {
-lookup_addr.SetOffset(offset - 1);
-
-  } else {
-// lookup_addr is the start

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-03 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

This version should address all comments and issue that I am aware of. Now the 
callbacks are registered with each debugger and the Progress class no longer 
has any global state. The Progress class calls a static method on the debugger 
which allows the Debugger class to iterate over the global debugger list and 
report progress to any debuggers that have callbacks. This also means that 
later we can modify the Progress class to allow debugger specific progress 
notifications by modifying the Progress class to take a debugger ID or pointer 
as an optional constructor argument and then that progress can be reported to 
only that debugger instance. I don't have any places I need this yet, so I 
haven't added support for it.




Comment at: lldb/include/lldb/Core/Progress.h:106
+  /// Total amount of work, llvm::None for non deterministic progress.
+  const uint64_t m_total;
+  /// Set to true when progress has been reported where m_completed == m_total

Using llvm::Optional was too messy. I tried, but since the callback requires a 
completed amount and total to be supplied at all times, it was much easier and 
cleaner to use UINT64_MAX since that would need to be sent anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-03 Thread Greg Clayton via Phabricator via lldb-commits

clayborg updated this revision to Diff 327871.
clayborg added a comment.

Update fixes:

- make progress callback and baton members of a lldb_private::Debugger object 
to allow each debugger to have their own callbacks.
- switch to have std::string title and clients use llvm::formatv() to create 
the title to avoid printf variadic args in progress constructor
- make the progress class thread safe using a mutex
- verified that lldb-vscode callback is threadsafe


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/Progress.h
  lldb/include/lldb/lldb-types.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/CMakeLists.txt
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/Progress.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -100,6 +100,12 @@
 
 typedef void (*RequestCallback)(const llvm::json::Object );
 
+static void LLDBProgressCallback(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total,
+ void *baton) {
+  g_vsc.SendProgressEvent(progress_id, message, completed, total);
+}
+
 enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
 SOCKET AcceptConnection(int portno) {
@@ -1352,6 +1358,8 @@
 // }
 void request_initialize(const llvm::json::Object ) {
   g_vsc.debugger = lldb::SBDebugger::Create(true /*source_init_files*/);
+  g_vsc.debugger.SetProgressCallback(LLDBProgressCallback, nullptr);
+
   // Create an empty target right away since we might get breakpoint requests
   // before we are given an executable to launch in a "launch" request, or a
   // executable when attaching to a process by process ID in a "attach"
@@ -1448,6 +1456,8 @@
   body.try_emplace("supportsDelayedStackTraceLoading", true);
   // The debug adapter supports the 'loadedSources' request.
   body.try_emplace("supportsLoadedSourcesRequest", false);
+  // The debug adapter supports sending progress reporting events.
+  body.try_emplace("supportsProgressReporting", true);
 
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
Index: lldb/tools/lldb-vscode/VSCode.h
===
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -132,6 +132,9 @@
 
   void SendOutput(OutputType o, const llvm::StringRef output);
 
+  void SendProgressEvent(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total);
+
   void __attribute__((format(printf, 3, 4)))
   SendFormattedOutput(OutputType o, const char *format, ...);
 
Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -40,7 +40,7 @@
   focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false),
   stop_at_entry(false), is_attach(false),
   reverse_request_seq(0), waiting_for_run_in_terminal(false) {
-  const char *log_file_path = getenv("LLDBVSCODE_LOG");
+  const char *log_file_path = "/tmp/vscode.txt"; // getenv("LLDBVSCODE_LOG");
 #if defined(_WIN32)
   // Windows opens stdout and stdin in text mode which converts \n to 13,10
   // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
@@ -225,6 +225,142 @@
   SendJSON(llvm::json::Value(std::move(event)));
 }
 
+// interface ProgressStartEvent extends Event {
+//   event: 'progressStart';
+//
+//   body: {
+// /**
+//  * An ID that must be used in subsequent 'progressUpdate' and
+//  'progressEnd'
+//  * events to make them refer to the same progress reporting.
+//  * IDs must be unique within a debug session.
+//  */
+// progressId: string;
+//
+// /**
+//  * Mandatory (short) title of the progress reporting. Shown in the UI to
+//  * describe the long running operation.
+//  */
+// title: string;
+//
+// /**
+//  * The request ID that this progress report is related to. If specified a
+//  * debug adapter is expected to emit
+//  * progress events for the long running request until the request has
+//  been
+//  * either completed or cancelled.
+//  * If the request ID is omitted, the progress

Re: [Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-03-03 Thread Caroline Tice via lldb-commits

Something went very wrong on the patch upload.  This does NOT look like the
patch I intended!

DO NOT REVIEW THIS UNTIL I CAN UPLOAD THE RIGHT PATCH!

-- Caroline
cmt...@google.com

On Tue, Mar 2, 2021 at 10:53 AM Shoaib Meenai via Phabricator <
revi...@reviews.llvm.org> wrote:

> smeenai added a comment.
>
> Is this rebased on main? The LLD for Mach-O changes look like they belong
> to a different diff.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D97786/new/
>
> https://reviews.llvm.org/D97786
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D97760: [lldb][NFC] Delete unused AddressResolverName

2021-03-03 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG820a8466097c: [lldb][NFC] Delete unused AddressResolverName 
(authored by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97760

Files:
  lldb/include/lldb/Core/AddressResolverName.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Core/AddressResolverName.cpp
  lldb/source/Core/CMakeLists.txt

Index: lldb/source/Core/CMakeLists.txt
===
--- lldb/source/Core/CMakeLists.txt
+++ lldb/source/Core/CMakeLists.txt
@@ -24,7 +24,6 @@
   AddressRange.cpp
   AddressResolver.cpp
   AddressResolverFileLine.cpp
-  AddressResolverName.cpp
   Communication.cpp
   Debugger.cpp
   Disassembler.cpp
Index: lldb/source/Core/AddressResolverName.cpp
===
--- lldb/source/Core/AddressResolverName.cpp
+++ /dev/null
@@ -1,198 +0,0 @@
-//===-- AddressResolverName.cpp ---===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "lldb/Core/AddressResolverName.h"
-
-#include "lldb/Core/Address.h"
-#include "lldb/Core/AddressRange.h"
-#include "lldb/Core/Module.h"
-#include "lldb/Symbol/Function.h"
-#include "lldb/Symbol/Symbol.h"
-#include "lldb/Symbol/SymbolContext.h"
-#include "lldb/Utility/Log.h"
-#include "lldb/Utility/Logging.h"
-#include "lldb/Utility/Stream.h"
-#include "lldb/lldb-enumerations.h"
-#include "lldb/lldb-forward.h"
-#include "lldb/lldb-types.h"
-#include "llvm/ADT/StringRef.h"
-
-#include 
-#include 
-#include 
-
-#include 
-
-using namespace lldb;
-using namespace lldb_private;
-
-AddressResolverName::AddressResolverName(const char *func_name,
- AddressResolver::MatchType type)
-: AddressResolver(), m_func_name(func_name), m_class_name(nullptr),
-  m_regex(), m_match_type(type) {
-  if (m_match_type == AddressResolver::Regexp) {
-m_regex = RegularExpression(m_func_name.GetStringRef());
-if (!m_regex.IsValid()) {
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-
-  if (log)
-log->Warning("function name regexp: \"%s\" did not compile.",
- m_func_name.AsCString());
-}
-  }
-}
-
-AddressResolverName::AddressResolverName(RegularExpression func_regex)
-: AddressResolver(), m_func_name(nullptr), m_class_name(nullptr),
-  m_regex(std::move(func_regex)), m_match_type(AddressResolver::Regexp) {}
-
-AddressResolverName::AddressResolverName(const char *class_name,
- const char *method,
- AddressResolver::MatchType type)
-: AddressResolver(), m_func_name(method), m_class_name(class_name),
-  m_regex(), m_match_type(type) {}
-
-AddressResolverName::~AddressResolverName() = default;
-
-// FIXME: Right now we look at the module level, and call the module's
-// "FindFunctions".
-// Greg says he will add function tables, maybe at the CompileUnit level to
-// accelerate function lookup.  At that point, we should switch the depth to
-// CompileUnit, and look in these tables.
-
-Searcher::CallbackReturn
-AddressResolverName::SearchCallback(SearchFilter ,
-SymbolContext , Address *addr) {
-  SymbolContextList func_list;
-  SymbolContextList sym_list;
-
-  bool skip_prologue = true;
-  uint32_t i;
-  SymbolContext sc;
-  Address func_addr;
-
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-
-  if (m_class_name) {
-if (log)
-  log->Warning("Class/method function specification not supported yet.\n");
-return Searcher::eCallbackReturnStop;
-  }
-
-  const bool include_symbols = false;
-  const bool include_inlines = true;
-  switch (m_match_type) {
-  case AddressResolver::Exact:
-if (context.module_sp) {
-  context.module_sp->FindSymbolsWithNameAndType(m_func_name,
-eSymbolTypeCode, sym_list);
-  context.module_sp->FindFunctions(m_func_name, CompilerDeclContext(),
-   eFunctionNameTypeAuto, include_symbols,
-   include_inlines, func_list);
-}
-break;
-
-  case AddressResolver::Regexp:
-if (context.module_sp) {
-  context.module_sp->FindSymbolsMatchingRegExAndType(
-  m_regex, eSymbolTypeCode, sym_list);
-  context.module_sp->FindFunctions(m_regex, include_symbols,
-   include_inlines, func_list);
-}
-break;

[Lldb-commits] [lldb] 820a846 - [lldb][NFC] Delete unused AddressResolverName

2021-03-03 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2021-03-03T13:30:02+01:00
New Revision: 820a8466097cd9336c81a14bcbe773807fbf96e4

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

LOG: [lldb][NFC] Delete unused AddressResolverName

That's all just dead code that hasn't been changed in years.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/source/API/SBTarget.cpp
lldb/source/Core/CMakeLists.txt

Removed: 
lldb/include/lldb/Core/AddressResolverName.h
lldb/source/Core/AddressResolverName.cpp



diff  --git a/lldb/include/lldb/Core/AddressResolverName.h 
b/lldb/include/lldb/Core/AddressResolverName.h
deleted file mode 100644
index 0ec1ef05b0ec..
--- a/lldb/include/lldb/Core/AddressResolverName.h
+++ /dev/null
@@ -1,63 +0,0 @@
-//===-- AddressResolverName.h ---*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLDB_CORE_ADDRESSRESOLVERNAME_H
-#define LLDB_CORE_ADDRESSRESOLVERNAME_H
-
-#include "lldb/Core/AddressResolver.h"
-#include "lldb/Core/SearchFilter.h"
-#include "lldb/Utility/ConstString.h"
-#include "lldb/Utility/RegularExpression.h"
-#include "lldb/lldb-defines.h"
-
-namespace lldb_private {
-class Address;
-class Stream;
-class SymbolContext;
-
-/// \class AddressResolverName AddressResolverName.h
-/// "lldb/Core/AddressResolverName.h" This class finds addresses for a given
-/// function name, either by exact match or by regular expression.
-
-class AddressResolverName : public AddressResolver {
-public:
-  AddressResolverName(const char *func_name,
-  AddressResolver::MatchType type = Exact);
-
-  // Creates a function breakpoint by regular expression.  Takes over control
-  // of the lifespan of func_regex.
-  AddressResolverName(RegularExpression func_regex);
-
-  AddressResolverName(const char *class_name, const char *method,
-  AddressResolver::MatchType type);
-
-  ~AddressResolverName() override;
-
-  Searcher::CallbackReturn SearchCallback(SearchFilter ,
-  SymbolContext ,
-  Address *addr) override;
-
-  lldb::SearchDepth GetDepth() override;
-
-  void GetDescription(Stream *s) override;
-
-protected:
-  ConstString m_func_name;
-  ConstString m_class_name; // FIXME: Not used yet.  The idea would be to stop
-// on methods of this class.
-  RegularExpression m_regex;
-  AddressResolver::MatchType m_match_type;
-
-private:
-  AddressResolverName(const AddressResolverName &) = delete;
-  const AddressResolverName =(const AddressResolverName &) = delete;
-};
-
-} // namespace lldb_private
-
-#endif // LLDB_CORE_ADDRESSRESOLVERNAME_H

diff  --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index b376bc4bd756..ad1ef6910701 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -32,7 +32,6 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Core/AddressResolver.h"
-#include "lldb/Core/AddressResolverName.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/Module.h"

diff  --git a/lldb/source/Core/AddressResolverName.cpp 
b/lldb/source/Core/AddressResolverName.cpp
deleted file mode 100644
index 51ab6435e8fb..
--- a/lldb/source/Core/AddressResolverName.cpp
+++ /dev/null
@@ -1,198 +0,0 @@
-//===-- AddressResolverName.cpp 
---===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "lldb/Core/AddressResolverName.h"
-
-#include "lldb/Core/Address.h"
-#include "lldb/Core/AddressRange.h"
-#include "lldb/Core/Module.h"
-#include "lldb/Symbol/Function.h"
-#include "lldb/Symbol/Symbol.h"
-#include "lldb/Symbol/SymbolContext.h"
-#include "lldb/Utility/Log.h"
-#include "lldb/Utility/Logging.h"
-#include "lldb/Utility/Stream.h"
-#include "lldb/lldb-enumerations.h"
-#include "lldb/lldb-forward.h"
-#include "lldb/lldb-types.h"
-#include "llvm/ADT/StringRef.h"
-
-#include 
-#include 
-#include 
-
-#include 
-
-using namespace lldb;
-using namespace lldb_private;
-
-AddressResolverName::AddressResolverName(const char *func_name,
-

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add memory tag reading to lldb-server

2021-03-03 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett marked an inline comment as done.
DavidSpickett added inline comments.



Comment at: 
lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py:19
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_qmemtags_packets(self):

omjavaid wrote:
> If skipUnlessAArch64MTELinuxCompiler can check for AArch64 and Linux then we 
> wont need above two decorators.
I'll merge them into one (at least one you use in tests, separate functions). 
Also I just realised this is not checking that the remote supports MTE, only 
worked because I've been using the one qemu instance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95601

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

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add memory tag reading to lldb-server

2021-03-03 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett marked an inline comment as done.
DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1357
+
+  if (error.Fail())
+return error;

omjavaid wrote:
> ptrace request is a success if number of tags requested is not equal to no of 
> tags read? If not then this and following condition may be redundant.
Well ptracewrapper doesn't check the iovec, but I'll check the kernel source to 
see if it's actually possible for it to fail that way.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3578
+
+  response.PutChar('m');
+  response.PutBytesAsRawHex8(tags.data(), tags.size());

omjavaid wrote:
> Just curious response starts with 'm'. Whats the design need for using m in 
> qMemTags response?
This is for future multi part replies ala qfThreadInfo 
(https://sourceware.org/gdb/current/onlinedocs/gdb/General-Query-Packets.html#General-Query-Packets).
 I'll add a comment with this too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95601

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

[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading to lldb

2021-03-03 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp:36
+  if (machine != llvm::Triple::aarch64 && machine != llvm::Triple::aarch64_be 
&&
+  machine != llvm::Triple::aarch64_32) {
+return nullptr;

omjavaid wrote:
> Do you think we should consider aarch64_32 as AArch64 architecture? 
> aarch64_32 AFAIK is used where 64bit kernel is running and 32bit executable. 
> Does MTE also apply on 32 bit executables running on 64bit kernel.
For MTE no but the rest of lldb considers it AArch64 for things like 
breakpoints, instruction emulation etc.

Though this Arch plugin only does MTE, in general they can control stepping 
behavior, things like that. So it makes sense to count it.

Plus for MTE we have to lookup the memory region via the kernel too. So if you 
were on aarch64_32 and you couldn't allocate MTE anyway, the debugger wouldn't 
be able to do anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95602

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

[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading to lldb

2021-03-03 Thread Muhammad Omair Javaid via Phabricator via lldb-commits

omjavaid added inline comments.



Comment at: lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp:36
+  if (machine != llvm::Triple::aarch64 && machine != llvm::Triple::aarch64_be 
&&
+  machine != llvm::Triple::aarch64_32) {
+return nullptr;

Do you think we should consider aarch64_32 as AArch64 architecture? aarch64_32 
AFAIK is used where 64bit kernel is running and 32bit executable. Does MTE also 
apply on 32 bit executables running on 64bit kernel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95602

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

[Lldb-commits] [PATCH] D97765: [lldb] Fix handling of `DW_AT_decl_file` according to D91014 (attempt #2)

2021-03-03 Thread Andy Yankovsky 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 rG3b47bd32f9df: [lldb] Fix handling of `DW_AT_decl_file` 
according to D91014 (attempt #2) (authored by werat).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97765

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  
lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s
  
lldb/test/Shell/SymbolFile/DWARF/Inputs/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu2.s

Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu2.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/Inputs/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu2.s
@@ -0,0 +1,160 @@
+	.text
+	.file	"inlinevar2.c"
+	.globl	other   # -- Begin function other
+	.type	other,@function
+other:  # @other
+.Lfunc_begin0:
+	.file	1 "" "inlinevar2.c"
+	.loc	1 3 0   # inlinevar2.c:3:0
+.Ltmp0:
+	.file	2 "" "./inlinevar.h"
+	.loc	2 2 16 prologue_end # ./inlinevar.h:2:16
+	movl	$42, %eax
+	.loc	2 3 10  # ./inlinevar.h:3:10
+	.loc	1 3 41  # inlinevar2.c:3:41
+	retq
+.Ltmp1:
+.Ltmp2:
+.Lfunc_end0:
+	.size	other, .Lfunc_end0-other
+# -- End function
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	17  # DW_TAG_compile_unit
+	.byte	1   # DW_CHILDREN_yes
+	.byte	37  # DW_AT_producer
+	.byte	14  # DW_FORM_strp
+	.byte	19  # DW_AT_language
+	.byte	5   # DW_FORM_data2
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	16  # DW_AT_stmt_list
+	.byte	23  # DW_FORM_sec_offset
+	.byte	27  # DW_AT_comp_dir
+	.byte	14  # DW_FORM_strp
+	.byte	17  # DW_AT_low_pc
+	.byte	1   # DW_FORM_addr
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	4   # Abbreviation Code
+	.byte	36  # DW_TAG_base_type
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	62  # DW_AT_encoding
+	.byte	11  # DW_FORM_data1
+	.byte	11  # DW_AT_byte_size
+	.byte	11  # DW_FORM_data1
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	6   # Abbreviation Code
+	.byte	46  # DW_TAG_subprogram
+	.byte	1   # DW_CHILDREN_yes
+	.byte	17  # DW_AT_low_pc
+	.byte	1   # DW_FORM_addr
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	64  # DW_AT_frame_base
+	.byte	24  # DW_FORM_exprloc
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	58  # DW_AT_decl_file
+	.byte	11  # DW_FORM_data1
+	.byte	59  # DW_AT_decl_line
+	.byte	11  # DW_FORM_data1
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	63  # DW_AT_external
+	.byte	25  # DW_FORM_flag_present
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	7   # Abbreviation Code
+	.byte	29  # DW_TAG_inlined_subroutine
+	.byte	1   # DW_CHILDREN_yes
+	.byte	49  # DW_AT_abstract_origin
+	.byte	0x10# DW_FORM_ref_addr
+	.byte	17  # DW_AT_low_pc
+	.byte	1   # DW_FORM_addr

[Lldb-commits] [lldb] 3b47bd3 - [lldb] Fix handling of `DW_AT_decl_file` according to D91014 (attempt #2)

2021-03-03 Thread Andy Yankovsky via lldb-commits


Author: Andy Yankovsky
Date: 2021-03-03T10:27:35+01:00
New Revision: 3b47bd32f9df4a57db98db5f35e680c7bd9fde3e

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

LOG: [lldb] Fix handling of `DW_AT_decl_file` according to D91014 (attempt #2)

Apply changes from https://reviews.llvm.org/D91014 to other places where DWARF 
entries are being processed.

Test case is provided by @jankratochvil.
The test is marked to run only on x64 and exclude Windows and Darwin, because 
the assembly is not OS-independent.

(First attempt https://reviews.llvm.org/D96778 broke the build bots)

Reviewed By: jankratochvil

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

Added: 

lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s

lldb/test/Shell/SymbolFile/DWARF/Inputs/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu2.s

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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 188a667ca564..af01a8f53518 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2201,7 +2201,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 case DW_AT_description:
 default:
 case DW_AT_decl_file:
-  decl.SetFile(die.GetCU()->GetFile(form_value.Unsigned()));
+  decl.SetFile(attributes.CompileUnitAtIndex(i)->GetFile(
+  form_value.Unsigned()));
   break;
 case DW_AT_decl_line:
   decl.SetLine(form_value.Unsigned());

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 7d273cb7df1b..587550961ec9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3126,8 +3126,8 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const 
SymbolContext ,
   continue;
 switch (attr) {
 case DW_AT_decl_file:
-  decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
+  decl.SetFile(
+  attributes.CompileUnitAtIndex(i)->GetFile(form_value.Unsigned()));
   break;
 case DW_AT_decl_line:
   decl.SetLine(form_value.Unsigned());

diff  --git 
a/lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s
 
b/lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s
new file mode 100644
index ..ef1481bbf800
--- /dev/null
+++ 
b/lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s
@@ -0,0 +1,171 @@
+# Check that DW_AT_decl_file of DW_AT_variable which is inherited by
+# DW_AT_abstract_origin from a 
diff erent DW_TAG_compile_unit is using the
+# DW_TAG_compile_unit->DW_AT_stmt_list where the DW_AT_decl_file is located 
(and
+# not where the DW_AT_abstract_origin is located).
+# DW_TAG_variable in CU 1 is using DW_AT_decl_file 3.
+# CU 1 has files: 
+# file_names[  1]: name: "inlinevarother.h"
+# file_names[  2]: name: "inlinevar1.c"
+# file_names[  3]: name: "inlinevar.h"
+# CU 2 has files: 
+# file_names[  1]: name: "inlinevar2.c"
+# file_names[  2]: name: "inlinevar.h"
+
+# UNSUPPORTED: system-darwin, system-windows
+# REQUIRES: target-x86_64
+
+# RUN: %clang_host -o %t %s \
+# RUN:   
%S/Inputs/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu2.s
+
+# RUN: %lldb %t \
+# RUN:   -o 'b other' -o r -o disas -o 'frame variable --show-declaration' \
+# RUN:   -o exit | FileCheck %s
+
+# CHECK: inlinevar.h:2: (int) var = {{.*}}
+# Unfixed LLDB did show only: (int) var = {{.*}}
+
+   .text
+   .file   "inlinevar1.c"
+   .file   1 "" "./inlinevarother.h"
+   .globl  main# -- Begin function main
+   .type   main,@function
+main:   # @main
+.Lfunc_begin1:
+   .file   2 "" "inlinevar1.c"
+   .loc2 4 0   # inlinevar1.c:4:0
+.Ltmp2:
+   .file   3 "" "./inlinevar.h"
+   .loc3 2 16 prologue_end # ./inlinevar.h:2:16
+   movl$42, %eax 
+   pushq   %rax
+   .loc3 3 10  # ./inlinevar.h:3:10
+.Ltmp3:
+   .loc2 5 20  # inlinevar1.c:5:20
+   callq   other
+   popq%rcx
+   .loc2 5 19  # inlinevar1.c:5:19
+   addl%ecx,

[Lldb-commits] [lldb] 3dcbfa2 - [debugserver] Fix more compiler warnings on arm64

[Lldb-commits] [PATCH] D97910: [lldb/Interpreter] Make OptionGroupPythonClassWithDict options non-required

[Lldb-commits] [PATCH] D95100: [lldb/Commands] Fix short option collision for `process launch`

[Lldb-commits] [PATCH] D97910: [lldb/Interpreter] Make OptionGroupPythonClassWithDict options non-required

[Lldb-commits] [PATCH] D97910: [lldb/Interpreter] Make OptionGroupPythonClassWithDict options non-required

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

[Lldb-commits] [lldb] 266bb78 - LanguageRuntime for 0th frame unwind, simplify getting pc-for-symbolication

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

[Lldb-commits] [PATCH] D97644: Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

Re: [Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

[Lldb-commits] [PATCH] D97760: [lldb][NFC] Delete unused AddressResolverName

[Lldb-commits] [lldb] 820a846 - [lldb][NFC] Delete unused AddressResolverName

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add memory tag reading to lldb-server

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add memory tag reading to lldb-server

[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading to lldb

[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading to lldb

[Lldb-commits] [PATCH] D97765: [lldb] Fix handling of `DW_AT_decl_file` according to D91014 (attempt #2)

[Lldb-commits] [lldb] 3b47bd3 - [lldb] Fix handling of `DW_AT_decl_file` according to D91014 (attempt #2)

24 matches

Site Navigation

Mail list logo

Footer information