Add error handling to the DataExtractor class

Summary:
This is motivated by D63591, where we realized that there isn't a really
good way of telling whether a DataExtractor is reading actual data, or
is it just returning default values because it reached the end of the
buffer.

This patch resolves that by providing a new "Cursor" class. A Cursor
object encapsulates two things:
- the current position/offset in the DataExtractor
- an error object

Storing the error object inside the Cursor enables one to use the same
pattern as the std::{io}stream API, where one can blindly perform a
sequence of reads and only check for errors once at the end of the
operation. Similarly to the stream API, as soon as we encounter one
error, all of the subsequent operations are skipped (return default
values) too, even if the would suceed with clear error state. Unlike the
std::stream API (but in line with other llvm APIs), we force the error
state to be checked through usage of llvm::Error.

Reviewers: probinson, dblaikie, JDevlieghere, aprantl, echristo

Subscribers: kristina, llvm-commits

Tags: #llvm

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370042 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/DebugInfo/DWARF/DWARFDataExtractor.h

@@ -36,13 +36,18 @@ public:
   /// Extracts a value and applies a relocation to the result if
   /// one exists for the given offset.
   uint64_t getRelocatedValue(uint32_t Size, uint64_t *Off,
-                             uint64_t *SectionIndex = nullptr) const;
+                             uint64_t *SectionIndex = nullptr,
+                             Error *Err = nullptr) const;
 
   /// Extracts an address-sized value and applies a relocation to the result if
   /// one exists for the given offset.
   uint64_t getRelocatedAddress(uint64_t *Off, uint64_t *SecIx = nullptr) const {
     return getRelocatedValue(getAddressSize(), Off, SecIx);
   }
+  uint64_t getRelocatedAddress(Cursor &C, uint64_t *SecIx = nullptr) const {
+    return getRelocatedValue(getAddressSize(), &getOffset(C), SecIx,
+                             &getError(C));
+  }
 
   /// Extracts a DWARF-encoded pointer in \p Offset using \p Encoding.
   /// There is a DWARF encoding that uses a PC-relative adjustment.

include/llvm/Support/DataExtractor.h

