Fix: Large stack allocation in compositors DebugInfo::graphviz
ClosedPublic
Actions

Authored by Michael (michael64) on Dec 19 2021, 7:12 PM.

Details

Reviewers

Ray Molenkamp (LazyDodo)

Commits

rBc6e7fc97443e: Fix: Large stack allocation in compositor

Summary

DebugInfo::graphviz uses a char[1000000] as local variable.
When this function is called this is allocated on the stack,
which has a size of just 1MB on mac and may cause a stack
overflow.

This patch allocates the memory on the heap and frees the memory
at the end of the function.

Diff Detail

Event Timeline

Michael (michael64) requested review of this revision.Dec 19 2021, 7:12 PM

Michael (michael64) created this revision.

Michael (michael64) mentioned this in T94230: Crash rendering with EEVEE in mesh edit-mode.Dec 19 2021, 7:20 PM

Jesse Yurkovich (deadpin) added a subscriber: Jesse Yurkovich (deadpin).Dec 20 2021, 10:56 PM

Jesse Yurkovich (deadpin) added inline comments.

source/blender/compositor/intern/COM_Debug.cc
434–436	`sizeof` is not going to work with a dynamically allocated array so this needs addressed.

Use a constant for the maximum text length to pass in the correct maxlen parameter into DebugInfo::graphviz_system(const ExecutionSystem *system, char *str, int maxlen).

Thank you @Jesse Yurkovich (deadpin) for catching my mistake!

Philipp Oeser (lichtwerk) added a task: T94230: Crash rendering with EEVEE in mesh edit-mode.Dec 22 2021, 2:44 PM

For the reasons outlined here this can't be a fix for T94230, however this is still a good fix to have.

This revision is now accepted and ready to land.Dec 22 2021, 6:05 PM

Ray Molenkamp (LazyDodo) removed a task: T94230: Crash rendering with EEVEE in mesh edit-mode.Dec 22 2021, 6:05 PM

Ray Molenkamp (LazyDodo) retitled this revision from Potential fix for T94230 to Fix: Large stack allocation in compositors DebugInfo::graphviz.Dec 22 2021, 6:10 PM

Ray Molenkamp (LazyDodo) edited the summary of this revision. (Show Details)

Closed by commit rBc6e7fc97443e: Fix: Large stack allocation in compositor (authored by Michael - (michael), committed by Ray Molenkamp (LazyDodo)). · Explain WhyDec 22 2021, 9:50 PM

This revision was automatically updated to reflect the committed changes.

Ray Molenkamp (LazyDodo) added a commit: rBc6e7fc97443e: Fix: Large stack allocation in compositor.

Thanks for the patch! committed! The right name is on the actual commit, but phab is wrongly matching it to the wrong user, sorry about that.