Skip to content

Conversation

@rootBrz
Copy link
Contributor

@rootBrz rootBrz commented Jan 2, 2026

Resolved a crash occurring when EventManager passes a dangling thisObj pointer. While the pointer was non-null, it pointed to garbage memory, causing the retrieval of a garbage baseForm address and a subsequent crash during GetModIndex().

Added a NOT_ID check to validate that thisObj is a valid TESObjectREFR before accessing its members to prevent crash.

CrashLogger.log

@korri123
Copy link
Member

korri123 commented Jan 2, 2026

Hmm, this means that the IsValidReference check/hack before the call to GetPermanentBaseForm is not sufficient. Jazz wrote that code actually, and I don't really even understand what it does, but he's MIA. I need to look into that since that's what is supposed to prevent this crash and see why that check can't just be an IS_ID(TESObjectREFR) call within that __try block instead.

@rootBrz
Copy link
Contributor Author

rootBrz commented Jan 2, 2026

The first bit check in IsValidReference seems unnecessary at all since the 3rd condition is the same, but we actually check whole vtable pointer instead of 1 bit.
Second condition looks random though, it doesn't even dereference the addr of ref, just check second half of bytes in it to match 0x108, can't really get my head around it.
Anyways I've added check for type inside GetPermanentBaseForm because there is already null check, can't hurt imo.

@korri123
Copy link
Member

korri123 commented Jan 3, 2026

We also need to check for classes that inherit TESObjectREFR. This was the code before Jazz micro-optimized it:

bool TryGetReference(TESObjectREFR* refr)
{
    // ### HACK HACK HACK
    // MarkEventList() may have been called for a BaseExtraList not associated with a TESObjectREFR
    bool bIsRefr = false;
    __try 
    {
        switch (*((UInt32*)refr)) {
            case kVtbl_PlayerCharacter:
            case kVtbl_Character:
            case kVtbl_Creature:
            case kVtbl_ArrowProjectile:
            case kVtbl_MagicBallProjectile:
            case kVtbl_MagicBoltProjectile:
            case kVtbl_MagicFogProjectile:
            case kVtbl_MagicSprayProjectile:
            case kVtbl_TESObjectREFR:
                bIsRefr = true;
        }
    }
    __except(EXCEPTION_EXECUTE_HANDLER) 
    {
        bIsRefr = false;
    }

    return bIsRefr;
}

Maybe best to just restore it to that.

@rootBrz
Copy link
Contributor Author

rootBrz commented Jan 3, 2026

Yeah, agree, no more crash with it.

@korri123 korri123 merged commit 2a2232b into xNVSE:master Jan 3, 2026
1 check passed
@korri123
Copy link
Member

korri123 commented Jan 3, 2026

Nice, thanks for the work and debugging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants