Skip to content

Commit b2d55a6

Browse files
committed
Fixed params optimization leaking default value array reference into script
1 parent d3d649b commit b2d55a6

2 files changed

Lines changed: 57 additions & 0 deletions

File tree

src/optimizer/optimizerModuleConstantFold.cpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,9 +228,57 @@ class OptimizerConstantFoldActionMap : public Singleton<OptimizerConstantFoldAct
228228

229229
node.children[0].value = ScriptConstantArray(); //dummy. Children are the contents
230230
node.children[0].type = InstructionType::push;
231+
232+
233+
// check if params array contains a value that can be modified by reference, to prevent it modifying the value in compiled code
234+
235+
const auto& paramsList = node.children[0].children;
236+
237+
for (auto& it : paramsList)
238+
{
239+
//#TODO throw warning on empty array passed to params?
240+
if (!it.children.empty() && std::holds_alternative<ScriptConstantArray>(it.value)) // is array, and is not empty
241+
{
242+
// it is a [name, default, allowedTypes] array
243+
auto& paramArguments = it.children;
244+
245+
if (paramArguments.size() < 2) // only [name], no defaults, don't care
246+
continue;
247+
248+
if (std::holds_alternative<ScriptConstantArray>(paramArguments[1].value))
249+
{
250+
// !!! [name, []] the array will be passed down by ref if parameter is not provided, and cause changes to propagate/persist to compiled code if modified by reference
251+
// We know that this will be a problem, lets insert a array copy
252+
253+
// replace ourselves with a + unary
254+
255+
// move our array out so we can move it over instead of copying
256+
auto myArgumentsArray = std::move(node.children[0]);
257+
// our only child is now default initialized, we have no children
258+
node.children.clear();
259+
260+
// convert ourselves to a unary + and add the array as argument
261+
OptimizerModuleBase::Node copyNode;
262+
copyNode.type = InstructionType::callUnary;
263+
copyNode.constant = false; // Lets prevent optimizing this copy out, shouldn't be done anyway but better be safe
264+
copyNode.value = STRINGTYPE("+"); // value is name of command
265+
copyNode.file = node.file;
266+
copyNode.line = myArgumentsArray.line;
267+
copyNode.offset = myArgumentsArray.offset;
268+
copyNode.children.emplace_back(std::move(myArgumentsArray)); // Add the arguments array back
269+
270+
node.children.emplace_back(std::move(copyNode));
271+
272+
// now changed params [...] to params +[...]
273+
break; // paramsList has been invalidated, cannot keep iterating
274+
}
275+
}
276+
}
277+
231278
}
232279
};
233280

281+
//#TODO optimize param too, if non-array constant default value, see params array checking
234282

235283

236284
}

src/scriptSerializer.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,15 @@ void blaBla(const CompiledCodeData& code, const std::vector<ScriptInstruction>&
8989

9090

9191
void ScriptSerializer::compiledToHumanReadable(const CompiledCodeData& code, std::ostream& output) {
92+
93+
//#TODO fix this
94+
//Check output of
95+
// params ["_unit", "_pos", ["_target", objNull], ["_buildings", []]];
96+
// + [1, 2, 3];
97+
// _buildings pushBack[1, 2, 3];
98+
99+
// array fuckup, no quotes on string constants inside the array
100+
92101
blaBla(code, std::get<0>(code.constants[code.codeIndex]).code, output);
93102
}
94103

0 commit comments

Comments
 (0)