Nested datamaps write in wrong memory space

[Kenzzer]

Help us help you

• [x] I have checked that my issue doesn't exist yet.
• [x] I have tried my absolute best to reduce the problem-space and have provided the absolute smallest test-case possible.
• [x] I can always reproduce the issue with the provided description below.

Environment

• Operating System version: Debian Buster
• Game/AppID (with version if applicable): 222350
• Current SourceMod version: 1.12.0.6976

• [x] I have updated SourceMod to the latest version and it still happens.
• [x] I have updated SourceMod to the latest snapshot and it still happens.
• [x] I have updated SourceMM to the latest snapshot and it still happens.

Description

Sourcemod doesn't seem to support nested datamaps. Instead using their 'absolute' offset to write in memory, even though it really shouldn't be doing that.

Problematic Code (or Steps to Reproduce)

Find an entity that derives CAI_BaseNPC, which should be simple there's at least one per branch of the source engine. It's an hl2 entity.

SetEntProp(entity, Prop_Data, "m_navType", 0);

Watch as now the property m_pMoveProbe gets overwritten with the value you set above.

PrintToServer("%d", GetEntProp(entity, Prop_Data, "m_pMoveProbe"));

Solution ?

I think the solution is that sourcemod should fail if operating on nested datamaps, or alternatively find their parent offset, to then retrieve the right memory address space to edit.

[asherkin]

Do you have a datamap dump of the game you tested showing the hierarchy here?

[Kenzzer]

datamaps_tf.txt Its a little old, some offsets def changed since then but shouldnt matter for the current issue.

- m_flMoveWaitFinished (Offset 2616) (Save)(4 Bytes)
- m_hOpeningDoor (Offset 2620) (Save)(4 Bytes)
 Sub-Class Table (1 Deep): m_pNavigator - CAI_Navigator
 - m_navType (Offset 12) (Save)(4 Bytes)
  Sub-Class Table (2 Deep): m_pPath - CAI_Path

[...]

 - m_hBigStepGroundEnt (Offset 124) (Save)(4 Bytes)
 - m_hLastBlockingEnt (Offset 128) (Save)(4 Bytes)
 Sub-Class Table (1 Deep): m_pLocalNavigator - CAI_LocalNavigator
 Sub-Class Table (1 Deep): m_pPathfinder - CAI_Pathfinder
 - m_flLastStaleLinkCheckTime (Offset 12) (Save)(4 Bytes)
 Sub-Class Table (1 Deep): m_pMoveProbe - CAI_MoveProbe
 - m_bIgnoreTransientEntities (Offset 8) (Save)(1 Bytes)
 - m_hLastBlockingEnt (Offset 16) (Save)(4 Bytes)
 Sub-Class Table (1 Deep): m_pMotor - CAI_Motor

[asherkin]

Original BZ bug: https://bugs.alliedmods.net/show_bug.cgi?id=5446

This does work well in the general case, so it'll be down to finding out what's odd about this bit of the hierarchy.

Useful for troubleshooting, printing was partially added in #235, but https://bugs.alliedmods.net/show_bug.cgi?id=5509 has a more complete patch that does the tables too.

[asherkin]

It looks like the outlier here is DEFINE_EMBEDDEDBYREF, which is like the DEFINE_EMBEDDED ones we support today but has an additional flag, FTYPEDESC_PTR that we're not handling. FTYPEDESC_PTR appears to indicate we should reset the absolute offset to 0 and change the base address we're using to the value referenced - this should be easy enough to do for SetEntProp and the internal funcs, but it going to make mincemeat of the FindDataMapInfo API (I reckon we just don't bother trying to return an absolute offset for them there).

Feels like we should definitely gain offset and flag printing for tables when fixing this, it would have stood out in the dump if we had.

https://cs.alliedmods.net/hl2sdk-tf2/search?q=DEFINE_EMBEDDEDBYREF&path=