Add (unsupported experimental) feature allowing byte aligned bitcode.

lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp

Index: lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp
index 6162c24cc93b9918ef1fa41e1156fea9dbd3f01d..8dc17e8279ba03b49b325dc64bd65e9bead0de45 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp
@@ -16,17 +16,58 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/StreamingMemoryObject.h"
 
-#include <limits>
 #include <cstring>
 #include <iomanip>
+#include <limits>
+#include <set>
 
 using namespace llvm;
 
+namespace {
+
+// The name for each ID tag.
+static const char *TagName[] = {
+  "Invalid",              // kInvalid
+  "PNaCl Version",        // kPNaClVersion
+  "Align bitcode records" // kAlignBitcodeRecords
+};
+
+// The name for each field type.
+static const char *FieldTypeName[] = {
+  "uint8[]", // kBufferType
+  "uint32",  // kUInt32Type
+  "flag",    // kFlagType
+  "unknown"  // kUnknownType
+};
+
+// The type associated with each ID tag.
+static const NaClBitcodeHeaderField::FieldType ExpectedType[] = {
+  NaClBitcodeHeaderField::kUnknownType, // kInvalid
+  NaClBitcodeHeaderField::kUInt32Type,  // kPNaClVersion
+  NaClBitcodeHeaderField::kFlagType     // kAlignBitcodeRecords
+};
+
+} // end of anonymous namespace
+
+const char *NaClBitcodeHeaderField::IDName(Tag ID) {
+  return ID > kTag_MAX ? "???" : TagName[ID];
+}
+
+const char *NaClBitcodeHeaderField::TypeName(FieldType FType) {
+  return FType > kFieldType_MAX ? "???" : FieldTypeName[FType];
+}
+
 NaClBitcodeHeaderField::NaClBitcodeHeaderField()
