[lldb][AArch64] Add memory tag writing to lldb

[DavidSpickett]

Summary

This adds memory tag writing to Process and the
GDB remote code. Supporting work for the
"memory tag write" command. (to follow)

Process WriteMemoryTags is similair to ReadMemoryTags.
It will pack the tags then call DoWriteMemoryTags.
That function will send the QMemTags packet to the gdb-remote.

The QMemTags packet follows the GDB specification in:
https://sourceware.org/gdb/current/onlinedocs/gdb/General-Query-Packets.html#General-Query-Packets

Note that lldb-server will be treating partial writes as
complete failures. So lldb doesn't need to handle the partial
write case in any special way.

[omjavaid]

lldb/include/lldb/Target/MemoryTagManager.h

35 ↗

(On Diff #355500)

I was wondering if you can explain reason for hosting this struct. Is there a association between MemoryTagManager and Tag Range.

I think same tag manager was associated with the whole of process address space? so why host tag manager pointer along with the range when we already have a pointer to process. This implies there could be different tag managers for different ranges? Our initial implementation introduced per architecture tag manager and for Process AArch64 we can use AArch64 Tag Manager for all our tag ranges. This appears to have over complicated range expansion.

[DavidSpickett]

lldb/include/lldb/Target/MemoryTagManager.h

35 ↗

(On Diff #355500)

The comment here is a bit misleading re. the "validity" of the manager. The manager itself is valid if you've got tagging, wherever it might be, you're right there.

The commit message has a better justification:

To save the commands then redoing that alignment
both memory tag manager methods now return a struct
of the range we checked and the manager itself.

So this is not "this manager is valid for this range", it's "here's a manager and an aligned range". Since we have to align the original range, then check the memory flags, then finally return the manager. Might as well give them the range too.

I agree that the comment here doesn't explain that well, I'll make it more precise.

[omjavaid]

lldb/include/lldb/Target/MemoryTagManager.h

35 ↗

(On Diff #355500)

The question I have is why we have to return the manager pointer with our expanded ranges. Tag ranges are per-process rather than per-tag-manager and we can get tag manager by keeping a reference to it in the process class then why return a manager alongside the expanded range.

lldb/source/Target/Process.cpp

6071

The point I was trying to establish above is that GetMemoryTagManagerImpl function here may remain as it was i-e GetMemoryTagManager. It could only run once on Process object creation. We know our process architecture so we can host a pointer to relevent tag manager in Process class. All the tag ranges corresponding to the current process address space should be able to get a pointer to MemoryTagManager as was being done previously.

6089

Main operation of this function is doing the expansion of ranges according to granule while the function name suggests that we are getting memory unique tag manager for addr, granules. Name doesnt even suggest that we are actually here to do range alignment/expansion.

[DavidSpickett]

lldb/source/Target/Process.cpp

6071

I see what you mean. I'm going to try this on top of main as a new change, then I'll refactor this based on that if it works out.

[DavidSpickett]

lldb/source/Target/Process.cpp

6071

It could only run once on Process object creation.

Turns out that the arch object, where the plugin is stored, changes 2/3 times between process creation and loading the program file. So we could store the pointer but we'd have to invalidate it on all these events. Ultimately we'd spend the time saved doing invalidation.

Which is probably why things like Target::GetCallableLoadAddress just get a new pointer each time.

You gave me some good ideas about simplifying this whole process though so I'm still going to work on that.

[DavidSpickett]

Rebase onto the earlier refactoring patch.

Now GetMemoryTagManager just deals with the manager,
no range handling. So this patch just adds WriteMemoryTags.

"memory tag write" will use MakeTaggedRange from
the memory tag manager instead of relying on the methods
I had here before.

[DavidSpickett]

Rebase, update use of SendPacketAndWaitForResponse.

lldb/include/lldb/Target/Process.h

/// \see Process::Notifications
   /// \return
   ///     If this architecture or process does not support memory tagging,
   ///     an error saying so.
   ///     If it does, either the memory tags or an error describing a
   ///     failure to read or unpack them.
   llvm::Expected<std::vector<lldb::addr_t>> ReadMemoryTags(lldb::addr_t addr,
                                                            size_t len);
 
+  /// Write memory tags for a range of memory.
+  /// (calls DoWriteMemoryTags to do the target specific work)
+  ///
+  /// \param[in] addr
+  ///     The address to start writing tags from. It is assumed that this
+  ///     address is granule aligned.
+  ///
+  /// \param[in] len
+  ///     The size of the range to write tags for. It is assumed that this
+  ///     is some multiple of the granule size. This len can be different
+  ///     from (number of tags * granule size) in the case where you want
+  ///     lldb-server to repeat tags across the range.
+  ///
+  /// \param[in] tags
+  ///     Allocation tags to be written. Since lldb-server can repeat tags for a
+  ///     range, the number of tags doesn't have to match the number of granules
+  ///     in the range. (though most of the time it will)
+  ///
+  /// \return
+  ///     A Status telling you if the write succeeded or not.
+  Status WriteMemoryTags(lldb::addr_t addr, size_t len,
+                         const std::vector<lldb::addr_t> &tags);
+
   /// Resolve dynamically loaded indirect functions.
   ///
   /// \param[in] address
   ///     The load address of the indirect function to resolve.
   ///
   /// \param[out] error
   ///     An error value in case the resolve fails.
   ///
protected:
   ///     if the read failed.
   virtual llvm::Expected<std::vector<uint8_t>>
   DoReadMemoryTags(lldb::addr_t addr, size_t len, int32_t type) {
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "%s does not support reading memory tags",
                                    GetPluginName().GetCString());
   }
 
+  /// Does the final operation to write memory tags. E.g. sending a GDB packet.
+  /// It assumes that WriteMemoryTags has checked that memory tagging is enabled
+  /// and has packed the tag data.
+  ///
+  /// \param[in] addr
+  ///    Start of address range to write memory tags for.
+  ///
+  /// \param[in] len
+  ///    Length of the memory range to write tags for (in bytes).
+  ///
+  /// \param[in] type
+  ///    Type of tags to read (get this from a MemoryTagManager)
+  ///
+  /// \param[in] tags
+  ///    Packed tags to be written.
+  ///
+  /// \return
+  ///     Status telling you whether the write succeeded.
+  virtual Status DoWriteMemoryTags(lldb::addr_t addr, size_t len, int32_t type,
+                                   const std::vector<uint8_t> &tags) {
+    return Status("%s does not support writing memory tags",
+                  GetPluginName().GetCString());
+  }
+
   // Type definitions
   typedef std::map<lldb::LanguageType, lldb::LanguageRuntimeSP>
       LanguageRuntimeCollection;
   typedef std::unordered_set<const void *> WarningsPointerSet;
   typedef std::map<uint64_t, WarningsPointerSet> WarningsCollection;
 
   struct PreResumeCallbackAndBaton {
     bool (*callback)(void *);

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

public:
 
   bool GetSharedCacheInfoSupported();
 
   bool GetMemoryTaggingSupported();
 
   lldb::DataBufferSP ReadMemoryTags(lldb::addr_t addr, size_t len,
                                     int32_t type);
 
+  Status WriteMemoryTags(lldb::addr_t addr, size_t len, int32_t type,
+                         const std::vector<uint8_t> &tags);
+
   /// Use qOffsets to query the offset used when relocating the target
   /// executable. If successful, the returned structure will contain at least
   /// one value in the offsets field.
   llvm::Optional<QOffsets> GetQOffsets();
 
   bool GetModuleInfo(const FileSpec &module_file_spec,
                      const ArchSpec &arch_spec, ModuleSpec &module_spec);
 

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

LLDB_LOGF(
         "GDBRemoteCommunicationClient::%s: Invalid data in qMemTags response",
         __FUNCTION__);
     return nullptr;
   }
 
   return buffer_sp;
 }
 
+Status GDBRemoteCommunicationClient::WriteMemoryTags(
+    lldb::addr_t addr, size_t len, int32_t type,
+    const std::vector<uint8_t> &tags) {
+  // Format QMemTags:address,length:type:tags
+  StreamString packet;
+  packet.Printf("QMemTags:%" PRIx64 ",%zx:%" PRIx32 ":", addr, len, type);
+  packet.PutBytesAsRawHex8(tags.data(), tags.size());
+
+  Status status;
+  StringExtractorGDBRemote response;
+  if (SendPacketAndWaitForResponse(packet.GetString(), response) !=
+          PacketResult::Success ||
+      !response.IsOKResponse()) {
+    status.SetErrorString("QMemTags packet failed");
+  }
+  return status;
+}
+
 bool GDBRemoteCommunicationClient::GetxPacketSupported() {
   if (m_supports_x == eLazyBoolCalculate) {
     StringExtractorGDBRemote response;
     m_supports_x = eLazyBoolNo;
     char packet[256];
     snprintf(packet, sizeof(packet), "x0,0");
     if (SendPacketAndWaitForResponse(packet, response) ==
         PacketResult::Success) {

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

protected:
 
   Status FlashDone();
 
   bool HasErased(FlashRange range);
 
   llvm::Expected<std::vector<uint8_t>>
   DoReadMemoryTags(lldb::addr_t addr, size_t len, int32_t type) override;
 
+  Status DoWriteMemoryTags(lldb::addr_t addr, size_t len, int32_t type,
+                           const std::vector<uint8_t> &tags) override;
+
 private:
   // For ProcessGDBRemote only
   std::string m_partial_profile_data;
   std::map<uint64_t, uint32_t> m_thread_id_to_used_usec_map;
   uint64_t m_last_signals_version = 0;
 
   static bool NewThreadNotifyBreakpointHit(void *baton,
                                            StoppointCallbackContext *context,

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

ProcessGDBRemote::DoReadMemoryTags(lldb::addr_t addr, size_t len,
   // Return the raw tag data
   llvm::ArrayRef<uint8_t> tag_data = buffer_sp->GetData();
   std::vector<uint8_t> got;
   got.reserve(tag_data.size());
   std::copy(tag_data.begin(), tag_data.end(), std::back_inserter(got));
   return got;
 }
 
+Status ProcessGDBRemote::DoWriteMemoryTags(lldb::addr_t addr, size_t len,
+                                           int32_t type,
+                                           const std::vector<uint8_t> &tags) {
+  // By now WriteMemoryTags should have validated that tagging is enabled
+  // for this target/process.
+  return m_gdb_comm.WriteMemoryTags(addr, len, type, tags);
+}
+
 Status ProcessGDBRemote::WriteObjectFile(
     std::vector<ObjectFile::LoadableData> entries) {
   Status error;
   // Sort the entries by address because some writes, like those to flash
   // memory, must happen in order of increasing address.
   std::stable_sort(
       std::begin(entries), std::end(entries),
       [](const ObjectFile::LoadableData a, const ObjectFile::LoadableData b) {

lldb/source/Target/Process.cpp

if (frame) {
       }
     }
   }
 
   return false;
 }
 
 llvm::Expected<const MemoryTagManager *> Process::GetMemoryTagManager() {
   Architecture *arch = GetTarget().GetArchitecturePlugin();
   const MemoryTagManager *tag_manager =
       arch ? arch->GetMemoryTagManager() : nullptr;
   if (!arch || !tag_manager) {
     return llvm::createStringError(
         llvm::inconvertibleErrorCode(),
         "This architecture does not support memory tagging",
         GetPluginName().GetCString());
   }
 
   if (!SupportsMemoryTagging()) {
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "Process does not support memory tagging");
   }
 
   return tag_manager;
 }
 
 llvm::Expected<std::vector<lldb::addr_t>>
 Process::ReadMemoryTags(lldb::addr_t addr, size_t len) {
   llvm::Expected<const MemoryTagManager *> tag_manager_or_err =
       GetMemoryTagManager();
   if (!tag_manager_or_err)
     return tag_manager_or_err.takeError();
 
   const MemoryTagManager *tag_manager = *tag_manager_or_err;
   llvm::Expected<std::vector<uint8_t>> tag_data =
       DoReadMemoryTags(addr, len, tag_manager->GetAllocationTagType());
   if (!tag_data)
     return tag_data.takeError();
 
   return tag_manager->UnpackTagsData(*tag_data,
                                      len / tag_manager->GetGranuleSize());
 }
+
+Status Process::WriteMemoryTags(lldb::addr_t addr, size_t len,
+                                const std::vector<lldb::addr_t> &tags) {
+  llvm::Expected<const MemoryTagManager *> tag_manager_or_err =
+      GetMemoryTagManager();
+  if (!tag_manager_or_err)
+    return Status(tag_manager_or_err.takeError());
+
+  const MemoryTagManager *tag_manager = *tag_manager_or_err;
+  llvm::Expected<std::vector<uint8_t>> packed_tags =
+      tag_manager->PackTags(tags);
+  if (!packed_tags) {
+    return Status(packed_tags.takeError());
+  }
+
+  return DoWriteMemoryTags(addr, len, tag_manager->GetAllocationTagType(),
+                           *packed_tags);
+}

lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/StructuredData.h"
 #include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include <future>
+#include <limits>
 
 using namespace lldb_private::process_gdb_remote;
 using namespace lldb_private;
 using namespace lldb;
 using namespace llvm;
 
 namespace {
 
TEST_F(GDBRemoteCommunicationClientTest, ReadMemoryTags) {
   check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "mhello",
                  llvm::None);
   // Data is not a complete hex char
   check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m9", llvm::None);
   // Data has a trailing hex char
   check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m01020",
                  llvm::None);
 }
+
+static void check_Qmemtags(TestClient &client, MockServer &server,
+                           lldb::addr_t addr, size_t len, int32_t type,
+                           const std::vector<uint8_t> &tags, const char *packet,
+                           llvm::StringRef response, bool should_succeed) {
+  const auto &WriteMemoryTags = [&]() {
+    std::future<Status> result = std::async(std::launch::async, [&] {
+      return client.WriteMemoryTags(addr, len, type, tags);
+    });
+
+    HandlePacket(server, packet, response);
+    return result.get();
+  };
+
+  auto result = WriteMemoryTags();
+  if (should_succeed)
+    ASSERT_TRUE(result.Success());
+  else
+    ASSERT_TRUE(result.Fail());
+}
+
+TEST_F(GDBRemoteCommunicationClientTest, WriteMemoryTags) {
+  check_Qmemtags(client, server, 0xABCD, 0x20, 1,
+                 std::vector<uint8_t>{0x12, 0x34}, "QMemTags:abcd,20:1:1234",
+                 "OK", true);
+
+  // The GDB layer doesn't care that the number of tags !=
+  // the length of the write.
+  check_Qmemtags(client, server, 0x4321, 0x20, 9, std::vector<uint8_t>{},
+                 "QMemTags:4321,20:9:", "OK", true);
+
+  check_Qmemtags(client, server, 0x8877, 0x123, 0x34,
+                 std::vector<uint8_t>{0x55, 0x66, 0x77},
+                 "QMemTags:8877,123:34:556677", "E01", false);
+
+  // Type is a signed integer but is packed as its raw bytes,
+  // instead of having a +/-.
+  check_Qmemtags(client, server, 0x456789, 0, -1, std::vector<uint8_t>{0x99},
+                 "QMemTags:456789,0:ffffffff:99", "E03", false);
+  check_Qmemtags(client, server, 0x456789, 0,
+                 std::numeric_limits<int32_t>::max(),
+                 std::vector<uint8_t>{0x99}, "QMemTags:456789,0:7fffffff:99",
+                 "E03", false);
+  check_Qmemtags(client, server, 0x456789, 0,
+                 std::numeric_limits<int32_t>::min(),
+                 std::vector<uint8_t>{0x99}, "QMemTags:456789,0:80000000:99",
+                 "E03", false);
+}