TheAssemblyArmada / Thyme

An open source re-implementation of Generals : Zero Hour written in C++.
GNU General Public License v2.0
584 stars 59 forks source link

w3d view crash #1145

Closed xezon closed 2 months ago

xezon commented 2 months ago
>   w3dview.exe!HLodClass::Render(RenderInfoClass & rinfo) Line 1350    C++
    w3dview.exe!SimpleSceneClass::Customized_Render(RenderInfoClass & rinfo) Line 201   C++
    w3dview.exe!SceneClass::Render(RenderInfoClass & rinfo) Line 86 C++
    w3dview.exe!W3D::Render(SceneClass * scene, CameraClass * cam, bool clear, bool clearz, const Vector3 & color) Line 340 C++
    w3dview.exe!CGraphicView::Render(int update, unsigned int time) Line 599    C++
    w3dview.exe!CGraphicView::WindowProc(unsigned int message, unsigned int wParam, long lParam) Line 87    C++
    mfc140.dll!5af08184()   Unknown
    [Frames below may be incorrect and/or missing, no symbols loaded for mfc140.dll]    
    mfc140.dll!5af083d4()   Unknown
    mfc140.dll!5ade7654()   Unknown
    user32.dll!758b181b()   Unknown
    user32.dll!758a7f6a()   Unknown
    user32.dll!758a68a1()   Unknown
    user32.dll!75898640()   Unknown
    mfc140.dll!5aef50fe()   Unknown
    mfc140.dll!5af1e244()   Unknown
    kernel32.dll!7647fcc9() Unknown
    ntdll.dll!__RtlUserThreadStart()    Unknown
    ntdll.dll!__RtlUserThreadStart@8()  Unknown
Exception thrown: read access violation.
VectorClass<HLodClass::ModelNodeClass>::operator[](...).**m_model** was nullptr.
pos = 0x00000006
  Name Value Type
