Update MinidumpYAML to use minidump::Exception for exception stream

[labath]

Adding @MaskRay and @grimar, as @jhenderson is out.

This ExceptionInformation stuff is pretty impressive, but I am wondering if it's really needed, particularly as lldb does not even use those fields right now. Also, while technically an array, the Parameter stuff isn't really used as an array, as each exception type (I actually know of only one: EXCEPTION_ACCESS_VIOLATION) defines fixed meanings for each position.

Therefore I think it would make sense to just spell out each member of that array as a separate member in the yaml representation, which should be a much simpler endeavour. We can use the "actual parameter count" field to suppress the fields that don't contain any value, if they really are zero, which should make the yaml output concise in the usual cases. I think something like this should be sufficient:

IO.mapOptional("Number of Parameters", Record.NumberParameters, 0);
for(unsigned I = 0; I < Exception::MaxParameters ++I) {
  if (I <Record.NumberParameters)
    mapRequiredHex(IO, "Parameter " + to_string(I),  Record.ExceptionInformation[I]);
  else
    mapOptionalHex(IO, "Parameter " + to_string(I),  Record.ExceptionInformation[I], 0);
}

I think that would strike a good balance between code complexity, output brevity, and being able to generate interesting and potentially invalid inputs for other tools (which is one of the main goals of yaml2obj, and so interpreting the input too strictly is not desired/helpful).

llvm/lib/ObjectYAML/MinidumpEmitter.cpp

127

Yeah, this isn't an issue specific to just this stream. There are other places which suffer from the same problem too (e.g. the "stack" field of a thread will usually match/point to the same blob as one of the MemoryList entries. So far, I have been ignoring that because this does not generally matter for the consumers, and it capturing that in yaml in a form which would still be easily editable/reducable would be tricky.

[JosephTremoulet]

rebase

[JosephTremoulet]

• Change Exception Information format per feedback

[JosephTremoulet]

• Add test with extraneous parameter

[JosephTremoulet]

In D68657#1703752, @labath wrote:

Therefore I think it would make sense to just spell out each member of that array as a separate member in the yaml representation, which should be a much simpler endeavour. We can use the "actual parameter count" field to suppress the fields that don't contain any value, if they really are zero, which should make the yaml output concise in the usual cases. I think something like this should be sufficient:
...
I think that would strike a good balance between code complexity, output brevity, and being able to generate interesting and potentially invalid inputs for other tools (which is one of the main goals of yaml2obj, and so interpreting the input too strictly is not desired/helpful).

Ok. Updated. I agree the code is simpler this way and the YAML just as readable, plus it's nice not having to worry about buffer overrun. Added testcases with missing and extraneous parameters.

[JosephTremoulet]

• Update test input yaml Exception stream

[labath]

Thanks. IIUC, all the existing tests just cover the yaml2obj direction. Could you add something for the other direction too? Maybe add an exception stream to test/tools/obj2yaml/basic-minidump.yaml?

llvm/include/llvm/ObjectYAML/MinidumpYAML.h

181–200

I've been trying to keep this somewhat sorted. Could you move this before the MemoryInfoListStream class? Also, in the previous patch we've moved the default constructors to front. It would be good to make this consistent with that.

llvm/lib/ObjectYAML/MinidumpYAML.cpp

392

