google / s2geometry

Computational geometry and spatial indexing on the sphere
http://s2geometry.io/
Apache License 2.0
2.29k stars 302 forks source link

Array out of bound warning. #355

Open Vedingrot opened 6 months ago

Vedingrot commented 6 months ago

Hi, there is -Warray-bound warning. It appears when compile with gcc13 and -Wall -O2.

warning:

In file included from /home/qemu_ved/libs2geometry-0.11.1/src/s2/r2.h:22,
                 from /home/qemu_ved/libs2geometry-0.11.1/src/s2/s2edge_clipping.h:37,
                 from /home/qemu_ved/libs2geometry-0.11.1/src/s2/s2edge_clipping.cc:18:
In member function ‘T util::math::internal_vector::BasicVector<VecTemplate, T, N>::operator[](int) const [with VecTemplate = Vector2; T = double; long unsigned int N = 2]’,
    inlined from ‘int S2::GetNextFace(int, const R2Point&, int, const S2PointUVW&, int)’ at /home/qemu_ved/libs2geometry-0.11.1/src/s2/s2edge_clipping.cc:195:25,
    inlined from ‘void S2::GetFaceSegments(const S2Point&, const S2Point&, FaceSegmentVector*)’ at /home/qemu_ved/libs2geometry-0.11.1/src/s2/s2edge_clipping.cc:249:23:
/home/qemu_ved/libs2geometry-0.11.1/src/s2/util/math/vector.h:97:49: warning: array subscript 5 is outside array bounds of ‘S2::FaceSegment [1]’ [-Warray-bounds=]
   97 |     return static_cast<const D&>(*this).Data()[b];
      |                                                 ^
/home/qemu_ved/libs2geometry-0.11.1/src/s2/s2edge_clipping.cc: In function ‘void S2::GetFaceSegments(const S2Point&, const S2Point&, FaceSegmentVector*)’:
/home/qemu_ved/libs2geometry-0.11.1/src/s2/s2edge_clipping.cc:212:15: note: at offset 40 into object ‘segment’ of size 40
  212 |   FaceSegment segment;
      |               ^~~~~~~

build:

mkdir build/ && cd build/
cmake -DCMAKE_POSITION_INDEPENDENT_CODE=ON \
           -DBUILD_TESTS=OFF  \
           -DCMAKE_CXX_STANDARD=17 \
          '-DCMAKE_CXX_FLAGS=-Wall -g -O2' \
           -DCMAKE_PREFIX_PATH=/home/qemu_ved/abseil_install_root/ ..
make -j10 VERBOSE=1

gcc version:

gcc -v
Using built-in specs.
COLLECT_GCC=x86_64-alt-linux-gcc
COLLECT_LTO_WRAPPER=/usr/lib64/gcc/x86_64-alt-linux/13/lto-wrapper
Target: x86_64-alt-linux
Configured with: ../configure --host=x86_64-alt-linux --build=x86_64-alt-linux --target=x86_64-alt-linux --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var/lib --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --disable-dependency-tracking --disable-silent-rules --without-included-gettext --enable-shared --program-suffix=-13 --with-slibdir=/lib64 --libexecdir=/usr/lib64 --with-bugurl=http://bugzilla.altlinux.org --enable-__cxa_atexit --enable-threads=posix --enable-checking=release --with-system-zlib --with-zstd --without-included-gettext --enable-multilib --enable-default-pie --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --with-arch_32=i586 --with-tune_32=generic --with-multilib-list=m64,m32,mx32 --with-gcc-major-version-only --enable-vtable-verify --enable-bootstrap --with-build-config=bootstrap-lto --enable-link-serialization=1 --enable-languages=c,c++,fortran,objc,obj-c++,ada,go,d,lto --enable-plugin
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.2.1 20240128 (ALT Sisyphus 13.2.1-alt3) (GCC) 

build_log.txt

jmr commented 6 months ago

I think this is a false positive.

The apparently offending access on s2edge_clipping.cc:195 is exit[1 - axis].

s2edge_clipping.cc:249 is GetNextFace(..., exit_axis, ...)

https://github.com/google/s2geometry/blob/7940c9f791819211782bd7df51632657172c17c2/src/s2/s2edge_clipping.cc#L249

exit_axis is always 0 or 1, so this should be in-bounds.

Can you demonstrate a runtime failure? Vector::operator[] has a DCHECK of its argument.