[lldb] Propagate destination type's byte order to memory writer

[kkcode0]

Summary

When a compiler attaches endianess to type, debugger should honor that when writing to variables of that type.

[kkcode0]

clang-format

[kkcode0]

typo

[DavidSpickett]

Seems like a noble goal but I don't have the experience to review.

I will just ask, how do you plan to test this? I guess this change is in response to an issue you hit, that's a good place to start from.

[kkcode0]

use ptracewrapper

lldb/include/lldb/Symbol/CompilerType.h

 //===-- CompilerType.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
 //
 //===----------------------------------------------------------------------===//
 
public:
 
   /// Return the size of the type in bytes.
   std::optional<uint64_t> GetByteSize(ExecutionContextScope *exe_scope) const;
   /// Return the size of the type in bits.
   std::optional<uint64_t> GetBitSize(ExecutionContextScope *exe_scope) const;
 
   lldb::Encoding GetEncoding(uint64_t &count) const;
 
+  lldb::ByteOrder GetByteOrder() const;
+
   lldb::Format GetFormat() const;
 
   std::optional<size_t> GetTypeBitAlign(ExecutionContextScope *exe_scope) const;
 
   uint32_t GetNumChildren(bool omit_empty_base_classes,
                           const ExecutionContext *exe_ctx) const;
 
   lldb::BasicType GetBasicTypeEnumeration() const;

lldb/include/lldb/Symbol/TypeSystem.h

 //===-- TypeSystem.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
 //
 //===----------------------------------------------------------------------===//
 
#endif
 
   virtual std::optional<uint64_t>
   GetBitSize(lldb::opaque_compiler_type_t type,
              ExecutionContextScope *exe_scope) = 0;
 
   virtual lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type,
                                      uint64_t &count) = 0;
 
+  virtual lldb::ByteOrder GetByteOrder(lldb::opaque_compiler_type_t type) = 0;
+
   virtual lldb::Format GetFormat(lldb::opaque_compiler_type_t type) = 0;
 
   virtual uint32_t GetNumChildren(lldb::opaque_compiler_type_t type,
                                   bool omit_empty_base_classes,
                                   const ExecutionContext *exe_ctx) = 0;
 
   virtual CompilerType GetBuiltinTypeByName(ConstString name);
 

lldb/include/lldb/Target/Process.h

 //===-- Process.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
 //
 //===----------------------------------------------------------------------===//
 
/// \see Process::Notifications
   ///     contains the data to write.
   ///
   /// \param[in] size
   ///     The number of bytes to write.
   ///
   /// \param[out] error
   ///     An error value in case the memory write fails.
   ///
+  /// \param[in] byte_order
+  ///     The byte order of destination type.
   /// \return
   ///     The number of bytes that were actually written.
   virtual size_t DoWriteMemory(lldb::addr_t vm_addr, const void *buf,
-                               size_t size, Status &error) {
+                               size_t size, Status &error,
+                               lldb::ByteOrder byte_order = endian::InlHostByteOrder()) {
     error.SetErrorStringWithFormatv(
         "error: {0} does not support writing to processes", GetPluginName());
     return 0;
   }
 
   /// Write all or part of a scalar value to memory.
   ///
   /// The value contained in \a scalar will be swapped to match the byte order
/// \see Process::Notifications
   ///     set to UINT32_MAX, then the size of \a scalar will be used.
   ///
   /// \param[out] error
   ///     An error value in case the memory write fails.
   ///
   /// \return
   ///     The number of bytes that were actually written.
   size_t WriteScalarToMemory(lldb::addr_t vm_addr, const Scalar &scalar,
-                             size_t size, Status &error);
+                             size_t size, Status &error,
+                             lldb::ByteOrder byte_order = endian::InlHostByteOrder());
 
   size_t ReadScalarIntegerFromMemory(lldb::addr_t addr, uint32_t byte_size,
                                      bool is_signed, Scalar &scalar,
                                      Status &error);
 
   /// Write memory to a process.
   ///
   /// This function will write memory to the current process's address space
