vendor/llvm/llvm-trunk-r321017

1 files changed, 231 insertions, 75 deletions

diff --git a/lib/XRay/Trace.cpp b/lib/XRay/Trace.cpp
index 6677063f944f..d1fcf1c35b36 100644
--- a/lib/XRay/Trace.cpp
+++ b/lib/XRay/Trace.cpp
@@ -21,6 +21,7 @@ using namespace llvm;
 using namespace llvm::xray;
 using llvm::yaml::Input;
 
+namespace {
 using XRayRecordStorage =
     std::aligned_storage<sizeof(XRayRecord), alignof(XRayRecord)>::type;
 
@@ -47,7 +48,7 @@ Error readBinaryFormatHeader(StringRef Data, XRayFileHeader &FileHeader) {
   FileHeader.NonstopTSC = Bitfield & 1uL << 1;
   FileHeader.CycleFrequency = HeaderExtractor.getU64(&OffsetPtr);
   std::memcpy(&FileHeader.FreeFormData, Data.bytes_begin() + OffsetPtr, 16);
-  if (FileHeader.Version != 1)
+  if (FileHeader.Version != 1 && FileHeader.Version != 2)
     return make_error<StringError>(
         Twine("Unsupported XRay file version: ") + Twine(FileHeader.Version),
         std::make_error_code(std::errc::invalid_argument));
@@ -56,7 +57,6 @@ Error readBinaryFormatHeader(StringRef Data, XRayFileHeader &FileHeader) {
 
 Error loadNaiveFormatLog(StringRef Data, XRayFileHeader &FileHeader,
                          std::vector<XRayRecord> &Records) {
-  // Check that there is at least a header
   if (Data.size() < 32)
     return make_error<StringError>(
         "Not enough bytes for an XRay log.",
@@ -82,26 +82,59 @@ Error loadNaiveFormatLog(StringRef Data, XRayFileHeader &FileHeader,
   for (auto S = Data.drop_front(32); !S.empty(); S = S.drop_front(32)) {
     DataExtractor RecordExtractor(S, true, 8);
     uint32_t OffsetPtr = 0;
-    Records.emplace_back();
-    auto &Record = Records.back();
-    Record.RecordType = RecordExtractor.getU16(&OffsetPtr);
-    Record.CPU = RecordExtractor.getU8(&OffsetPtr);
-    auto Type = RecordExtractor.getU8(&OffsetPtr);
-    switch (Type) {
-    case 0:
-      Record.Type = RecordTypes::ENTER;
+    switch (auto RecordType = RecordExtractor.getU16(&OffsetPtr)) {
+    case 0: { // Normal records.
+      Records.emplace_back();
+      auto &Record = Records.back();
+      Record.RecordType = RecordType;
+      Record.CPU = RecordExtractor.getU8(&OffsetPtr);
+      auto Type = RecordExtractor.getU8(&OffsetPtr);
+      switch (Type) {
+      case 0:
+        Record.Type = RecordTypes::ENTER;
+        break;
+      case 1:
+        Record.Type = RecordTypes::EXIT;
+        break;
+      case 2:
+        Record.Type = RecordTypes::TAIL_EXIT;
+        break;
+      case 3:
+        Record.Type = RecordTypes::ENTER_ARG;
+        break;
+      default:
+        return make_error<StringError>(
+            Twine("Unknown record type '") + Twine(int{Type}) + "'",
+            std::make_error_code(std::errc::executable_format_error));
+      }
+      Record.FuncId = RecordExtractor.getSigned(&OffsetPtr, sizeof(int32_t));
+      Record.TSC = RecordExtractor.getU64(&OffsetPtr);
+      Record.TId = RecordExtractor.getU32(&OffsetPtr);
       break;
-    case 1:
-      Record.Type = RecordTypes::EXIT;
+    }
+    case 1: { // Arg payload record.
+      auto &Record = Records.back();
+      // Advance two bytes to avoid padding.
+      OffsetPtr += 2;
+      int32_t FuncId = RecordExtractor.getSigned(&OffsetPtr, sizeof(int32_t));
+      auto TId = RecordExtractor.getU32(&OffsetPtr);
+      if (Record.FuncId != FuncId || Record.TId != TId)
+        return make_error<StringError>(
+            Twine("Corrupted log, found payload following non-matching "
+                  "function + thread record. Record for ") +
+                Twine(Record.FuncId) + " != " + Twine(FuncId),
+            std::make_error_code(std::errc::executable_format_error));
+      // Advance another four bytes to avoid padding.
+      OffsetPtr += 4;
+      auto Arg = RecordExtractor.getU64(&OffsetPtr);
+      Record.CallArgs.push_back(Arg);
       break;
+    }
     default:
       return make_error<StringError>(
-          Twine("Unknown record type '") + Twine(int{Type}) + "'",
+          Twine("Unknown record type == ") + Twine(RecordType),
           std::make_error_code(std::errc::executable_format_error));
     }
-    Record.FuncId = RecordExtractor.getSigned(&OffsetPtr, sizeof(int32_t));
-    Record.TSC = RecordExtractor.getU64(&OffsetPtr);
-    Record.TId = RecordExtractor.getU32(&OffsetPtr);
   }
   return Error::success();
 }
@@ -125,6 +158,8 @@ struct FDRState {
     FUNCTION_SEQUENCE,
     SCAN_TO_END_OF_THREAD_BUF,
     CUSTOM_EVENT_DATA,
+    CALL_ARGUMENT,
+    BUFFER_EXTENTS,
   };
   Token Expects;
 
@@ -134,7 +169,7 @@ struct FDRState {
   uint64_t CurrentBufferConsumed;
 };
 
-Twine fdrStateToTwine(const FDRState::Token &state) {
+const char *fdrStateToTwine(const FDRState::Token &state) {
   switch (state) {
   case FDRState::Token::NEW_BUFFER_RECORD_OR_EOF:
     return "NEW_BUFFER_RECORD_OR_EOF";
@@ -148,6 +183,10 @@ Twine fdrStateToTwine(const FDRState::Token &state) {
     return "SCAN_TO_END_OF_THREAD_BUF";
   case FDRState::Token::CUSTOM_EVENT_DATA:
     return "CUSTOM_EVENT_DATA";
+  case FDRState::Token::CALL_ARGUMENT:
+    return "CALL_ARGUMENT";
+  case FDRState::Token::BUFFER_EXTENTS:
+    return "BUFFER_EXTENTS";
   }
   return "UNKNOWN";
 }
@@ -158,7 +197,9 @@ Error processFDRNewBufferRecord(FDRState &State, uint8_t RecordFirstByte,
 
   if (State.Expects != FDRState::Token::NEW_BUFFER_RECORD_OR_EOF)
     return make_error<StringError>(
-        "Malformed log. Read New Buffer record kind out of sequence",
+        Twine("Malformed log. Read New Buffer record kind out of sequence; "
+              "expected: ") +
+            fdrStateToTwine(State.Expects),
         std::make_error_code(std::errc::executable_format_error));
   uint32_t OffsetPtr = 1; // 1 byte into record.
   State.ThreadId = RecordExtractor.getU16(&OffsetPtr);
@@ -171,7 +212,9 @@ Error processFDREndOfBufferRecord(FDRState &State, uint8_t RecordFirstByte,
                                   DataExtractor &RecordExtractor) {
   if (State.Expects == FDRState::Token::NEW_BUFFER_RECORD_OR_EOF)
     return make_error<StringError>(
-        "Malformed log. Received EOB message without current buffer.",
+        Twine("Malformed log. Received EOB message without current buffer; "
+              "expected: ") +
+            fdrStateToTwine(State.Expects),
         std::make_error_code(std::errc::executable_format_error));
   State.Expects = FDRState::Token::SCAN_TO_END_OF_THREAD_BUF;
   return Error::success();
@@ -183,7 +226,9 @@ Error processFDRNewCPUIdRecord(FDRState &State, uint8_t RecordFirstByte,
   if (State.Expects != FDRState::Token::FUNCTION_SEQUENCE &&
       State.Expects != FDRState::Token::NEW_CPU_ID_RECORD)
     return make_error<StringError>(
-        "Malformed log. Read NewCPUId record kind out of sequence",
+        Twine("Malformed log. Read NewCPUId record kind out of sequence; "
+              "expected: ") +
+            fdrStateToTwine(State.Expects),
         std::make_error_code(std::errc::executable_format_error));
   uint32_t OffsetPtr = 1; // Read starting after the first byte.
   State.CPUId = RecordExtractor.getU16(&OffsetPtr);
@@ -197,7 +242,9 @@ Error processFDRTSCWrapRecord(FDRState &State, uint8_t RecordFirstByte,
                               DataExtractor &RecordExtractor) {
   if (State.Expects != FDRState::Token::FUNCTION_SEQUENCE)
     return make_error<StringError>(
-        "Malformed log. Read TSCWrap record kind out of sequence",
+        Twine("Malformed log. Read TSCWrap record kind out of sequence; "
+              "expecting: ") +
+            fdrStateToTwine(State.Expects),
         std::make_error_code(std::errc::executable_format_error));
   uint32_t OffsetPtr = 1; // Read starting after the first byte.
   State.BaseTSC = RecordExtractor.getU64(&OffsetPtr);
@@ -209,10 +256,13 @@ Error processFDRWallTimeRecord(FDRState &State, uint8_t RecordFirstByte,
                                DataExtractor &RecordExtractor) {
   if (State.Expects != FDRState::Token::WALLCLOCK_RECORD)
     return make_error<StringError>(
-        "Malformed log. Read Wallclock record kind out of sequence",
+        Twine("Malformed log. Read Wallclock record kind out of sequence; "
+              "expecting: ") +
+            fdrStateToTwine(State.Expects),
         std::make_error_code(std::errc::executable_format_error));
-  // We don't encode the wall time into any of the records.
-  // XRayRecords are concerned with the TSC instead.
+
+  // TODO: Someday, reconcile the TSC ticks to wall clock time for presentation
+  // purposes. For now, we're ignoring these records.
   State.Expects = FDRState::Token::NEW_CPU_ID_RECORD;
   return Error::success();
 }
@@ -222,27 +272,63 @@ Error processCustomEventMarker(FDRState &State, uint8_t RecordFirstByte,
                                DataExtractor &RecordExtractor,
                                size_t &RecordSize) {
   // We can encounter a CustomEventMarker anywhere in the log, so we can handle
-  // it regardless of the expectation. However, we do se the expectation to read
-  // a set number of fixed bytes, as described in the metadata.
+  // it regardless of the expectation. However, we do set the expectation to
+  // read a set number of fixed bytes, as described in the metadata.
   uint32_t OffsetPtr = 1; // Read after the first byte.
   uint32_t DataSize = RecordExtractor.getU32(&OffsetPtr);
   uint64_t TSC = RecordExtractor.getU64(&OffsetPtr);
 
-  // FIXME: Actually represent the record through the API. For now we only skip
-  // through the data.
+  // FIXME: Actually represent the record through the API. For now we only
+  // skip through the data.
   (void)TSC;
   RecordSize = 16 + DataSize;
   return Error::success();
 }
 
+/// State transition when an BufferExtents record is encountered.
+Error processBufferExtents(FDRState &State, uint8_t RecordFirstByte,
+                           DataExtractor &RecordExtractor) {
+  if (State.Expects != FDRState::Token::BUFFER_EXTENTS)
+    return make_error<StringError>(
+        Twine("Malformed log. Buffer Extents unexpected; expected: ") +
+            fdrStateToTwine(State.Expects),
+        std::make_error_code(std::errc::executable_format_error));
+  uint32_t OffsetPtr = 1; // Read after the first byte.
+  State.CurrentBufferSize = RecordExtractor.getU64(&OffsetPtr);
+  State.Expects = FDRState::Token::NEW_BUFFER_RECORD_OR_EOF;
+  return Error::success();
+}
+
+/// State transition when a CallArgumentRecord is encountered.
+Error processFDRCallArgumentRecord(FDRState &State, uint8_t RecordFirstByte,
+                                   DataExtractor &RecordExtractor,
+                                   std::vector<XRayRecord> &Records) {
+  uint32_t OffsetPtr = 1; // Read starting after the first byte.
+  auto &Enter = Records.back();
+
+  if (Enter.Type != RecordTypes::ENTER)
+    return make_error<StringError>(
+        "CallArgument needs to be right after a function entry",
+        std::make_error_code(std::errc::executable_format_error));
+  Enter.Type = RecordTypes::ENTER_ARG;
+  Enter.CallArgs.emplace_back(RecordExtractor.getU64(&OffsetPtr));
+  return Error::success();
+}
+
 /// Advances the state machine for reading the FDR record type by reading one
 /// Metadata Record and updating the State appropriately based on the kind of
 /// record encountered. The RecordKind is encoded in the first byte of the
 /// Record, which the caller should pass in because they have already read it
 /// to determine that this is a metadata record as opposed to a function record.
+///
+/// Beginning with Version 2 of the FDR log, we do not depend on the size of the
+/// buffer, but rather use the extents to determine how far to read in the log
+/// for this particular buffer.
 Error processFDRMetadataRecord(FDRState &State, uint8_t RecordFirstByte,
                                DataExtractor &RecordExtractor,
-                               size_t &RecordSize) {
+                               size_t &RecordSize,
+                               std::vector<XRayRecord> &Records,
+                               uint16_t Version) {
   // The remaining 7 bits are the RecordKind enum.
   uint8_t RecordKind = RecordFirstByte >> 1;
   switch (RecordKind) {
@@ -252,6 +338,10 @@ Error processFDRMetadataRecord(FDRState &State, uint8_t RecordFirstByte,
       return E;
     break;
   case 1: // EndOfBuffer
+    if (Version >= 2)
+      return make_error<StringError>(
+          "Since Version 2 of FDR logging, we no longer support EOB records.",
+          std::make_error_code(std::errc::executable_format_error));
     if (auto E = processFDREndOfBufferRecord(State, RecordFirstByte,
                                              RecordExtractor))
       return E;
@@ -276,6 +366,15 @@ Error processFDRMetadataRecord(FDRState &State, uint8_t RecordFirstByte,
                                           RecordExtractor, RecordSize))
       return E;
     break;
+  case 6: // CallArgument
+    if (auto E = processFDRCallArgumentRecord(State, RecordFirstByte,
+                                              RecordExtractor, Records))
+      return E;
+    break;
+  case 7: // BufferExtents
+    if (auto E = processBufferExtents(State, RecordFirstByte, RecordExtractor))
+      return E;
+    break;
   default:
     // Widen the record type to uint16_t to prevent conversion to char.
     return make_error<StringError>(
@@ -320,12 +419,13 @@ Error processFDRFunctionRecord(FDRState &State, uint8_t RecordFirstByte,
       Record.Type = RecordTypes::ENTER;
       break;
     case static_cast<uint8_t>(RecordTypes::EXIT):
-    case 2: // TAIL_EXIT is not yet defined in RecordTypes.
       Record.Type = RecordTypes::EXIT;
       break;
+    case static_cast<uint8_t>(RecordTypes::TAIL_EXIT):
+      Record.Type = RecordTypes::TAIL_EXIT;
+      break;
     default:
-      // When initializing the error, convert to uint16_t so that the record
-      // type isn't interpreted as a char.
+      // Cast to an unsigned integer to not interpret the record type as a char.
       return make_error<StringError>(
           Twine("Illegal function record type: ")
               .concat(Twine(static_cast<unsigned>(RecordType))),
@@ -333,7 +433,7 @@ Error processFDRFunctionRecord(FDRState &State, uint8_t RecordFirstByte,
     }
     Record.CPU = State.CPUId;
     Record.TId = State.ThreadId;
-    // Back up to read first 32 bits, including the 8 we pulled RecordType
+    // Back up to read first 32 bits, including the 4 we pulled RecordType
     // and RecordKind out of. The remaining 28 are FunctionId.
     uint32_t OffsetPtr = 0;
     // Despite function Id being a signed int on XRayRecord,
@@ -346,9 +446,9 @@ Error processFDRFunctionRecord(FDRState &State, uint8_t RecordFirstByte,
     // FunctionRecords have a 32 bit delta from the previous absolute TSC
     // or TSC delta. If this would overflow, we should read a TSCWrap record
     // with an absolute TSC reading.
-    uint64_t new_tsc = State.BaseTSC + RecordExtractor.getU32(&OffsetPtr);
-    State.BaseTSC = new_tsc;
-    Record.TSC = new_tsc;
+    uint64_t NewTSC = State.BaseTSC + RecordExtractor.getU32(&OffsetPtr);
+    State.BaseTSC = NewTSC;
+    Record.TSC = NewTSC;
   }
   return Error::success();
 }
@@ -360,17 +460,19 @@ Error processFDRFunctionRecord(FDRState &State, uint8_t RecordFirstByte,
 ///
 /// The following is an attempt to document the grammar of the format, which is
 /// parsed by this function for little-endian machines. Since the format makes
-/// use of BitFields, when we support big-Endian architectures, we will need to
+/// use of BitFields, when we support big-endian architectures, we will need to
 /// adjust not only the endianness parameter to llvm's RecordExtractor, but also
 /// the bit twiddling logic, which is consistent with the little-endian
 /// convention that BitFields within a struct will first be packed into the
 /// least significant bits the address they belong to.
 ///
-/// We expect a format complying with the grammar in the following pseudo-EBNF.
+/// We expect a format complying with the grammar in the following pseudo-EBNF
+/// in Version 1 of the FDR log.
 ///
 /// FDRLog: XRayFileHeader ThreadBuffer*
-/// XRayFileHeader: 32 bits to identify the log as FDR with machine metadata.
-/// ThreadBuffer: BufSize NewBuffer WallClockTime NewCPUId FunctionSequence EOB
+/// XRayFileHeader: 32 bytes to identify the log as FDR with machine metadata.
+///     Includes BufferSize
+/// ThreadBuffer: NewBuffer WallClockTime NewCPUId FunctionSequence EOB
 /// BufSize: 8 byte unsigned integer indicating how large the buffer is.
 /// NewBuffer: 16 byte metadata record with Thread Id.
 /// WallClockTime: 16 byte metadata record with human readable time.
@@ -379,6 +481,15 @@ Error processFDRFunctionRecord(FDRState &State, uint8_t RecordFirstByte,
 /// FunctionSequence: NewCPUId | TSCWrap | FunctionRecord
 /// TSCWrap: 16 byte metadata record with a full 64 bit TSC reading.
 /// FunctionRecord: 8 byte record with FunctionId, entry/exit, and TSC delta.
+///
+/// In Version 2, we make the following changes:
+///
+/// ThreadBuffer: BufferExtents NewBuffer WallClockTime NewCPUId
+///               FunctionSequence
+/// BufferExtents: 16 byte metdata record describing how many usable bytes are
+///                in the buffer. This is measured from the start of the buffer
+///                and must always be at least 48 (bytes).
+/// EOB: *deprecated*
 Error loadFDRLog(StringRef Data, XRayFileHeader &FileHeader,
                  std::vector<XRayRecord> &Records) {
   if (Data.size() < 32)
@@ -404,8 +515,22 @@ Error loadFDRLog(StringRef Data, XRayFileHeader &FileHeader,
     uint32_t ExtraDataOffset = 0;
     BufferSize = ExtraDataExtractor.getU64(&ExtraDataOffset);
   }
-  FDRState State{0,          0, 0, FDRState::Token::NEW_BUFFER_RECORD_OR_EOF,
-                 BufferSize, 0};
+
+  FDRState::Token InitialExpectation;
+  switch (FileHeader.Version) {
+  case 1:
+    InitialExpectation = FDRState::Token::NEW_BUFFER_RECORD_OR_EOF;
+    break;
+  case 2:
+    InitialExpectation = FDRState::Token::BUFFER_EXTENTS;
+    break;
+  default:
+    return make_error<StringError>(
+        Twine("Unsupported version '") + Twine(FileHeader.Version) + "'",
+        std::make_error_code(std::errc::executable_format_error));
+  }
+  FDRState State{0, 0, 0, InitialExpectation, BufferSize, 0};
+
   // RecordSize will tell the loop how far to seek ahead based on the record
   // type that we have just read.
   size_t RecordSize = 0;
@@ -414,11 +539,10 @@ Error loadFDRLog(StringRef Data, XRayFileHeader &FileHeader,
     uint32_t OffsetPtr = 0;
     if (State.Expects == FDRState::Token::SCAN_TO_END_OF_THREAD_BUF) {
       RecordSize = State.CurrentBufferSize - State.CurrentBufferConsumed;
-      if (S.size() < State.CurrentBufferSize - State.CurrentBufferConsumed) {
+      if (S.size() < RecordSize) {
         return make_error<StringError>(
-            Twine("Incomplete thread buffer. Expected ") +
-                Twine(State.CurrentBufferSize - State.CurrentBufferConsumed) +
-                " remaining bytes but found " + Twine(S.size()),
+            Twine("Incomplete thread buffer. Expected at least ") +
+                Twine(RecordSize) + " bytes but found " + Twine(S.size()),
             make_error_code(std::errc::invalid_argument));
       }
       State.CurrentBufferConsumed = 0;
@@ -427,24 +551,43 @@ Error loadFDRLog(StringRef Data, XRayFileHeader &FileHeader,
     }
     uint8_t BitField = RecordExtractor.getU8(&OffsetPtr);
     bool isMetadataRecord = BitField & 0x01uL;
+    bool isBufferExtents =
+        (BitField >> 1) == 7; // BufferExtents record kind == 7
     if (isMetadataRecord) {
       RecordSize = 16;
-      if (auto E = processFDRMetadataRecord(State, BitField, RecordExtractor,
-                                            RecordSize))
+      if (auto E =
+              processFDRMetadataRecord(State, BitField, RecordExtractor,
+                                       RecordSize, Records, FileHeader.Version))
         return E;
-      State.CurrentBufferConsumed += RecordSize;
     } else { // Process Function Record
       RecordSize = 8;
       if (auto E = processFDRFunctionRecord(State, BitField, RecordExtractor,
                                             Records))
         return E;
+    }
+
+    // The BufferExtents record is technically not part of the buffer, so we
+    // don't count the size of that record against the buffer's actual size.
+    if (!isBufferExtents)
       State.CurrentBufferConsumed += RecordSize;
+    assert(State.CurrentBufferConsumed <= State.CurrentBufferSize);
+    if (FileHeader.Version == 2 &&
+        State.CurrentBufferSize == State.CurrentBufferConsumed) {
+      // In Version 2 of the log, we don't need to scan to the end of the thread
+      // buffer if we've already consumed all the bytes we need to.
+      State.Expects = FDRState::Token::BUFFER_EXTENTS;
+      State.CurrentBufferSize = BufferSize;
+      State.CurrentBufferConsumed = 0;
     }
   }
-  // There are two conditions
-  if (State.Expects != FDRState::Token::NEW_BUFFER_RECORD_OR_EOF &&
-      !(State.Expects == FDRState::Token::SCAN_TO_END_OF_THREAD_BUF &&
-        State.CurrentBufferSize == State.CurrentBufferConsumed))
+
+  // Having iterated over everything we've been given, we've either consumed
+  // everything and ended up in the end state, or were told to skip the rest.
+  bool Finished = State.Expects == FDRState::Token::SCAN_TO_END_OF_THREAD_BUF &&
+                  State.CurrentBufferSize == State.CurrentBufferConsumed;
+  if ((State.Expects != FDRState::Token::NEW_BUFFER_RECORD_OR_EOF &&
+       State.Expects != FDRState::Token::BUFFER_EXTENTS) &&
+      !Finished)
     return make_error<StringError>(
         Twine("Encountered EOF with unexpected state expectation ") +
             fdrStateToTwine(State.Expects) +
@@ -457,7 +600,6 @@ Error loadFDRLog(StringRef Data, XRayFileHeader &FileHeader,
 
 Error loadYAMLLog(StringRef Data, XRayFileHeader &FileHeader,
                   std::vector<XRayRecord> &Records) {
-  // Load the documents from the MappedFile.
   YAMLXRayTrace Trace;
   Input In(Data);
   In >> Trace;
@@ -478,11 +620,12 @@ Error loadYAMLLog(StringRef Data, XRayFileHeader &FileHeader,
   Records.clear();
   std::transform(Trace.Records.begin(), Trace.Records.end(),
                  std::back_inserter(Records), [&](const YAMLXRayRecord &R) {
-                   return XRayRecord{R.RecordType, R.CPU, R.Type,
-                                     R.FuncId,     R.TSC, R.TId};
+                   return XRayRecord{R.RecordType, R.CPU, R.Type,    R.FuncId,
+                                     R.TSC,        R.TId, R.CallArgs};
                  });
   return Error::success();
 }
+} // namespace
 
 Expected<Trace> llvm::xray::loadTraceFile(StringRef Filename, bool Sort) {
   int Fd;
@@ -491,7 +634,6 @@ Expected<Trace> llvm::xray::loadTraceFile(StringRef Filename, bool Sort) {
         Twine("Cannot read log from '") + Filename + "'", EC);
   }
 
-  // Attempt to get the filesize.
   uint64_t FileSize;
   if (auto EC = sys::fs::file_size(Filename, FileSize)) {
     return make_error<StringError>(
@@ -503,7 +645,7 @@ Expected<Trace> llvm::xray::loadTraceFile(StringRef Filename, bool Sort) {
         std::make_error_code(std::errc::executable_format_error));
   }
 
-  // Attempt to mmap the file.
+  // Map the opened file into memory and use a StringRef to access it later.
   std::error_code EC;
   sys::fs::mapped_file_region MappedFile(
       Fd, sys::fs::mapped_file_region::mapmode::readonly, FileSize, 0, EC);
@@ -511,16 +653,18 @@ Expected<Trace> llvm::xray::loadTraceFile(StringRef Filename, bool Sort) {
     return make_error<StringError>(
         Twine("Cannot read log from '") + Filename + "'", EC);
   }
+  auto Data = StringRef(MappedFile.data(), MappedFile.size());
 
   // Attempt to detect the file type using file magic. We have a slight bias
   // towards the binary format, and we do this by making sure that the first 4
   // bytes of the binary file is some combination of the following byte
-  // patterns:
+  // patterns: (observe the code loading them assumes they're little endian)
   //
-  //   0x0001 0x0000 - version 1, "naive" format
-  //   0x0001 0x0001 - version 1, "flight data recorder" format
+  //   0x01 0x00 0x00 0x00 - version 1, "naive" format
+  //   0x01 0x00 0x01 0x00 - version 1, "flight data recorder" format
+  //   0x02 0x00 0x01 0x00 - version 2, "flight data recorder" format
   //
-  // YAML files dont' typically have those first four bytes as valid text so we
+  // YAML files don't typically have those first four bytes as valid text so we
   // try loading assuming YAML if we don't find these bytes.
   //
   // Only if we can't load either the binary or the YAML format will we yield an
@@ -534,23 +678,35 @@ Expected<Trace> llvm::xray::loadTraceFile(StringRef Filename, bool Sort) {
   enum BinaryFormatType { NAIVE_FORMAT = 0, FLIGHT_DATA_RECORDER_FORMAT = 1 };
 
   Trace T;
-  if (Version == 1 && Type == NAIVE_FORMAT) {
-    if (auto E =
-            loadNaiveFormatLog(StringRef(MappedFile.data(), MappedFile.size()),
-                               T.FileHeader, T.Records))
-      return std::move(E);
-  } else if (Version == 1 && Type == FLIGHT_DATA_RECORDER_FORMAT) {
-    if (auto E = loadFDRLog(StringRef(MappedFile.data(), MappedFile.size()),
-                            T.FileHeader, T.Records))
-      return std::move(E);
-  } else {
-    if (auto E = loadYAMLLog(StringRef(MappedFile.data(), MappedFile.size()),
-                             T.FileHeader, T.Records))
+  switch (Type) {
+  case NAIVE_FORMAT:
+    if (Version == 1 || Version == 2) {
+      if (auto E = loadNaiveFormatLog(Data, T.FileHeader, T.Records))
+        return std::move(E);
+    } else {
+      return make_error<StringError>(
+          Twine("Unsupported version for Basic/Naive Mode logging: ") +
+              Twine(Version),
+          std::make_error_code(std::errc::executable_format_error));
+    }
+    break;
+  case FLIGHT_DATA_RECORDER_FORMAT:
+    if (Version == 1 || Version == 2) {
+      if (auto E = loadFDRLog(Data, T.FileHeader, T.Records))
+        return std::move(E);
+    } else {
+      return make_error<StringError>(
+          Twine("Unsupported version for FDR Mode logging: ") + Twine(Version),
+          std::make_error_code(std::errc::executable_format_error));
+    }
+    break;
+  default:
+    if (auto E = loadYAMLLog(Data, T.FileHeader, T.Records))
       return std::move(E);
   }
 
   if (Sort)
-    std::sort(T.Records.begin(), T.Records.end(),
+    std::stable_sort(T.Records.begin(), T.Records.end(),
               [&](const XRayRecord &L, const XRayRecord &R) {
                 return L.TSC < R.TSC;
               });