Changeset 930

Timestamp:

05/27/20 12:14:12 (5 years ago)

Author:

Maciej Komosinski

Message:

Replaced std::sort() with qsort() in Vector.sort(). std::sort() requires "strict weak ordering" and may (and does) crash when given a non-compliant comparator, and we can't control what kind of user script comparator function will be passed to Vector.sort().

File:

• cpp/frams/vm/classes/collectionobj.cpp (modified) (3 diffs)

cpp/frams/vm/classes/collectionobj.cpp

@@ -1 +1 @@
 // This file is a part of Framsticks SDK.  http://www.framsticks.com/
-// Copyright (C) 1999-2019  Maciej Komosinski and Szymon Ulatowski.
+// Copyright (C) 1999-2020  Maciej Komosinski and Szymon Ulatowski.
 // See LICENSE.txt for details.
 
@@ -270 +270 @@
         VMachine *vm;
         VMVEComparator(VMachine::JumpTargetObject *_jto) :jto(_jto), vm(jto->vm) {}
-        bool operator()(const ExtValue *a, const ExtValue *b);
+        static int compare(const void *a, const void *b, void* _this);
 };
 
-bool VMVEComparator::operator()(const ExtValue *a, const ExtValue *b)
-{
+int VMVEComparator::compare(const void *a, const void *b, void* _this)
+{
+        VMachine *vm = ((VMVEComparator*)_this)->vm;
+        VMachine::JumpTargetObject *jto = ((VMVEComparator*)_this)->jto;
         if (!VMCode::prepareDynamicJumpTarget(jto->pc, jto->code))
                 return false;
 
-        vm->push(*a);
-        vm->push(*b);
+        vm->push(**(const ExtValue **)a);
+        vm->push(**(const ExtValue **)b);
         vm->pushNewCallState();
         vm->jumpDynamicJumpTarget(jto->pc);
         vm->run();
         vm->popCallState();
-        bool ret;
+        int ret;
         ExtValue& retval = vm->getValue();
         if (retval.type == TInvalid)
         {
-                ret = false;
+                ret = 0;
                 logPrintf("VectorElementComparator", "", LOG_ERROR, "Comparison function returned no value");
         }
         else
-                ret = (retval.getInt() != 0);
+                ret = (retval.getInt() != 0) ? -1 : 1;
         vm->drop(2);
         return ret;
@@ -306 +308 @@
                 VMVEComparator cmp(jto);
                 ExtValue **first = (ExtValue**)&data.getref(0);
-                std::sort(first, first + data.size(), cmp);
+                //Originally was the same as below: std::sort(first, first + data.size(), cmp);
+                //However, std::sort() requires "strict weak ordering" and may crash (and indeed crashes, "undefined behavior") when given a non-compliant comparator.
+                //We use qsort() instead because we can't control what kind of user script comparator function will be passed to Vector.sort(), and qsort() seems to behave safely for every function.
+                qsort_r(first, data.size(), sizeof(ExtValue*), cmp.compare, &cmp);
         }
         else