/// \see Process::Notifications
   ///
   /// \param[in] buf
   ///     A byte buffer that is at least \a size bytes long that
   ///     contains the data to write.
   ///
   /// \param[in] size
   ///     The number of bytes to write.
   ///
+  /// \param[in] byte_order
+  ///     The byte order of destination type.
+  ///
   /// \return
   ///     The number of bytes that were actually written.
   // TODO: change this to take an ArrayRef<uint8_t>
   size_t WriteMemory(lldb::addr_t vm_addr, const void *buf, size_t size,
-                     Status &error);
+                     Status &error,
+                     lldb::ByteOrder byte_order = endian::InlHostByteOrder());
 
   /// Actually allocate memory in the process.
   ///
   /// This function will allocate memory in the process's address space.  This
   /// can't rely on the generic function calling mechanism, since that
   /// requires this function.
   ///
   /// \param[in] size
protected:
   bool GetEventsPrivate(lldb::EventSP &event_sp,
                         const Timeout<std::micro> &timeout, bool control_only);
 
   lldb::StateType
   GetStateChangedEventsPrivate(lldb::EventSP &event_sp,
                                const Timeout<std::micro> &timeout);
 
   size_t WriteMemoryPrivate(lldb::addr_t addr, const void *buf, size_t size,
-                            Status &error);
+                            Status &error,
+                            lldb::ByteOrder byte_order = endian::InlHostByteOrder());
 
   void AppendSTDOUT(const char *s, size_t len);
 
   void AppendSTDERR(const char *s, size_t len);
 
   void BroadcastAsyncProfileData(const std::string &one_profile_data);
 
   static void STDIOReadThreadBytesReceived(void *baton, const void *src,

lldb/source/Core/ValueObject.cpp

 //===-- ValueObject.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
 //
 //===----------------------------------------------------------------------===//
 
bool ValueObject::SetData(DataExtractor &data, Status &error) {
   // type is valid.
   if (!UpdateValueIfNeeded(false)) {
     error.SetErrorString("unable to read value");
     return false;
   }
 
   uint64_t count = 0;
   const Encoding encoding = GetCompilerType().GetEncoding(count);
+  const ByteOrder byte_order = GetCompilerType().GetByteOrder();
+
 
   const size_t byte_size = GetByteSize().value_or(0);
 
   Value::ValueType value_type = m_value.GetValueType();
 
   switch (value_type) {
   case Value::ValueType::Invalid:
     error.SetErrorString("invalid location");
bool ValueObject::SetData(DataExtractor &data, Status &error) {
   case Value::ValueType::LoadAddress: {
     // If it is a load address, then the scalar value is the storage location
     // of the data, and we have to shove this value down to that load location.
     ExecutionContext exe_ctx(GetExecutionContextRef());
     Process *process = exe_ctx.GetProcessPtr();
     if (process) {
       addr_t target_addr = m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
       size_t bytes_written = process->WriteMemory(
-          target_addr, data.GetDataStart(), byte_size, error);
+          target_addr, data.GetDataStart(), byte_size, error, byte_order);
       if (!error.Success())
         return false;
       if (bytes_written != byte_size) {
         error.SetErrorString("unable to write value to memory");
         return false;
       }
     }
   } break;
bool ValueObject::SetValueFromCString(const char *value_str, Status &error) {
   // type is valid.
   if (!UpdateValueIfNeeded(false)) {
     error.SetErrorString("unable to read value");
     return false;
   }
 
   uint64_t count = 0;
   const Encoding encoding = GetCompilerType().GetEncoding(count);
+  const ByteOrder byte_order = GetCompilerType().GetByteOrder();
 
   const size_t byte_size = GetByteSize().value_or(0);
 
   Value::ValueType value_type = m_value.GetValueType();
 
   if (value_type == Value::ValueType::Scalar) {
     // If the value is already a scalar, then let the scalar change itself:
     m_value.GetScalar().SetValueFromCString(value_str, encoding, byte_size);
if (error.Success()) {
         // location of the data, and we have to shove this value down to that
         // load location.
         ExecutionContext exe_ctx(GetExecutionContextRef());
         Process *process = exe_ctx.GetProcessPtr();
         if (process) {
           addr_t target_addr =
               m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
           size_t bytes_written = process->WriteScalarToMemory(
-              target_addr, new_scalar, byte_size, error);
+              target_addr, new_scalar, byte_size, error, byte_order);
           if (!error.Success())
             return false;
           if (bytes_written != byte_size) {
             error.SetErrorString("unable to write value to memory");
             return false;
           }
         }
       } break;

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

 //===-- ProcessGDBRemote.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
 //
 //===----------------------------------------------------------------------===//
 
public:
   // Process Memory
   size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
                       Status &error) override;
 
   Status
   WriteObjectFile(std::vector<ObjectFile::LoadableData> entries) override;
 
   size_t DoWriteMemory(lldb::addr_t addr, const void *buf, size_t size,
-                       Status &error) override;
+                       Status &error,
+                       lldb::ByteOrder byte_order = endian::InlHostByteOrder()) override;
 
   lldb::addr_t DoAllocateMemory(size_t size, uint32_t permissions,
                                 Status &error) override;
 
   Status DoDeallocateMemory(lldb::addr_t ptr) override;
 
   // Process STDIO
   size_t PutSTDIN(const char *buf, size_t buf_size, Status &error) override;

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

 //===-- ProcessGDBRemote.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
 //
 //===----------------------------------------------------------------------===//
 
if (m_gdb_comm.SendPacketAndWaitForResponse("vFlashDone", response,
     }
   } else {
     status.SetErrorStringWithFormat("failed to send flash done packet");
   }
   return status;
 }
 
 size_t ProcessGDBRemote::DoWriteMemory(addr_t addr, const void *buf,
-                                       size_t size, Status &error) {
+                                       size_t size, Status &error,
+                                       ByteOrder byte_order) {
   GetMaxMemorySize();
   // M and m packets take 2 bytes for 1 byte of memory
   size_t max_memory_size = m_max_memory_size / 2;
   if (size > max_memory_size) {
     // Keep memory read sizes down to a sane limit. This function will be
     // called multiple times in order to complete the task by
     // lldb_private::Process so it is ok to do this.
     size = max_memory_size;
if (is_flash) {
     error = FlashErase(addr, size);
     if (!error.Success())
       return 0;
     packet.Printf("vFlashWrite:%" PRIx64 ":", addr);
     packet.PutEscapedBytes(buf, size);
   } else {
     packet.Printf("M%" PRIx64 ",%" PRIx64 ":", addr, (uint64_t)size);
     packet.PutBytesAsRawHex8(buf, size, endian::InlHostByteOrder(),
-                             endian::InlHostByteOrder());
+                             byte_order);
   }
   StringExtractorGDBRemote response;
   if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response,
                                               GetInterruptTimeout()) ==
       GDBRemoteCommunication::PacketResult::Success) {
     if (response.IsOKResponse()) {
       error.Clear();
       return size;

lldb/source/Plugins/Process/scripted/ScriptedProcess.h

 //===-- ScriptedProcess.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
 //
 //===----------------------------------------------------------------------===//
 
public:
   void RefreshStateAfterStop() override;
 
   bool IsAlive() override;
 
   size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
                       Status &error) override;
 
   size_t DoWriteMemory(lldb::addr_t vm_addr, const void *buf, size_t size,
-                       Status &error) override;
+                       Status &error,
+                       lldb::ByteOrder byte_order = endian::InlHostByteOrder()) override;
 
   Status EnableBreakpointSite(BreakpointSite *bp_site) override;
 
   ArchSpec GetArchitecture();
 
   Status
   GetMemoryRegions(lldb_private::MemoryRegionInfos &region_list) override;
 

lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp

 //===-- ScriptedProcess.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
 //
 //===----------------------------------------------------------------------===//
 
size_t ScriptedProcess::DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
 
   // FIXME: We should use the diagnostic system to report a warning if the
   // `bytes_copied` is different from `size`.
 
   return bytes_copied;
 }
 
 size_t ScriptedProcess::DoWriteMemory(lldb::addr_t vm_addr, const void *buf,
-                                      size_t size, Status &error) {
+                                      size_t size, Status &error,
+                                      ByteOrder byte_order) {
   lldb::DataExtractorSP data_extractor_sp = std::make_shared<DataExtractor>(
       buf, size, GetByteOrder(), GetAddressByteSize());
 
   if (!data_extractor_sp || !data_extractor_sp->GetByteSize())
     return 0;
 
   lldb::offset_t bytes_written =
       GetInterface().WriteMemoryAtAddress(vm_addr, data_extractor_sp, error);

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

 //===-- TypeSystemClang.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
 //
 //===----------------------------------------------------------------------===//
 
#endif
   }
 
   std::optional<uint64_t> GetBitSize(lldb::opaque_compiler_type_t type,
                                      ExecutionContextScope *exe_scope) override;
 
   lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type,
                              uint64_t &count) override;
 
+  lldb::ByteOrder GetByteOrder(lldb::opaque_compiler_type_t type) override;
+
   lldb::Format GetFormat(lldb::opaque_compiler_type_t type) override;
 
   std::optional<size_t>
   GetTypeBitAlign(lldb::opaque_compiler_type_t type,
                   ExecutionContextScope *exe_scope) override;
 
   uint32_t GetNumChildren(lldb::opaque_compiler_type_t type,
                           bool omit_empty_base_classes,

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

 //===-- TypeSystemClang.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 "clang/Basic/RISCVVTypes.def"
   case clang::Type::ConstantMatrix:
   case clang::Type::DependentSizedMatrix:
     break;
   }
   count = 0;
   return lldb::eEncodingInvalid;
 }
 