This file has a helper function for this (mapOptional(IO, "name", value, 0). I'd consider changing the field name to "Number of Parameters" even though it does not match the field name, as it reads weird without that. I'm not sure why the microsoft naming is inconsistent here -- most of the other minidump structs have "of" in their name already (BaseOfImage, SizeOfImage, etc.), but at least we can be consistent.

406–412

Could you remove this bit too? While it is technically invalid, this is not something that yaml2obj needs to care about (as it does not prevent successful serialization), and it would be nice to be able to use it to generate a test case with an invalid number (because that is something lldb should care about and expect/handle)..

[JosephTremoulet]

Address review feedback

  - Add Exception stream to minidump-basic.yaml to test obj2yaml
    direction
  - Reorder ExceptionStream definition and constructors
  - Use the mapOptional helper
  - Replace "Number Parameters" with "Number of Parameters" in YAML
  - Stop using mapping traits validation for number of parameters,
    update test to ensure correctly de-yamlizing an exception record
    with an out-of-bounds number of parameters

[JosephTremoulet]

Added Exception stream to minidump-basic.yaml as suggested.

llvm/lib/ObjectYAML/MinidumpYAML.cpp

392

Updated to use the helper, and changed the name in the YAML to "Number of Parameters". Let me know if it's important to you to also change the name of the field in the llvm::minidump::Exception type to NumberOfParameters -- I wasn't sure if you were suggesting that, and regardless my preference would be to leave that as-is to match breakpad aside from casing, as otherwise it's hard to know where to stop (e.g. change "ExceptionInformation" to "Parameters" to match "NumberOfParameters" and the YAML? Reconcile the several different ways that alignment padding fields are named? etc.)

[grimar]

llvm/include/llvm/ObjectYAML/MinidumpYAML.h

113–114

I'd avoid memset:

ExceptionStream()
    : Stream(StreamKind::Exception, minidump::StreamType::Exception),
      MDExceptionStream({}) {

llvm/lib/ObjectYAML/MinidumpYAML.cpp

530

We often avoid using auto when return type is not obvious.

llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

143

I'd add a comment for each test to describe what they do.

[JosephTremoulet]

• Apply review feedback (-auto, -memset, +comments)

[JosephTremoulet]

• Fix Expected<> types

[JosephTremoulet]

• ...and fix namespace...

[labath]

llvm/lib/ObjectYAML/MinidumpYAML.cpp

376–382

I've been thinking about this for a while now, and while that idea has some appeal, I am not sure if this would ever really be a "good" idea. Currently, this format allows you to easily create syntactically-valid-but-probably-nonsensical minidumps (multiple thread list streams, multiple threads with the same ID, etc..). All of that would be more difficult if we started depending on strict "correctness" of other parts of the minidump in order to compute something here.

Even if I was doing this, I'd probably implement this differently -- make the context always optional, but then check for consistency higher up (the validate call of the entire minidump object or something). Anyway, maybe just delete this todo? :)

392

I wasn't sure about that either -- that's why it was phrased so vaguely. While I generally tried to stick to the original naming, I have already done some renames to the field names in existing structures -- IIRC, it was limited to unifying the naming for the various "reserved"/"unused" fields. Anyway, I think that keeping the original name in this particular case is fine.

llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

310–316

Maybe:

EXPECT_THAT_EXPECTED(ExpectedFile, Failed<StringError>(
    testing::Property(&StringError::convertToErrorCode,
         make_error_code(errc::invalid_argument))));

?

Though, this might actually be best off as a lit test where you just FileCheck the exact error message.

[JosephTremoulet]

• Remove TODO, lit-ify negative test and tighten check

[JosephTremoulet]

llvm/lib/ObjectYAML/MinidumpYAML.cpp

376–382

I've removed the TODO, but I reserve the right to think this would be a good idea :). Speficially because I think you could still model in YAML anything you could put in a minidump file, by explicitly providing the ThreadContext even when it has a default.

(This is in contrast to the other validation stuff I had in earlier revisions, where I was just misunderstanding the point of yaml validation -- so thanks for explaining it!)

llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

310–316

Moved to lit, thanks for the suggestion. The obvious place seemed to be llvm/test/ObjectYAML, but there weren't any minidump tests there so I added a minidump subdirectory for the new test. Please let me know if there was a better place that I just overlooked.

[labath]

llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

310–316

there are some in test/tools/yaml2obj. I'd put it next to those.

[JosephTremoulet]

• Move test

[labath]

Thanks. I think this is great. @grimar, do you have any more comments?

llvm/lib/ObjectYAML/MinidumpYAML.cpp

376–382

Fair enough. :)

[grimar]

LGTM with 2 nits.

llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml

4

nit: It is became common to use ## for comments in yaml2obj and actually many other LLVM tools test cases.
(here and below)

llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

143

nit: Missing a full stop (here and below).

[JosephTremoulet]

llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml

4

Sorry, when you say "here and below", do you just mean lines 3 and 4-5, or do you also mean the CHECK line (line 20)? Grepping in test/tools it appears to be very uncommon to use ## for CHECK lines, but that's what I'd have otherwise assumed "and below" referred to...

[JosephTremoulet]

• punctuation fixes

[grimar]

llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml

4

Sorry, I think I misread the line 20, i.e. I read it as a comment. Indeed we do not use ## for CHECK lines.

[MaskRay]

llvm/lib/ObjectYAML/MinidumpYAML.cpp

388

You may use ("Parameter " + Twine(Index)).str() to get rid of the "llvm/Support/FormatVariadic.h" dependency.

[JosephTremoulet]

llvm/lib/ObjectYAML/MinidumpYAML.cpp

388

Is #including FormatVariadic.h a bad thing? I did it this way to avoid heap allocation.

[JosephTremoulet]

• Rebase
• Use Twine instead of formatv

[JosephTremoulet]

llvm/lib/ObjectYAML/MinidumpYAML.cpp

388

...finally realized that Twine avoids the heap too :). Thanks for the suggestion, updated.

lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml

 --- !minidump
 Streams:         
   - Type:            ThreadList
     Threads:         
       - Thread Id:       0x000074DD
         Context:         0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000002020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040109600000000000100000000000000000000000000000068E7D0C8FF7F000068E7D0C8FF7F000097E6D0C8FF7F000010109600000000000000000000000000020000000000000088E4D0C8FF7F0000603FFF85C77F0000F00340000000000080E7D0C8FF7F000000000000000000000000000000000000E0034000000000007F0300000000000000000000000000000000000000000000801F0000FFFF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF252525252525252525252525252525250000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
         Stack:           
           Start of Memory Range: 0x00007FFFC8D0E000
           Content:               DEADBEEFBAADF00D
   - Type:            ModuleList
     Modules:         
       - Base of Image:   0x0000000000400000
         Size of Image:   0x00001000
         Module Name:     '/tmp/test/linux-x86_64'
         CodeView Record: 4C457042E35C283BC327C28762DB788BF5A4078BE2351448
   - Type:            Exception
-    Content:         DD740000000000000B00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000D0040000F8310000
+    Thread ID:       0x000074DD
+    Exception Record:
+      Exception Code:  0x0000000B
+    Thread Context:  00000000
   - Type:            SystemInfo
     Processor Arch:  AMD64
     Processor Level: 6
     Processor Revision: 15876
     Number of Processors: 40
     Platform ID:     Linux
     CSD Version:     'Linux 3.13.0-91-generic'
     CPU:             

llvm/include/llvm/ObjectYAML/MinidumpYAML.h

 /// corresponds to the Stream Type field in the minidump file. The "Kind" field
 /// specifies how are we going to treat it. For highly specialized streams (e.g.
 /// SystemInfo), there is a 1:1 mapping between Types and Kinds, but in general
 /// one stream Kind can be used to represent multiple stream Types (e.g. any
 /// unrecognised stream Type will be handled via RawContentStream). The mapping
 /// from Types to Kinds is fixed and given by the static getKind function.
 struct Stream {
   enum class StreamKind {
+    Exception,
     MemoryInfoList,
     MemoryList,
     ModuleList,
     RawContent,
     SystemInfo,
     TextContent,
     ThreadList,
   };
struct ParsedMemoryDescriptor {
   yaml::BinaryRef Content;
 };
 } // namespace detail
 
 using ModuleListStream = detail::ListStream<detail::ParsedModule>;
 using ThreadListStream = detail::ListStream<detail::ParsedThread>;
 using MemoryListStream = detail::ListStream<detail::ParsedMemoryDescriptor>;
 
