Enable ability to have multiple PNaCl wire format versions.

Modifies PNaCl bitcode reader/writer to accept PNaClVersion=1 as supported,
and all other versions are unsupported and unreadable. The PNaCl bitcode
reader/writer will generate appropriate messages (including what version
is unsupported if applicable).

Also allows command-line option --pnacl-version for setting the PNaClVersion
in the PNaCl bitcode writer.

Also fixes some problems on PNaCl bitcode headers, using common support to
determine when the read/written PNaCl bitcode file is valid.

BUG=None
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/19400002

8 files changed, 76 insertions, 32 deletions

diff --git a/include/llvm/Bitcode/NaCl/NaClBitcodeHeader.h b/include/llvm/Bitcode/NaCl/NaClBitcodeHeader.h
index 8febf95564..4f5f83a17a 100644
--- a/include/llvm/Bitcode/NaCl/NaClBitcodeHeader.h
+++ b/include/llvm/Bitcode/NaCl/NaClBitcodeHeader.h
@@ -151,6 +151,12 @@ public:
   /// is readable and supported.
   void InstallFields();
 
+  /// \brief Adds a field to the list of fields in a header. Takes ownership
+  /// of fields added.
+  void push_back(NaClBitcodeHeaderField *Field) {
+    Fields.push_back(Field);
+  }
+
   /// \brief Read the PNaCl bitcode header, The format of the header is:
   ///
   ///    1) 'PEXE' - The four character sequence defining the magic number.
@@ -172,9 +178,9 @@ public:
   // \brief Returns the number of bytes read to consume the header.
   size_t getHeaderSize() { return HeaderSize; }
 
-  /// \brief Returns C string describing why the header describes
-  /// an unsupported PNaCl Bitcode file. Returns 0 if supported.
-  const std::string Unsupported() const { return UnsupportedMessage; }
+  /// \brief Returns string describing why the header describes
+  /// an unsupported PNaCl Bitcode file.
+  const std::string &Unsupported() const { return UnsupportedMessage; }
 
   /// \brief Returns true if supported. That is, it can be run in the
   /// browser.
