Fix locking for printing error messages. Same as CL https://codereview.chromium.org/1071423003 (which has LGTM). BUG= https://code.google.com/p/nativeclient/issues/detail?id=4138 Review URL: https://codereview.chromium.org/1097563003
diff --git a/src/IceClFlags.cpp b/src/IceClFlags.cpp index 9e24665..fda4e06 100644 --- a/src/IceClFlags.cpp +++ b/src/IceClFlags.cpp
@@ -245,6 +245,43 @@ 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;
diff --git a/src/IceClFlags.h b/src/IceClFlags.h index 0762002..c89e695 100644 --- a/src/IceClFlags.h +++ b/src/IceClFlags.h
@@ -26,27 +26,10 @@ 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);
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp index 35cdcf7..ce46239 100644 --- a/src/PNaClTranslator.cpp +++ b/src/PNaClTranslator.cpp
@@ -486,10 +486,12 @@ 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();
diff --git a/unittest/BitcodeMunge.cpp b/unittest/BitcodeMunge.cpp index fd7100c..689279e 100644 --- a/unittest/BitcodeMunge.cpp +++ b/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());
diff --git a/unittest/BitcodeMunge.h b/unittest/BitcodeMunge.h index 6b639a7..0b99864 100644 --- a/unittest/BitcodeMunge.h +++ b/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 @@ 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 @@ 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
diff --git a/unittest/IceParseInstsTest.cpp b/unittest/IceParseInstsTest.cpp index 5d45888..191eff9 100644 --- a/unittest/IceParseInstsTest.cpp +++ b/unittest/IceParseInstsTest.cpp
@@ -63,6 +63,12 @@ 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.