From d523c326c523f672aa4c466eb35a06c64e7f7b5b Mon Sep 17 00:00:00 2001
From: Bastien Bouclet <bastien.bouclet@gmail.com>
Date: Wed, 1 Jun 2016 18:03:25 +0200
Subject: [PATCH] STARK: Dont try to visit the same block multiple times when
 decompiling

Unless the block is trivial. The original compiler seems to reuse "end" commands.
---
 engines/stark/tools/block.cpp      |  8 ++++++++
 engines/stark/tools/block.h        |  3 +++
 engines/stark/tools/decompiler.cpp | 14 +++++++++++++-
 engines/stark/tools/decompiler.h   |  1 +
 4 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/engines/stark/tools/block.cpp b/engines/stark/tools/block.cpp
index d63b9790b8a..37b2b30f9eb 100644
--- a/engines/stark/tools/block.cpp
+++ b/engines/stark/tools/block.cpp
@@ -307,6 +307,14 @@ CFGCommand *Block::getConditionCommand() const {
 	}
 }
 
+bool Block::allowDuplication() const {
+	// Allow simple termination blocks to be duplicated in the decompiled output
+	bool isScriptEnd = !_follower && !_trueBranch && !_falseBranch;
+	bool isShort = _commands.size() < 5;
+
+	return isScriptEnd && isShort;
+}
+
 ControlStructure::ControlStructure(ControlStructureType t) :
 		type(t),
 		condition(nullptr),
diff --git a/engines/stark/tools/block.h b/engines/stark/tools/block.h
index 17e0869b121..8f37eb443ee 100644
--- a/engines/stark/tools/block.h
+++ b/engines/stark/tools/block.h
@@ -87,6 +87,9 @@ public:
 	bool isInfiniteLoopStart() const;
 	void setInfiniteLoopStart(bool infiniteLoopStart);
 
+	/** Can this block appear multiple times in the decompiled output? */
+	bool allowDuplication() const;
+
 	// Graph query methods
 	bool hasPredecessor(Block *predecessor) const;
 	bool hasSuccessor(Block *successor) const;
diff --git a/engines/stark/tools/decompiler.cpp b/engines/stark/tools/decompiler.cpp
index 051077ef975..1e5cb676614 100644
--- a/engines/stark/tools/decompiler.cpp
+++ b/engines/stark/tools/decompiler.cpp
@@ -290,6 +290,18 @@ void Decompiler::buildASTFromBlock(ASTBlock *parent, Block *block, Block *stopBl
 		stopBlock = block;
 	}
 
+	{
+		bool alreadyVisited = Common::find(_visitedBlocks.begin(), _visitedBlocks.end(), block) != _visitedBlocks.end();
+		if (alreadyVisited && !block->allowDuplication()) {
+			// FIXME: We just return for now when an already visited block is visited again.
+			// Obviously, this leads to invalid decompiled code, which is caught by the verification step.
+			// To fix, either handle the cases leading to multiple visits, or generate gotos.
+			return;
+		}
+	}
+
+	_visitedBlocks.push_back(block);
+
 	Common::Array<CFGCommand *> commands = block->getLinearCommands();
 	for (uint i = 0; i < commands.size(); i++) {
 		parent->addNode(new ASTCommand(parent, commands[i]));
@@ -376,7 +388,7 @@ bool Decompiler::verifyCommandInAST(CFGCommand *cfgCommand) {
 		return false;
 	}
 
-	if (list.size() > 1) {
+	if (list.size() > 1 && !cfgCommand->getBlock()->allowDuplication()) {
 		_error = Common::String::format("Command %d found %d times in the AST", cfgCommand->getIndex(), list.size());
 		return false;
 	}
diff --git a/engines/stark/tools/decompiler.h b/engines/stark/tools/decompiler.h
index 7bde855b32f..0a5402dee9b 100644
--- a/engines/stark/tools/decompiler.h
+++ b/engines/stark/tools/decompiler.h
@@ -92,6 +92,7 @@ private:
 
 	ASTNode *_astHead;
 	Common::Array<Block *> _visitedInfiniteLoopStarts;
+	Common::Array<Block *> _visitedBlocks;
 };
 
 } // End of namespace Tools