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

GUI: Multiple fixes to grid widget #3960

Merged
merged 4 commits into from Jun 9, 2022
Merged

Conversation

lephilousophe
Copy link
Member

@lephilousophe lephilousophe commented Jun 4, 2022

This PR fixes #13499.
In addition, before this, filter became inconsistent when we changed the grouping option.
Additionally, the selected game is now preserved when switching between grid and list and restored at startup.
Finally a small memory optimization is done by not storing the grid items twice but only storing pointers to original list.

@sev-
Copy link
Member

@sev- sev- commented Jun 9, 2022

Thank you!

@sev- sev- merged commit 507aaae into scummvm:master Jun 9, 2022
8 checks passed
@sev-
Copy link
Member

@sev- sev- commented Jun 9, 2022

Oh, it now crashes for me on the start:

==38465==ERROR: AddressSanitizer: heap-use-after-free on address 0x61d0004a3880 at pc 0x00010b450524 bp 0x7ffee644b8d0 sp 0x7ffee644b8c8
READ of size 1 at 0x61d0004a3880 thread T0
    #0 0x10b450523 in GUI::GridWidget::calcEntrySizes() grid.cpp:835
    #1 0x10b447388 in GUI::GridWidget::sortGroups() grid.cpp:522
    #2 0x10b44e84f in GUI::GridWidget::groupEntries() grid.cpp:461
    #3 0x10b304d59 in GUI::LauncherGrid::groupEntries(Common::Array<Common::ConfigManager::Domain const*> const&) launcher.cpp:1413
    #4 0x10b3070c3 in GUI::LauncherGrid::updateListing() launcher.cpp:1546
    #5 0x10b30812c in GUI::LauncherGrid::build() launcher.cpp:1590
    #6 0x10b301882 in GUI::LauncherGrid::LauncherGrid(Common::String const&, GUI::LauncherChooser*) launcher.cpp:1307
    #7 0x10b2f62a4 in GUI::LauncherGrid::LauncherGrid(Common::String const&, GUI::LauncherChooser*) launcher.cpp:1306
    #8 0x10b2f60cb in GUI::LauncherChooser::selectLauncher() launcher.cpp:946
    #9 0x10988422d in launcherDialog() main.cpp:104
    #10 0x109881c04 in scummvm_main main.cpp:565
    #11 0x109877676 in main macosx-main.cpp:44
    #12 0x7fff205ebf3c in start+0x0 (libdyld.dylib:x86_64+0x15f3c)

0x61d0004a3880 is located 0 bytes inside of 2176-byte region [0x61d0004a3880,0x61d0004a4100)
freed by thread T0 here:
    #0 0x10f6f54e9 in wrap_free+0xa9 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x444e9)
    #1 0x10b310297 in Common::Array<GUI::GridItemInfo>::freeStorage(GUI::GridItemInfo*, unsigned int) array.h:401
    #2 0x10b3109ff in Common::Array<GUI::GridItemInfo>::insert_aux(GUI::GridItemInfo*, GUI::GridItemInfo const*, GUI::GridItemInfo const*) array.h:440
    #3 0x10b307955 in Common::Array<GUI::GridItemInfo>::push_back(GUI::GridItemInfo const&) array.h:144
    #4 0x10b446aca in GUI::GridWidget::sortGroups() grid.cpp:484
    #5 0x10b44e84f in GUI::GridWidget::groupEntries() grid.cpp:461
    #6 0x10b304d59 in GUI::LauncherGrid::groupEntries(Common::Array<Common::ConfigManager::Domain const*> const&) launcher.cpp:1413
    #7 0x10b3070c3 in GUI::LauncherGrid::updateListing() launcher.cpp:1546
    #8 0x10b30812c in GUI::LauncherGrid::build() launcher.cpp:1590
    #9 0x10b301882 in GUI::LauncherGrid::LauncherGrid(Common::String const&, GUI::LauncherChooser*) launcher.cpp:1307
    #10 0x10b2f62a4 in GUI::LauncherGrid::LauncherGrid(Common::String const&, GUI::LauncherChooser*) launcher.cpp:1306
    #11 0x10b2f60cb in GUI::LauncherChooser::selectLauncher() launcher.cpp:946
    #12 0x10988422d in launcherDialog() main.cpp:104
    #13 0x109881c04 in scummvm_main main.cpp:565
    #14 0x109877676 in main macosx-main.cpp:44
    #15 0x7fff205ebf3c in start+0x0 (libdyld.dylib:x86_64+0x15f3c)

previously allocated by thread T0 here:
    #0 0x10f6f53a0 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x443a0)
    #1 0x10b31149e in Common::Array<GUI::GridItemInfo>::allocCapacity(unsigned int) array.h:389
    #2 0x10b310785 in Common::Array<GUI::GridItemInfo>::insert_aux(GUI::GridItemInfo*, GUI::GridItemInfo const*, GUI::GridItemInfo const*) array.h:429
    #3 0x10b307955 in Common::Array<GUI::GridItemInfo>::push_back(GUI::GridItemInfo const&) array.h:144
    #4 0x10b446aca in GUI::GridWidget::sortGroups() grid.cpp:484
    #5 0x10b44e84f in GUI::GridWidget::groupEntries() grid.cpp:461
    #6 0x10b304d59 in GUI::LauncherGrid::groupEntries(Common::Array<Common::ConfigManager::Domain const*> const&) launcher.cpp:1413
    #7 0x10b3070c3 in GUI::LauncherGrid::updateListing() launcher.cpp:1546
    #8 0x10b30812c in GUI::LauncherGrid::build() launcher.cpp:1590
    #9 0x10b301882 in GUI::LauncherGrid::LauncherGrid(Common::String const&, GUI::LauncherChooser*) launcher.cpp:1307
    #10 0x10b2f62a4 in GUI::LauncherGrid::LauncherGrid(Common::String const&, GUI::LauncherChooser*) launcher.cpp:1306
    #11 0x10b2f60cb in GUI::LauncherChooser::selectLauncher() launcher.cpp:946
    #12 0x10988422d in launcherDialog() main.cpp:104
    #13 0x109881c04 in scummvm_main main.cpp:565
    #14 0x109877676 in main macosx-main.cpp:44
    #15 0x7fff205ebf3c in start+0x0 (libdyld.dylib:x86_64+0x15f3c)

@sev-
Copy link
Member

@sev- sev- commented Jun 9, 2022

This PR also did not fix groupedlist.cpp, I did that in a follow-up commit.

The above crash happens both on startup, if grid is selected, and when switching from list to grid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants