Skip to content

Conversation

@rfomin
Copy link
Collaborator

@rfomin rfomin commented Jul 26, 2025

There are only "left shift of negative value" errors left, which may not be worth fixing.

@rfomin rfomin changed the title Fix errors reported bu UBSan Fix errors reported by UBSan Jul 26, 2025
Copy link
Collaborator

@Pedro-Beirao Pedro-Beirao 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 thanks!

Its weird that it complains about P_ReplaceMobjWithIndex, but whatever.

@rfomin
Copy link
Collaborator Author

rfomin commented Jul 27, 2025

Its weird that it complains about P_ReplaceMobjWithIndex, but whatever.

The actual problem is the P_SAVE_TYPE_REF macro. One way to fix the alignment issue is to use an additional memcpy, but then the saving code will be slower.

Kraflab removed the padding in the old Doom code. See: https://github.com/coelckers/prboom-plus/blob/969515162c5aebea4ad7a125ee178dcad3d576ad/prboom2/src/p_saveg.c#L53
It seems that we can resolve the issue by restoring and updating it.

How important is misalignment? I remember at least one crash related to this issue in Woof on Linux. There are a lot of changes in this PR, so more testing is needed.

@rfomin rfomin marked this pull request as draft July 27, 2025 11:07
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