aviggiano / redis-roaring

Roaring Bitmaps for Redis
MIT License
348 stars 56 forks source link

‘oldruns’ may be used after ‘realloc’ [-Wuse-after-free],I encountered the following exception when compiling with GCC 12. Will it have any impact on runtime? #106

Closed xl-xueling closed 7 months ago

xl-xueling commented 7 months ago

/opt/test/redis-roaring/src/roaring.c:5099:28: warning: pointer ‘oldruns’ may be used after ‘realloc’ [-Wuse-after-free] 5099 | if (src->runs == NULL) free(oldruns); // should never happen? | ^~~~~ /opt/test/redis-roaring/src/roaring.c:5098:28: note: call to ‘realloc’ here 5098 | src->runs = (rle16_t )realloc(oldruns, src->capacity sizeof(rle16_t)); | ^~~~~~~~~~~~~ /opt/test/redis-roaring/src/roaring.c: In function ‘run_container_grow’: /opt/test/redis-roaring/src/roaring.c:5139:32: warning: pointer ‘oldruns’ may be used after ‘realloc’ [-Wuse-after-free] 5139 | if (run->runs == NULL) free(oldruns); | ^~~~~ /opt/test/redis-roaring/src/roaring.c:5138:24: note: call to ‘realloc’ here 5138 | (rle16_t )realloc(oldruns, run->capacity sizeof(rle16_t)); | ^~~~~~~~~~~~~ /opt/test/redis-roaring/src/roaring.c: In function ‘array_container_shrink_to_fit’: /opt/test/redis-roaring/src/roaring.c:6525:31: warning: pointer ‘oldarray’ may be used after ‘realloc’ [-Wuse-after-free] 6525 | if (src->array == NULL) free(oldarray); // should never happen? | ^~~~~~ /opt/test/redis-roaring/src/roaring.c:6524:21: note: call to ‘realloc’ here 6524 | (uint16_t )realloc(oldarray, src->capacity sizeof(uint16_t)); | ^~~~~~~~~~~ /opt/test/redis-roaring/src/roaring.c: In function ‘array_container_grow’: /opt/test/redis-roaring/src/roaring.c:6562:39: warning: pointer ‘array’ may be used after ‘realloc’ [-Wuse-after-free] 6562 | if (container->array == NULL) free(array);

xl-xueling commented 7 months ago

I modified the configuration in the CMakeLists.txt file as follows: set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99 -Wall -Werror -Wno-error=use-after-free"), which allowed the compilation to pass. However, I am uncertain whether there will be any issues during runtime.

xl-xueling commented 7 months ago

and I compiled the project on Debian 12.4 system.

lemire commented 7 months ago

@xl-xueling Why do you say that they are 'exceptions'? What do you mean? There is no exception in C as far as I know.

You printed an output with warnings. Warnings are specifically not errors.

xl-xueling commented 7 months ago

@lemire hi , I tested on Debian 10 and Debian 11 systems, and everything was fine. However, when compiling on Debian 12(gcc12), I encountered errors, not warnings.

The Logs:

