Bug 218350 - kernelshark-v2.3.0 crash because of assert failed in ksmodel_set_upper_edge
Summary: kernelshark-v2.3.0 crash because of assert failed in ksmodel_set_upper_edge
Status: NEW
Alias: None
Product: Tools
Classification: Unclassified
Component: Trace-cmd/Kernelshark (show other bugs)
Hardware: All Linux
: P3 high
Assignee: Default virtual assignee for Trace-cmd and kernelshark
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-01-06 18:19 UTC by Benjamin Robin
Modified: 2024-01-11 19:13 UTC (History)
1 user (show)

See Also:
Kernel Version:
Subsystem:
Regression: No
Bisected commit-id:


Attachments
Patch (851 bytes, application/mbox)
2024-01-10 19:06 UTC, Yordan Karadzhov
Details
Patch-v2 (1.27 KB, application/mbox)
2024-01-10 20:22 UTC, Benjamin Robin
Details
Patch-v2 (1.40 KB, patch)
2024-01-10 20:53 UTC, Benjamin Robin
Details | Diff
Patch-v2 (1.64 KB, patch)
2024-01-10 20:56 UTC, Benjamin Robin
Details | Diff
Patch-v2 (1.64 KB, patch)
2024-01-10 21:09 UTC, Benjamin Robin
Details | Diff

Description Benjamin Robin 2024-01-06 18:19:17 UTC
Environment:
 - KDE with Oxygen theme
 - kernelshark-v2.3.0 (Qt6 build)

Note: I do not have any issue with kernelshark-v2.2.1. These bugs are triggered by the
migration to Qt6.

Here my analysis:

In KsGLWidget::_defaultPlots(), _model.reset() is called, which will call endResetModel().
This will trigger (synchronously) a call to KsGLWidget::resizeGL():
 - At this point _model.histo() is empty, since previously reset, so both min and max
   histo fields are 0.
 - A call to ksmodel_set_bining() will update max value to an invalid value (too small)
 - _model.fill(_data) is called "KsGraphModel::fill()", which call ksmodel_fill ->
   ksmodel_set_upper_edge
 - The kshark_find_entry_by_time() is going to return BSEARCH_ALL_GREATER (since max is
   smaller than all timestamp) => ASSERT

There is another problem:
 - From KsGLWidget::_defaultPlots(), _model.reset() is called, which will call 
   endResetModel()
 - This will trigger (synchronously) a call to KsGLWidget::resizeGL()
 - Which is going to execute _model.fill(), which is going to call endResetModel()
 - This will trigger (synchronously) a call to KsGLWidget::resizeGL()
 - ... => Stackoverflow

I tried to fix it by adding to resizeGL():

    if (_model.histo()->min == 0 && _model.histo()->max == 0)
        return;

    if (_model.histo()->n_bins == nBins)
        return;

With this "fix" the application can open a trace-cmd file without crashing.
But there is still a lot of issue
Comment 1 Yordan Karadzhov 2024-01-10 19:06:56 UTC
Created attachment 305693 [details]
Patch
Comment 2 Yordan Karadzhov 2024-01-10 19:10:00 UTC
Hi Benjamin,

Thanks a lot for reporting the issue!

Please test the attached pach and tell me if it fixes that problem that you see.
Cheers,

Yordan
Comment 3 Yordan Karadzhov 2024-01-10 19:10:57 UTC
Hi Benjamin,

Thanks a lot for reporting the issue!

Please test the attached patch and tell me if it fixes that problem that you see.
Cheers,

Yordan
Comment 4 Benjamin Robin 2024-01-10 20:22:59 UTC
Created attachment 305694 [details]
Patch-v2

Yes your patch works, thanks for the idea, but I really don't like keeping the 
update() function in KsGLWidget class since it overrides QWidget::update() and 
QWidget::update() is not virtual.
Also I prefer the new connection mechanism, but I failed to connect directly to
QWidget::update(), so I used a lambda.
So I made a new v2 patch
Comment 5 Benjamin Robin 2024-01-10 20:53:06 UTC
Created attachment 305695 [details]
Patch-v2

Updated patch, also rename update function
Comment 6 Benjamin Robin 2024-01-10 20:56:38 UTC
Created attachment 305696 [details]
Patch-v2

Fix bad commit
Comment 7 Benjamin Robin 2024-01-10 21:09:27 UTC
Created attachment 305697 [details]
Patch-v2

Sorry for the spam, updated patch (again) I used this time the proper way to realize the connect (which I did learn)
Comment 8 Yordan Karadzhov 2024-01-11 17:21:10 UTC
Great!!!

Please send your patch to the mailing list (linux-trace-devel@vger.kernel.org) and I will be happy to take it.

Y.
Comment 9 Benjamin Robin 2024-01-11 19:13:36 UTC
Did send it, hopefully properly. Thanks

Note You need to log in before you can comment on or make changes to this bug.