Modify DataEncoder to be able to encode data in an object owned buffer.

DataEncoder was previously made to modify data within an existing buffer. As the code progressed, new clients started using DataEncoder to create binary data. In these cases the use of this class was possibly, but only if you knew exactly how large your buffer would be ahead of time. This patchs adds the ability for DataEncoder to own a buffer that can be dynamically resized as data is appended to the buffer.

Change in this patch:
- Allow a DataEncoder object to be created that owns a DataBufferHeap object that can dynamically grow as data is appended
- Add new methods that start with "Append" to append data to the buffer and grow it as needed
- Adds full testing of the API to assure modifications don't regress any functionality
- Has two constructors: one that uses caller owned data and one that creates an object with object owned data
- "Append" methods only work if the object owns it own data
- Removes the ability to specify a shared memory buffer as no one was using this functionality. This allows us to switch to a case where the object owns its own data in a DataBufferHeap that can be resized as data is added

"Put" methods work on both caller and object owned data.
"Append" methods work on only object owned data where we can grow the buffer. These methods will return false if called on a DataEncoder object that has caller owned data.

The main reason for these modifications is to be able to use the DateEncoder objects instead of llvm::gsym::FileWriter in https://reviews.llvm.org/D113789. This patch wants to add the ability to create symbol table caching to LLDB and the code needs to build binary caches and save them to disk.

Reviewed By: labath

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

Author: clayborg

lldb/include/lldb/Utility/DataEncoder.h

@@ -16,6 +16,9 @@
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+
 #include <cstddef>
 #include <cstdint>
 
