Minidump: Add support for reading/writing strings

Summary:
Strings in minidump files are stored as a 32-bit length field, giving
the length of the string in *bytes*, which is followed by the
appropriate number of UTF16 code units. The string is also supposed to
be null-terminated, and the null-terminator is not a part of the length
field. This patch:
- adds support for reading these strings out of the minidump file (this
  implementation does not depend on proper null-termination)
- adds support for writing them to a minidump file
- using the previous two pieces implements proper (de)serialization of
  the CSDVersion field of the SystemInfo stream. Previously, this was
  only read/written as hex, and no attempt was made to access the
  referenced string -- now this string is read and written correctly.

The changes are tested via yaml2obj|obj2yaml round-trip as well as a
unit test which checks the corner cases of the string deserialization
logic.

Reviewers: jhenderson, zturner, clayborg

Subscribers: llvm-commits, aprantl, markmentovai, amccarth, lldb-commits

Tags: #llvm

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

llvm-svn: 357749

2 files changed, 69 insertions, 5 deletions

diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index d0d49b4e8d3..3e3255a2f17 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -8,6 +8,7 @@
 
 #include "llvm/Object/Minidump.h"
 #include "llvm/Object/Error.h"
+#include "llvm/Support/ConvertUTF.h"
 
 using namespace llvm;
 using namespace llvm::object;
@@ -21,6 +22,33 @@ MinidumpFile::getRawStream(minidump::StreamType Type) const {
   return None;
 }
 
+Expected<std::string> MinidumpFile::getString(size_t Offset) const {
+  // Minidump strings consist of a 32-bit length field, which gives the size of
+  // the string in *bytes*. This is followed by the actual string encoded in
+  // UTF16.
+  auto ExpectedSize =
+      getDataSliceAs<support::ulittle32_t>(getData(), Offset, 1);
+  if (!ExpectedSize)
+    return ExpectedSize.takeError();
+  size_t Size = (*ExpectedSize)[0];
+  if (Size % 2 != 0)
+    return createError("String size not even");
+  Size /= 2;
+  if (Size == 0)
+    return "";
+
+  Offset += sizeof(support::ulittle32_t);
+  auto ExpectedData = getDataSliceAs<UTF16>(getData(), Offset, Size);
+  if (!ExpectedData)
+    return ExpectedData.takeError();
+
+  std::string Result;
+  if (!convertUTF16ToUTF8String(*ExpectedData, Result))
+    return createError("String decoding failed");
+
+  return Result;
+}
+
 Expected<ArrayRef<uint8_t>>
 MinidumpFile::getDataSlice(ArrayRef<uint8_t> Data, size_t Offset, size_t Size) {
   // Check for overflow.
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index a2e0b209da5..38d23d9dfc6 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ObjectYAML/MinidumpYAML.h"
+#include "llvm/Support/ConvertUTF.h"
 
 using namespace llvm;
 using namespace llvm::MinidumpYAML;
@@ -39,6 +40,8 @@ public:
     return allocateArray(makeArrayRef(Data));
   }
 
+  size_t allocateString(StringRef Str);
+
   void writeTo(raw_ostream &OS) const;
 
 private:
@@ -48,6 +51,26 @@ private:
 };
 } // namespace
 
+size_t BlobAllocator::allocateString(StringRef Str) {
+  SmallVector<UTF16, 32> WStr;
+  bool OK = convertUTF8ToUTF16String(Str, WStr);
+  assert(OK && "Invalid UTF8 in Str?");
+  (void)OK;
+
+  SmallVector<support::ulittle16_t, 32> EndianStr(WStr.size() + 1,
+                                                  support::ulittle16_t());
+  copy(WStr, EndianStr.begin());
+  return allocateCallback(
+      sizeof(uint32_t) + EndianStr.size() * sizeof(support::ulittle16_t),
+      [EndianStr](raw_ostream &OS) {
+        // Length does not include the null-terminator.
+        support::ulittle32_t Length(2 * (EndianStr.size() - 1));
+        OS.write(reinterpret_cast<const char *>(&Length), sizeof(Length));
+        OS.write(reinterpret_cast<const char *>(EndianStr.begin()),
+                 sizeof(support::ulittle16_t) * EndianStr.size());
+      });
+}
+
 void BlobAllocator::writeTo(raw_ostream &OS) const {
   size_t BeginOffset = OS.tell();
   for (const auto &Callback : Callbacks)
@@ -269,7 +292,7 @@ static void streamMapping(yaml::IO &IO, SystemInfoStream &Stream) {
   mapOptional(IO, "Minor Version", Info.MinorVersion, 0);
   mapOptional(IO, "Build Number", Info.BuildNumber, 0);
   IO.mapRequired("Platform ID", Info.PlatformId);
-  mapOptionalHex(IO, "CSD Version RVA", Info.CSDVersionRVA, 0);
+  IO.mapOptional("CSD Version", Stream.CSDVersion, "");
   mapOptionalHex(IO, "Suite Mask", Info.SuiteMask, 0);
   mapOptionalHex(IO, "Reserved", Info.Reserved, 0);
   switch (static_cast<ProcessorArchitecture>(Info.ProcessorArch)) {
@@ -337,6 +360,7 @@ static Directory layout(BlobAllocator &File, Stream &S) {
   Directory Result;
   Result.Type = S.Type;
   Result.Location.RVA = File.tell();
+  Optional<size_t> DataEnd;
   switch (S.Kind) {
   case Stream::StreamKind::RawContent: {
     RawContentStream &Raw = cast<RawContentStream>(S);
@@ -347,14 +371,22 @@ static Directory layout(BlobAllocator &File, Stream &S) {
     });
     break;
   }
-  case Stream::StreamKind::SystemInfo:
-    File.allocateObject(cast<SystemInfoStream>(S).Info);
+  case Stream::StreamKind::SystemInfo: {
+    SystemInfoStream &SystemInfo = cast<SystemInfoStream>(S);
+    File.allocateObject(SystemInfo.Info);
+    // The CSD string is not a part of the stream.
+    DataEnd = File.tell();
+    SystemInfo.Info.CSDVersionRVA = File.allocateString(SystemInfo.CSDVersion);
     break;
+  }
   case Stream::StreamKind::TextContent:
     File.allocateArray(arrayRefFromStringRef(cast<TextContentStream>(S).Text));
     break;
   }
-  Result.Location.DataSize = File.tell() - Result.Location.RVA;
+  // If DataEnd is not set, we assume everything we generated is a part of the
+  // stream.
+  Result.Location.DataSize =
+      DataEnd.getValueOr(File.tell()) - Result.Location.RVA;
   return Result;
 }
 
@@ -395,7 +427,11 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
     auto ExpectedInfo = File.getSystemInfo();
     if (!ExpectedInfo)
       return ExpectedInfo.takeError();
-    return make_unique<SystemInfoStream>(*ExpectedInfo);
+    auto ExpectedCSDVersion = File.getString(ExpectedInfo->CSDVersionRVA);
+    if (!ExpectedCSDVersion)
+      return ExpectedInfo.takeError();
+    return make_unique<SystemInfoStream>(*ExpectedInfo,
+                                         std::move(*ExpectedCSDVersion));
   }
   case StreamKind::TextContent:
     return make_unique<TextContentStream>(