From 5d85fb4f407cd661fb904f68e2c9cc27ddcc0331 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 22 Jun 2015 11:31:04 -0400 Subject: [PATCH] Fix assertion failure on unmatched function or macro The fix in commit v3.2.3~3^2 (Fix assertion failure on unmatched foreach in function, 2015-05-18) broke handling of unmatched non-loop blocks because it assumed all function blockers removed during error unwinding were for loops, essentially switching the set of mishandled cases. The purpose of the loop block push/pop operations is to define a scope matching the lifetime of the loop function blockers. Since our function blockers already have the proper lifetime, simply move the push/pop operations to their constructor/destructor. Extend the RunCMake.Syntax test with a case covering this. --- Source/cmForEachCommand.cxx | 22 ++++++++++++------- Source/cmForEachCommand.h | 5 +++-- Source/cmMakefile.cxx | 1 - Source/cmMakefile.h | 9 -------- Source/cmWhileCommand.cxx | 17 +++++++++----- Source/cmWhileCommand.h | 5 +++-- .../Syntax/FunctionUnmatched-result.txt | 1 + .../Syntax/FunctionUnmatched-stderr.txt | 6 +++++ Tests/RunCMake/Syntax/FunctionUnmatched.cmake | 2 ++ .../RunCMake/Syntax/MacroUnmatched-result.txt | 1 + .../RunCMake/Syntax/MacroUnmatched-stderr.txt | 6 +++++ Tests/RunCMake/Syntax/MacroUnmatched.cmake | 2 ++ Tests/RunCMake/Syntax/RunCMakeTest.cmake | 2 ++ 13 files changed, 52 insertions(+), 27 deletions(-) create mode 100644 Tests/RunCMake/Syntax/FunctionUnmatched-result.txt create mode 100644 Tests/RunCMake/Syntax/FunctionUnmatched-stderr.txt create mode 100644 Tests/RunCMake/Syntax/FunctionUnmatched.cmake create mode 100644 Tests/RunCMake/Syntax/MacroUnmatched-result.txt create mode 100644 Tests/RunCMake/Syntax/MacroUnmatched-stderr.txt create mode 100644 Tests/RunCMake/Syntax/MacroUnmatched.cmake diff --git a/Source/cmForEachCommand.cxx b/Source/cmForEachCommand.cxx index 8e3510d63..33cb23492 100644 --- a/Source/cmForEachCommand.cxx +++ b/Source/cmForEachCommand.cxx @@ -13,6 +13,17 @@ #include +cmForEachFunctionBlocker::cmForEachFunctionBlocker(cmMakefile* mf): + Makefile(mf), Depth(0) +{ + this->Makefile->PushLoopBlock(); +} + +cmForEachFunctionBlocker::~cmForEachFunctionBlocker() +{ + this->Makefile->PopLoopBlock(); +} + bool cmForEachFunctionBlocker:: IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, cmExecutionStatus &inStatus) @@ -27,8 +38,6 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, // if this is the endofreach for this statement if (!this->Depth) { - cmMakefile::LoopBlockPop loopBlockPop(&mf); - // Remove the function blocker for this scope or bail. cmsys::auto_ptr fb(mf.RemoveFunctionBlocker(this, lff)); @@ -130,7 +139,7 @@ bool cmForEachCommand } // create a function blocker - cmForEachFunctionBlocker *f = new cmForEachFunctionBlocker(); + cmForEachFunctionBlocker *f = new cmForEachFunctionBlocker(this->Makefile); if ( args.size() > 1 ) { if ( args[1] == "RANGE" ) @@ -206,15 +215,14 @@ bool cmForEachCommand } this->Makefile->AddFunctionBlocker(f); - this->Makefile->PushLoopBlock(); - return true; } //---------------------------------------------------------------------------- bool cmForEachCommand::HandleInMode(std::vector const& args) { - cmsys::auto_ptr f(new cmForEachFunctionBlocker()); + cmsys::auto_ptr + f(new cmForEachFunctionBlocker(this->Makefile)); f->Args.push_back(args[0]); enum Doing { DoingNone, DoingLists, DoingItems }; @@ -252,7 +260,5 @@ bool cmForEachCommand::HandleInMode(std::vector const& args) this->Makefile->AddFunctionBlocker(f.release()); // TODO: pass auto_ptr - this->Makefile->PushLoopBlock(); - return true; } diff --git a/Source/cmForEachCommand.h b/Source/cmForEachCommand.h index 9b7c85a4d..36e88088f 100644 --- a/Source/cmForEachCommand.h +++ b/Source/cmForEachCommand.h @@ -19,8 +19,8 @@ class cmForEachFunctionBlocker : public cmFunctionBlocker { public: - cmForEachFunctionBlocker() {this->Depth = 0;} - virtual ~cmForEachFunctionBlocker() {} + cmForEachFunctionBlocker(cmMakefile* mf); + ~cmForEachFunctionBlocker(); virtual bool IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, cmExecutionStatus &); @@ -29,6 +29,7 @@ public: std::vector Args; std::vector Functions; private: + cmMakefile* Makefile; int Depth; }; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 61a175c78..ba914e151 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -3284,7 +3284,6 @@ void cmMakefile::PopFunctionBlockerBarrier(bool reportError) this->FunctionBlockerBarriers.back(); while(this->FunctionBlockers.size() > barrier) { - cmMakefile::LoopBlockPop loopBlockPop(this); cmsys::auto_ptr fb(this->FunctionBlockers.back()); this->FunctionBlockers.pop_back(); if(reportError) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index bff8c1288..53a0805a8 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -125,15 +125,6 @@ public: }; friend class LexicalPushPop; - class LoopBlockPop - { - public: - LoopBlockPop(cmMakefile* mf) { this->Makefile = mf; } - ~LoopBlockPop() { this->Makefile->PopLoopBlock(); } - private: - cmMakefile* Makefile; - }; - /** * Try running cmake and building a file. This is used for dynalically * loaded commands, not as part of the usual build process. diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx index 5170eadbe..012c580f0 100644 --- a/Source/cmWhileCommand.cxx +++ b/Source/cmWhileCommand.cxx @@ -12,6 +12,17 @@ #include "cmWhileCommand.h" #include "cmConditionEvaluator.h" +cmWhileFunctionBlocker::cmWhileFunctionBlocker(cmMakefile* mf): + Makefile(mf), Depth(0) +{ + this->Makefile->PushLoopBlock(); +} + +cmWhileFunctionBlocker::~cmWhileFunctionBlocker() +{ + this->Makefile->PopLoopBlock(); +} + bool cmWhileFunctionBlocker:: IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, cmExecutionStatus &inStatus) @@ -27,8 +38,6 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, // if this is the endwhile for this while loop then execute if (!this->Depth) { - cmMakefile::LoopBlockPop loopBlockPop(&mf); - // Remove the function blocker for this scope or bail. cmsys::auto_ptr fb(mf.RemoveFunctionBlocker(this, lff)); @@ -140,12 +149,10 @@ bool cmWhileCommand } // create a function blocker - cmWhileFunctionBlocker *f = new cmWhileFunctionBlocker(); + cmWhileFunctionBlocker *f = new cmWhileFunctionBlocker(this->Makefile); f->Args = args; this->Makefile->AddFunctionBlocker(f); - this->Makefile->PushLoopBlock(); - return true; } diff --git a/Source/cmWhileCommand.h b/Source/cmWhileCommand.h index 9fafffc09..85a0bd327 100644 --- a/Source/cmWhileCommand.h +++ b/Source/cmWhileCommand.h @@ -19,8 +19,8 @@ class cmWhileFunctionBlocker : public cmFunctionBlocker { public: - cmWhileFunctionBlocker() {this->Depth=0;} - virtual ~cmWhileFunctionBlocker() {} + cmWhileFunctionBlocker(cmMakefile* mf); + ~cmWhileFunctionBlocker(); virtual bool IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, cmExecutionStatus &); @@ -29,6 +29,7 @@ public: std::vector Args; std::vector Functions; private: + cmMakefile* Makefile; int Depth; }; diff --git a/Tests/RunCMake/Syntax/FunctionUnmatched-result.txt b/Tests/RunCMake/Syntax/FunctionUnmatched-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/Syntax/FunctionUnmatched-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/Syntax/FunctionUnmatched-stderr.txt b/Tests/RunCMake/Syntax/FunctionUnmatched-stderr.txt new file mode 100644 index 000000000..776a8f260 --- /dev/null +++ b/Tests/RunCMake/Syntax/FunctionUnmatched-stderr.txt @@ -0,0 +1,6 @@ +^CMake Error at CMakeLists.txt:[0-9]+ \(include\): + A logical block opening on the line + + .*/Tests/RunCMake/Syntax/FunctionUnmatched.cmake:[0-9]+ \(function\) + + is not closed.$ diff --git a/Tests/RunCMake/Syntax/FunctionUnmatched.cmake b/Tests/RunCMake/Syntax/FunctionUnmatched.cmake new file mode 100644 index 000000000..515b6bf63 --- /dev/null +++ b/Tests/RunCMake/Syntax/FunctionUnmatched.cmake @@ -0,0 +1,2 @@ +function(f) +#endfunction() # missing diff --git a/Tests/RunCMake/Syntax/MacroUnmatched-result.txt b/Tests/RunCMake/Syntax/MacroUnmatched-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/Syntax/MacroUnmatched-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/Syntax/MacroUnmatched-stderr.txt b/Tests/RunCMake/Syntax/MacroUnmatched-stderr.txt new file mode 100644 index 000000000..1699c436d --- /dev/null +++ b/Tests/RunCMake/Syntax/MacroUnmatched-stderr.txt @@ -0,0 +1,6 @@ +^CMake Error at CMakeLists.txt:[0-9]+ \(include\): + A logical block opening on the line + + .*/Tests/RunCMake/Syntax/MacroUnmatched.cmake:[0-9]+ \(macro\) + + is not closed.$ diff --git a/Tests/RunCMake/Syntax/MacroUnmatched.cmake b/Tests/RunCMake/Syntax/MacroUnmatched.cmake new file mode 100644 index 000000000..302d96ec3 --- /dev/null +++ b/Tests/RunCMake/Syntax/MacroUnmatched.cmake @@ -0,0 +1,2 @@ +macro(m) +#endmacro() # missing diff --git a/Tests/RunCMake/Syntax/RunCMakeTest.cmake b/Tests/RunCMake/Syntax/RunCMakeTest.cmake index c43128087..fd012b9c4 100644 --- a/Tests/RunCMake/Syntax/RunCMakeTest.cmake +++ b/Tests/RunCMake/Syntax/RunCMakeTest.cmake @@ -110,5 +110,7 @@ run_cmake(CMP0053-NameWithEscapedSpacesQuoted) run_cmake(CMP0053-NameWithEscapedTabsQuoted) # Function and macro tests. +run_cmake(FunctionUnmatched) run_cmake(FunctionUnmatchedForeach) +run_cmake(MacroUnmatched) run_cmake(MacroUnmatchedForeach)