GCC warning about overflow in SPEX
mmuetzel opened this issue · 8 comments
GCC warns about an out of bounds access in SPEX.
See, e.g., the Ubuntu runner in CI:
[ 38%] Building C object SPEX/CMakeFiles/SPEX_static.dir/SPEX_Util/Source/SPEX_matrix_allocate.c.o
In file included from /home/runner/work/SuiteSparse/SuiteSparse/SPEX/SPEX_Util/Source/SPEX_matrix_allocate.c:22:
/home/runner/work/SuiteSparse/SuiteSparse/SPEX/SPEX_Util/Source/SPEX_matrix_allocate.c: In function ‘SPEX_matrix_allocate’:
/home/runner/work/SuiteSparse/SuiteSparse/SPEX/SPEX_Util/Source/SPEX_matrix_allocate.c:98:17: warning: ‘SPEX_mpq_init’ accessing 32 bytes in a region of size 4 [-Wstringop-overflow=]
98 | SPEX_CHECK (SPEX_mpq_init (A->scale)) ;
/home/runner/work/SuiteSparse/SuiteSparse/SPEX/SPEX_Util/Source/spex_util_internal.h:100:13: note: in definition of macro ‘SPEX_CHECK’
100 | info = (method) ; \
| ^~~~~~
/home/runner/work/SuiteSparse/SuiteSparse/SPEX/SPEX_Util/Source/SPEX_matrix_allocate.c:98:17: note: referencing argument 1 of type ‘__mpq_struct *’
98 | SPEX_CHECK (SPEX_mpq_init (A->scale)) ;
/home/runner/work/SuiteSparse/SuiteSparse/SPEX/SPEX_Util/Source/spex_util_internal.h:100:13: note: in definition of macro ‘SPEX_CHECK’
100 | info = (method) ; \
| ^~~~~~
In file included from /home/runner/work/SuiteSparse/SuiteSparse/SPEX/SPEX_Util/Source/spex_util_internal.h:108,
from /home/runner/work/SuiteSparse/SuiteSparse/SPEX/SPEX_Util/Source/SPEX_matrix_allocate.c:22:
/home/runner/work/SuiteSparse/SuiteSparse/SPEX/Include/SPEX.h:802:11: note: in a call to function ‘SPEX_mpq_init’
802 | SPEX_info SPEX_mpq_init (mpq_t x) ;
| ^~~~~~~~~~~~~
I don't know if this is a bogus warning or if there is really an out-of-bounds issue.
My SPEX co-authors and I have seen this too. We've looked at it closely and we are fairly sure that it is a spurious warning.
A->scale is an mpq_t type, and so is the input to SPEX_mpq_init. We have also tested this code under valgrind, and it doesn't report any errors.
It's a puzzle as to why some gcc versions (not all) complain about this statement. gcc 9.4.0 and gcc 13.2.0 don't complain, at least on my machine with MPFR 4.2.0, but gcc 11.4.0 does complain in the CI (with MPFR 4.1.0).
That leads me to believe that the warning is spurious.
I tried it with MPFR 4.1.0 and gcc 13.2.0, and got no warnings, on my local machine.
Correction... the function mpq_init is in GMP, not MPFR. SPEX_mpq_init is a simple wrapper around mpq_init, described here: https://gmplib.org/manual/Initializing-Rationals
and it looks to me that we are calling it correctly. sizeof(mpq_t) and sizeof (A->scale) are both 32.
I installed gcc 11.4.0 on my desktop via spack. Using it to compile SPEX causes this warning to be emitted. If instead I use gcc 9.4.0 or gcc 13.2.0 (the latter being the most recent version of gcc on spack), then I get no warning.
I tried -Wstringop-overflow=n for n = 0 to 4 (I think 0 disables the warning but I'm not sure). n=0 makes the warning go away with gcc 11.4.0.
gcc 13.2.0 reports no warning, regardless of the -Wstringop-overflow=n flag, for any n.
Since a more recent gcc is silent about this (13.2.0), and an older gcc complains about it (11.4.0), and a still older gcc is also silent (9.4.0) I think this must be a spurious warning from an overly zealous check in gcc 11.4.0 that is now gone in gcc 13.2.0.
I think it's because of this bug in gcc 11, that was fixed in gcc 12:
I fixed this issue just now, by simply disabling -Wstringop-overflow for gcc 11 when compiling SPEX. See #716
I think this is fixed now; I just need to check the CI results.
The logs look fine now.