Fix locking for printing error messages.

src/IceClFlags.cpp

@@ -245,6 +245,43 @@ void ClFlags::parseFlags(int argc, char **argv) {
   AppName = IceString(argv[0]);
 }
 
+void ClFlags::resetClFlags(ClFlags &OutFlags) {
+  // bool fields
+  OutFlags.AllowErrorRecovery = false;
+  OutFlags.AllowUninitializedGlobals = false;
+  OutFlags.DataSections = false;
+  OutFlags.DecorateAsm = false;
+  OutFlags.DisableInternal = false;
+  OutFlags.DisableIRGeneration = false;
+  OutFlags.DisableTranslation = false;
+  OutFlags.DumpStats = false;
+  OutFlags.FunctionSections = false;
+  OutFlags.GenerateUnitTestMessages = false;
+  OutFlags.PhiEdgeSplit = false;
+  OutFlags.RandomNopInsertion = false;
+  OutFlags.RandomRegAlloc = false;
+  OutFlags.SubzeroTimingEnabled = false;
+  OutFlags.TimeEachFunction = false;
+  OutFlags.UseSandboxing = false;
+  // Enum and integer fields.
+  OutFlags.Opt = Opt_m1;
+  OutFlags.OutFileType = FT_Iasm;
+  OutFlags.RandomMaxNopsPerInstruction = 0;
+  OutFlags.RandomNopProbabilityAsPercentage = 0;
+  OutFlags.TArch = TargetArch_NUM;
+  OutFlags.VMask = IceV_None;
+  // IceString fields.
+  OutFlags.DefaultFunctionPrefix = "";
+  OutFlags.DefaultGlobalPrefix = "";
+  OutFlags.TestPrefix = "";
+  OutFlags.TimingFocusOn = "";
+  OutFlags.TranslateOnly = "";
+  OutFlags.VerboseFocusOn = "";
+  // size_t and 64-bit fields.
+  OutFlags.NumTranslationThreads = 0;
+  OutFlags.RandomSeed = 0;
+}
+
 void ClFlags::getParsedClFlags(ClFlags &OutFlags) {
   if (::DisableIRGeneration)
     ::DisableTranslation = true;

src/IceClFlags.h

@@ -26,27 +26,10 @@ class ClFlags {
   ClFlags &operator=(const ClFlags &) = delete;
 
 public:
-  ClFlags()
-      : // bool fields.
-        AllowErrorRecovery(false),
-        AllowUninitializedGlobals(false), DataSections(false),
-        DecorateAsm(false), DisableInternal(false), DisableIRGeneration(false),
-        DisableTranslation(false), DumpStats(false), FunctionSections(false),
-        GenerateUnitTestMessages(false), PhiEdgeSplit(false),
-        RandomNopInsertion(false), RandomRegAlloc(false),
-        SubzeroTimingEnabled(false), TimeEachFunction(false),
-        UseSandboxing(false),
-        // Enum and integer fields.
-        Opt(Opt_m1), OutFileType(FT_Iasm), RandomMaxNopsPerInstruction(0),
-        RandomNopProbabilityAsPercentage(0), TArch(TargetArch_NUM),
-        VMask(IceV_None),
-        // IceString fields.
-        DefaultFunctionPrefix(""), DefaultGlobalPrefix(""), TestPrefix(""),
-        TimingFocusOn(""), TranslateOnly(""), VerboseFocusOn(""),
-        // size_t and 64-bit fields.
-        NumTranslationThreads(0), RandomSeed(0) {}
+  ClFlags() { resetClFlags(*this); }
 
   static void parseFlags(int argc, char *argv[]);
+  static void resetClFlags(ClFlags &OutFlags);
   static void getParsedClFlags(ClFlags &OutFlags);
   static void getParsedClFlagsExtra(ClFlagsExtra &OutFlagsExtra);
 

src/PNaClTranslator.cpp

@@ -486,10 +486,12 @@ bool TopLevelParser::ErrorAt(naclbitc::ErrorLevel Level, uint64_t Bit,
   ErrorStatus.assign(Ice::EC_Bitcode);
   ++NumErrors;
   Ice::GlobalContext *Context = Translator.getContext();
-  Ice::OstreamLocker L(Context);
-  raw_ostream &OldErrStream = setErrStream(Context->getStrDump());
-  NaClBitcodeParser::ErrorAt(Level, Bit, Message);
-  setErrStream(OldErrStream);
+  { // Lock while printing out error message.
+    Ice::OstreamLocker L(Context);
+    raw_ostream &OldErrStream = setErrStream(Context->getStrDump());
+    NaClBitcodeParser::ErrorAt(Level, Bit, Message);
+    setErrStream(OldErrStream);
+  }
   if (Level >= naclbitc::Error &&
       !Translator.getFlags().getAllowErrorRecovery())
     Fatal();

unittest/BitcodeMunge.cpp

@@ -19,19 +19,25 @@
 
 namespace IceTest {
 
-bool IceTest::SubzeroBitcodeMunger::runTest(const char *TestName,
-                                            const uint64_t Munges[],
-                                            size_t MungeSize) {
-  const bool AddHeader = true;
-  setupTest(TestName, Munges, MungeSize, AddHeader);
+void IceTest::SubzeroBitcodeMunger::resetFlags() {
+  Ice::ClFlags::resetClFlags(Flags);
+  resetMungeFlags();
+}
 
-  Ice::ClFlags Flags;
+void IceTest::SubzeroBitcodeMunger::resetMungeFlags() {
   Flags.setAllowErrorRecovery(true);
   Flags.setGenerateUnitTestMessages(true);
   Flags.setOptLevel(Ice::Opt_m1);
   Flags.setOutFileType(Ice::FT_Iasm);
   Flags.setTargetArch(Ice::Target_X8632);
   Flags.setVerbose(Ice::IceV_Instructions);
+}
+
+bool IceTest::SubzeroBitcodeMunger::runTest(const char *TestName,
+                                            const uint64_t Munges[],
+                                            size_t MungeSize) {
+  const bool AddHeader = true;
+  setupTest(TestName, Munges, MungeSize, AddHeader);
   Ice::GlobalContext Ctx(DumpStream, DumpStream, nullptr, Flags);
   Ice::PNaClTranslator Translator(&Ctx);
   Translator.translateBuffer(TestName, MungedInput.get());

unittest/BitcodeMunge.h

@@ -17,6 +17,8 @@
 
 #include "llvm/Bitcode/NaCl/NaClBitcodeMunge.h"
 
+#include "IceClFlags.h"
+
 namespace IceTest {
 
 // Class to run tests on Subzero's bitcode parser. Runs a Subzero
@@ -27,7 +29,9 @@ class SubzeroBitcodeMunger : public llvm::NaClBitcodeMunger {
 public:
   SubzeroBitcodeMunger(const uint64_t Records[], size_t RecordSize,
                        uint64_t RecordTerminator)
-      : llvm::NaClBitcodeMunger(Records, RecordSize, RecordTerminator) {}
+      : llvm::NaClBitcodeMunger(Records, RecordSize, RecordTerminator) {
+    resetMungeFlags();
+  }
 
   /// Runs PNaClTranslator to translate bitcode records (with defined
   /// record Munges), and puts output into DumpResults. Returns true
@@ -39,6 +43,15 @@ public:
     uint64_t NoMunges[] = {0};
     return runTest(TestName, NoMunges, 0);
   }
+
+  /// Sets flags back to default assumptions for munging.
+  void resetFlags();
+
+  /// Flags to use to run tests. Use to change default assumptions.
+  Ice::ClFlags Flags;
+
+private:
+  void resetMungeFlags();
 };
 
 } // end of namespace IceTest

unittest/IceParseInstsTest.cpp

@@ -63,6 +63,12 @@ TEST(IceParseInstsTest, NonexistentCallArg) {
   EXPECT_FALSE(Munger.runTest("Nonexistent call arg"));
   EXPECT_EQ("Error(66:4): Invalid function record: <34 0 4 2 100>\n",
             Munger.getTestResults());
+
+  // Show that we generate a fatal error when not allowing error recovery.
+  Munger.Flags.setAllowErrorRecovery(false);
+  EXPECT_DEATH(
+      Munger.runTest("Nonexistent call arg"),
+      ".*ERROR: Unable to continue.*");
 }
 
 /// Test how we recognize alignments in alloca instructions.