Changeset 930


Ignore:
Timestamp:
05/27/20 12:14:12 (4 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:
1 edited

Legend:

Unmodified
Added
Removed
  • cpp/frams/vm/classes/collectionobj.cpp

    r868 r930  
    11// This file is a part of Framsticks SDK.  http://www.framsticks.com/
    2 // Copyright (C) 1999-2019  Maciej Komosinski and Szymon Ulatowski.
     2// Copyright (C) 1999-2020  Maciej Komosinski and Szymon Ulatowski.
    33// See LICENSE.txt for details.
    44
     
    270270        VMachine *vm;
    271271        VMVEComparator(VMachine::JumpTargetObject *_jto) :jto(_jto), vm(jto->vm) {}
    272         bool operator()(const ExtValue *a, const ExtValue *b);
     272        static int compare(const void *a, const void *b, void* _this);
    273273};
    274274
    275 bool VMVEComparator::operator()(const ExtValue *a, const ExtValue *b)
    276 {
     275int VMVEComparator::compare(const void *a, const void *b, void* _this)
     276{
     277        VMachine *vm = ((VMVEComparator*)_this)->vm;
     278        VMachine::JumpTargetObject *jto = ((VMVEComparator*)_this)->jto;
    277279        if (!VMCode::prepareDynamicJumpTarget(jto->pc, jto->code))
    278280                return false;
    279281
    280         vm->push(*a);
    281         vm->push(*b);
     282        vm->push(**(const ExtValue **)a);
     283        vm->push(**(const ExtValue **)b);
    282284        vm->pushNewCallState();
    283285        vm->jumpDynamicJumpTarget(jto->pc);
    284286        vm->run();
    285287        vm->popCallState();
    286         bool ret;
     288        int ret;
    287289        ExtValue& retval = vm->getValue();
    288290        if (retval.type == TInvalid)
    289291        {
    290                 ret = false;
     292                ret = 0;
    291293                logPrintf("VectorElementComparator", "", LOG_ERROR, "Comparison function returned no value");
    292294        }
    293295        else
    294                 ret = (retval.getInt() != 0);
     296                ret = (retval.getInt() != 0) ? -1 : 1;
    295297        vm->drop(2);
    296298        return ret;
     
    306308                VMVEComparator cmp(jto);
    307309                ExtValue **first = (ExtValue**)&data.getref(0);
    308                 std::sort(first, first + data.size(), cmp);
     310                //Originally was the same as below: std::sort(first, first + data.size(), cmp);
     311                //However, std::sort() requires "strict weak ordering" and may crash (and indeed crashes, "undefined behavior") when given a non-compliant comparator.
     312                //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.
     313                qsort_r(first, data.size(), sizeof(ExtValue*), cmp.compare, &cmp);
    309314        }
    310315        else
Note: See TracChangeset for help on using the changeset viewer.