+/// ExceptionStream minidump stream.
+struct ExceptionStream : public Stream {
+  minidump::ExceptionStream MDExceptionStream;
+  yaml::BinaryRef ThreadContext;
+
+  ExceptionStream()
+      : Stream(StreamKind::Exception, minidump::StreamType::Exception),
+        MDExceptionStream({}) {}
+
+  explicit ExceptionStream(const minidump::ExceptionStream &MDExceptionStream,
+                           ArrayRef<uint8_t> ThreadContext)
+      : Stream(StreamKind::Exception, minidump::StreamType::Exception),
+        MDExceptionStream(MDExceptionStream), ThreadContext(ThreadContext) {}
+
+  static bool classof(const Stream *S) {
+    return S->Kind == StreamKind::Exception;
+  }
+};
+
 /// A structure containing the list of MemoryInfo entries comprising a
 /// MemoryInfoList stream.
 struct MemoryInfoListStream : public Stream {
   std::vector<minidump::MemoryInfo> Infos;
 
   MemoryInfoListStream()
       : Stream(StreamKind::MemoryInfoList,
                minidump::StreamType::MemoryInfoList) {}
explicit SystemInfoStream(const minidump::SystemInfo &Info,
       : Stream(StreamKind::SystemInfo, minidump::StreamType::SystemInfo),
         Info(Info), CSDVersion(std::move(CSDVersion)) {}
 
   static bool classof(const Stream *S) {
     return S->Kind == StreamKind::SystemInfo;
   }
 };
 
 /// A StringRef, which is printed using YAML block notation.
 LLVM_YAML_STRONG_TYPEDEF(StringRef, BlockStringRef)
 
 /// A minidump stream containing textual data (typically, the contents of a
 /// /proc/<pid> file on linux).
 struct TextContentStream : public Stream {
   BlockStringRef Text;
 
   TextContentStream(minidump::StreamType Type, StringRef Text = {})
       : Stream(StreamKind::TextContent, Type), Text(Text) {}
 
   static bool classof(const Stream *S) {
     return S->Kind == StreamKind::TextContent;
   }
 };
 
 /// The top level structure representing a minidump object, consisting of a
 /// minidump header, and zero or more streams. To construct an Object from a
 /// minidump file, use the static create function. To serialize to/from yaml,
 /// use the appropriate streaming operator on a yaml stream.
 struct Object {
   Object() = default;
   Object(const Object &) = delete;
   Object &operator=(const Object &) = delete;
   Object(Object &&) = default;
   Object &operator=(Object &&) = default;
 
   Object(const minidump::Header &Header,

 
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::minidump::ProcessorArchitecture)
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::minidump::OSPlatform)
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::minidump::StreamType)
 
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::ArmInfo)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::OtherInfo)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::X86Info)
+LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::Exception)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::MemoryInfo)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::VSFixedFileInfo)
 
 LLVM_YAML_DECLARE_MAPPING_TRAITS(
     llvm::MinidumpYAML::MemoryListStream::entry_type)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(
     llvm::MinidumpYAML::ModuleListStream::entry_type)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(

llvm/lib/ObjectYAML/MinidumpEmitter.cpp

void BlobAllocator::writeTo(raw_ostream &OS) const {
   (void)BeginOffset;
 }
 
 static LocationDescriptor layout(BlobAllocator &File, yaml::BinaryRef Data) {
   return {support::ulittle32_t(Data.binary_size()),
           support::ulittle32_t(File.allocateBytes(Data))};
 }
 
+static size_t layout(BlobAllocator &File, MinidumpYAML::ExceptionStream &S) {
+  File.allocateObject(S.MDExceptionStream);
+
+  size_t DataEnd = File.tell();
+
+  // Lay out the thread context data, (which is not a part of the stream).
+  // TODO: This usually (always?) matches the thread context of the
+  // corresponding thread, and may overlap memory regions as well.  We could
+  // add a level of indirection to the MinidumpYAML format (like an array of
+  // Blobs that the LocationDescriptors index into) to be able to distinguish
+  // the cases where location descriptions overlap vs happen to reference
+  // identical data.
+  S.MDExceptionStream.ThreadContext = layout(File, S.ThreadContext);
+
+  return DataEnd;
+}
+
 static void layout(BlobAllocator &File, MemoryListStream::entry_type &Range) {
   Range.Entry.Memory = layout(File, Range.Content);
 }
 
 static void layout(BlobAllocator &File, ModuleListStream::entry_type &M) {
   M.Entry.ModuleNameRVA = File.allocateString(M.Name);
 
   M.Entry.CvRecord = layout(File, M.CvRecord);

 }
 
 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::Exception:
+    DataEnd = layout(File, cast<MinidumpYAML::ExceptionStream>(S));
+    break;
   case Stream::StreamKind::MemoryInfoList: {
     MemoryInfoListStream &InfoList = cast<MemoryInfoListStream>(S);
     File.allocateNewObject<minidump::MemoryInfoListHeader>(
         sizeof(minidump::MemoryInfoListHeader), sizeof(minidump::MemoryInfo),
         InfoList.Infos.size());
     File.allocateArray(makeArrayRef(InfoList.Infos));
     break;
   }

llvm/lib/ObjectYAML/MinidumpYAML.cpp

static inline void mapOptionalHex(yaml::IO &IO, const char *Key,
                                   typename EndianType::value_type Default) {
   mapOptionalAs<typename HexType<EndianType>::type>(IO, Key, Val, Default);
 }
 
 Stream::~Stream() = default;
 
 Stream::StreamKind Stream::getKind(StreamType Type) {
   switch (Type) {
+  case StreamType::Exception:
+    return StreamKind::Exception;
   case StreamType::MemoryInfoList:
     return StreamKind::MemoryInfoList;
   case StreamType::MemoryList:
     return StreamKind::MemoryList;
   case StreamType::ModuleList:
     return StreamKind::ModuleList;
   case StreamType::SystemInfo:
     return StreamKind::SystemInfo;
Stream::StreamKind Stream::getKind(StreamType Type) {
   default:
     return StreamKind::RawContent;
   }
 }
 
 std::unique_ptr<Stream> Stream::create(StreamType Type) {
   StreamKind Kind = getKind(Type);
   switch (Kind) {
+  case StreamKind::Exception:
+    return std::make_unique<ExceptionStream>();
   case StreamKind::MemoryInfoList:
     return std::make_unique<MemoryInfoListStream>();
   case StreamKind::MemoryList:
     return std::make_unique<MemoryListStream>();
   case StreamKind::ModuleList:
     return std::make_unique<ModuleListStream>();
   case StreamKind::RawContent:
     return std::make_unique<RawContentStream>(Type);
void yaml::MappingTraits<VSFixedFileInfo>::mapping(IO &IO,
   mapOptionalHex(IO, "File Date Low", Info.FileDateLow, 0);
 }
 
 void yaml::MappingTraits<ModuleListStream::entry_type>::mapping(
     IO &IO, ModuleListStream::entry_type &M) {
   mapRequiredHex(IO, "Base of Image", M.Entry.BaseOfImage);
   mapRequiredHex(IO, "Size of Image", M.Entry.SizeOfImage);
   mapOptionalHex(IO, "Checksum", M.Entry.Checksum, 0);
-  IO.mapOptional("Time Date Stamp", M.Entry.TimeDateStamp,
-                 support::ulittle32_t(0));
+  mapOptional(IO, "Time Date Stamp", M.Entry.TimeDateStamp, 0);
   IO.mapRequired("Module Name", M.Name);
   IO.mapOptional("Version Info", M.Entry.VersionInfo, VSFixedFileInfo());
   IO.mapRequired("CodeView Record", M.CvRecord);
   IO.mapOptional("Misc Record", M.MiscRecord, yaml::BinaryRef());
   mapOptionalHex(IO, "Reserved0", M.Entry.Reserved0, 0);
   mapOptionalHex(IO, "Reserved1", M.Entry.Reserved1, 0);
 }
 
void yaml::MappingTraits<ThreadListStream::entry_type>::mapping(
   IO.mapRequired("Context", T.Context);
   IO.mapRequired("Stack", T.Entry.Stack, T.Stack);
 }
 
 static void streamMapping(yaml::IO &IO, ThreadListStream &Stream) {
   IO.mapRequired("Threads", Stream.Entries);
 }
 
+static void streamMapping(yaml::IO &IO, MinidumpYAML::ExceptionStream &Stream) {
+  mapRequiredHex(IO, "Thread ID", Stream.MDExceptionStream.ThreadId);
+  IO.mapRequired("Exception Record", Stream.MDExceptionStream.ExceptionRecord);
+  IO.mapRequired("Thread Context", Stream.ThreadContext);
+}
+
+void yaml::MappingTraits<minidump::Exception>::mapping(
+    yaml::IO &IO, minidump::Exception &Exception) {
+  mapRequiredHex(IO, "Exception Code", Exception.ExceptionCode);
+  mapOptionalHex(IO, "Exception Flags", Exception.ExceptionFlags, 0);
+  mapOptionalHex(IO, "Exception Record", Exception.ExceptionRecord, 0);
+  mapOptionalHex(IO, "Exception Address", Exception.ExceptionAddress, 0);
+  mapOptional(IO, "Number of Parameters", Exception.NumberParameters, 0);
+
+  for (size_t Index = 0; Index < Exception.MaxParameters; ++Index) {
+    SmallString<16> Name("Parameter ");
+    Twine(Index).toVector(Name);
+    support::ulittle64_t &Field = Exception.ExceptionInformation[Index];
+
+    if (Index < Exception.NumberParameters)
+      mapRequiredHex(IO, Name.c_str(), Field);
+    else
+      mapOptionalHex(IO, Name.c_str(), Field, 0);
+  }
+}
+
 void yaml::MappingTraits<std::unique_ptr<Stream>>::mapping(
     yaml::IO &IO, std::unique_ptr<MinidumpYAML::Stream> &S) {
   StreamType Type;
   if (IO.outputting())
     Type = S->Type;
   IO.mapRequired("Type", Type);
 
   if (!IO.outputting())
     S = MinidumpYAML::Stream::create(Type);
   switch (S->Kind) {
+  case MinidumpYAML::Stream::StreamKind::Exception:
+    streamMapping(IO, llvm::cast<MinidumpYAML::ExceptionStream>(*S));
+    break;
   case MinidumpYAML::Stream::StreamKind::MemoryInfoList:
     streamMapping(IO, llvm::cast<MemoryInfoListStream>(*S));
     break;
   case MinidumpYAML::Stream::StreamKind::MemoryList:
     streamMapping(IO, llvm::cast<MemoryListStream>(*S));
     break;
   case MinidumpYAML::Stream::StreamKind::ModuleList:
     streamMapping(IO, llvm::cast<ModuleListStream>(*S));
     break;
void yaml::MappingTraits<std::unique_ptr<Stream>>::mapping(
   }
 }
 
 StringRef yaml::MappingTraits<std::unique_ptr<Stream>>::validate(
     yaml::IO &IO, std::unique_ptr<MinidumpYAML::Stream> &S) {
   switch (S->Kind) {
   case MinidumpYAML::Stream::StreamKind::RawContent:
     return streamValidate(cast<RawContentStream>(*S));
+  case MinidumpYAML::Stream::StreamKind::Exception:
   case MinidumpYAML::Stream::StreamKind::MemoryInfoList:
   case MinidumpYAML::Stream::StreamKind::MemoryList:
   case MinidumpYAML::Stream::StreamKind::ModuleList:
   case MinidumpYAML::Stream::StreamKind::SystemInfo:
   case MinidumpYAML::Stream::StreamKind::TextContent:
   case MinidumpYAML::Stream::StreamKind::ThreadList:
     return "";
   }
   llvm_unreachable("Fully covered switch above!");
 }
 
 void yaml::MappingTraits<Object>::mapping(IO &IO, Object &O) {
   IO.mapTag("!minidump", true);
   mapOptionalHex(IO, "Signature", O.Header.Signature, Header::MagicSignature);
   mapOptionalHex(IO, "Version", O.Header.Version, Header::MagicVersion);
   mapOptionalHex(IO, "Flags", O.Header.Flags, 0);
   IO.mapRequired("Streams", O.Streams);
 }
 
 Expected<std::unique_ptr<Stream>>
 Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
   StreamKind Kind = getKind(StreamDesc.Type);
   switch (Kind) {
+  case StreamKind::Exception: {
+    Expected<const minidump::ExceptionStream &> ExpectedExceptionStream =
+        File.getExceptionStream();
+    if (!ExpectedExceptionStream)
+      return ExpectedExceptionStream.takeError();
+    Expected<ArrayRef<uint8_t>> ExpectedThreadContext =
+        File.getRawData(ExpectedExceptionStream->ThreadContext);
+    if (!ExpectedThreadContext)
+      return ExpectedThreadContext.takeError();
+    return std::make_unique<ExceptionStream>(*ExpectedExceptionStream,
+                                             *ExpectedThreadContext);
+  }
   case StreamKind::MemoryInfoList: {
     if (auto ExpectedList = File.getMemoryInfoList())
       return std::make_unique<MemoryInfoListStream>(*ExpectedList);
     else
       return ExpectedList.takeError();
   }
   case StreamKind::MemoryList: {
     auto ExpectedList = File.getMemoryList();
if (!ExpectedInfo)
       return ExpectedInfo.takeError();
     auto ExpectedCSDVersion = File.getString(ExpectedInfo->CSDVersionRVA);
     if (!ExpectedCSDVersion)
       return ExpectedInfo.takeError();
     return std::make_unique<SystemInfoStream>(*ExpectedInfo,
                                                std::move(*ExpectedCSDVersion));
   }
   case StreamKind::TextContent:
     return std::make_unique<TextContentStream>(
         StreamDesc.Type, toStringRef(File.getRawStream(StreamDesc)));
   case StreamKind::ThreadList: {
     auto ExpectedList = File.getThreadList();
     if (!ExpectedList)
       return ExpectedList.takeError();
     std::vector<ThreadListStream::entry_type> Threads;
     for (const Thread &T : *ExpectedList) {
       auto ExpectedStack = File.getRawData(T.Stack.Memory);

llvm/test/tools/obj2yaml/basic-minidump.yaml

- Type:            ThreadList
     Threads:
       - Thread Id:         0x5C5D5E5F
         Priority Class:    0x60616263
         Environment Block: 0x6465666768696A6B
         Context:           '7C7D7E7F80818283'
         Stack:
           Start of Memory Range: 0x6C6D6E6F70717273
           Content:               '7475767778797A7B'
+  - Type:            Exception
+    Thread ID:       0x7
+    Exception Record:
+      Exception Code:       0x10
+      Exception Flags:      0x5
+      Exception Record:     0x0102030405060708
+      Exception Address:    0x0A0B0C0D0E0F1011
+      Number of Parameters: 2
+      Parameter 0: 0x22
+      Parameter 1: 0x24
+    Thread Context:  '8182838485868788'
   - Type:            MemoryList
     Memory Ranges:   
       - Start of Memory Range: 0x7C7D7E7F80818283
         Content:               '8485868788'
   - Type:            MemoryInfoList
     Memory Ranges:
       - Base Address:    0x0000000000000000
         Allocation Protect: [  ]

 # CHECK-NEXT:     Threads:
 # CHECK-NEXT:       - Thread Id:         0x5C5D5E5F
 # CHECK-NEXT:         Priority Class:    0x60616263
 # CHECK-NEXT:         Environment Block: 0x6465666768696A6B
 # CHECK-NEXT:         Context:           7C7D7E7F80818283
 # CHECK-NEXT:         Stack:
 # CHECK-NEXT:           Start of Memory Range: 0x6C6D6E6F70717273
 # CHECK-NEXT:           Content:               7475767778797A7B
+# CHECK-NEXT:   - Type:            Exception
+# CHECK-NEXT:     Thread ID:       0x00000007
+# CHECK-NEXT:     Exception Record:
+# CHECK-NEXT:       Exception Code:       0x00000010
+# CHECK-NEXT:       Exception Flags:      0x00000005
+# CHECK-NEXT:       Exception Record:     0x0102030405060708
+# CHECK-NEXT:       Exception Address:    0x0A0B0C0D0E0F1011
+# CHECK-NEXT:       Number of Parameters: 2
+# CHECK-NEXT:       Parameter 0: 0x0000000000000022
+# CHECK-NEXT:       Parameter 1: 0x0000000000000024
+# CHECK-NEXT:     Thread Context:  '8182838485868788'
 # CHECK-NEXT:   - Type:            MemoryList
 # CHECK-NEXT:     Memory Ranges:   
 # CHECK-NEXT:       - Start of Memory Range: 0x7C7D7E7F80818283
 # CHECK-NEXT:         Content:               '8485868788'
 # CHECK-NEXT:   - Type:            MemoryInfoList
 # CHECK-NEXT:     Memory Ranges:
 # CHECK-NEXT:       - Base Address:       0x0000000000000000
 # CHECK-NEXT:         Allocation Protect: [  ]

llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml

+# RUN: not yaml2obj %s -o /dev/null 2>&1 | FileCheck %s
+
+## Test that we report an error for an ExceptionStream where the specified
+## number of parameters is greater than the number of ExceptionInformation
+## elements listed.
+
+--- !minidump
+Streams:
+  - Type:            Exception
+    Thread ID:  0x7
+    Exception Record:
+      Exception Code:  0x23
+      Exception Flags: 0x5
+      Exception Record: 0x0102030405060708
+      Exception Address: 0x0a0b0c0d0e0f1011
+      Number of Parameters: 4
+      Parameter 0: 0x99
+      Parameter 1: 0x23
+      Parameter 2: 0x42
+# CHECK: error: missing required key 'Parameter 3'
+    Thread Context:  3DeadBeefDefacedABadCafe)");

llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

Streams:
   ASSERT_THAT_EXPECTED(ExpectedSysInfo, Succeeded());
   const SystemInfo &SysInfo = *ExpectedSysInfo;
   EXPECT_EQ(ProcessorArchitecture::PPC, SysInfo.ProcessorArch);
   EXPECT_EQ(OSPlatform::Linux, SysInfo.PlatformId);
   EXPECT_EQ(
       (ArrayRef<uint8_t>{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
       makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
 }
+
+// Test that we can parse a normal-looking ExceptionStream.
+TEST(MinidumpYAML, ExceptionStream) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:            Exception
+    Thread ID:  0x7
+    Exception Record:
+      Exception Code:  0x23
+      Exception Flags: 0x5
+      Exception Record: 0x0102030405060708
+      Exception Address: 0x0a0b0c0d0e0f1011
+      Number of Parameters: 2
+      Parameter 0: 0x22
+      Parameter 1: 0x24
+    Thread Context:  3DeadBeefDefacedABadCafe)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  Expected<const minidump::ExceptionStream &> ExpectedStream =
+      File.getExceptionStream();
+
+  ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+  const minidump::ExceptionStream &Stream = *ExpectedStream;
+  EXPECT_EQ(0x7u, Stream.ThreadId);
+  const minidump::Exception &Exception = Stream.ExceptionRecord;
+  EXPECT_EQ(0x23u, Exception.ExceptionCode);
+  EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+  EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+  EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+  EXPECT_EQ(2u, Exception.NumberParameters);
+  EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+  EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+  Expected<ArrayRef<uint8_t>> ExpectedContext =
+      File.getRawData(Stream.ThreadContext);
+  ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+  EXPECT_EQ((ArrayRef<uint8_t>{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+                               0xab, 0xad, 0xca, 0xfe}),
+            *ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation.
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:            Exception
+    Thread ID:  0x7
+    Exception Record:
+      Exception Code:  0x23
+      Exception Flags: 0x5
+      Exception Record: 0x0102030405060708
+      Exception Address: 0x0a0b0c0d0e0f1011
+    Thread Context:  3DeadBeefDefacedABadCafe)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  Expected<const minidump::ExceptionStream &> ExpectedStream =
+      File.getExceptionStream();
+
+  ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+  const minidump::ExceptionStream &Stream = *ExpectedStream;
+  EXPECT_EQ(0x7u, Stream.ThreadId);
+  const minidump::Exception &Exception = Stream.ExceptionRecord;
+  EXPECT_EQ(0x23u, Exception.ExceptionCode);
+  EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+  EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+  EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+  EXPECT_EQ(0u, Exception.NumberParameters);
+
+  Expected<ArrayRef<uint8_t>> ExpectedContext =
+      File.getRawData(Stream.ThreadContext);
+  ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+  EXPECT_EQ((ArrayRef<uint8_t>{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+                               0xab, 0xad, 0xca, 0xfe}),
+            *ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:            Exception
+    Thread ID:  0x8
+    Exception Record:
+      Exception Code: 0
+      Number of Parameters: 16
+      Parameter 0: 0x0
+      Parameter 1: 0xff
+      Parameter 2: 0xee
+      Parameter 3: 0xdd
+      Parameter 4: 0xcc
+      Parameter 5: 0xbb
+      Parameter 6: 0xaa
+      Parameter 7: 0x99
+      Parameter 8: 0x88
+      Parameter 9: 0x77
+      Parameter 10: 0x66
+      Parameter 11: 0x55
+      Parameter 12: 0x44
+      Parameter 13: 0x33
+      Parameter 14: 0x22
+    Thread Context:  3DeadBeefDefacedABadCafe)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  Expected<const minidump::ExceptionStream &> ExpectedStream =
+      File.getExceptionStream();
+
+  ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+  const minidump::ExceptionStream &Stream = *ExpectedStream;
+  EXPECT_EQ(0x8u, Stream.ThreadId);
+  const minidump::Exception &Exception = Stream.ExceptionRecord;
+  EXPECT_EQ(0x0u, Exception.ExceptionCode);
+  EXPECT_EQ(0x0u, Exception.ExceptionFlags);
+  EXPECT_EQ(0x00u, Exception.ExceptionRecord);
+  EXPECT_EQ(0x0u, Exception.ExceptionAddress);
+  EXPECT_EQ(16u, Exception.NumberParameters);
+  EXPECT_EQ(0x0u, Exception.ExceptionInformation[0]);
+  for (int Index = 1; Index < 15; ++Index) {
+    EXPECT_EQ(0x110u - Index * 0x11, Exception.ExceptionInformation[Index]);
+  }
+
+  Expected<ArrayRef<uint8_t>> ExpectedContext =
+      File.getRawData(Stream.ThreadContext);
+  ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+  EXPECT_EQ((ArrayRef<uint8_t>{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+                               0xab, 0xad, 0xca, 0xfe}),
+            *ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the number of
+// ExceptionInformation parameters provided is greater than the
+// specified Number of Parameters.
+TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:            Exception
+    Thread ID:  0x7
+    Exception Record:
+      Exception Code:  0x23
+      Exception Flags: 0x5
+      Exception Record: 0x0102030405060708
+      Exception Address: 0x0a0b0c0d0e0f1011
+      Number of Parameters: 2
+      Parameter 0: 0x99
+      Parameter 1: 0x23
+      Parameter 2: 0x42
+    Thread Context:  3DeadBeefDefacedABadCafe)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  Expected<const minidump::ExceptionStream &> ExpectedStream =
+      File.getExceptionStream();
+
+  ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+  const minidump::ExceptionStream &Stream = *ExpectedStream;
+  EXPECT_EQ(0x7u, Stream.ThreadId);
+  const minidump::Exception &Exception = Stream.ExceptionRecord;
+  EXPECT_EQ(0x23u, Exception.ExceptionCode);
+  EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+  EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+  EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+  EXPECT_EQ(2u, Exception.NumberParameters);
+  EXPECT_EQ(0x99u, Exception.ExceptionInformation[0]);
+  EXPECT_EQ(0x23u, Exception.ExceptionInformation[1]);
+  EXPECT_EQ(0x42u, Exception.ExceptionInformation[2]);
+
+  Expected<ArrayRef<uint8_t>> ExpectedContext =
+      File.getRawData(Stream.ThreadContext);
+  ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+  EXPECT_EQ((ArrayRef<uint8_t>{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+                               0xab, 0xad, 0xca, 0xfe}),
+            *ExpectedContext);
+}