@@ -11,6 +11,7 @@
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DataTypes.h"
+#include "llvm/Support/Error.h"
 
 namespace llvm {
 
@@ -42,6 +43,38 @@ class DataExtractor {
   uint8_t IsLittleEndian;
   uint8_t AddressSize;
 public:
+  /// A class representing a position in a DataExtractor, as well as any error
+  /// encountered during extraction. It enables one to extract a sequence of
+  /// values without error-checking and then checking for errors in bulk at the
+  /// end. The class holds an Error object, so failing to check the result of
+  /// the parse will result in a runtime error. The error flag is sticky and
+  /// will cause all subsequent extraction functions to fail without even
+  /// attempting to parse and without updating the Cursor offset. After clearing
+  /// the error flag, one can again use the Cursor object for parsing.
+  class Cursor {
+    uint64_t Offset;
+    Error Err;
+
+    friend class DataExtractor;
+
+  public:
+    /// Construct a cursor for extraction from the given offset.
+    explicit Cursor(uint64_t Offset) : Offset(Offset), Err(Error::success()) {}
+
+    /// Checks whether the cursor is valid (i.e. no errors were encountered). In
+    /// case of errors, this does not clear the error flag -- one must call
+    /// takeError() instead.
+    explicit operator bool() { return !Err; }
+
+    /// Return the current position of this Cursor. In the error state this is
+    /// the position of the Cursor before the first error was encountered.
+    uint64_t tell() const { return Offset; }
+
+    /// Return error contained inside this Cursor, if any. Clears the internal
+    /// Cursor state.
+    Error takeError() { return std::move(Err); }
+  };
+
   /// Construct with a buffer that is owned by the caller.
   ///
   /// This constructor allows us to use data that is owned by the
@@ -124,10 +157,24 @@ public:
   /// @param[in] byte_size
   ///     The size in byte of the integer to extract.
   ///
+  /// @param[in,out] Err
+  ///     A pointer to an Error object. Upon return the Error object is set to
+  ///     indicate the result (success/failure) of the function. If the Error
+  ///     object is already set when calling this function, no extraction is
+  ///     performed.
+  ///
   /// @return
   ///     The unsigned integer value that was extracted, or zero on
   ///     failure.
-  uint64_t getUnsigned(uint64_t *offset_ptr, uint32_t byte_size) const;
+  uint64_t getUnsigned(uint64_t *offset_ptr, uint32_t byte_size,
+                       Error *Err = nullptr) const;
+
+  /// Extract an unsigned integer of the given size from the location given by
+  /// the cursor. In case of an extraction error, or if the cursor is already in
+  /// an error state, zero is returned.
+  uint64_t getUnsigned(Cursor &C, uint32_t Size) const {
+    return getUnsigned(&C.Offset, Size, &C.Err);
+  }
 
   /// Extract an signed integer of size \a byte_size from \a *offset_ptr.
   ///
@@ -175,6 +222,11 @@ public:
     return getUnsigned(offset_ptr, AddressSize);
   }
 
+  /// Extract a pointer-sized unsigned integer from the location given by the
+  /// cursor. In case of an extraction error, or if the cursor is already in
+  /// an error state, zero is returned.
+  uint64_t getAddress(Cursor &C) const { return getUnsigned(C, AddressSize); }
+
   /// Extract a uint8_t value from \a *offset_ptr.
   ///
   /// Extract a single uint8_t from the binary data at the offset
@@ -187,9 +239,20 @@ public:
   ///     enough bytes to extract this value, the offset will be left
   ///     unmodified.
   ///
+  /// @param[in,out] Err
+  ///     A pointer to an Error object. Upon return the Error object is set to
+  ///     indicate the result (success/failure) of the function. If the Error
+  ///     object is already set when calling this function, no extraction is
+  ///     performed.
+  ///
   /// @return
   ///     The extracted uint8_t value.
-  uint8_t getU8(uint64_t *offset_ptr) const;
+  uint8_t getU8(uint64_t *offset_ptr, Error *Err = nullptr) const;
+
+  /// Extract a single uint8_t value from the location given by the cursor. In
+  /// case of an extraction error, or if the cursor is already in an error
+  /// state, zero is returned.
+  uint8_t getU8(Cursor &C) const { return getU8(&C.Offset, &C.Err); }
 
   /// Extract \a count uint8_t values from \a *offset_ptr.
   ///
@@ -216,6 +279,26 @@ public:
   ///     NULL otherise.
   uint8_t *getU8(uint64_t *offset_ptr, uint8_t *dst, uint32_t count) const;
 
+  /// Extract \a Count uint8_t values from the location given by the cursor and
+  /// store them into the destination buffer. In case of an extraction error, or
+  /// if the cursor is already in an error state, a nullptr is returned and the
+  /// destination buffer is left unchanged.
+  uint8_t *getU8(Cursor &C, uint8_t *Dst, uint32_t Count) const;
+
+  /// Extract \a Count uint8_t values from the location given by the cursor and
+  /// store them into the destination vector. The vector is resized to fit the
+  /// extracted data. In case of an extraction error, or if the cursor is
+  /// already in an error state, the destination vector is left unchanged and
+  /// cursor is placed into an error state.
+  void getU8(Cursor &C, SmallVectorImpl<uint8_t> &Dst, uint32_t Count) const {
+    if (isValidOffsetForDataOfSize(C.Offset, Count))
+      Dst.resize(Count);
+
+    // This relies on the fact that getU8 will not attempt to write to the
+    // buffer if isValidOffsetForDataOfSize(C.Offset, Count) is false.
+    getU8(C, Dst.data(), Count);
+  }
+
   //------------------------------------------------------------------
   /// Extract a uint16_t value from \a *offset_ptr.
   ///
@@ -229,10 +312,21 @@ public:
   ///     enough bytes to extract this value, the offset will be left
   ///     unmodified.
   ///
+  /// @param[in,out] Err
+  ///     A pointer to an Error object. Upon return the Error object is set to
+  ///     indicate the result (success/failure) of the function. If the Error
+  ///     object is already set when calling this function, no extraction is
+  ///     performed.
+  ///
   /// @return
   ///     The extracted uint16_t value.
   //------------------------------------------------------------------
-  uint16_t getU16(uint64_t *offset_ptr) const;
+  uint16_t getU16(uint64_t *offset_ptr, Error *Err = nullptr) const;
+
+  /// Extract a single uint16_t value from the location given by the cursor. In
+  /// case of an extraction error, or if the cursor is already in an error
+  /// state, zero is returned.
+  uint16_t getU16(Cursor &C) const { return getU16(&C.Offset, &C.Err); }
 
   /// Extract \a count uint16_t values from \a *offset_ptr.
   ///
@@ -288,9 +382,20 @@ public:
   ///     enough bytes to extract this value, the offset will be left
   ///     unmodified.
   ///
+  /// @param[in,out] Err
+  ///     A pointer to an Error object. Upon return the Error object is set to
+  ///     indicate the result (success/failure) of the function. If the Error
+  ///     object is already set when calling this function, no extraction is
+  ///     performed.
+  ///
   /// @return
   ///     The extracted uint32_t value.
-  uint32_t getU32(uint64_t *offset_ptr) const;
+  uint32_t getU32(uint64_t *offset_ptr, Error *Err = nullptr) const;
+
+  /// Extract a single uint32_t value from the location given by the cursor. In
+  /// case of an extraction error, or if the cursor is already in an error
+  /// state, zero is returned.
+  uint32_t getU32(Cursor &C) const { return getU32(&C.Offset, &C.Err); }
 
   /// Extract \a count uint32_t values from \a *offset_ptr.
   ///
@@ -329,9 +434,20 @@ public:
   ///     enough bytes to extract this value, the offset will be left
   ///     unmodified.
   ///
+  /// @param[in,out] Err
+  ///     A pointer to an Error object. Upon return the Error object is set to
+  ///     indicate the result (success/failure) of the function. If the Error
+  ///     object is already set when calling this function, no extraction is
+  ///     performed.
+  ///
   /// @return
   ///     The extracted uint64_t value.
-  uint64_t getU64(uint64_t *offset_ptr) const;
+  uint64_t getU64(uint64_t *offset_ptr, Error *Err = nullptr) const;
+
+  /// Extract a single uint64_t value from the location given by the cursor. In
+  /// case of an extraction error, or if the cursor is already in an error
+  /// state, zero is returned.
+  uint64_t getU64(Cursor &C) const { return getU64(&C.Offset, &C.Err); }
 
   /// Extract \a count uint64_t values from \a *offset_ptr.
   ///
@@ -390,9 +506,30 @@ public:
   ///     enough bytes to extract this value, the offset will be left
   ///     unmodified.
   ///
+  /// @param[in,out] Err
+  ///     A pointer to an Error object. Upon return the Error object is set to
+  ///     indicate the result (success/failure) of the function. If the Error
+  ///     object is already set when calling this function, no extraction is
+  ///     performed.
+  ///
   /// @return
   ///     The extracted unsigned integer value.
-  uint64_t getULEB128(uint64_t *offset_ptr) const;
+  uint64_t getULEB128(uint64_t *offset_ptr, llvm::Error *Err = nullptr) const;
+
+  /// Extract an unsigned ULEB128 value from the location given by the cursor.
+  /// In case of an extraction error, or if the cursor is already in an error
+  /// state, zero is returned.
+  uint64_t getULEB128(Cursor &C) const { return getULEB128(&C.Offset, &C.Err); }
+
+  /// Advance the Cursor position by the given number of bytes. No-op if the
+  /// cursor is in an error state.
+  void skip(Cursor &C, uint64_t Length) const;
+
+  /// Return true iff the cursor is at the end of the buffer, regardless of the
+  /// error state of the cursor. The only way both eof and error states can be
+  /// true is if one attempts a read while the cursor is at the very end of the
+  /// data buffer.
+  bool eof(const Cursor &C) const { return Data.size() == C.Offset; }
 
   /// Test the validity of \a offset.
   ///
@@ -420,6 +557,12 @@ public:
   bool isValidOffsetForAddress(uint64_t offset) const {
     return isValidOffsetForDataOfSize(offset, AddressSize);
   }
+
+protected:
+  // Make it possible for subclasses to access these fields without making them
+  // public.
+  static uint64_t &getOffset(Cursor &C) { return C.Offset; }
+  static Error &getError(Cursor &C) { return C.Err; }
 };
 
 } // namespace llvm