m_lod,7 0x040a6c50 {{m_maxScreenSize=3.40282347e+38 m_nonPixelCost=222.000000 m_pixelCostPerArea=0.00000000 ...}, ...} HLodClass::ModelArrayClass[0x00000007]
  ▶ [0x00000000] {m_maxScreenSize=3.40282347e+38 m_nonPixelCost=222.000000 m_pixelCostPerArea=0.00000000 ...} HLodClass::ModelArrayClass
  ▶ [0x00000001] {m_maxScreenSize=1.62692106e-36 m_nonPixelCost=1.62692106e-36 m_pixelCostPerArea=7.347e-40#DEN ...} HLodClass::ModelArrayClass
  ▶ [0x00000002] {m_maxScreenSize=1.194e-39#DEN m_nonPixelCost=7.591e-40#DEN m_pixelCostPerArea=0.219558448 ...} HLodClass::ModelArrayClass
  ▶ [0x00000003] {m_maxScreenSize=1.62802524e-36 m_nonPixelCost=2.15074239e-30 m_pixelCostPerArea=0.00000000 ...} HLodClass::ModelArrayClass
  ▶ [0x00000004] {m_maxScreenSize=0.00000000 m_nonPixelCost=1.286e-39#DEN m_pixelCostPerArea=1.378e-39#DEN ...} HLodClass::ModelArrayClass
  ▶ [0x00000005] {m_maxScreenSize=6.429e-40#DEN m_nonPixelCost=3.674e-40#DEN m_pixelCostPerArea=9.184e-41#DEN ...} HLodClass::ModelArrayClass
  ◢ [0x00000006] {m_maxScreenSize=1.286e-39#DEN m_nonPixelCost=1.378e-39#DEN m_pixelCostPerArea=1.561e-39#DEN ...} HLodClass::ModelArrayClass
  ▶ DynamicVectorClass {m_activeCount=0x00020004 m_growthStep=0x00040002 } DynamicVectorClass
  m_maxScreenSize 1.286e-39#DEN float
  m_nonPixelCost 1.378e-39#DEN float
  m_pixelCostPerArea 1.561e-39#DEN float
  m_benefitFactor 1.653e-39#DEN float

m_lod[1] to m_lod[6] look garbage.

  Name Value Type
  mlod->m_activeCount 0x0000000f int
  Name Value Type
m_lod->m_vector,0xf 0x052b818c {{m_model=0x004f3e20 {w3dview.exe!void(* DX8RigidFVFCategoryContainer::`vftable'[8])()} {...} ...}, ...} HLodClass::ModelNodeClass[0x0000000f]
  ▶ [0x00000000] {m_model=0x004f3e20 {w3dview.exe!void(* DX8RigidFVFCategoryContainer::`vftable'[8])()} {m_bits=0x0046c730 ...} ...} HLodClass::ModelNodeClass
  ▶ [0x00000001] {m_model=0x004f3df8 {w3dview.exe!void(* MultiListClass::`vftable'[2])()} {m_bits=...} ...} HLodClass::ModelNodeClass
  ▶ [0x00000002] {m_model=0x014277e0 {m_bits=0x040a72a8 m_transform={Row=... } m_objectScale=1.63238097e-36 ...} m_boneIndex=...} HLodClass::ModelNodeClass
  ▶ [0x00000003] {m_model=0x00000000 m_boneIndex=0x00000000 } HLodClass::ModelNodeClass
  ▶ [0x00000004] {m_model=0x004f3df8 {w3dview.exe!void(* MultiListClass::`vftable'[2])()} {m_bits=...} ...} HLodClass::ModelNodeClass
  ▶ [0x00000005] {m_model=0x052b81b0 {m_bits=0x052b81c8 m_transform={Row=... } m_objectScale=3.57184028e-38 ...} m_boneIndex=...} HLodClass::ModelNodeClass
  ▶ [0x00000006] {m_model=0x00000000 m_boneIndex=0x00000000 } HLodClass::ModelNodeClass
  ▶ [0x00000007] {m_model=0x004f3df8 {w3dview.exe!void(* MultiListClass::`vftable'[2])()} {m_bits=...} ...} HLodClass::ModelNodeClass
  ▶ [0x00000008] {m_model=0x052b81c8 {m_bits=0x052b81e0 m_transform={Row=... } m_objectScale=8.06427007e-36 ...} m_boneIndex=...} HLodClass::ModelNodeClass
  ▶ [0x00000009] {m_model=0x00000000 m_boneIndex=0x00000000 } HLodClass::ModelNodeClass
  ▶ [0x0000000a] {m_model=0x004f3df8 {w3dview.exe!void(* MultiListClass::`vftable'[2])()} {m_bits=...} ...} HLodClass::ModelNodeClass
  ▶ [0x0000000b] {m_model=0x052b81e0 {m_bits=0x01427830 m_transform={Row=... } m_objectScale=8.06428729e-36 ...} m_boneIndex=...} HLodClass::ModelNodeClass
  ▶ [0x0000000c] {m_model=0x00000000 m_boneIndex=0x00000000 } HLodClass::ModelNodeClass
  ▶ [0x0000000d] {m_model=0x004f3df8 {w3dview.exe!void(* MultiListClass::`vftable'[2])()} {m_bits=...} ...} HLodClass::ModelNodeClass
  ▶ [0x0000000e] {m_model=0x01427830 {m_bits=0x0efc4874 m_transform={Row=... } m_objectScale=7.473e-39#DEN ...} m_boneIndex=...} HLodClass::ModelNodeClass

Is this perhaps meant to access m_lod->m_vector instead?

Is HLodClass::Render implemented correctly?

xezon commented 2 months ago

Ah it does access m_vector. Ok.

template<typename T> T &VectorClass<T>::operator[](int index)
{
    captainslog_assert(unsigned(index) < unsigned(m_vectorMax));
    return m_vector[index];
}
xezon commented 2 months ago

Ok looks like m_vector contains garbage to begin with

  Name Value Type
m_lod->m_vector,0x10 0x052b818c {{m_model=0x004f3e20 {w3dview.exe!void(* DX8RigidFVFCategoryContainer::`vftable'[8])()} {...} ...}, ...} HLodClass::ModelNodeClass[0x00000010]
  ◢ [0x00000000] {m_model=0x004f3e20 {w3dview.exe!void(* DX8RigidFVFCategoryContainer::`vftable'[8])()} {m_bits=0x0046c730 ...} ...} HLodClass::ModelNodeClass
  ◢ m_model 0x004f3e20 {w3dview.exe!void(* DX8RigidFVFCategoryContainer::`vftable'[8])()} {m_bits=0x0046c730 m_transform=...} RenderObjClass *
  ▶ RefCountClass {m_numRefs=0x0046b700 } RefCountClass
  ▶ PersistClass {...} PersistClass
  ◢ MultiListObjectClass {m_listNode=0x004695d0 {w3dview.exe!DX8RigidFVFCategoryContainer::Add_Delayed_Visible_Material_Pass(MaterialPassClass , MeshClass )} {...} } MultiListObjectClass
  ◢ __vfptr 0x0046a630 {w3dview.exe!DX8RigidFVFCategoryContainer::Check_If_Mesh_Fits(MeshModelClass *)} {0x8bec8b55} void
  [0x00000000] 0x8bec8b55 void *
  ▶ m_listNode 0x004695d0 {w3dview.exe!DX8RigidFVFCategoryContainer::Add_Delayed_Visible_Material_Pass(MaterialPassClass , MeshClass )} {...} MultiListNodeClass *
  m_bits 0x0046c730 unsigned long
  ▶ m_transform {Row=0x004f3e3c {{X=7.362e-39#DEN Y=6.482e-39#DEN Z=6.494e-39#DEN ...}, {X=6.492e-39#DEN Y=6.488e-39#DEN ...}, ...} } Matrix3D
  m_objectScale 6.482e-39#DEN float
  m_houseColor 0x00502b90 unsigned int
  ▶ m_cachedBoundingSphere {Center={X=6.481e-39#DEN Y=0.00000000 Z=3.37500000 } Radius=7.363e-39#DEN } SphereClass
  ▶ m_cachedBoundingBox {m_center={X=6.524e-39#DEN Y=6.507e-39#DEN Z=7.363e-39#DEN } m_extent={X=6.524e-39#DEN Y=6.507e-39#DEN ...} } AABoxClass
  m_nativeScreenSize 6.506e-39#DEN float
  m_isTransformIdentity true (0xb0) bool
  ▶ m_scene 0x00470410 {w3dview.exe!VectorClass<FontCharsBuffer >::Resize(int, FontCharsBuffer const *)} {m_ambientLight=...} SceneClass *
  ▶ m_container 0x0046eba0 {w3dview.exe!VectorClass<FontCharsBuffer *>::Clear(void)} {m_bits=0x0446c704 m_transform=...} RenderObjClass *
  m_userData 0x0046fba0 {w3dview.exe!VectorClass<FontCharsBuffer >::ID(FontCharsBuffer const &)} void *
  ▶ m_unknown 0x0046fbf0 {w3dview.exe!VectorClass<FontCharsBuffer >::ID(FontCharsBuffer const *)} {...} RenderObjUnk *
  m_boneIndex 0x01427790 int
xezon commented 2 months ago

I wonder if something is missing in this constructor:

template<typename T>
VectorClass<T>::VectorClass(int size, const T *array) :
    m_vector(nullptr), m_vectorMax(size), m_isValid(true), m_isAllocated(false)
{
    //    Allocate the vector. The default constructor will be called for every
    //    object in this vector.
    if (size > 0) {
        if (array != nullptr) {
            m_vector = new ((void *)array) T[size];
        } else {
            m_vector = new T[size];
            m_isAllocated = true;
        }
    }
}

It allocates m_vector, but leaves it in unitialized state. It does not copy array.

xezon commented 2 months ago

If I understand this correctly it will use "array" as buffer to construct on. And that array cannot be deallocated as long as that VectorClass is used. So if we lose "array", then hell breaks lose.

There appear to be no callers for the placement new code paths (2). That is good.

So the VectorClass element is

    class ModelNodeClass
    {
    public:
        RenderObjClass *m_model;
        int m_boneIndex;

        bool operator==(const ModelNodeClass &src) { return m_model == src.m_model && m_boneIndex == src.m_boneIndex; }
        bool operator!=(const ModelNodeClass &src) { return m_model != src.m_model || m_boneIndex != src.m_boneIndex; }
    };

It has no constructor. So this will construct with garbage pointers.