+lldb::ByteOrder TypeSystemClang::GetByteOrder(lldb::opaque_compiler_type_t type) {
+    return endian::InlHostByteOrder();
+}
+
 lldb::Format TypeSystemClang::GetFormat(lldb::opaque_compiler_type_t type) {
   if (!type)
     return lldb::eFormatDefault;
 
   clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
 
   switch (qual_type->getTypeClass()) {
   case clang::Type::Atomic:

lldb/source/Symbol/CompilerType.cpp

 //===-- CompilerType.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
 //
 //===----------------------------------------------------------------------===//
 

 
 lldb::Encoding CompilerType::GetEncoding(uint64_t &count) const {
   if (IsValid())
     if (auto type_system_sp = GetTypeSystem())
       return type_system_sp->GetEncoding(m_type, count);
   return lldb::eEncodingInvalid;
 }
 
+lldb::ByteOrder CompilerType::GetByteOrder() const {
+  if (IsValid())
+    if (auto type_system_sp = GetTypeSystem())
+      return type_system_sp->GetByteOrder(m_type);
+  return endian::InlHostByteOrder();
+}
+
 lldb::Format CompilerType::GetFormat() const {
   if (IsValid())
     if (auto type_system_sp = GetTypeSystem())
       return type_system_sp->GetFormat(m_type);
   return lldb::eFormatDefault;
 }
 
 uint32_t CompilerType::GetNumChildren(bool omit_empty_base_classes,

lldb/source/Target/Process.cpp

 //===-- Process.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
 //
 //===----------------------------------------------------------------------===//
 
if (addr_byte_size <= 4)
     scalar = (uint32_t)ptr_value;
   else
     scalar = ptr_value;
   return WriteScalarToMemory(vm_addr, scalar, addr_byte_size, error) ==
          addr_byte_size;
 }
 
 size_t Process::WriteMemoryPrivate(addr_t addr, const void *buf, size_t size,
-                                   Status &error) {
+                                   Status &error, ByteOrder byte_order) {
   size_t bytes_written = 0;
   const uint8_t *bytes = (const uint8_t *)buf;
 
   while (bytes_written < size) {
     const size_t curr_size = size - bytes_written;
     const size_t curr_bytes_written = DoWriteMemory(
-        addr + bytes_written, bytes + bytes_written, curr_size, error);
+        addr + bytes_written, bytes + bytes_written, curr_size, error, byte_order);
     bytes_written += curr_bytes_written;
     if (curr_bytes_written == curr_size || curr_bytes_written == 0)
       break;
   }
   return bytes_written;
 }
 
 size_t Process::WriteMemory(addr_t addr, const void *buf, size_t size,
-                            Status &error) {
+                            Status &error, ByteOrder byte_order) {
   if (ABISP abi_sp = GetABI())
     addr = abi_sp->FixAnyAddress(addr);
 
 #if defined(ENABLE_MEMORY_CACHING)
   m_memory_cache.Flush(addr, size);
 #endif
 
   if (buf == nullptr || size == 0)
     return 0;
 
   m_mod_id.BumpMemoryID();
 
   // We need to write any data that would go where any current software traps
   // (enabled software breakpoints) any software traps (breakpoints) that we
   // may have placed in our tasks memory.
 
   BreakpointSiteList bp_sites_in_range;
   if (!m_breakpoint_site_list.FindInRange(addr, addr + size, bp_sites_in_range))
-    return WriteMemoryPrivate(addr, buf, size, error);
+    return WriteMemoryPrivate(addr, buf, size, error, byte_order);
 
   // No breakpoint sites overlap
   if (bp_sites_in_range.IsEmpty())
-    return WriteMemoryPrivate(addr, buf, size, error);
+    return WriteMemoryPrivate(addr, buf, size, error, byte_order);
 
   const uint8_t *ubuf = (const uint8_t *)buf;
   uint64_t bytes_written = 0;
 
   bp_sites_in_range.ForEach([this, addr, size, &bytes_written, &ubuf,
-                             &error](BreakpointSite *bp) -> void {
+                             &error, byte_order](BreakpointSite *bp) -> void {
     if (error.Fail())
       return;
 
     if (bp->GetType() != BreakpointSite::eSoftware)
       return;
 
     addr_t intersect_addr;
     size_t intersect_size;
bp_sites_in_range.ForEach([this, addr, size, &bytes_written, &ubuf,
 
     // Check for bytes before this breakpoint
     const addr_t curr_addr = addr + bytes_written;
     if (intersect_addr > curr_addr) {
       // There are some bytes before this breakpoint that we need to just
       // write to memory
       size_t curr_size = intersect_addr - curr_addr;
       size_t curr_bytes_written =
-          WriteMemoryPrivate(curr_addr, ubuf + bytes_written, curr_size, error);
+          WriteMemoryPrivate(curr_addr, ubuf + bytes_written, curr_size, error, byte_order);
       bytes_written += curr_bytes_written;
       if (curr_bytes_written != curr_size) {
         // We weren't able to write all of the requested bytes, we are
         // done looping and will return the number of bytes that we have
         // written so far.
         if (error.Success())
           error.SetErrorToGenericError();
       }
     }
     // Now write any bytes that would cover up any software breakpoints
     // directly into the breakpoint opcode buffer
     ::memcpy(bp->GetSavedOpcodeBytes() + opcode_offset, ubuf + bytes_written,
              intersect_size);
     bytes_written += intersect_size;
   });
 
   // Write any remaining bytes after the last breakpoint if we have any left
   if (bytes_written < size)
     bytes_written +=
         WriteMemoryPrivate(addr + bytes_written, ubuf + bytes_written,
-                           size - bytes_written, error);
+                           size - bytes_written, error, byte_order);
 
   return bytes_written;
 }
 
 size_t Process::WriteScalarToMemory(addr_t addr, const Scalar &scalar,
-                                    size_t byte_size, Status &error) {
+                                    size_t byte_size, Status &error,
+                                    ByteOrder byte_order) {
   if (byte_size == UINT32_MAX)
     byte_size = scalar.GetByteSize();
   if (byte_size > 0) {
     uint8_t buf[32];
     const size_t mem_size =
         scalar.GetAsMemoryData(buf, byte_size, GetByteOrder(), error);
     if (mem_size > 0)
-      return WriteMemory(addr, buf, mem_size, error);
+      return WriteMemory(addr, buf, mem_size, error, byte_order);
     else
       error.SetErrorString("failed to get scalar as memory data");
   } else {
     error.SetErrorString("invalid scalar value");
   }
   return 0;
 }