Fix handling errors in switch instruction by PNaCl bitcode reader.

lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp

 //===- NaClBitcodeReader.cpp ----------------------------------------------===//
 //     Internal NaClBitcodeReader implementation
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
@@ skipping 1492 matching lines @@
       for (size_t i = 0; i != NumCases; ++i) {
         // The PNaCl bitcode format has vestigial support for case
         // ranges, but we no longer support reading them because
         // no-one produced them.
         // See https://code.google.com/p/nativeclient/issues/detail?id=3758
         if (CurIdx + 3 >= Record.size())
           return Error(InvalidRecord,
                        "Incomplete case entry in SWITCH record");
         uint64_t NumItems = Record[CurIdx++];
         bool isSingleNumber = Record[CurIdx++];
-        if (NumItems != 1 || !isSingleNumber)
+        if (NumItems != 1 || !isSingleNumber) {
+          delete SI;

[JF, 2015/08/18 17:07:46]

Can this file as well as upstream lib/Bitcode/Reader/BitcodeReader.cpp use a
unique_ptr instead? All temporaries would have a well-bounded lifetime, and
would transfer ownership to I (also changed to unique_ptr) cleanly.

Or are switches special compared to other ops?

[Karl, 2015/08/19 19:39:00]

Switches are not special. The code was written this way because the upstream was
written this way. What makes this case different that most is that you can't
create a complete switch instruction with a single constructor. Rather, you
create a switch instruction with just the default branch, and then add each of
the cases by calling method addCase().

Hence, the switch is created up front, before the cases are processed. This
results in having to delete the switch if there is an error in the case.

I agree that a unique_ptr would be better in this case, and that the upstream
code should be fixed to use it as well. Fixing this CL to use unique_ptr in this
case.

           return Error(InvalidRecord,
                        "Case ranges are not supported in PNaCl bitcode");
+        }
 
         APInt CaseValue(ValueBitWidth,
                         NaClDecodeSignRotatedValue(Record[CurIdx++]));
         BasicBlock *DestBB = getBasicBlock(Record[CurIdx++]);
+        if (DestBB == nullptr) {
+          delete SI;
+          return Error(InvalidValue, "Invalid branch in SWITCH case");
+        }
         SI->addCase(ConstantInt::get(Context, CaseValue), DestBB);
       }
       I = SI;
       break;
     }
     case naclbitc::FUNC_CODE_INST_UNREACHABLE: // UNREACHABLE
       I = new UnreachableInst(Context);
       break;
     case naclbitc::FUNC_CODE_INST_PHI: { // PHI: [ty, val0,bb0, ...]
       if (Record.size() < 1 || ((Record.size()-1)&1))
@@ skipping 462 matching lines @@
   if (std::error_code EC = M->materializeAllPermanently()) {
     delete M;
     return EC;
   }
 
   // TODO: Restore the use-lists to the in-memory state when the bitcode was
   // written.  We must defer until the Module has been fully materialized.
 
   return M;
 }