Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide AVOID_UB for some bugs found in GCC compiler testing #1785

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Thar0
Copy link
Contributor

@Thar0 Thar0 commented Jan 7, 2025

Special thanks to @fig02 for the GCC playthrough that uncovered these, and for everyone else who looked into and discussed the problems.

The z_message_nes.c AVOID_UB is particularly nasty.. suggestions welcome

Copy link
Collaborator

@hensldm hensldm left a comment

Choose a reason for hiding this comment

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

Can't think of anything better for Message_DecodeNes unfortunately.

@AngheloAlf AngheloAlf added Merge-ready All reviewers satisfied, just waiting for CI and removed Needs-second-approval Second approval labels Jan 7, 2025
Copy link
Contributor

@fig02 fig02 left a comment

Choose a reason for hiding this comment

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

commenting on the bug in the dog actor would be good to include, dunno if thats planned for another time though

Comment on lines +462 to +467
#ifdef AVOID_UB
//! @bug Z2_DEKU_KINGCutsceneData_008258 provides a cue id of 6, reading out-of-bounds
//! of this array that happens to be 0 padding.
// Avoid UB: Add an explicit array member rather than relying on 0 padding.
DEKU_KING_ANIM_IDLE,
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

should this mention mm3d and how it proves that it was infact an original bug?

Copy link
Contributor

Choose a reason for hiding this comment

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

i would be interested to see how MM3D proves this to be padding, because it does currently match without the AVOID_UB tag

Copy link
Contributor

Choose a reason for hiding this comment

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

see discord (gcc thread)

@hensldm hensldm added the Waiting-for-author Author needs fix to conflicts or address reviews label Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean up Merge-ready All reviewers satisfied, just waiting for CI Waiting-for-author Author needs fix to conflicts or address reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants