TableGetSortSpecs can return structure with corrupt ImGuiTableSortSpecs::Specs #4233
Description
Version/Branch of Dear ImGui:
Version: 1.84 WIP (18303)
Branch: master (commit d5828cd)
Back-end/Renderer/Compiler/OS
Back-ends: imgui_impl_opengl3.cpp + imgui_impl_glfw.cpp
Compiler: clang 10
Operating System: linux (ubuntu)
My Issue/Question:
TableGetSortSpecs can return structure with a corrupt ImGuiTableSortSpecs::Specs if table sort order hasn't been dirtied.
The contents of the array referenced by ImGuiTableSortSpecs::Specs might also be well formed (but invalid for the table) or point to pure junk.
This matters if you want to know the sort spec for a table even if the spec hasn't changed.
e.g. Your backing store occasionally invalidates and you need to reapply sorting.
Cause is described below. In this context an 'ephemeral' alloc is a mem alloc that is only valid for the duration of a BeginTableEx/EndTable span.
bool ImGui::BeginTableEx(...)
{
... // snip - irrelevant
// !! Ephemeral storage allocated in `TempData`
ImGuiTableTempData* temp_data = table->TempData = &g.TablesTempDataStack[g.CurrentTableStackIdx];
temp_data->TableIndex = table_idx;
... // snip - irrelevant
}
ImGuiTableSortSpecs* ImGui::TableGetSortSpecs()
{
... // snip - doesn't matter
if (table->IsSortSpecsDirty)
TableSortSpecsBuild(table); // <--- SortSpecs BUT ONLY IF DIRTY
// `ImGuiTableSortSpecs::Specs` points to invalid ephemeral alloc from previous begin/end for this table
return &table->SortSpecs;
}
void ImGui::TableSortSpecsBuild(ImGuiTable* table)
{
IM_ASSERT(table->IsSortSpecsDirty);
TableSortSpecsSanitize(table);
// `Specs` points to ephemeral memory
ImGuiTableTempData* temp_data = table->TempData;
temp_data->SortSpecsMulti.resize(table->SortSpecsCount <= 1 ? 0 : table->SortSpecsCount);
ImGuiTableColumnSortSpecs* sort_specs =
(table->SortSpecsCount == 0) ? NULL :
(table->SortSpecsCount == 1) ? &temp_data->SortSpecsSingle :
temp_data->SortSpecsMulti.Data;
... // snip - doesn't matter
table->SortSpecs.Specs = sort_specs; // <-- POINTS TO EPHEMERAL ALLOC
table->SortSpecs.SpecsCount = table->SortSpecsCount;
table->SortSpecs.SpecsDirty = true; // Mark as dirty for user
table->IsSortSpecsDirty = false; // Mark as not dirty for us
}
Header/docs states the following invariants for TableGetSortSpecs:
// - Call TableGetSortSpecs() to retrieve latest sort specs for the table. NULL when not sorting.
// - When 'SpecsDirty == true' you should sort your data.
// - Lifetime: don't hold on this pointer over multiple frames or past any subsequent call to BeginTable().
Docs for ImGuiTableSortSpecs don't impose any additional invariants.
Screenshots/Video
N/A. Not a layout/visual issue.
Example
This demo draws a full-display window containing two sortable tables.
Click on column header B to change the sorting spec of the first table. This will trigger an assertion when drawing the second table, demonstrating that its sort spec has been (visibly) corrupted.
This sample could trigger an immediate crash if Specs were allocated using a mechanism that doesn't reuse previous allocations.
On my system/run the Specs member points to the same address for both tables. (Legal, but impl doesn't update the array for the second table.)
Source Code:
using namespace ImGui;
auto drawTable = [&](char const* name, std::vector<char const*> columns) {
if (BeginTable(name, columns.size(), ImGuiTableFlags_Sortable)) {
for (auto&& col : columns)
TableSetupColumn(col, ImGuiTableColumnFlags_DefaultSort);
TableHeadersRow();
if (auto specs = TableGetSortSpecs()) {
std::cerr << specs << "\n";
for (int i = 0; i < specs->SpecsCount; ++i) {
auto&& info = specs->Specs[i];
assert(0 <= info.ColumnIndex);
assert(size_t(info.ColumnIndex) < columns.size());
}
}
}
EndTable();
};
auto& io = GetIO();
SetNextWindowSize(io.DisplaySize, ImGuiCond_Always);
SetNextWindowPos({0, 0}, ImGuiCond_Always, {0, 0});
if (Begin("__background__", {})) {
std::cerr << "drawing tables...\n";
drawTable("###table_col_2", {"A", "B"});
drawTable("###table_col_1", {"C"});
}
End();
Full about dump:
Dear ImGui 1.84 WIP (18303)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201703
define: __linux__
define: __GNUC__=4
define: __clang_version__=10.0.1
--------------------------------
io.BackendPlatformName: imgui_impl_glfw
io.BackendRendererName: imgui_impl_opengl3
io.ConfigFlags: 0x00000001
NavEnableKeyboard
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
HasMouseCursors
HasSetMousePos
RendererHasVtxOffset
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,2048
io.DisplaySize: 1848.00,1016.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 5.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00