close
Skip to content

Skip memcpy in ImVector::operator= if Data isn't initialized#8874

Closed
i25e wants to merge 1 commit into
ocornut:masterfrom
i25e:imvectorcopyassignment
Closed

Skip memcpy in ImVector::operator= if Data isn't initialized#8874
i25e wants to merge 1 commit into
ocornut:masterfrom
i25e:imvectorcopyassignment

Conversation

@i25e
Copy link
Copy Markdown
Contributor

@i25e i25e commented Aug 10, 2025

ImVector::operator= doesn't properly copy a vector with Size = 0 and non-nil Data. It always sets the destination vector's Data pointer to NULL before resizing it to hold the same amount of items in the source vector. But when src.Size = 0 the destination vector won't have its Data pointer initialized, and src.Data is not null which results in a call to memcpy(NULL, src.Data, 0). This causes a crash when the program is compiled with -fsanitize=undefined -fno-sanitize-recover. The fix is to check if src.Data and this->Data are both non-nil.

I encountered this by accidentally using auto instead of auto& and causing a copy assignment of ImGuiIO::InputQueueCharacters. EndFrame() calls InputQueueCharacters.resize(0) which sets its Size to 0 but leaves its Data untouched (which is non-NULL once keys have been pressed). Afterwards, trying to make a new copy of the object returned from GetIO() invokes ImVector's copy constructor which calls ImVector::operator= and causes the error above.

Here's is a stripped down example tested with c++ -fsanitize=undefined $(pkg-config --cflags --libs sdl2) -I imgui imgui/backends/imgui_impl_opengl3.cpp imgui/backends/imgui_impl_sdl2.cpp imgui/*.cpp test.cc:

#include <SDL.h>
#include "imgui.h"
#include "backends/imgui_impl_sdl2.h"
#include "backends/imgui_impl_opengl3.h"

int
main(void)
{
    SDL_Window *window = SDL_CreateWindow("",
                                          SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED,
                                          128, 128, SDL_WINDOW_OPENGL);
    SDL_GLContext context = SDL_GL_CreateContext(window);

    ImGui::CreateContext();
    ImGui_ImplSDL2_InitForOpenGL(window, context);
    ImGui_ImplOpenGL3_Init("#version 330");

    for (int quit = 0; quit == 0; ) {
        ImGui_ImplOpenGL3_NewFrame();
        ImGui_ImplSDL2_NewFrame();
        ImGui::NewFrame();

        for (SDL_Event e; SDL_PollEvent(&e); ) {
            ImGui_ImplSDL2_ProcessEvent(&e);
            switch (e.type) {
            case SDL_QUIT:
                quit = 1;
                break;
            }

            ImGuiIO io = ImGui::GetIO(); // causes undefined behavior once keys are pressed
            ImVector<ImWchar> iqc = ImGui::GetIO().InputQueueCharacters; // same issue
        }

        ImGui::Render();
        ImGui_ImplOpenGL3_RenderDrawData(ImGui::GetDrawData());
        SDL_GL_SwapWindow(window);
    }

    // Minimal example
    ImVector<int> a, b;
    b.push_back(1); // b.Size = 1, b.Data != NULL
    b.resize(0);    // b.Size = 0, b.Data != NULL
    a = b;          // a::resize(0) doesn't initialize a.Data
                    // memcpy(NULL, b.Data, 0) is called
}

ocornut pushed a commit that referenced this pull request Aug 11, 2025
@ocornut
Copy link
Copy Markdown
Owner

ocornut commented Aug 11, 2025

Thank you Ian! This is now merged as ea075ed.

@ocornut ocornut closed this Aug 11, 2025
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.

2 participants