[Lldb-commits] [lldb] 7e770f9 - Revert "Add SBValue::GetValueAsAddress API for removing non-addressing metadata"

Jason Molenda via lldb-commits lldb-commits at lists.llvm.org
Thu Mar 2 14:37:42 PST 2023


Author: Jason Molenda
Date: 2023-03-02T14:36:37-08:00
New Revision: 7e770f9c1713046cb7b724b51e928186ec123c26

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

LOG: Revert "Add SBValue::GetValueAsAddress API for removing non-addressing metadata"

Revert while I investigate two CI bot failures;
the more important is the lldb-arm-ubuntu where
the FixAddress is removing the 0th bit so we're
adding the `actual=` decorator on a string pointer,

```
Got output:
(char *) strptr = 0x00400817 (actual=0x400816) ptr = [{ },{H}]
```

in TestDataFormatterSmartArray.py line 229.

This reverts commit 4d635be2dbadc77522eddc9668697385a3b9f8b4.

Added: 
    

Modified: 
    lldb/bindings/interface/SBValueDocstrings.i
    lldb/include/lldb/API/SBValue.h
    lldb/include/lldb/Core/ValueObject.h
    lldb/source/API/SBValue.cpp
    lldb/source/Core/ValueObject.cpp
    lldb/source/DataFormatters/ValueObjectPrinter.cpp

Removed: 
    lldb/test/API/api/clear-sbvalue-nonadressable-bits/Makefile
    lldb/test/API/api/clear-sbvalue-nonadressable-bits/TestClearSBValueNonAddressableBits.py
    lldb/test/API/api/clear-sbvalue-nonadressable-bits/main.c


################################################################################
diff  --git a/lldb/bindings/interface/SBValueDocstrings.i b/lldb/bindings/interface/SBValueDocstrings.i
index 0bdd3867aa234..6bab923e8b35a 100644
--- a/lldb/bindings/interface/SBValueDocstrings.i
+++ b/lldb/bindings/interface/SBValueDocstrings.i
@@ -135,24 +135,6 @@ linked list."
 %feature("docstring", "Expands nested expressions like .a->b[0].c[1]->d."
 ) lldb::SBValue::GetValueForExpressionPath;
 
