Fix T102225: Crash opening menus
ClosedPublic
Actions

Authored by Sergey Sharybin (sergey) on Nov 3 2022, 10:45 AM.

Details

Reviewers

Julian Eisel (Severin)
Jacques Lucke (JacquesLucke)

Maniphest Tasks

T102225: Blender 3.4.0 Alpha Crash Guide on click menu. 1.0.0

Commits

rB09b9e1e95ee4: Fix T102225: Crash opening menus

Summary

Seems to be introduced by 99e5024e97f.

The crash is caused by the difference in the expected alignment
of the uiPopupMenu which is 16 bytes and the actual alignment
returned by the MEM_mallocN() which is 8 bytes due to the memory
head.

Now made it so that MEM_new() can be used for types with any
alignment.

Diff Detail

Repository

rB Blender

Branch

fix_T102225 (branched from master)

Build Status

Buildable 24470
Build 24470: arc lint + arc unit

Event Timeline

Sergey Sharybin (sergey) requested review of this revision.Nov 3 2022, 10:45 AM

Sergey Sharybin (sergey) created this revision.

Harbormaster completed remote builds in B24470: Diff 57215.Nov 3 2022, 10:45 AM

Sergey Sharybin (sergey) added a task: T102225: Blender 3.4.0 Alpha Crash Guide on click menu. 1.0.0.Nov 3 2022, 10:46 AM

Works for me. Don't quite get why it's needed since before my change we were just using MEM_cnew(), so weird that it breaks now. But according to Sergey it's some optimization in some implementations of std::function.

Should we also do this for the other new like functions?

This revision is now accepted and ready to land.Nov 3 2022, 12:38 PM

After some second round of thoughts, there are few things we can consider doing:

Make both MEM_new and MEM_cnew aware of the type alignment
Align to 16 bytes instead of the alignof(T) to match the alignment behavior on the macOS
Make it so MEM_mallocN and friends follow the typical alignment on the platform (which is 16 bytes on macOS)

One of the thoughts in the original patch to limit the change was to reduce change of side effects for cases when objects are allocated more heavily. Would be unideal to increase memory slop.

Jacques Lucke (JacquesLucke) accepted this revision.Nov 3 2022, 2:01 PM

Closed by commit rB09b9e1e95ee4: Fix T102225: Crash opening menus (authored by Sergey Sharybin (sergey)). · Explain WhyNov 3 2022, 2:33 PM

This revision was automatically updated to reflect the committed changes.

Sergey Sharybin (sergey) added a commit: rB09b9e1e95ee4: Fix T102225: Crash opening menus.