[ 10%] Building C object CMakeFiles/unit.dir/src/data-structure.c.o /usr/bin/cc -I/opt/soft/redis-roaring/deps/CRoaring -I/opt/soft/redis-roaring/deps/hiredis -I/opt/soft/redis-roaring/deps/sds -I/opt/soft/redis-roaring/src -std=gnu99 -Wall -Werror -g -MD -MT CMakeFiles/unit.dir/src/data-structure.c.o -MF CMakeFiles/unit.dir/src/data-structure.c.o.d -o CMakeFiles/unit.dir/src/data-structure.c.o -c /opt/soft/redis-roaring/src/data-structure.c [ 20%] Building C object CMakeFiles/unit.dir/src/roaring.c.o /usr/bin/cc -I/opt/soft/redis-roaring/deps/CRoaring -I/opt/soft/redis-roaring/deps/hiredis -I/opt/soft/redis-roaring/deps/sds -I/opt/soft/redis-roaring/src -std=gnu99 -Wall -Werror -g -MD -MT CMakeFiles/unit.dir/src/roaring.c.o -MF CMakeFiles/unit.dir/src/roaring.c.o.d -o CMakeFiles/unit.dir/src/roaring.c.o -c /opt/soft/redis-roaring/src/roaring.c /opt/soft/redis-roaring/src/roaring.c: In function ‘run_container_shrink_to_fit’: /opt/soft/redis-roaring/src/roaring.c:5099:28: error: pointer ‘oldruns’ may be used after ‘realloc’ [-Werror=use-after-free] 5099 | if (src->runs == NULL) free(oldruns); // should never happen? | ^~~~~ /opt/soft/redis-roaring/src/roaring.c:5098:28: note: call to ‘realloc’ here 5098 | src->runs = (rle16_t )realloc(oldruns, src->capacity sizeof(rle16_t)); | ^~~~~~~~~~~~~ /opt/soft/redis-roaring/src/roaring.c: In function ‘run_container_grow’: /opt/soft/redis-roaring/src/roaring.c:5139:32: error: pointer ‘oldruns’ may be used after ‘realloc’ [-Werror=use-after-free] 5139 | if (run->runs == NULL) free(oldruns); | ^~~~~ /opt/soft/redis-roaring/src/roaring.c:5138:24: note: call to ‘realloc’ here 5138 | (rle16_t )realloc(oldruns, run->capacity sizeof(rle16_t)); | ^~~~~~~~~~~~~ /opt/soft/redis-roaring/src/roaring.c: In function ‘array_container_shrink_to_fit’: /opt/soft/redis-roaring/src/roaring.c:6525:31: error: pointer ‘oldarray’ may be used after ‘realloc’ [-Werror=use-after-free] 6525 | if (src->array == NULL) free(oldarray); // should never happen? | ^~~~~~ /opt/soft/redis-roaring/src/roaring.c:6524:21: note: call to ‘realloc’ here 6524 | (uint16_t )realloc(oldarray, src->capacity sizeof(uint16_t)); | ^~~~~~~~~~~ /opt/soft/redis-roaring/src/roaring.c: In function ‘array_container_grow’: /opt/soft/redis-roaring/src/roaring.c:6562:39: error: pointer ‘array’ may be used after ‘realloc’ [-Werror=use-after-free] 6562 | if (container->array == NULL) free(array); | ^~~ /opt/soft/redis-roaring/src/roaring.c:6561:25: note: call to ‘realloc’ here 6561 | (uint16_t )realloc(array, new_capacity sizeof(uint16_t)); | ^~~~~~~~~~~ cc1: all warnings being treated as errors make[2]: [CMakeFiles/unit.dir/build.make:93: CMakeFiles/unit.dir/src/roaring.c.o] Error 1 make[2]: Leaving directory '/opt/soft/redis-roaring/build' make[1]: [CMakeFiles/Makefile2:90: CMakeFiles/unit.dir/all] Error 2 make[1]: Leaving directory '/opt/soft/redis-roaring/build' make: ** [Makefile:104: all] Error 2 find: ‘libredis-roaring’: No such file or directory

lemire commented 7 months ago

@xl-xueling

I encountered errors, not warnings.

No. You did not. You encountered warnings.

You are compiling with -Werror which deliberately turns warnings into errors.

It comes from this line in the project:

https://github.com/aviggiano/redis-roaring/blob/4fa6e4ebebe72cea5d23b4791812e09d4973d35c/CMakeLists.txt#L5

See the -Werror? Remove this and the code will build.

I'll see about issue a PR on this project.

xl-xueling commented 7 months ago

ok,i see, I just want to confirm whether this line of code poses any risk of memory leaks.

lemire commented 7 months ago

I have issued a PR which solves the issue: https://github.com/aviggiano/redis-roaring/pull/107

xl-xueling commented 7 months ago

ok, thanks ~