-%feature("docstring", "     
-      Return the value as an address.  On failure, LLDB_INVALID_ADDRESS
-      will be returned.  On architectures like AArch64, where the
-      top (unaddressable) bits can be used for authentication,
-      memory tagging, or top byte ignore,  this method will return
-      the value with those top bits cleared.
-
-      GetValueAsUnsigned returns the actual value, with the
-      authentication/Top Byte Ignore/Memory Tagging Extension bits.
-
-      Calling this on a random value which is not a pointer is
-      incorrect.  Call GetType().IsPointerType() if in doubt.
-
-      An SB API program may want to show both the literal byte value
-      and the address it refers to in memory.  These two SBValue
-      methods allow SB API writers to behave appropriately for their
-      interface.") lldb::SBValue::GetValueAsAddress;
-
 %feature("doctstring", "
     Returns the number for children.
 

diff  --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h
index 6409bb18678aa..1cba492486c63 100644
--- a/lldb/include/lldb/API/SBValue.h
+++ b/lldb/include/lldb/API/SBValue.h
@@ -62,8 +62,6 @@ class LLDB_API SBValue {
 
   uint64_t GetValueAsUnsigned(uint64_t fail_value = 0);
 
-  lldb::addr_t GetValueAsAddress();
-
   ValueType GetValueType();
 
   // If you call this on a newly created ValueObject, it will always return

diff  --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h
index d249fc27bd789..a666d0bab1730 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -564,10 +564,6 @@ class ValueObject {
 
   lldb::addr_t GetPointerValue(AddressType *address_type = nullptr);
 
-  /// Remove TBI/MTE/ptrauth bits from address, if those are defined on this
-  /// target/ABI.
-  lldb::addr_t GetStrippedPointerValue(lldb::addr_t address);
-
   lldb::ValueObjectSP GetSyntheticChild(ConstString key) const;
 
   lldb::ValueObjectSP GetSyntheticArrayMember(size_t index, bool can_create);

diff  --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 2c853c7579aa5..ecd16ccf83a8f 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -30,7 +30,6 @@
 #include "lldb/Symbol/Type.h"
 #include "lldb/Symbol/Variable.h"
 #include "lldb/Symbol/VariableList.h"
-#include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StackFrame.h"
@@ -925,25 +924,6 @@ uint64_t SBValue::GetValueAsUnsigned(uint64_t fail_value) {
   return fail_value;
 }
 
-lldb::addr_t SBValue::GetValueAsAddress() {
-  addr_t fail_value = LLDB_INVALID_ADDRESS;
-  ValueLocker locker;
-  lldb::ValueObjectSP value_sp(GetSP(locker));
-  if (value_sp) {
-    bool success = true;
-    uint64_t ret_val = fail_value;
-    ret_val = value_sp->GetValueAsUnsigned(fail_value, &success);
-    if (!success)
-      return fail_value;
-    if (ProcessSP process_sp = m_opaque_sp->GetProcessSP())
-      if (ABISP abi_sp = process_sp->GetABI())
-        return abi_sp->FixCodeAddress(ret_val);
-    return ret_val;
-  }
-
-  return fail_value;
-}
-
 bool SBValue::MightHaveChildren() {
   LLDB_INSTRUMENT_VA(this);
 

diff  --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 821f2b31871b0..6e79a04d024e5 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -31,7 +31,6 @@
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Symbol/Type.h"
 #include "lldb/Symbol/Variable.h"
-#include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Language.h"
 #include "lldb/Target/LanguageRuntime.h"
@@ -1454,14 +1453,6 @@ addr_t ValueObject::GetAddressOf(bool scalar_is_load_address,
   return LLDB_INVALID_ADDRESS;
 }
 
-addr_t ValueObject::GetStrippedPointerValue(addr_t address) {
-  ExecutionContext exe_ctx(GetExecutionContextRef());
-  if (Process *process = exe_ctx.GetProcessPtr())
-    if (ABISP abi_sp = process->GetABI())
-      return abi_sp->FixCodeAddress(address);
-  return address;
-}
-
 addr_t ValueObject::GetPointerValue(AddressType *address_type) {
   addr_t address = LLDB_INVALID_ADDRESS;
   if (address_type)
@@ -1477,15 +1468,11 @@ addr_t ValueObject::GetPointerValue(AddressType *address_type) {
     address = m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
     break;
 
-  case Value::ValueType::HostAddress: {
-    lldb::offset_t data_offset = 0;
-    address = m_data.GetAddress(&data_offset);
-  } break;
-
+  case Value::ValueType::HostAddress:
   case Value::ValueType::LoadAddress:
   case Value::ValueType::FileAddress: {
     lldb::offset_t data_offset = 0;
-    address = GetStrippedPointerValue(m_data.GetAddress(&data_offset));
+    address = m_data.GetAddress(&data_offset);
   } break;
   }
 
@@ -3024,11 +3011,6 @@ lldb::ValueObjectSP ValueObject::CreateValueObjectFromAddress(
   if (type) {
     CompilerType pointer_type(type.GetPointerType());
     if (pointer_type) {
-      if (Process *process = exe_ctx.GetProcessPtr()) {
-        if (ABISP abi_sp = process->GetABI()) {
-          address = abi_sp->FixCodeAddress(address);
-        }
-      }
       lldb::DataBufferSP buffer(
           new lldb_private::DataBufferHeap(&address, sizeof(lldb::addr_t)));
       lldb::ValueObjectSP ptr_result_valobj_sp(ValueObjectConstResult::Create(

diff  --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index 4f73cee482bcb..17e9877bd5242 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -440,13 +440,6 @@ bool ValueObjectPrinter::PrintValueAndSummaryIfNeeded(bool &value_printed,
           if (!m_options.m_hide_name)
             m_stream->PutChar(' ');
           m_stream->PutCString(m_value);
-          if (IsPointerValue(m_valobj->GetCompilerType())) {
-            uint64_t orig_value = m_valobj->GetValueAsUnsigned(0);
-            addr_t stripped = m_valobj->GetPointerValue();
-            if (stripped != orig_value) {
-              m_stream->Printf(" (actual=0x%" PRIx64 ")", stripped);
-            }
-          }
           value_printed = true;
         }
       }

diff  --git a/lldb/test/API/api/clear-sbvalue-nonadressable-bits/Makefile b/lldb/test/API/api/clear-sbvalue-nonadressable-bits/Makefile
deleted file mode 100644
index 10495940055b6..0000000000000
--- a/lldb/test/API/api/clear-sbvalue-nonadressable-bits/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-C_SOURCES := main.c
-
-include Makefile.rules

diff  --git a/lldb/test/API/api/clear-sbvalue-nonadressable-bits/TestClearSBValueNonAddressableBits.py b/lldb/test/API/api/clear-sbvalue-nonadressable-bits/TestClearSBValueNonAddressableBits.py
deleted file mode 100644
index 0de98ec98d2ba..0000000000000
--- a/lldb/test/API/api/clear-sbvalue-nonadressable-bits/TestClearSBValueNonAddressableBits.py
+++ /dev/null
@@ -1,58 +0,0 @@
-"""Test that SBValue clears non-addressable bits"""
-
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-class TestClearSBValueNonAddressableBits(TestBase):
-
-    NO_DEBUG_INFO_TESTCASE = True
-
-    # On AArch64 systems, the top bits that are not used for
-    # addressing may be used for TBI, MTE, and/or pointer 
-    # authentication.
-    @skipIf(archs=no_match(['aarch64', 'arm64', 'arm64e']))
-    
-    # Only run this test on systems where TBI is known to be
-    # enabled, so the address mask will clear the TBI bits.
-    @skipUnlessPlatform(["linux"]+lldbplatformutil.getDarwinOSTriples())
-    def test(self):
-        self.source = 'main.c'
-        self.build()
-        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
-                           "break here", lldb.SBFileSpec(self.source, False))
-
-        if self.TraceOn():
-            self.runCmd ("frame variable")
-            self.runCmd ("frame variable &count &global")
-
-        frame = thread.GetFrameAtIndex(0)
-
-        count_p = frame.FindVariable("count_p")
-        count_invalid_p = frame.FindVariable("count_invalid_p")
-        self.assertEqual(count_p.GetValueAsUnsigned(), count_invalid_p.GetValueAsAddress())
-        self.assertNotEqual(count_invalid_p.GetValueAsUnsigned(), count_invalid_p.GetValueAsAddress())
-        self.assertEqual(5, count_p.Dereference().GetValueAsUnsigned())
-        self.assertEqual(5, count_invalid_p.Dereference().GetValueAsUnsigned())
-        strm = lldb.SBStream()
-        count_invalid_p.GetDescription(strm)
-        self.assertIn("actual=0x", strm.GetData())
-
-        global_p = frame.FindVariable("global_p")
-        global_invalid_p = frame.FindVariable("global_invalid_p")
-        self.assertEqual(global_p.GetValueAsUnsigned(), global_invalid_p.GetValueAsAddress())
-        self.assertNotEqual(global_invalid_p.GetValueAsUnsigned(), global_invalid_p.GetValueAsAddress())
-        self.assertEqual(10, global_p.Dereference().GetValueAsUnsigned())
-        self.assertEqual(10, global_invalid_p.Dereference().GetValueAsUnsigned())
-        strm = lldb.SBStream()
-        global_invalid_p.GetDescription(strm)
-        self.assertIn("actual=0x", strm.GetData())
-
-        main_p = frame.FindVariable("main_p")
-        main_invalid_p = frame.FindVariable("main_invalid_p")
-        self.assertEqual(main_p.GetValueAsUnsigned(), main_invalid_p.GetValueAsAddress())
-        strm = lldb.SBStream()
-        main_invalid_p.GetDescription(strm)
-        self.assertIn("main at main.c:", strm.GetData())
-        self.assertIn("actual=0x", strm.GetData())

diff  --git a/lldb/test/API/api/clear-sbvalue-nonadressable-bits/main.c b/lldb/test/API/api/clear-sbvalue-nonadressable-bits/main.c
deleted file mode 100644
index 4eaca414f4752..0000000000000
--- a/lldb/test/API/api/clear-sbvalue-nonadressable-bits/main.c
+++ /dev/null
@@ -1,25 +0,0 @@
-int global = 10;
-
-int main() {
-  int count = 5;
-  int *count_p = &count;
-
-  // Add some metadata in the top byte (this will crash unless the
-  // test is running with TBI enabled, but we won't dereference it)
-
-  intptr_t scratch = (intptr_t)count_p;
-  scratch |= (3ULL << 60);
-  int *count_invalid_p = (int *)scratch;
-
-  int (*main_p)() = main;
-  scratch = (intptr_t)main_p;
-  scratch |= (3ULL << 60);
-  int (*main_invalid_p)() = (int (*)())scratch;
-
-  int *global_p = &global;
-  scratch = (intptr_t)global_p;
-  scratch |= (3ULL << 60);
-  int *global_invalid_p = (int *)scratch;
-
-  return count; // break here
-}


        


More information about the lldb-commits mailing list