nothings / stb

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

stb_ds.h: shputs with the same key twice can cause crash #1332

Closed skippuku closed 2 years ago

skippuku commented 2 years ago

Describe the bug Under certain circumstances (depending on random seed and at what point the key is inserted again) if a struct is inserted and it has the same string key as a previously inserted struct, it can cause the key not to be found or even a crash.

Crash only happens when using sh_new_strdup. When the map is setup with sh_new_arena, the key is simply not found. Manual memory management gives the same result as sh_new_arena.

To Reproduce compile and run this test program: https://gist.github.com/cyman-ide/bac6538fba0b3197544b2a5d33ed2ace

Expected behavior Linked test program runs without failures.

Screenshots Test run with sh_new_strdup: image

Test run with sh_new_arena (or manual string management): image

nothings commented 2 years ago

This is a bug introduced by an incomplete change in https://github.com/nothings/stb/pull/993. (Before that change, strdup/arena string hash tables didn't support struct API calls at all.)

The code to detect the presence of an existing key is split into two loops, and only one of them was fixed in the above change.

The correct code is in lines 1394-1397 at https://github.com/nothings/stb/blob/master/stb_ds.h#L1394:

            stbds_temp(a) = bucket->index[i];
            if (mode >= STBDS_HM_STRING)
              stbds_temp_key(a) = * (char **) ((char *) raw_a + elemsize*bucket->index[i] + keyoffset);
            return STBDS_ARR_TO_HASH(a,elemsize);

The same code needs to be replaced into 1412-1413 at https://github.com/nothings/stb/blob/master/stb_ds.h#L1412, which is currently:

            stbds_temp(a) = bucket->index[i];
            return STBDS_ARR_TO_HASH(a,elemsize);

Fixed for next release, but please try this change locally and verify it fixes your problem. If not, reopen.