@@ -26,21 +29,34 @@ namespace lldb_private {
 /// An binary data encoding class.
 ///
 /// DataEncoder is a class that can encode binary data (swapping if needed) to
-/// a data buffer. The data buffer can be caller owned, or can be shared data
-/// that can be shared between multiple DataEncoder or DataEncoder instances.
+/// a data buffer. The DataEncoder can be constructed with data that will be
+/// copied into the internally owned buffer. This allows data to be modified
+/// in the internal buffer. The DataEncoder object can also be constructed with
+/// just a byte order and address size and data can be appended to the
+/// internally owned buffer.
+///
+/// Clients can get a shared pointer to the data buffer when done modifying or
+/// creating the data to keep the data around after the lifetime of a
+/// DataEncoder object. \see GetDataBuffer
 ///
-/// \see DataBuffer
+/// Client can get a reference to the object owned data as an array by calling
+/// the GetData method. \see GetData
 class DataEncoder {
 public:
   /// Default constructor.
   ///
-  /// Initialize all members to a default empty state.
+  /// Initialize all members to a default empty state and create a empty memory
+  /// buffer that can be appended to. The ByteOrder and address size will be set
+  /// to match the current host system.
   DataEncoder();
 
-  /// Construct with a buffer that is owned by the caller.
+  /// Construct an encoder that copies the specified data into the object owned
+  /// data buffer.
   ///
-  /// This constructor allows us to use data that is owned by the caller. The
-  /// data must stay around as long as this object is valid.
+  /// This constructor is designed to be used when you have a data buffer and
+  /// want to modify values within the buffer. A copy of the data will be made
+  /// in the internally owned buffer and that data can be fixed up and appended
+  /// to.
   ///
   /// \param[in] data
   ///     A pointer to caller owned data.
@@ -49,54 +65,37 @@ class DataEncoder {
   ///     The length in bytes of \a data.
   ///
   /// \param[in] byte_order
-  ///     A byte order of the data that we are extracting from.
+  ///     A byte order for the data that will be encoded.
   ///
   /// \param[in] addr_size
-  ///     A new address byte size value.
-  DataEncoder(void *data, uint32_t data_length, lldb::ByteOrder byte_order,
-              uint8_t addr_size);
+  ///     A size of an address in bytes. \see PutAddress, AppendAddress
+  DataEncoder(const void *data, uint32_t data_length,
+              lldb::ByteOrder byte_order, uint8_t addr_size);
 
-  /// Construct with shared data.
+  /// Construct an encoder that owns a heap based memory buffer.
   ///
-  /// Copies the data shared pointer which adds a reference to the contained
-  /// in \a data_sp. The shared data reference is reference counted to ensure
-  /// the data lives as long as anyone still has a valid shared pointer to the
-  /// data in \a data_sp.
-  ///
-  /// \param[in] data_sp
-  ///     A shared pointer to data.
+  /// This allows clients to create binary data from scratch by appending values
+  /// with the methods that start with "Append".
   ///
   /// \param[in] byte_order
-  ///     A byte order of the data that we are extracting from.
+  ///     A byte order for the data that will be encoded.
   ///
   /// \param[in] addr_size
-  ///     A new address byte size value.
-  DataEncoder(const lldb::DataBufferSP &data_sp, lldb::ByteOrder byte_order,
-              uint8_t addr_size);
+  ///     A size of an address in bytes. \see PutAddress, AppendAddress
+  DataEncoder(lldb::ByteOrder byte_order, uint8_t addr_size);
 
-  /// Destructor
-  ///
-  /// If this object contains a valid shared data reference, the reference
-  /// count on the data will be decremented, and if zero, the data will be
-  /// freed.
   ~DataEncoder();
 
-  /// Clears the object state.
-  ///
-  /// Clears the object contents back to a default invalid state, and release
-  /// any references to shared data that this object may contain.
-  void Clear();
-
   /// Encode an unsigned integer of size \a byte_size to \a offset.
   ///
   /// Encode a single integer value at \a offset and return the offset that
   /// follows the newly encoded integer when the data is successfully encoded
-  /// into the existing data. There must be enough room in the data, else
-  /// UINT32_MAX will be returned to indicate that encoding failed.
+  /// into the existing data. There must be enough room in the existing data,
+  /// else UINT32_MAX will be returned to indicate that encoding failed.
   ///
   /// \param[in] offset
-  ///     The offset within the contained data at which to put the
-  ///     encoded integer.
+  ///     The offset within the contained data at which to put the encoded
+  ///     integer.
   ///
   /// \param[in] byte_size
   ///     The size in byte of the integer to encode.
@@ -111,6 +110,64 @@ class DataEncoder {
   ///     was successfully encoded, UINT32_MAX if the encoding failed.
   uint32_t PutUnsigned(uint32_t offset, uint32_t byte_size, uint64_t value);
 
+  /// Encode an unsigned integer at offset \a offset.
+  ///
+  /// Encode a single unsigned integer value at \a offset and return the offset
+  /// that follows the newly encoded integer when the data is successfully
+  /// encoded into the existing data. There must be enough room in the data,
+  /// else UINT32_MAX will be returned to indicate that encoding failed.
+  ///
+  /// \param[in] offset
+  ///     The offset within the contained data at which to put the encoded
+  ///     integer.
+  ///
+  /// \param[in] value
+  ///     The integer value to write.
+  ///
+  /// \return
+  ///     The next offset in the bytes of this data if the integer was
+  ///     successfully encoded, UINT32_MAX if the encoding failed.
+  uint32_t PutU8(uint32_t offset, uint8_t value);
+  uint32_t PutU16(uint32_t offset, uint16_t value);
+  uint32_t PutU32(uint32_t offset, uint32_t value);
+  uint32_t PutU64(uint32_t offset, uint64_t value);
+
+  /// Append a unsigned integer to the end of the owned data.
+  ///
+  /// \param value
+  ///   A unsigned integer value to append.
+  void AppendU8(uint8_t value);
+  void AppendU16(uint16_t value);
+  void AppendU32(uint32_t value);
+  void AppendU64(uint64_t value);
+
+  /// Append an address sized integer to the end of the owned data.
+  ///
+  /// \param addr
+  ///    A unsigned integer address value to append. The size of the address
+  ///    will be determined by the address size specified in the constructor.
+  void AppendAddress(lldb::addr_t addr);
+
+  /// Append a bytes to the end of the owned data.
+  ///
+  /// Append the bytes contained in the string reference. This function will
+  /// not append a NULL termination character for a C string. Use the
+  /// AppendCString function for this purpose.
+  ///
+  /// \param data
+  ///     A string reference that contains bytes to append.
+  void AppendData(llvm::StringRef data);
+
+  /// Append a C string to the end of the owned data.
+  ///
+  /// Append the bytes contained in the string reference along with an extra
+  /// NULL termination character if the StringRef bytes doesn't include one as
+  /// the last byte.
+  ///
+  /// \param data
+  ///     A string reference that contains bytes to append.
+  void AppendCString(llvm::StringRef data);
+
   /// Encode an arbitrary number of bytes.
   ///
   /// \param[in] offset
@@ -131,11 +188,10 @@ class DataEncoder {
   /// Encode an address in the existing buffer at \a offset bytes into the
   /// buffer.
   ///
-  /// Encode a single address (honoring the m_addr_size member) to the data
-  /// and return the next offset where subsequent data would go. pointed to by
-  /// \a offset_ptr. The size of the extracted address comes from the \a
-  /// m_addr_size member variable and should be set correctly prior to
-  /// extracting any address values.
+  /// Encode a single address to the data and return the next offset where
+  /// subsequent data would go. The size of the address comes from the \a
+  /// m_addr_size member variable and should be set correctly prior to encoding
+  /// any address values.
   ///
   /// \param[in] offset
   ///     The offset where to encode the address.
@@ -150,7 +206,10 @@ class DataEncoder {
 
   /// Put a C string to \a offset.
   ///
-  /// Encodes a C string into the existing data including the terminating
+  /// Encodes a C string into the existing data including the terminating. If
+  /// there is not enough room in the buffer to fit the entire C string and the
+  /// NULL terminator in the existing buffer bounds, then this function will
+  /// fail.
   ///
   /// \param[in] offset
   ///     The offset where to encode the string.
@@ -159,18 +218,32 @@ class DataEncoder {
   ///     The string to encode.
   ///
   /// \return
-  ///     A pointer to the C string value in the data. If the offset
-  ///     pointed to by \a offset_ptr is out of bounds, or if the
-  ///     offset plus the length of the C string is out of bounds,
-  ///     NULL will be returned.
+  ///     The next valid offset within data if the put operation was successful,
+  ///     else UINT32_MAX to indicate the put failed.
   uint32_t PutCString(uint32_t offset, const char *cstr);
 
-private:
-  uint32_t PutU8(uint32_t offset, uint8_t value);
-  uint32_t PutU16(uint32_t offset, uint16_t value);
-  uint32_t PutU32(uint32_t offset, uint32_t value);
-  uint32_t PutU64(uint32_t offset, uint64_t value);
+  /// Get a shared copy of the heap based memory buffer owned by this object.
+  ///
+  /// This allows a data encoder to be used to create a data buffer that can
+  /// be extracted and used elsewhere after this object is destroyed.
+  ///
+  /// \return
+  ///     A shared pointer to the DataBufferHeap that contains the data that was
+  ///     encoded into this object.
+  std::shared_ptr<lldb_private::DataBufferHeap> GetDataBuffer() {
+    return m_data_sp;
+  }
 
+  /// Get a access to the bytes that this references.
+  ///
+  /// This value will always return the data that this object references even if
+  /// the object was constructed with caller owned data.
+  ///
+  /// \return
+  ///     A array reference to the data that this object references.
+  llvm::ArrayRef<uint8_t> GetData() const;
+
+private:
   uint32_t BytesLeft(uint32_t offset) const {
     const uint32_t size = GetByteSize();
     if (size > offset)
@@ -187,31 +260,6 @@ class DataEncoder {
     return length <= BytesLeft(offset);
   }
 
-  /// Adopt a subset of shared data in \a data_sp.
-  ///
-  /// Copies the data shared pointer which adds a reference to the contained
-  /// in \a data_sp. The shared data reference is reference counted to ensure
-  /// the data lives as long as anyone still has a valid shared pointer to the
-  /// data in \a data_sp. The byte order and address byte size settings remain
-  /// the same. If \a offset is not a valid offset in \a data_sp, then no
-  /// reference to the shared data will be added. If there are not \a length
-  /// bytes available in \a data starting at \a offset, the length will be
-  /// truncated to contains as many bytes as possible.
-  ///
-  /// \param[in] data_sp
-  ///     A shared pointer to data.
-  ///
-  /// \param[in] offset
-  ///     The offset into \a data_sp at which the subset starts.
-  ///
-  /// \param[in] length
-  ///     The length in bytes of the subset of \a data_sp.
-  ///
-  /// \return
-  ///     The number of bytes that this object now contains.
-  uint32_t SetData(const lldb::DataBufferSP &data_sp, uint32_t offset = 0,
-                   uint32_t length = UINT32_MAX);
-
   /// Test the validity of \a offset.
   ///
   /// \return
@@ -223,25 +271,17 @@ class DataEncoder {
   ///
   /// \return
   ///     The total number of bytes of data this object refers to.
-  size_t GetByteSize() const { return m_end - m_start; }
-
-  /// A pointer to the first byte of data.
-  uint8_t *m_start = nullptr;
+  size_t GetByteSize() const;
 
-  /// A pointer to the byte that is past the end of the data.
-  uint8_t *m_end = nullptr;
+  /// The shared pointer to data that can grow as data is added
+  std::shared_ptr<lldb_private::DataBufferHeap> m_data_sp;
 
-  /// The byte order of the data we are extracting from.
+  /// The byte order of the data we are encoding to.
   lldb::ByteOrder m_byte_order;
 
-  /// The address size to use when extracting pointers or
-  /// addresses
+  /// The address size to use when encoding pointers or addresses.
   uint8_t m_addr_size;
 
-  /// The shared pointer to data that can
-  /// be shared among multiple instances
-  mutable lldb::DataBufferSP m_data_sp;
-
   DataEncoder(const DataEncoder &) = delete;
   const DataEncoder &operator=(const DataEncoder &) = delete;
 };

lldb/include/lldb/lldb-forward.h

@@ -65,6 +65,7 @@ class DWARFCallFrameInfo;
 class DWARFDataExtractor;
 class DWARFExpression;
 class DataBuffer;
+class DataBufferHeap;
 class DataEncoder;
 class DataExtractor;
 class Debugger;

lldb/source/Expression/DWARFExpression.cpp

@@ -460,22 +460,19 @@ bool DWARFExpression::Update_DW_OP_addr(lldb::addr_t file_addr) {
       // first, then modify it, and if all goes well, we then replace the data
       // for this expression
 
-      // So first we copy the data into a heap buffer
-      std::unique_ptr<DataBufferHeap> head_data_up(
-          new DataBufferHeap(m_data.GetDataStart(), m_data.GetByteSize()));
-
-      // Make en encoder so we can write the address into the buffer using the
-      // correct byte order (endianness)
-      DataEncoder encoder(head_data_up->GetBytes(), head_data_up->GetByteSize(),
+      // Make en encoder that contains a copy of the location expression data
+      // so we can write the address into the buffer using the correct byte
+      // order.
+      DataEncoder encoder(m_data.GetDataStart(), m_data.GetByteSize(),
                           m_data.GetByteOrder(), addr_byte_size);
 
       // Replace the address in the new buffer
-      if (encoder.PutUnsigned(offset, addr_byte_size, file_addr) == UINT32_MAX)
+      if (encoder.PutAddress(offset, file_addr) == UINT32_MAX)
         return false;
 
       // All went well, so now we can reset the data using a shared pointer to
       // the heap data so "m_data" will now correctly manage the heap data.
-      m_data.SetData(DataBufferSP(head_data_up.release()));
+      m_data.SetData(encoder.GetDataBuffer());
       return true;
     } else {
       const offset_t op_arg_size = GetOpcodeDataSize(m_data, offset, op);
@@ -521,15 +518,11 @@ bool DWARFExpression::LinkThreadLocalStorage(
   // We have to make a copy of the data as we don't know if this data is from a
   // read only memory mapped buffer, so we duplicate all of the data first,
   // then modify it, and if all goes well, we then replace the data for this
-  // expression
-
-  // So first we copy the data into a heap buffer
-  std::shared_ptr<DataBufferHeap> heap_data_sp(
-      new DataBufferHeap(m_data.GetDataStart(), m_data.GetByteSize()));
+  // expression.
 
-  // Make en encoder so we can write the address into the buffer using the
-  // correct byte order (endianness)
-  DataEncoder encoder(heap_data_sp->GetBytes(), heap_data_sp->GetByteSize(),
+  // Make en encoder that contains a copy of the location expression data so we
+  // can write the address into the buffer using the correct byte order.
+  DataEncoder encoder(m_data.GetDataStart(), m_data.GetByteSize(),
                       m_data.GetByteOrder(), addr_byte_size);
 
   lldb::offset_t offset = 0;
@@ -603,7 +596,7 @@ bool DWARFExpression::LinkThreadLocalStorage(
   // and read the
   // TLS data
   m_module_wp = new_module_sp;
-  m_data.SetData(heap_data_sp);
+  m_data.SetData(encoder.GetDataBuffer());
   return true;
 }
 
@@ -2817,4 +2810,3 @@ bool DWARFExpression::MatchesOperand(StackFrame &frame,
     return MatchRegOp(*reg)(operand);
   }
 }
-