[lldb] Make StackID call Fix{Code,Data} pointers (llvm#152796)

In architectures where pointers may contain metadata, such as arm64e, it
is important to ignore those bits when comparing two different StackIDs,
as the metadata may not be the same even if the pointers are.

This patch is a step towards allowing consumers of pointers to decide
whether they want to keep or remove metadata, as opposed to discarding
metadata at the moment pointers are created. See
llvm#150537.

This was tested running the LLDB test suite on arm64e.

(cherry picked from commit 9c8e716)

Author: felipepiovezan

lldb/include/lldb/Target/StackID.h

@@ -14,11 +14,16 @@
 
 namespace lldb_private {
 
+class Process;
+
 class StackID {
 public:
   // Constructors and Destructors
   StackID() = default;
 
+  explicit StackID(lldb::addr_t pc, lldb::addr_t cfa,
+                   SymbolContextScope *symbol_scope, Process *process);
+
   StackID(const StackID &rhs) = default;
 
   ~StackID() = default;
@@ -68,11 +73,8 @@ class StackID {
 protected:
   friend class StackFrame;
 
-  explicit StackID(lldb::addr_t pc, lldb::addr_t cfa) : m_pc(pc), m_cfa(cfa) {}
-
-  void SetPC(lldb::addr_t pc) { m_pc = pc; }
-
-  void SetCFA(lldb::addr_t cfa) { m_cfa = cfa; }
+  void SetPC(lldb::addr_t pc, Process *process);
+  void SetCFA(lldb::addr_t cfa, Process *process);
 
   lldb::addr_t m_pc =
       LLDB_INVALID_ADDRESS; // The pc value for the function/symbol for this

lldb/source/Target/StackFrame.cpp

@@ -62,7 +62,8 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
                        const SymbolContext *sc_ptr)
     : m_thread_wp(thread_sp), m_frame_index(frame_idx),
       m_concrete_frame_index(unwind_frame_index), m_reg_context_sp(),
-      m_id(pc, cfa), m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(),
+      m_id(pc, cfa, nullptr, thread_sp->GetProcess().get()),
+      m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(),
       m_frame_base_error(), m_cfa_is_valid(cfa_is_valid),
       m_stack_frame_kind(kind),
       m_behaves_like_zeroth_frame(behaves_like_zeroth_frame),
@@ -72,7 +73,7 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
   // recursive functions properly aren't confused with one another on a history
   // stack.
   if (IsHistorical() && !m_cfa_is_valid) {
-    m_id.SetCFA(m_frame_index);
+    m_id.SetCFA(m_frame_index, thread_sp->GetProcess().get());
   }
 
   if (sc_ptr != nullptr) {
@@ -88,9 +89,11 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
                        const SymbolContext *sc_ptr)
     : m_thread_wp(thread_sp), m_frame_index(frame_idx),
       m_concrete_frame_index(unwind_frame_index),
-      m_reg_context_sp(reg_context_sp), m_id(pc, cfa), m_frame_code_addr(pc),
-      m_sc(), m_flags(), m_frame_base(), m_frame_base_error(),
-      m_cfa_is_valid(true), m_stack_frame_kind(StackFrame::Kind::Regular),
+      m_reg_context_sp(reg_context_sp),
+      m_id(pc, cfa, nullptr, thread_sp->GetProcess().get()),
+      m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(),
+      m_frame_base_error(), m_cfa_is_valid(true),
+      m_stack_frame_kind(StackFrame::Kind::Regular),
       m_behaves_like_zeroth_frame(behaves_like_zeroth_frame),
       m_variable_list_sp(), m_variable_list_value_objects(),
       m_recognized_frame_sp(), m_disassembly(), m_mutex() {
@@ -114,7 +117,8 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
     : m_thread_wp(thread_sp), m_frame_index(frame_idx),
       m_concrete_frame_index(unwind_frame_index),
       m_reg_context_sp(reg_context_sp),
-      m_id(pc_addr.GetLoadAddress(thread_sp->CalculateTarget().get()), cfa),
+      m_id(pc_addr.GetLoadAddress(thread_sp->CalculateTarget().get()), cfa,
+           nullptr, thread_sp->GetProcess().get()),
       m_frame_code_addr(pc_addr), m_sc(), m_flags(), m_frame_base(),
       m_frame_base_error(), m_cfa_is_valid(true),
       m_stack_frame_kind(StackFrame::Kind::Regular),
@@ -2054,7 +2058,9 @@ void StackFrame::UpdatePreviousFrameFromCurrentFrame(StackFrame &curr_frame) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   assert(GetStackID() ==
          curr_frame.GetStackID());     // TODO: remove this after some testing
-  m_id.SetPC(curr_frame.m_id.GetPC()); // Update the Stack ID PC value
+  m_id.SetPC(
+      curr_frame.m_id.GetPC(),
+      curr_frame.CalculateProcess().get()); // Update the Stack ID PC value
   assert(GetThread() == curr_frame.GetThread());
   m_frame_index = curr_frame.m_frame_index;
   m_concrete_frame_index = curr_frame.m_concrete_frame_index;

lldb/source/Target/StackID.cpp

@@ -14,6 +14,7 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/LLDBLog.h"
+#include "lldb/Target/Process.h"
 #include "lldb/Utility/Stream.h"
 
 using namespace lldb_private;
@@ -32,6 +33,23 @@ bool StackID::IsCFAOnStack(Process &process) const {
   return m_cfa_on_stack == eLazyBoolYes;
 }
 
+StackID::StackID(lldb::addr_t pc, lldb::addr_t cfa,
+                 SymbolContextScope *symbol_scope, Process *process)
+    : m_pc(pc), m_cfa(cfa), m_symbol_scope(symbol_scope) {
+  if (process) {
+    m_pc = process->FixCodeAddress(m_pc);
+    m_cfa = process->FixDataAddress(m_cfa);
+  }
+}
+
+void StackID::SetPC(lldb::addr_t pc, Process *process) {
+  m_pc = process ? process->FixCodeAddress(pc) : pc;
+}
+
+void StackID::SetCFA(lldb::addr_t cfa, Process *process) {
+  m_cfa = process ? process->FixDataAddress(cfa) : cfa;
+}
+
 void StackID::Dump(Stream *s) {
   s->Printf("StackID (pc = 0x%16.16" PRIx64 ", cfa = 0x%16.16" PRIx64
             ", cfa_on_stack = %d, symbol_scope = %p",