-    : ID(kInvalid), FType(kBufferType), Len(0), Data(0) {}
+  : ID(kInvalid), FType(kBufferType), Len(0), Data(0) {}
+
+NaClBitcodeHeaderField::NaClBitcodeHeaderField(Tag MyID)
+  : ID(MyID), FType(kFlagType), Len(0), Data(0) {
+  assert(MyID <= kTag_MAX);
+}
 
 NaClBitcodeHeaderField::NaClBitcodeHeaderField(Tag MyID, uint32_t MyValue)
     : ID(MyID), FType(kUInt32Type), Len(4), Data(new uint8_t[4]) {
+  assert(MyID <= kTag_MAX);
   Data[0] = static_cast<uint8_t>(MyValue & 0xFF);
   Data[1] = static_cast<uint8_t>((MyValue >> 8) & 0xFF);
   Data[2] = static_cast<uint8_t>((MyValue >> 16) & 0xFF);
@@ -44,6 +85,7 @@ uint32_t NaClBitcodeHeaderField::GetUInt32Value() const {
 NaClBitcodeHeaderField::NaClBitcodeHeaderField(Tag MyID, size_t MyLen,
                                                uint8_t *MyData)
     : ID(MyID), FType(kBufferType), Len(MyLen), Data(new uint8_t[MyLen]) {
+  assert(MyID <= kTag_MAX);
   for (size_t i = 0; i < MyLen; ++i) {
     Data[i] = MyData[i];
   }
@@ -94,16 +136,11 @@ bool NaClBitcodeHeaderField::Read(const uint8_t *Buf, size_t BufLen) {
 std::string NaClBitcodeHeaderField::Contents() const {
   std::string buffer;
   raw_string_ostream ss(buffer);
-  switch (ID) {
-  case kPNaClVersion:
-    ss << "PNaCl Version";
-    break;
-  case kInvalid:
-    ss << "Invalid";
-    break;
-  }
-  ss << ": ";
+  ss << IDName() << ": ";
   switch (FType) {
+  case kFlagType:
+    ss << "true";
+    break;
   case kUInt32Type:
     ss << GetUInt32Value();
     break;
@@ -116,6 +153,9 @@ std::string NaClBitcodeHeaderField::Contents() const {
     }
     ss << "]";
     break;
+  case kUnknownType:
+    ss << "unknown value";
+    break;
   }
   return ss.str();
 }
@@ -244,29 +284,68 @@ NaClBitcodeHeaderField *GetPNaClVersionPtr(NaClBitcodeHeader *Header) {
 }
 
 void NaClBitcodeHeader::InstallFields() {
-  // Assume supported until contradicted.
-  bool UpdatedUnsupportedMessage = false;
   IsSupportedFlag = true;
   IsReadableFlag = true;
-  UnsupportedMessage = "Supported";
+  AlignBitcodeRecords = false;
   PNaClVersion = 0;
-  if (NaClBitcodeHeaderField *Version = GetPNaClVersionPtr(this)) {
-    PNaClVersion = Version->GetUInt32Value();
-  }
-  if (PNaClVersion != 2) {
-    IsSupportedFlag = false;
-    IsReadableFlag = false;
-    UpdatedUnsupportedMessage = true;
-    UnsupportedMessage.clear();
-    raw_string_ostream UnsupportedStream(UnsupportedMessage);
-    UnsupportedStream << "Unsupported PNaCl bitcode version: "
-                      << PNaClVersion << "\n";
-    UnsupportedStream.flush();
-  }
-  if (Fields.size() != 1) {
+  UnsupportedMessage.clear();
+  std::set<unsigned> FieldIDs;
+
+  auto ReportProblem = [&](bool IsReadable=false) {
+    UnsupportedMessage.append("\n");
     IsSupportedFlag = false;
-    IsReadableFlag = false;
-    if (!UpdatedUnsupportedMessage)
-      UnsupportedMessage = "Unknown header field(s) found";
+    IsReadableFlag = IsReadable;

[jvoung (off chromium), 2015/05/07 18:11:17]

Can a second call to ReportProblem cause you to overwrite a previous setting of
IsReadableFlag (from false -> true again)? The loop sometimes will "continue;"
after a ReportProblem... call, so it seems possible (but maybe not)?

[Karl, 2015/05/07 22:18:54]

The intent is continue with the next field, and not report any more problems
with the current field.

The current code does this directly or indirectly. However, we should make it
clear that this is what we want. Changing all calls to ReportProblem... be
immediately followed by a continue.

[jvoung (off chromium), 2015/05/08 17:55:01]

I mean that since you are processing all the fields even after hitting an
problem, what if there was an early unreadable problem, followed by
"ReportProblemWithContents(Field, true)" at the end? Then IsReadable flips from
false to true, even though it should stay false (I think). E.g., header has a
bad bitcode version followed by the align records flag.

So the update would be safer if it was like "IsReadableFlag = IsReadableFlag &&
IsReadable"?

[Karl, 2015/05/08 21:09:00]

Good point. Fixing.

+  };
+
+  auto ReportProblemWithContents = [&](NaClBitcodeHeaderField *Field,
+				       bool IsReadable=false) {

[jvoung (off chromium), 2015/05/07 18:11:17]

spaces instead of tabs

[Karl, 2015/05/07 22:18:54]

Done.

+    UnsupportedMessage.append(": ");
+    UnsupportedMessage.append(Field->Contents());
+    ReportProblem(IsReadable);
+  };
+
+  for (size_t i = 0, e = NumberFields(); i < e; ++i) {
+    // Start by checking expected properties for any field
+    NaClBitcodeHeaderField *Field = GetField(i);
+    if (!FieldIDs.insert(Field->GetID()).second) {
+      UnsupportedMessage.append("Specified multiple times: ");
+      UnsupportedMessage.append(Field->IDName());
+      ReportProblem();
+      continue;
+    }
+    NaClBitcodeHeaderField::FieldType ExpectedTy = ExpectedType[Field->GetID()];
+    if (Field->GetType() != ExpectedTy) {
+      UnsupportedMessage.append("Expects type ");
+      UnsupportedMessage.append(NaClBitcodeHeaderField::TypeName(ExpectedTy));
+      ReportProblemWithContents(Field);
+      continue;
+    }
+    if (Field->GetType() == NaClBitcodeHeaderField::kUnknownType) {
+      UnsupportedMessage.append("Unknown value");
+      ReportProblemWithContents(Field);
+      continue;
+    }
+
+    // Check specific ID values and install.
+    switch (Field->GetID()) {
+    case NaClBitcodeHeaderField::kInvalid:
+      UnsupportedMessage.append("Unsupported");
+      ReportProblemWithContents(Field);
+      break;
+    case NaClBitcodeHeaderField::kPNaClVersion:
+      PNaClVersion = Field->GetUInt32Value();
+      if (PNaClVersion != 2) {
+	UnsupportedMessage.append("Unsupported");

[jvoung (off chromium), 2015/05/07 18:11:17]

got some tabs

[Karl, 2015/05/07 22:18:55]

Done.

+	ReportProblemWithContents(Field);
+      }
+      break;
+    case NaClBitcodeHeaderField::kAlignBitcodeRecords:
+      AlignBitcodeRecords = true;
+      UnsupportedMessage.append("Unsupported");
+      ReportProblemWithContents(Field, true);
+      break;
+    }
   }
+  if (UnsupportedMessage.empty())
+    UnsupportedMessage = "Supported";
 }