R2Northstar / NorthstarLauncher

Launcher used to modify Titanfall 2 to allow mods to be loaded
MIT License
271 stars 125 forks source link

Use old Mutex constructor to deal with redist incompatibility #704

Closed Jan200101 closed 1 month ago

Jan200101 commented 1 month ago

Microsoft is a very cool company that tests their stuff.

Jan200101 commented 1 month ago

Tested by two different people on Discord and confirmed working

Jan200101 commented 1 month ago

I figured I should probably explain the issue at hand:

a newer release of the Windows SDK ships a new version of the STL. It changed the Mutex constructor to be constrexpr by default

#ifdef _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
    _Mutex_base(int _Flags = 0) noexcept {
        _Mtx_init_in_situ(_Mymtx(), _Flags | _Mtx_try);
    }
#else // ^^^ defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) / !defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) vvv
    constexpr _Mutex_base(int _Flags = 0) noexcept {
        _Mtx_storage._Critical_section = {};
        _Mtx_storage._Thread_id        = -1;
        _Mtx_storage._Type             = _Flags | _Mtx_try;
        _Mtx_storage._Count            = 0;
    }
#endif // ^^^ !defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) ^^^

https://github.com/microsoft/STL/blob/e36ee6c2b9bc6f5b1f70776c18cf5d3a93a69798/stl/inc/mutex#L35-L46

This is the opposite of the original behavior, where it was opt-in instead of opt-out.

The non constexpr constructor calls _Mtx_init_in_situ which is implemented as follows:

_CRTIMP2_PURE void __cdecl _Mtx_init_in_situ(_Mtx_t mtx, int type) noexcept { // initialize mutex in situ
    new (&mtx->_Critical_section) _Stl_critical_section;
    mtx->_Thread_id = -1;
    mtx->_Type      = type;
    mtx->_Count     = 0;
}

https://github.com/microsoft/STL/blob/e36ee6c2b9bc6f5b1f70776c18cf5d3a93a69798/stl/src/mutex.cpp#L41-L46

its almost the same as the new constructor, except that _Critical_section is being heap allocated. The type of _Critical_section changed between versions and was previously just a pointer and now a struct with two pointers for ABI compatibility

struct _Stl_critical_section {
    void* _Unused       = nullptr; // TRANSITION, ABI: was the vptr
    _Smtx_t _M_srw_lock = nullptr;
};

https://github.com/microsoft/STL/blob/e36ee6c2b9bc6f5b1f70776c18cf5d3a93a69798/stl/inc/__msvc_threads_core.hpp#L30-L33

in the constexpr constructor the _Unused member is never allocated thus resulting in a crash.


Why does Northstar still work for some users then?

Microsoft tried updating the STL while attempting to not break ABI by expanding the existing data type and leaving existing stuff as is. But this didn't quite work. Old versions of msvcp140.dll ship the following code

if (mtx->thread_id != static_cast<long>(GetCurrentThreadId())) {
    mtx->_get_cs()->lock();
}

https://github.com/microsoft/STL/blob/28a6aab88a37e5cceef1fb16045296935f6ac53b/stl/src/mutex.cpp#L100

This will attempt to dereference the _Critical_section._Unused causing a NULL dereference and thus crashing.

a newer version of the redist uses this code

if (mtx->_Thread_id != current_thread_id) {
    AcquireSRWLockExclusive(get_srw_lock(mtx));
}

https://github.com/microsoft/STL/blob/e36ee6c2b9bc6f5b1f70776c18cf5d3a93a69798/stl/src/mutex.cpp#L93-L95

which does not reference the old pointer.


Fixes to this are: