PNaCl wire format: Disallow duplicate FORWARDTYPEREFs for the same value ID

Change the reader to be stricter so that duplicate FORWARDTYPEREFs are
rejected.

Also fix typo in error message.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3507
TEST=see "Invalid FORWARDTYPEREF record" if EmitFnForwardTypeRef()
  is changed to produce unnecessary FORWARDTYPEREFs

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

2 files changed, 16 insertions, 21 deletions

diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
index 593727a2f4..9782be1ec9 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
@@ -290,23 +290,21 @@ Value *NaClBitcodeReaderValueList::getValueFwdRef(unsigned Idx) {
   return 0;
 }
 
-Value *NaClBitcodeReaderValueList::getOrCreateValueFwdRef(
-    unsigned Idx, Type *Ty) {
+bool NaClBitcodeReaderValueList::createValueFwdRef(unsigned Idx, Type *Ty) {
   if (Idx >= size())
     resize(Idx + 1);
 
-  if (Value *V = ValuePtrs[Idx]) {
-    assert((Ty == 0 || Ty == V->getType()) && "Type mismatch in value table!");
-    return V;
-  }
+  // Return an error if this a duplicate definition of Idx.
+  if (ValuePtrs[Idx])
+    return true;
 
   // No type specified, must be invalid reference.
-  if (Ty == 0) return 0;
+  if (Ty == 0)
+    return true;
 
-  // Create and return a placeholder, which will later be RAUW'd.
-  Value *V = new Argument(Ty);
-  ValuePtrs[Idx] = V;
-  return V;
+  // Create a placeholder, which will later be RAUW'd.
+  ValuePtrs[Idx] = new Argument(Ty);
+  return false;
 }
 
 /// ResolveConstantForwardRefs - Once all constants are read, this method bulk
@@ -2235,9 +2233,9 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
     }
     case naclbitc::FUNC_CODE_INST_FORWARDTYPEREF:
       // Build corresponding forward reference.
-      if (Record.size() != 2)
-        return Error("Invald FORWARDTYPEREF record");
-      getOrCreateFnValueByID(Record[0], getTypeByID(Record[1]));
+      if (Record.size() != 2 ||
+          ValueList.createValueFwdRef(Record[0], getTypeByID(Record[1])))
+        return Error("Invalid FORWARDTYPEREF record");
       continue;
     }
 
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
index 5256e07134..cacd971286 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
@@ -77,8 +77,10 @@ public:
     ValuePtrs.resize(N);
   }
 
-  // Gets or creates the forward reference value for Idx with the given type.
-  Value *getOrCreateValueFwdRef(unsigned Idx, Type *Ty);
+  // Declares the type of the forward-referenced value Idx.  Returns
+  // true if an error occurred.  It is an error if Idx's type has
+  // already been declared.
+  bool createValueFwdRef(unsigned Idx, Type *Ty);
 
   Constant *getConstantFwdRef(unsigned Idx, Type *Ty);
 
@@ -207,11 +209,6 @@ private:
              (!AcceptSupportedBitcodeOnly && Header.IsReadable()));
   }
   Type *getTypeByID(unsigned ID);
-  // Gets or creates the (function-level) forward referenced value for
-  // ID with the given type.
-  Value *getOrCreateFnValueByID(unsigned ID, Type *Ty) {
-    return ValueList.getOrCreateValueFwdRef(ID, Ty);
-  }
   // Returns the value associated with ID. The value must already exist,
   // or a forward referenced value created by getOrCreateFnVaueByID.
   Value *getFnValueByID(unsigned ID) {