Skip to content

Conversation

@Stubbjax
Copy link

@Stubbjax Stubbjax commented Jan 30, 2026

This change fixes a crash caused by spawning a CreateDebris object with an assigned Shadow by initializing the shadow name to an empty string.

When the shadow name is uninitialized, checks for '\0' and strlen <= 1 fail and garbage data is instead passed to various locations that do not expect it nor know how to deal with it.

@Stubbjax Stubbjax self-assigned this Jan 30, 2026
@Stubbjax Stubbjax added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Stability Concerns stability of the runtime Crash This is a crash, very bad Mod Relates to Mods or modding labels Jan 30, 2026
@greptile-apps
Copy link

greptile-apps bot commented Jan 30, 2026

Greptile Overview

Greptile Summary

This PR fixes a critical crash caused by uninitialized memory in the Shadow::ShadowTypeInfo struct. When creating debris objects with assigned shadows, the uninitialized m_ShadowName array contained garbage data that caused checks for '\0' and strlen <= 1 to fail in W3DProjectedShadow.cpp, leading to crashes.

The fix adds a default constructor to ShadowTypeInfo that properly initializes all members:

  • m_ShadowName[0] = '\0' - prevents garbage data in string checks
  • All numeric fields initialized to sensible defaults (0.0f for floats, false for booleans, SHADOW_NONE for type)

Additionally, the PR removes redundant manual initializations in W3DDebrisDraw.cpp and W3DDefaultDraw.cpp that are now handled by the constructor. This follows the DRY principle and ensures consistent initialization across all instantiation sites.

The changes are mirrored across both Generals/ (vanilla) and GeneralsMD/ (Zero Hour) codebases.

Confidence Score: 5/5

  • This PR is safe to merge and fixes a critical crash bug
  • The fix properly addresses a critical uninitialized memory bug by adding a default constructor that initializes all struct members. The solution follows C++ best practices, eliminates redundant code, and has no side effects. All changes are consistent across both codebases (Generals and GeneralsMD).
  • No files require special attention

Important Files Changed

Filename Overview
Generals/Code/GameEngine/Include/GameClient/Shadow.h Added default constructor to ShadowTypeInfo that initializes all members including m_ShadowName[0] = '\0' to prevent uninitialized memory access
GeneralsMD/Code/GameEngine/Include/GameClient/Shadow.h Added default constructor to ShadowTypeInfo that initializes all members including m_ShadowName[0] = '\0' to prevent uninitialized memory access

Sequence Diagram

sequenceDiagram
    participant Client as Game Client
    participant Debris as W3DDebrisDraw
    participant Shadow as ShadowTypeInfo
    participant Manager as W3DShadowManager
    participant Projected as W3DProjectedShadow
    
    Note over Client,Projected: Before Fix (Crash Scenario)
    Client->>Debris: Create debris with shadow
    Debris->>Shadow: Instantiate ShadowTypeInfo
    Note over Shadow: m_ShadowName contains<br/>garbage data (uninitialized)
    Debris->>Shadow: Set m_type = t
    Debris->>Manager: addShadow(renderObject, &shadowInfo)
    Manager->>Projected: Process shadow creation
    Projected->>Projected: Check strlen(m_ShadowName) <= 1
    Note over Projected: FAIL: Garbage data makes<br/>strlen unpredictable
    Projected->>Projected: Check m_ShadowName[0] != '\0'
    Note over Projected: FAIL: Garbage data present
    Projected->>Projected: strcpy(texture_name, m_ShadowName)
    Note over Projected: CRASH: Passes garbage<br/>to texture loader
    
    Note over Client,Projected: After Fix (Proper Initialization)
    Client->>Debris: Create debris with shadow
    Debris->>Shadow: Instantiate ShadowTypeInfo
    Note over Shadow: Constructor initializes:<br/>m_ShadowName[0] = '\0'<br/>All members initialized
    Debris->>Shadow: Set m_type = t
    Debris->>Manager: addShadow(renderObject, &shadowInfo)
    Manager->>Projected: Process shadow creation
    Projected->>Projected: Check strlen(m_ShadowName) <= 1
    Note over Projected: SUCCESS: strlen = 0
    Projected->>Projected: Use default texture
    Note over Projected: No crash, proper behavior
Loading

@Caball009
Copy link

Caball009 commented Jan 30, 2026

Perhaps there should be a default constructor for Shadow::ShadowTypeInfo where this is handled, instead of relying on each call construction site to do the right thing.

@Stubbjax
Copy link
Author

Perhaps there should be a default constructor for Shadow::ShadowTypeInfo where this is handled, instead of relying on each call construction site to do the right thing.

Good thinking. Updated!

@Mauller
Copy link

Mauller commented Jan 30, 2026

Does this tend to only get hit under mod use?

@alanblack166
Copy link

alanblack166 commented Jan 30, 2026

Does this tend to only get hit under mod use?

Zero Hour does not have CreateDebris objects with a defined Shadow parameter.

Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Needs to be rebased and merged after #2225

@xezon xezon changed the title fix: Resolve crash by initializing debris shadow name to an empty string fix(shadow): Fix crash by initializing debris shadow name to an empty string Jan 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Crash This is a crash, very bad Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker Mod Relates to Mods or modding Stability Concerns stability of the runtime ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ObjectCreationList CreateDebris object with SHADOW_DECAL Shadow parameter has null texture

5 participants