nothings / stb

stb single-file public domain libraries for C/C++
https://twitter.com/nothings
Other
26.31k stars 7.69k forks source link

stb_ds.h : Valgrind detects a direct memory leak #1488

Open avivbeeri opened 1 year ago

avivbeeri commented 1 year ago

Hi there,

I use stb_ds.h in a project of mine for hash maps. When I compile and run it with valgrind's memory leak checks enabled on linux, it detects a "direct leak".

Direct leak of 256 byte(s) in 1 object(s) allocated from:
    #0 0x4ba289 in __interceptor_realloc (BuildId: 06492acfb83cea26dc432b497c12f283ec931dab)
    #1 0x4f8863 in stbds_arrgrowf src/include/stb_ds.h:785:7
    #2 0x4f9337 in stbds_hmget_key_ts src/include/stb_ds.h:1286:9
    #3 0x4f9718 in stbds_hmget_key src/include/stb_ds.h:1315:13

This seems to land on this line: 785| b = STBDS_REALLOC(NULL, (a) ? stbds_header(a) : 0, elemsize * min_cap + sizeof(stbds_array_header));

I can't seem to figure out what the actual leak is, because this all seems sensible to me.

nothings commented 1 year ago

That's just the line it's allocated from, the leak is that it's never being freed. Are you not freeing the hashmap you're creating?

avivbeeri commented 1 year ago

I definitely do. Strangely, valgrind seemed satisfied after I gave the hash map a default value.

My use-case is a little complicated, so I'll try to extract a minimal reproduction when I get a chance.

josealvim commented 2 months ago

Had a similar problem and I can confirm that valgrind does not report a leak when I set a default value for the hash map. I'll set try and set up a minimal example momentarily.

Edit: I tried to recreate it from the ground up, but it seems pretty finicky. The path that supposedly leaks is part of an unification algorithm, it just copies the map corresponding with the variable bindings so it can backtrack if the unification fails. It seems to leak in an unexpected function, which performs variable resolution in a variable binding context:

static Id bind_resolve(struct bind* bind, Id id) {
    while (id < 0) {
        Id resolved = hmget(bind, id);
        if (resolved == 0)
            return id;
        id = resolved;
    }
    return id;
}

It doesn't seem to care if I replace the if == 0 with a hmgeti >= 0, if I don't set a default value, it leaks regardless. With a default of 0 those two alternatives work fine and valgrind doesn't accuse any other leaks in the program.

Now, apparently if I change the function to take a struct bind** and pass *bind to hmget it makes valgrind happy without the hmdefault. Presumably something's happening to the hash map and the caller doesn't get to know their pointer got messed around with. I'd have expected hmget to be more const-y which is why I didn't consider passing a pointer to the map.