lib/DebugInfo/DWARF/DWARFDataExtractor.cpp

@@ -13,13 +13,14 @@
 using namespace llvm;
 
 uint64_t DWARFDataExtractor::getRelocatedValue(uint32_t Size, uint64_t *Off,
-                                               uint64_t *SecNdx) const {
+                                               uint64_t *SecNdx,
+                                               Error *Err) const {
   if (SecNdx)
     *SecNdx = object::SectionedAddress::UndefSection;
   if (!Section)
-    return getUnsigned(Off, Size);
+    return getUnsigned(Off, Size, Err);
   Optional<RelocAddrEntry> E = Obj->find(*Section, *Off);
-  uint64_t A = getUnsigned(Off, Size);
+  uint64_t A = getUnsigned(Off, Size, Err);
   if (!E)
     return A;
   if (SecNdx)

lib/Support/DataExtractor.cpp

@@ -7,104 +7,131 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/DataExtractor.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Host.h"
-#include "llvm/Support/SwapByteOrder.h"
 #include "llvm/Support/LEB128.h"
+#include "llvm/Support/SwapByteOrder.h"
+
 using namespace llvm;
 
+static void unexpectedEndReached(Error *E) {
+  if (E)
+    *E = createStringError(errc::illegal_byte_sequence,
+                           "unexpected end of data");
+}
+
+static bool isError(Error *E) { return E && *E; }
+
 template <typename T>
 static T getU(uint64_t *offset_ptr, const DataExtractor *de,
-              bool isLittleEndian, const char *Data) {
+              bool isLittleEndian, const char *Data, llvm::Error *Err) {
+  ErrorAsOutParameter ErrAsOut(Err);
   T val = 0;
-  uint64_t offset = *offset_ptr;
-  if (de->isValidOffsetForDataOfSize(offset, sizeof(val))) {
-    std::memcpy(&val, &Data[offset], sizeof(val));
-    if (sys::IsLittleEndianHost != isLittleEndian)
-      sys::swapByteOrder(val);
+  if (isError(Err))
+    return val;
 
-    // Advance the offset
-    *offset_ptr += sizeof(val);
+  uint64_t offset = *offset_ptr;
+  if (!de->isValidOffsetForDataOfSize(offset, sizeof(T))) {
+    unexpectedEndReached(Err);
+    return val;
   }
+  std::memcpy(&val, &Data[offset], sizeof(val));
+  if (sys::IsLittleEndianHost != isLittleEndian)
+    sys::swapByteOrder(val);
+
+  // Advance the offset
+  *offset_ptr += sizeof(val);
   return val;
 }
 
 template <typename T>
 static T *getUs(uint64_t *offset_ptr, T *dst, uint32_t count,
-                const DataExtractor *de, bool isLittleEndian, const char *Data){
+                const DataExtractor *de, bool isLittleEndian, const char *Data,
+                llvm::Error *Err) {
+  ErrorAsOutParameter ErrAsOut(Err);
+  if (isError(Err))
+    return nullptr;
+
   uint64_t offset = *offset_ptr;
 
-  if (count > 0 && de->isValidOffsetForDataOfSize(offset, sizeof(*dst)*count)) {
-    for (T *value_ptr = dst, *end = dst + count; value_ptr != end;
-        ++value_ptr, offset += sizeof(*dst))
-      *value_ptr = getU<T>(offset_ptr, de, isLittleEndian, Data);
-    // Advance the offset
-    *offset_ptr = offset;
-    // Return a non-NULL pointer to the converted data as an indicator of
-    // success
-    return dst;
+  if (!de->isValidOffsetForDataOfSize(offset, sizeof(*dst) * count)) {
+    unexpectedEndReached(Err);
+    return nullptr;
   }
-  return nullptr;
+  for (T *value_ptr = dst, *end = dst + count; value_ptr != end;
+       ++value_ptr, offset += sizeof(*dst))
+    *value_ptr = getU<T>(offset_ptr, de, isLittleEndian, Data, Err);
+  // Advance the offset
+  *offset_ptr = offset;
+  // Return a non-NULL pointer to the converted data as an indicator of
+  // success
+  return dst;
 }
 
-uint8_t DataExtractor::getU8(uint64_t *offset_ptr) const {
-  return getU<uint8_t>(offset_ptr, this, IsLittleEndian, Data.data());
+uint8_t DataExtractor::getU8(uint64_t *offset_ptr, llvm::Error *Err) const {
+  return getU<uint8_t>(offset_ptr, this, IsLittleEndian, Data.data(), Err);
 }
 
 uint8_t *
 DataExtractor::getU8(uint64_t *offset_ptr, uint8_t *dst, uint32_t count) const {
   return getUs<uint8_t>(offset_ptr, dst, count, this, IsLittleEndian,
-                       Data.data());
+                        Data.data(), nullptr);
+}
+
+uint8_t *DataExtractor::getU8(Cursor &C, uint8_t *Dst, uint32_t Count) const {
+  return getUs<uint8_t>(&C.Offset, Dst, Count, this, IsLittleEndian,
+                        Data.data(), &C.Err);
 }
 
-uint16_t DataExtractor::getU16(uint64_t *offset_ptr) const {
-  return getU<uint16_t>(offset_ptr, this, IsLittleEndian, Data.data());
+uint16_t DataExtractor::getU16(uint64_t *offset_ptr, llvm::Error *Err) const {
+  return getU<uint16_t>(offset_ptr, this, IsLittleEndian, Data.data(), Err);
 }
 
 uint16_t *DataExtractor::getU16(uint64_t *offset_ptr, uint16_t *dst,
                                 uint32_t count) const {
   return getUs<uint16_t>(offset_ptr, dst, count, this, IsLittleEndian,
-                        Data.data());
+                         Data.data(), nullptr);
 }
 
 uint32_t DataExtractor::getU24(uint64_t *offset_ptr) const {
   uint24_t ExtractedVal =
-      getU<uint24_t>(offset_ptr, this, IsLittleEndian, Data.data());
+      getU<uint24_t>(offset_ptr, this, IsLittleEndian, Data.data(), nullptr);
   // The 3 bytes are in the correct byte order for the host.
   return ExtractedVal.getAsUint32(sys::IsLittleEndianHost);
 }
 
-uint32_t DataExtractor::getU32(uint64_t *offset_ptr) const {
-  return getU<uint32_t>(offset_ptr, this, IsLittleEndian, Data.data());
+uint32_t DataExtractor::getU32(uint64_t *offset_ptr, llvm::Error *Err) const {
+  return getU<uint32_t>(offset_ptr, this, IsLittleEndian, Data.data(), Err);
 }
 
 uint32_t *DataExtractor::getU32(uint64_t *offset_ptr, uint32_t *dst,
                                 uint32_t count) const {
   return getUs<uint32_t>(offset_ptr, dst, count, this, IsLittleEndian,
-                        Data.data());
+                         Data.data(), nullptr);
 }
 
-uint64_t DataExtractor::getU64(uint64_t *offset_ptr) const {
-  return getU<uint64_t>(offset_ptr, this, IsLittleEndian, Data.data());
+uint64_t DataExtractor::getU64(uint64_t *offset_ptr, llvm::Error *Err) const {
+  return getU<uint64_t>(offset_ptr, this, IsLittleEndian, Data.data(), Err);
 }
 
 uint64_t *DataExtractor::getU64(uint64_t *offset_ptr, uint64_t *dst,
                                 uint32_t count) const {
   return getUs<uint64_t>(offset_ptr, dst, count, this, IsLittleEndian,
-                        Data.data());
+                         Data.data(), nullptr);
 }
 
-uint64_t
-DataExtractor::getUnsigned(uint64_t *offset_ptr, uint32_t byte_size) const {
+uint64_t DataExtractor::getUnsigned(uint64_t *offset_ptr, uint32_t byte_size,
+                                    llvm::Error *Err) const {
   switch (byte_size) {
   case 1:
-    return getU8(offset_ptr);
+    return getU8(offset_ptr, Err);
   case 2:
-    return getU16(offset_ptr);
+    return getU16(offset_ptr, Err);
   case 4:
-    return getU32(offset_ptr);
+    return getU32(offset_ptr, Err);
   case 8:
-    return getU64(offset_ptr);
+    return getU64(offset_ptr, Err);
   }
   llvm_unreachable("getUnsigned unhandled case!");
 }
@@ -144,16 +171,23 @@ StringRef DataExtractor::getCStrRef(uint64_t *offset_ptr) const {
   return StringRef();
 }
 
-uint64_t DataExtractor::getULEB128(uint64_t *offset_ptr) const {
+uint64_t DataExtractor::getULEB128(uint64_t *offset_ptr,
+                                   llvm::Error *Err) const {
   assert(*offset_ptr <= Data.size());
+  ErrorAsOutParameter ErrAsOut(Err);
+  if (isError(Err))
+    return 0;
 
   const char *error;
   unsigned bytes_read;
   uint64_t result = decodeULEB128(
       reinterpret_cast<const uint8_t *>(Data.data() + *offset_ptr), &bytes_read,
       reinterpret_cast<const uint8_t *>(Data.data() + Data.size()), &error);
-  if (error)
+  if (error) {
+    if (Err)
+      *Err = createStringError(errc::illegal_byte_sequence, error);
     return 0;
+  }
   *offset_ptr += bytes_read;
   return result;
 }
@@ -171,3 +205,14 @@ int64_t DataExtractor::getSLEB128(uint64_t *offset_ptr) const {
   *offset_ptr += bytes_read;
   return result;
 }
+
+void DataExtractor::skip(Cursor &C, uint64_t Length) const {
+  ErrorAsOutParameter ErrAsOut(&C.Err);
+  if (isError(&C.Err))
+    return;
+
+  if (isValidOffsetForDataOfSize(C.Offset, Length))
+    C.Offset += Length;
+  else
+    unexpectedEndReached(&C.Err);
+}

unittests/Support/DataExtractorTest.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/DataExtractor.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 using namespace llvm;
 
@@ -126,4 +127,146 @@ TEST(DataExtractorTest, LEB128_error) {
   EXPECT_EQ(0U, DE.getSLEB128(&Offset));
   EXPECT_EQ(0U, Offset);
 }
+
+TEST(DataExtractorTest, Cursor_tell) {
+  DataExtractor DE(StringRef("AB"), false, 8);
+  DataExtractor::Cursor C(0);
+  // A successful read operation advances the cursor
+  EXPECT_EQ('A', DE.getU8(C));
+  EXPECT_EQ(1u, C.tell());
+
+  // An unsuccessful one doesn't.
+  EXPECT_EQ(0u, DE.getU16(C));
+  EXPECT_EQ(1u, C.tell());
+
+  // And neither do any subsequent operations.
+  EXPECT_EQ(0, DE.getU8(C));
+  EXPECT_EQ(1u, C.tell());
+
+  consumeError(C.takeError());
+}
+
+TEST(DataExtractorTest, Cursor_takeError) {
+  DataExtractor DE(StringRef("AB"), false, 8);
+  DataExtractor::Cursor C(0);
+  // Initially, the cursor is in the "success" state.
+  EXPECT_THAT_ERROR(C.takeError(), Succeeded());
+
+  // It remains "success" after a successful read.
+  EXPECT_EQ('A', DE.getU8(C));
+  EXPECT_THAT_ERROR(C.takeError(), Succeeded());
+
+  // An unsuccessful read sets the error state.
+  EXPECT_EQ(0u, DE.getU32(C));
+  EXPECT_THAT_ERROR(C.takeError(), Failed());
+
+  // Once set the error sticks until explicitly cleared.
+  EXPECT_EQ(0u, DE.getU32(C));
+  EXPECT_EQ(0, DE.getU8(C));
+  EXPECT_THAT_ERROR(C.takeError(), Failed());
+
+  // At which point reads can be succeed again.
+  EXPECT_EQ('B', DE.getU8(C));
+  EXPECT_THAT_ERROR(C.takeError(), Succeeded());
+}
+
+TEST(DataExtractorTest, Cursor_chaining) {
+  DataExtractor DE(StringRef("ABCD"), false, 8);
+  DataExtractor::Cursor C(0);
+
+  // Multiple reads can be chained without trigerring any assertions.
+  EXPECT_EQ('A', DE.getU8(C));
+  EXPECT_EQ('B', DE.getU8(C));
+  EXPECT_EQ('C', DE.getU8(C));
+  EXPECT_EQ('D', DE.getU8(C));
+  // And the error checked at the end.
+  EXPECT_THAT_ERROR(C.takeError(), Succeeded());
+}
+
+#if defined(GTEST_HAS_DEATH_TEST) && defined(_DEBUG)
+TEST(DataExtractorDeathTest, Cursor) {
+  DataExtractor DE(StringRef("AB"), false, 8);
+
+  // Even an unused cursor must be checked for errors:
+  EXPECT_DEATH(DataExtractor::Cursor(0),
+               "Success values must still be checked prior to being destroyed");
+
+  {
+    auto C = std::make_unique<DataExtractor::Cursor>(0);
+    EXPECT_EQ(0u, DE.getU32(*C));
+    // It must also be checked after an unsuccessful operation.
+    // destruction.
+    EXPECT_DEATH(C.reset(), "unexpected end of data");
+    EXPECT_THAT_ERROR(C->takeError(), Failed());
+  }
+  {
+    auto C = std::make_unique<DataExtractor::Cursor>(0);
+    EXPECT_EQ('A', DE.getU8(*C));
+    // Same goes for a successful one.
+    EXPECT_DEATH(
+        C.reset(),
+        "Success values must still be checked prior to being destroyed");
+    EXPECT_THAT_ERROR(C->takeError(), Succeeded());
+  }
+  {
+    auto C = std::make_unique<DataExtractor::Cursor>(0);
+    EXPECT_EQ('A', DE.getU8(*C));
+    EXPECT_EQ(0u, DE.getU32(*C));
+    // Even if a successful operation is followed by an unsuccessful one.
+    EXPECT_DEATH(C.reset(), "unexpected end of data");
+    EXPECT_THAT_ERROR(C->takeError(), Failed());
+  }
+  {
+    auto C = std::make_unique<DataExtractor::Cursor>(0);
+    EXPECT_EQ(0u, DE.getU32(*C));
+    EXPECT_EQ(0, DE.getU8(*C));
+    // Even if an unsuccessful operation is followed by one that would normally
+    // succeed.
+    EXPECT_DEATH(C.reset(), "unexpected end of data");
+    EXPECT_THAT_ERROR(C->takeError(), Failed());
+  }
+}
+#endif
+
+TEST(DataExtractorTest, getU8_vector) {
+  DataExtractor DE(StringRef("AB"), false, 8);
+  DataExtractor::Cursor C(0);
+  SmallVector<uint8_t, 2> S;
+
+  DE.getU8(C, S, 4);
+  EXPECT_THAT_ERROR(C.takeError(), Failed());
+  EXPECT_EQ("", toStringRef(S));
+
+  DE.getU8(C, S, 2);
+  EXPECT_THAT_ERROR(C.takeError(), Succeeded());
+  EXPECT_EQ("AB", toStringRef(S));
+}
+
+TEST(DataExtractorTest, skip) {
+  DataExtractor DE(StringRef("AB"), false, 8);
+  DataExtractor::Cursor C(0);
+
+  DE.skip(C, 4);
+  EXPECT_THAT_ERROR(C.takeError(), Failed());
+  EXPECT_EQ(0u, C.tell());
+
+  DE.skip(C, 2);
+  EXPECT_THAT_ERROR(C.takeError(), Succeeded());
+  EXPECT_EQ(2u, C.tell());
+}
+
+TEST(DataExtractorTest, eof) {
+  DataExtractor DE(StringRef("A"), false, 8);
+  DataExtractor::Cursor C(0);
+
+  EXPECT_FALSE(DE.eof(C));
+
+  EXPECT_EQ(0, DE.getU16(C));
+  EXPECT_FALSE(DE.eof(C));
+  EXPECT_THAT_ERROR(C.takeError(), Failed());
+
+  EXPECT_EQ('A', DE.getU8(C));
+  EXPECT_TRUE(DE.eof(C));
+  EXPECT_THAT_ERROR(C.takeError(), Succeeded());
+}
 }