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

Valgrind "Invalid read of size 8" when using hashtable_get_values + array_remove_at #96

Closed
IvanRibakov opened this issue Jan 21, 2019 · 1 comment
Labels

Comments

@IvanRibakov
Copy link

Minimal scenario to reproduce the issue:

HashTable *ht;
hashtable_new(&ht);
hashtable_add(ht, "1", "1");
hashtable_add(ht, "2", "2");
hashtable_add(ht, "3", "3");

Array *arr;
hashtable_get_values(ht, &arr);

printf("Array size: %i\n", (int)array_size(arr));

array_remove_at(arr, 0, NULL);

printf("Array size: %i\n", (int)array_size(arr));

Expecting to see

Array size: 3
Array size: 2

Getting

Array size: 3
==19255== Invalid read of size 8
==19255==    at 0x4C2DECE: memcpy@GLIBC_2.2.5 (vg_replace_strmem.c:1021)
==19255==    by 0x5E41A48: array_remove_at (in /.../reqs/lib/libcollectc.so)
==19255==    ...
==19255==  Address 0xb01b358 is 0 bytes after a block of size 24 alloc'd
==19255==    at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==19255==    by 0x5E414D4: array_new_conf (in /.../reqs/lib/libcollectc.so)
==19255==    by 0x5E459FF: hashtable_get_values (in /.../reqs/lib/libcollectc.so)
==19255==    ...
==19255==
Array size: 2

If I create array manually using array_new, I don't get the same issue. Looking at the source code of array_remove_at I can see there is a call to memmove (https://github.com/srdja/Collections-C/blob/master/src/array.c#L317) which I'm guessing is calling memcpy internally, but unfortunately I have no guesses as to why that produces the error.

Any ideas what might be causing it and whether there is a way to work around it?

@srdja srdja added the bug label Feb 21, 2019
@srdja
Copy link
Owner

srdja commented Feb 21, 2019

Good find! I've pushed a fix. It was an off by on error in array_remove_at in which size was used instead of the highest index to get the moved block size.

@srdja srdja closed this as completed Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants