table pointer is NULL about hashmap

[hoxsiew]

This project has been fairly widely used, including in several products in at least two companies, so I hesitate to suspect there >is a fundamental issue with the code.

You can add my company, PercayAI . We use it extensively in both our academic and commercial bioinformatics tool backends.

[DavidLeeds]

Make sure to call hashmap_init() and ensure no error is returned prior to using the hashmap. Does that resolve your issue?

[kencb]

Yes, I do. hashmap_base_init() will clear the struct but the table pointer is never allocated. It's just null.

[DavidLeeds]

The data structure doesn't allocate any memory until you put or reserve. This is by design. If you are referring to this code in rehash()

hashmap/src/hashmap.c

Line 187 in dcf30be

for (entry = old_table; entry < &old_table[old_size]; ++entry) {

, the operation on the null pointer is not an issue, because we are never accessing memory at that location. It is just calculating an offset.

What compiler are you using? This code has been tested on GCC and Clang, but I suppose it is possible that it could run into an issue with another compiler. This project has been fairly widely used, including in several products in at least two companies, so I hesitate to suspect there is a fundamental issue with the code.

[kencb]

Yes that is the code in question. Compiler is clang and the error comes from the memory debugging I have turned on (in Xcode) so it's not a compiler or building problem. With no diagnostics enabled it would run just fine. Personally I prefer 'clean' code in that regard so it would be nice if a null pointer was handled differently here. I changed it to return early if the pointer is null without doing the comparison.

[DavidLeeds]

I would argue that there is nothing unclean about &old_table[old_size], since it is identical to &(*(old_table + old_size)). Nothing actually attempts to access the memory at old_table + old_size = 0 + 0 = 0.

That said, we could certainly change the code to avoid triggering static analysis tools that aren't aware of this.

for (entry = old_table; entry < old_table + old_size; ++entry) { 

Would you like to put up a pull request with the changes needed to satisfy your tool?

[kencb]

Yeah I don't really claim the code is unclean, just that the memory diagnostic is warning about it when running the code and I like to avoid that even though it's something that would be turned off in a release build. Same as I prefer to get any compiler warnings fixed when building.

Overall I like the hashmap so far. I was looking for something that is simple, I can provide the key, hash and compare functions and does not depend on lots of STL stuff.

The simple fix that works is to return right before the for-loop if old_table is NULL. That is the end result even if the for-loop comparison is done.

if (old_table == NULL) return 0;

[FluffyFoxUwU]

The data structure doesn't allocate any memory until you put or reserve. This is by design. If you are referring to this code in rehash()

hashmap/src/hashmap.c

Line 187 in dcf30be

for (entry = old_table; entry < &old_table[old_size]; ++entry) {

, the operation on the null pointer is not an issue, because we are never accessing memory at that location. It is just calculating an offset.

What compiler are you using? This code has been tested on GCC and Clang, but I suppose it is possible that it could run into an issue with another compiler. This project has been fairly widely used, including in several products in at least two companies, so I hesitate to suspect there is a fundamental issue with the code.

UBSan detect that bit of code as undefined behaviour and should be changed