diff --git a/include/llvm/Bitcode/NaCl/NaClReaderWriter.h b/include/llvm/Bitcode/NaCl/NaClReaderWriter.h
index 53feb8ab86..1eb188a83e 100644
--- a/include/llvm/Bitcode/NaCl/NaClReaderWriter.h
+++ b/include/llvm/Bitcode/NaCl/NaClReaderWriter.h
@@ -30,6 +30,15 @@ namespace llvm {
   /// this takes ownership of 'buffer' and returns a non-null pointer.  On
   /// error, this returns null, *does not* take ownership of Buffer, and fills
   /// in *ErrMsg with an error description if ErrMsg is non-null.
+  ///
+  /// The AcceptSupportedOnly argument is used to decide which PNaCl versions
+  /// of the PNaCl bitcode to accept. There are three forms:
+  ///    1) Readable and supported.
+  ///    2) Readable and unsupported. Allows testing of code before becoming
+  ///       supported, as well as running experiments on the bitcode format.
+  ///    3) Unreadable.
+  /// When AcceptSupportedOnly is true, only form 1 is allowed. When
+  /// AcceptSupportedOnly is false, forms 1 and 2 are allowed.
   Module *getNaClLazyBitcodeModule(MemoryBuffer *Buffer,
                                    LLVMContext &Context,
                                    std::string *ErrMsg = 0,
@@ -39,6 +48,9 @@ namespace llvm {
   /// and prepare for lazy deserialization and streaming of function bodies.
   /// On error, this returns null, and fills in *ErrMsg with an error
   /// description if ErrMsg is non-null.
+  ///
+  /// See getNaClLazyBitcodeModule for an explanation of argument
+  /// AcceptSupportedOnly.
   Module *getNaClStreamedBitcodeModule(const std::string &name,
                                        DataStreamer *streamer,
                                        LLVMContext &Context,
@@ -49,6 +61,9 @@ namespace llvm {
   /// returning the module.  If an error occurs, this returns null and
   /// fills in *ErrMsg if it is non-null.  This method *never* takes
   /// ownership of Buffer.
+  ///
+  /// See getNaClLazyBitcodeModule for an explanation of argument
+  /// AcceptSupportedOnly.
   Module *NaClParseBitcodeFile(MemoryBuffer *Buffer, LLVMContext &Context,
                                std::string *ErrMsg = 0,
                                bool AcceptSupportedOnly = true);
@@ -57,7 +72,16 @@ namespace llvm {
   /// specified raw output stream, using PNaCl wire format.  For
   /// streams where it matters, the given stream should be in "binary"
   /// mode.
-  void NaClWriteBitcodeToFile(const Module *M, raw_ostream &Out);
+  ///
+  /// The AcceptSupportedOnly argument is used to decide which PNaCl versions
+  /// of the PNaCl bitcode to generate. There are two forms:
+  ///    1) Writable and supported.
+  ///    2) Writable and unsupported. Allows testing of code before becoming
+  ///       supported, as well as running experiments on the bitcode format.
+  /// When AcceptSupportedOnly is true, only form 1 is allowed. When
+  /// AcceptSupportedOnly is false, forms 1 and 2 are allowed.
+  void NaClWriteBitcodeToFile(const Module *M, raw_ostream &Out,
+                              bool AcceptSupportedOnly = true);
 
   /// isNaClBitcode - Return true if the given bytes are the magic bytes for
   /// PNaCl bitcode wire format.
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp
index aa73b9cffa..2dd2af5dc3 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeHeader.cpp
@@ -249,8 +249,12 @@ void NaClBitcodeHeader::InstallFields() {
   if (PNaClVersion != 1) {
     IsSupportedFlag = false;
     IsReadableFlag = false;
-    UnsupportedMessage = "Unsupported Version";
     UpdatedUnsupportedMessage = true;
+    UnsupportedMessage.clear();
+    raw_string_ostream UnsupportedStream(UnsupportedMessage);
+    UnsupportedStream << "Unsupported PNaCl bitcode version: "
+                      << PNaClVersion << "\n";
+    UnsupportedStream.flush();
   }
   if (Fields.size() != 1) {
     IsSupportedFlag = false;
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
index 09a94c08cf..c7eb1ac683 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
@@ -1504,7 +1504,7 @@ bool NaClBitcodeReader::ParseBitcodeInto(Module *M) {
   M->setDataLayout("e-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-"
                    "f32:32:32-f64:64:64-p:32:32:32-v128:32:32");
 
-  if (InitStream()) return true;
+  if (InitStream()) return Error(Header.Unsupported());
 
   // We expect a number of well-defined blocks, though we don't necessarily
   // need to understand them all.
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
index 3b9d8982d5..fe7b5c1e9e 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
@@ -207,15 +207,11 @@ public:
   virtual bool MaterializeModule(Module *M, std::string *ErrInfo = 0);
   virtual void Dematerialize(GlobalValue *GV);
 
-  bool Error(const char *Str) {
-    ErrorString = Str;
-    return true;
-  }
   bool Error(const std::string &Str) {
     ErrorString = Str;
     return true;
   }
-  const char *getErrorString() const { return ErrorString.c_str(); }
+  const std::string &getErrorString() const { return ErrorString; }
 
   /// @brief Main interface to parsing a bitcode buffer.
   /// @returns true if an error occurred.
@@ -227,6 +223,9 @@ private:
     return !(Header.IsSupported() ||
              (!AcceptSupportedBitcodeOnly && Header.IsReadable()));
   }
+  uint32_t GetPNaClVersion() const {
+    return Header.GetPNaClVersion();
+  }
   Type *getTypeByID(unsigned ID);
   // Returns the value associated with ID. The value must already exist,
   // or a forward referenced value created by getOrCreateFnVaueByID.
diff --git a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
index 223d1bf416..e41ef6062c 100644
--- a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
+++ b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
@@ -35,6 +35,11 @@
 #include <map>
 using namespace llvm;
 
+static cl::opt<unsigned>
+PNaClVersion("pnacl-version",
+             cl::desc("Specify PNaCl bitcode version to write"),
+             cl::init(1));
+
 /// These are manifest constants used by the bitcode writer. They do
 /// not need to be kept in sync with the reader, but need to be
 /// consistent within this file.
@@ -1635,7 +1640,7 @@ static void WriteModule(const Module *M, NaClBitstreamWriter &Stream) {
   Stream.EmitRecord(naclbitc::MODULE_CODE_VERSION, Vals);
 
   // Analyze the module, enumerating globals, functions, etc.
-  NaClValueEnumerator VE(M);
+  NaClValueEnumerator VE(M, PNaClVersion);
 
   // Emit blockinfo, which defines the standard abbreviations etc.
   WriteBlockInfo(VE, Stream);
@@ -1666,9 +1671,9 @@ static void WriteModule(const Module *M, NaClBitstreamWriter &Stream) {
 // out to files (the parsing works for arbitrary sizes).
 static const size_t kMaxVariableFieldSize = 256;
 
-// Write out the given fields to the bitstream.
-static void WriteHeaderFields(
-    const std::vector<NaClBitcodeHeaderField*> &Fields,
+// Write out the given Header to the bitstream.
+static void WriteHeader(
+    const NaClBitcodeHeader &Header,
     NaClBitstreamWriter& Stream) {
   // Emit placeholder for number of bytes used to hold header fields.
   // This value is necessary so that the streamable reader can preallocate
@@ -1676,17 +1681,16 @@ static void WriteHeaderFields(
   Stream.Emit(0, naclbitc::BlockSizeWidth);
   unsigned BytesForHeader = 0;
 
-  unsigned NumberFields = Fields.size();
+  unsigned NumberFields = Header.NumberFields();
   if (NumberFields > 0xFFFF)
     report_fatal_error("Too many header fields");
 
   uint8_t Buffer[kMaxVariableFieldSize];
-  for (std::vector<NaClBitcodeHeaderField*>::const_iterator
-           Iter = Fields.begin(), IterEnd = Fields.end();
-       Iter != IterEnd; ++Iter) {
-    if (!(*Iter)->Write(Buffer, kMaxVariableFieldSize))
+  for (unsigned F = 0; F < NumberFields; ++F) {
+    NaClBitcodeHeaderField *Field = Header.GetField(F);
+    if (!Field->Write(Buffer, kMaxVariableFieldSize))
       report_fatal_error("Header field too big to generate");
-    size_t limit = (*Iter)->GetTotalSize();
+    size_t limit = Field->GetTotalSize();
     for (size_t i = 0; i < limit; i++) {
       Stream.Emit(Buffer[i], 8);
     }
@@ -1702,12 +1706,10 @@ static void WriteHeaderFields(
   Stream.BackpatchWord(NaClBitcodeHeader::WordSize, Value);
 }
 
-// Define the version of PNaCl bitcode we are generating.
-static const uint16_t kPNaClVersion = 1;
-
 /// WriteBitcodeToFile - Write the specified module to the specified output
 /// stream.
-void llvm::NaClWriteBitcodeToFile(const Module *M, raw_ostream &Out) {
+void llvm::NaClWriteBitcodeToFile(const Module *M, raw_ostream &Out,
+                                  bool AcceptSupportedOnly) {
   SmallVector<char, 0> Buffer;
   Buffer.reserve(256*1024);
 
@@ -1721,13 +1723,18 @@ void llvm::NaClWriteBitcodeToFile(const Module *M, raw_ostream &Out) {
     Stream.Emit((unsigned)'X', 8);
     Stream.Emit((unsigned)'E', 8);
 
-    // Collect header fields to add.
+    // Define header and install into stream.
     {
-      std::vector<NaClBitcodeHeaderField*> HeaderFields;
-      HeaderFields.push_back(
+      NaClBitcodeHeader Header;
+      Header.push_back(
           new NaClBitcodeHeaderField(NaClBitcodeHeaderField::kPNaClVersion,
-                                     kPNaClVersion));
-      WriteHeaderFields(HeaderFields, Stream);
+                                     PNaClVersion));
+      Header.InstallFields();
+      if (!(Header.IsSupported() ||
+            (!AcceptSupportedOnly && Header.IsReadable()))) {
+        report_fatal_error(Header.Unsupported());
+      }
+      WriteHeader(Header, Stream);
     }
 
     // Emit the module.
diff --git a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
index 5e2484a3f3..01b4ff8df5 100644
--- a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
+++ b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
@@ -32,7 +32,8 @@ static bool isIntOrIntVectorValue(const std::pair<const Value*, unsigned> &V) {
 }
 
 /// NaClValueEnumerator - Enumerate module-level information.
-NaClValueEnumerator::NaClValueEnumerator(const Module *M) {
+NaClValueEnumerator::NaClValueEnumerator(const Module *M, uint32_t PNaClVersion)
+    : PNaClVersion(PNaClVersion) {
   // Create map for counting frequency of types, and set field
   // TypeCountMap accordingly.  Note: Pointer field TypeCountMap is
   // used to deal with the fact that types are added through various
diff --git a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h
index e98a091ba8..21bbb2fb76 100644
--- a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h
+++ b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h
@@ -84,10 +84,13 @@ private:
   // The number of global variable IDs defined in the bitcode file.
   unsigned NumGlobalVarIDs;
 
+  // The version of PNaCl bitcode to generate.
+  uint32_t PNaClVersion;
+
   NaClValueEnumerator(const NaClValueEnumerator &) LLVM_DELETED_FUNCTION;
   void operator=(const NaClValueEnumerator &) LLVM_DELETED_FUNCTION;
 public:
-  NaClValueEnumerator(const Module *M);
+  NaClValueEnumerator(const Module *M, uint32_t PNaClVersion);
 
   void dump() const;
   void print(raw_ostream &OS, const ValueMapType &Map, const char *Name) const;