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

fix: crash if samples have different amount of costs associated #636

Merged
merged 2 commits into from
May 23, 2024

Conversation

lievenhey
Copy link
Contributor

If we have a recording with the following layout: A B C where A: part where function f is accessed
B: list samples
C: part where function g is accessed
then f won't have the lost events cost while g does. This will cause a crash when the model tries to access it

fixes: #629

src/models/callercalleemodel.h Outdated Show resolved Hide resolved
src/models/callercalleemodel.h Outdated Show resolved Hide resolved
src/models/callercalleemodel.h Outdated Show resolved Hide resolved
src/models/callercalleemodel.h Outdated Show resolved Hide resolved
@lievenhey lievenhey force-pushed the fix-flamegraph branch 3 times, most recently from f437e74 to a06a35e Compare May 16, 2024 14:39
src/models/data.h Outdated Show resolved Hide resolved
src/models/data.cpp Outdated Show resolved Hide resolved
src/models/data.cpp Outdated Show resolved Hide resolved
@lievenhey lievenhey force-pushed the fix-flamegraph branch 2 times, most recently from 96603d4 to 76a06b9 Compare May 16, 2024 20:42
src/models/data.cpp Outdated Show resolved Hide resolved
src/models/data.h Outdated Show resolved Hide resolved
src/models/data.h Outdated Show resolved Hide resolved
src/models/data.h Outdated Show resolved Hide resolved
src/models/data.cpp Outdated Show resolved Hide resolved
If we have a recording with the following layout: A B C where A: part
where function f is accessed
B: lost samples
C: part where function g is accessed
then f won't have the lost events cost while g does. This will cause a
crash when the model tries to access it.

This patch replaces the typedef ItemCost with a real wrapper that will
catch the out range access.

fixes: #629
Copy link
Member

@milianw milianw left a comment

Choose a reason for hiding this comment

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

lgtm, thanks

@milianw milianw merged commit 053d979 into master May 23, 2024
25 checks passed
@milianw milianw deleted the fix-flamegraph branch May 23, 2024 08:39
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.

Crash(Assertion failure) when opening large data
2 participants