google / s2geometry

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

Bug with address sanitizer #382

Open FAguGomez opened 2 months ago

FAguGomez commented 2 months ago

Hello, I'm trying to use this library to process some shapefiles. I've compiled my app using address sanitizer and this is stopping my program execution (see details below). Now I've been roaming around looking for something suspicious in my app but to no avail. I managed to create a minimal example to reproduce this error.

This are the used libraries and compiler versions:

Now, this is the command line that I use to compile abseil:

cmake -S . -B build/ -DBUILD_SHARED_LIBS=OFF -DBUILD_TESTING=OFF -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=17 -DABSL_PROPAGATE_CXX_STD=ON -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DCMAKE_INSTALL_PREFIX=$INSTALL_PREFIX
cmake --build build/ -j4
cmake --install build/

And this one for S2:

cmake  -S . -B build/ -DBUILD_SHARED_LIBS=OFF -DBUILD_TESTS=OFF -DCMAKE_PREFIX_PATH=$INSTALL_PREFIX -DCMAKE_CXX_STANDARD=17 -DOPENSSL_INCLUDE_DIR=/usr/include/openssl -DCMAKE_INSTALL_PREFIX=$INSTALL_PREFIX 
make -C build -j4
make -C build install

The minimal example is the following:

#include <vector>
#include <s2/s2loop.h>
#include <s2/s2point.h>
#include <s2/s2latlng.h>
#include <s2/s2polygon.h>

int main()
{
    std::vector<S2Point> points;
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392984, -0.743239).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392983, -0.743238).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392982, -0.743237).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.39298, -0.743237).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392978, -0.743237).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392976, -0.743238).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392974, -0.743239).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392973, -0.743241).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392972, -0.743242).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392971, -0.743243).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392971, -0.743245).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392972, -0.743245).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392974, -0.743246).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392975, -0.743246).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392976, -0.743246).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392978, -0.743245).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.39298, -0.743244).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392982, -0.743243).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392983, -0.743241).ToPoint()));
    points.push_back(S2Point(S2LatLng::FromDegrees(0.392984, -0.74324).ToPoint()));

    std::vector<std::unique_ptr<S2Loop>> loops;

    // This is the line where the error occurs
    loops.push_back(std::make_unique<S2Loop>(points)); 

    loops.back()->Normalize();

    auto polygon = new S2Polygon(std::move(loops));
    delete(polygon);
}

This is how I compiled the example:

g++ -O0 -ggdb3 -Wall -fsanitize=address -I $INSTALL_PREFIX/include/ -L $INSTALL_PREFIX/lib/ -o s2test s2test.cpp -ls2 -labsl_log_internal_message -labsl_examine_stack -labsl_symbolize -labsl_synchronization -labsl_base -labsl_hash -labsl_low_level_hash -labsl_city -labsl_civil_time -labsl_stacktrace -labsl_debugging_internal -labsl_demangle_internal -labsl_demangle_rust -labsl_decode_rust_punycode -labsl_failure_signal_handler -labsl_flags_internal -labsl_flags_commandlineflag -labsl_flags_commandlineflag_internal -labsl_flags_config -labsl_flags_marshalling -labsl_flags_parse -labsl_flags_reflection -labsl_flags_private_handle_accessor -labsl_flags_program_name -labsl_flags_usage -labsl_flags_usage_internal -labsl_graphcycles_internal -labsl_hashtablez_sampler -labsl_int128 -labsl_kernel_timeout_internal -labsl_log_entry -labsl_log_flags -labsl_log_globals -labsl_log_initialize -labsl_log_internal_check_op -labsl_log_internal_fnmatch -labsl_log_internal_format -labsl_log_internal_globals -labsl_log_internal_log_sink_set -labsl_log_sink -labsl_log_internal_nullguard -labsl_log_internal_proto -labsl_malloc_internal -labsl_random_internal_seed_material -labsl_random_seed_gen_exception -labsl_random_seed_sequences -labsl_raw_hash_set -labsl_raw_logging_internal -labsl_spinlock_wait -labsl_strerror -labsl_str_format_internal -labsl_strings -labsl_time -labsl_time_zone -labsl_utf8_for_code_point -lcrypto

And finally this is the error stack trace:

==38==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000000e (pc 0x5d83215a4018 bp 0x7ffcd398c990 sp 0x7ffcd398c980 T0)
==38==The signal is caused by a READ memory access.
==38==Hint: address points to the zero page.
    #0 0x5d83215a4018 in absl::lts_20240722::container_internal::btree_node<absl::lts_20240722::container_internal::map_params<S2CellId, S2ShapeIndexCell*, std::less<S2CellId>, std::allocator<std::pair<S2CellId const, S2ShapeIndexCell*> >, 256, false> >::finish() const INSTALL_PREFIX/include/absl/container/internal/btree.h:694
    #1 0x5d83215a22f0 in absl::lts_20240722::container_internal::btree<absl::lts_20240722::container_internal::map_params<S2CellId, S2ShapeIndexCell*, std::less<S2CellId>, std::allocator<std::pair<S2CellId const, S2ShapeIndexCell*> >, 256, false> >::end() const INSTALL_PREFIX/install/include/absl/container/internal/btree.h:1433
    #2 0x5d83215b8021 in absl::lts_20240722::container_internal::btree_container<absl::lts_20240722::container_internal::btree<absl::lts_20240722::container_internal::map_params<S2CellId, S2ShapeIndexCell*, std::less<S2CellId>, std::allocator<std::pair<S2CellId const, S2ShapeIndexCell*> >, 256, false> > >::end() const (/s2test+0x34021)
    #3 0x5d83215b4edc in MutableS2ShapeIndex::Iterator::InitStale(MutableS2ShapeIndex const*, S2ShapeIndex::InitialPosition) (/s2test+0x30edc)
    #4 0x5d83215ab9f0 in MutableS2ShapeIndex::Minimize() (/s2test+0x279f0)
    #5 0x5d83215ac025 in MutableS2ShapeIndex::ReleaseAll() (/s2test+0x28025)
    #6 0x5d83215ac082 in MutableS2ShapeIndex::Clear() (/s2test+0x28082)
    #7 0x5d83215f7254 in S2Loop::ClearIndex() (/s2test+0x73254)
    #8 0x5d83215f7286 in S2Loop::Init(absl::lts_20240722::Span<S2Point const>) (/s2test+0x73286)
    #9 0x5d83215f7116 in S2Loop::S2Loop(absl::lts_20240722::Span<S2Point const>, S2Debug) (/s2test+0x73116)
    #10 0x5d83215f7056 in S2Loop::S2Loop(absl::lts_20240722::Span<S2Point const>) (/s2test+0x73056)
    #11 0x5d83215a1b0f in std::__detail::_MakeUniq<S2Loop>::__single_object std::make_unique<S2Loop, std::vector<S2Point, std::allocator<S2Point> >&>(std::vector<S2Point, std::allocator<S2Point> >&) /usr/include/c++/12/bits/unique_ptr.h:1065
    #12 0x5d832159df0f in main /s2test.cpp:33
    #13 0x7af609157249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #14 0x7af609157304 in __libc_start_main_impl ../csu/libc-start.c:360
    #15 0x5d832159bfe0 in _start (/s2test+0x17fe0)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV INSTALL_PREFIX/include/absl/container/internal/btree.h:694 in absl::lts_20240722::container_internal::btree_node<absl::lts_20240722::container_internal::map_params<S2CellId, S2ShapeIndexCell*, std::less<S2CellId>, std::allocator<std::pair<S2CellId const, S2ShapeIndexCell*> >, 256, false> >::finish() const
==38==ABORTING

I found a workaround by compiling both libraries with address sanitizer by using this flag -DCMAKE_CXX_FLAGS=-fsanitize=address and adding this attribute __attribute__((no_sanitize_address)) to the s2 line where the error occurs which is the following:

inline void __attribute__((no_sanitize_address)) MutableS2ShapeIndex::Iterator::InitStale(
    const MutableS2ShapeIndex* index, InitialPosition pos) {
  index_ = index;
  end_ = index_->cell_map_.end();
  if (pos == BEGIN) {
    iter_ = index_->cell_map_.begin();
  } else {
    iter_ = end_;
  }
  Refresh();
}

I'm still a newbie when it comes to to this library, so feel free to point anything that seems off. Thank you!

smcallis commented 2 months ago

This looks for all the world to me like a segfault inside of absl::btree_map. One thing I notice is that the iterator code looks a little out of date from what's at HEAD now, can you update to the latest and make sure it still happens? Does it segfault with -fsanitize=memory too? If so can you run with -fsanitize-memory-track-origins to see if we can see where the originating nullptr comes from?

Sean

On Mon, Sep 23, 2024 at 2:12 PM FAguGomez @.***> wrote:

Hello, I'm trying to use this library to process some shapefiles. I've compiled my app using address sanitizer and this is stopping my program execution (see details below). Now I've been roaming around looking for something suspicious in my app but to no avail. I managed to create a minimal example to reproduce this error.

This are the used libraries and compiler versions:

  • abseil (version 20240722.0)
  • s2geometry (version 0.11.1)
  • gcc (Ubuntu 9.5.0-6ubuntu2) 9.5.0.

Now, this is the command line that I use to compile abseil:

cmake -S . -B build/ -DBUILD_SHARED_LIBS=OFF -DBUILD_TESTING=OFF -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=17 -DABSL_PROPAGATE_CXX_STD=ON -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DCMAKE_INSTALL_PREFIX=$INSTALL_PREFIX cmake --build build/ -j4 cmake --install build/

And this one for S2:

cmake -S . -B build/ -DBUILD_SHARED_LIBS=OFF -DBUILD_TESTS=OFF -DCMAKE_PREFIX_PATH=$INSTALL_PREFIX -DCMAKE_CXX_STANDARD=17 -DOPENSSL_INCLUDE_DIR=/usr/include/openssl -DCMAKE_INSTALL_PREFIX=$INSTALL_PREFIX make -C build -j4 make -C build install

The minimal example is the following:

include

include <s2/s2loop.h>

include <s2/s2point.h>

include <s2/s2latlng.h>

include <s2/s2polygon.h>

int main() { std::vector points; points.push_back(S2Point(S2LatLng::FromDegrees(0.392984, -0.743239).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392983, -0.743238).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392982, -0.743237).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.39298, -0.743237).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392978, -0.743237).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392976, -0.743238).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392974, -0.743239).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392973, -0.743241).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392972, -0.743242).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392971, -0.743243).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392971, -0.743245).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392972, -0.743245).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392974, -0.743246).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392975, -0.743246).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392976, -0.743246).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392978, -0.743245).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.39298, -0.743244).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392982, -0.743243).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392983, -0.743241).ToPoint())); points.push_back(S2Point(S2LatLng::FromDegrees(0.392984, -0.74324).ToPoint()));

std::vector<std::unique_ptr<S2Loop>> loops;

// This is the line where the error occurs
loops.push_back(std::make_unique<S2Loop>(points));

loops.back()->Normalize();

auto polygon = new S2Polygon(std::move(loops));
delete(polygon);

}

This is how I compiled the example:

g++ -O0 -ggdb3 -Wall -fsanitize=address -I $INSTALL_PREFIX/include/ -L $INSTALL_PREFIX/lib/ -o s2test s2test.cpp -ls2 -labsl_log_internal_message -labsl_examine_stack -labsl_symbolize -labsl_synchronization -labsl_base -labsl_hash -labsl_low_level_hash -labsl_city -labsl_civil_time -labsl_stacktrace -labsl_debugging_internal -labsl_demangle_internal -labsl_demangle_rust -labsl_decode_rust_punycode -labsl_failure_signal_handler -labsl_flags_internal -labsl_flags_commandlineflag -labsl_flags_commandlineflag_internal -labsl_flags_config -labsl_flags_marshalling -labsl_flags_parse -labsl_flags_reflection -labsl_flags_private_handle_accessor -labsl_flags_program_name -labsl_flags_usage -labsl_flags_usage_internal -labsl_graphcycles_internal -labsl_hashtablez_sampler -labsl_int128 -labsl_kernel_timeout_internal -labsl_log_entry -labsl_log_flags -labsl_log_globals -labsl_log_initialize -labsl_log_internal_check_op -labsl_log_internal_fnmatch -labsl_log_internal_format -labsl_log_internal_globals -labsl_log_internal_log_sink_set -labsl_log_sink -labsl_log_internal_nullguard -labsl_log_internal_proto -labsl_malloc_internal -labsl_random_internal_seed_material -labsl_random_seed_gen_exception -labsl_random_seed_sequences -labsl_raw_hash_set -labsl_raw_logging_internal -labsl_spinlock_wait -labsl_strerror -labsl_str_format_internal -labsl_strings -labsl_time -labsl_time_zone -labsl_utf8_for_code_point -lcrypto

And finally this is the error stack trace:

==38==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000000e (pc 0x5d83215a4018 bp 0x7ffcd398c990 sp 0x7ffcd398c980 T0) ==38==The signal is caused by a READ memory access. ==38==Hint: address points to the zero page.

0 0x5d83215a4018 in absl::lts_20240722::container_internal::btree_node<absl::lts_20240722::container_internal::map_params<S2CellId, S2ShapeIndexCell, std::less, std::allocator<std::pair<S2CellId const, S2ShapeIndexCell> >, 256, false> >::finish() const INSTALL_PREFIX/include/absl/container/internal/btree.h:694

#1 0x5d83215a22f0 in absl::lts_20240722::container_internal::btree<absl::lts_20240722::container_internal::map_params<S2CellId, S2ShapeIndexCell*, std::less<S2CellId>, std::allocator<std::pair<S2CellId const, S2ShapeIndexCell*> >, 256, false> >::end() const INSTALL_PREFIX/install/include/absl/container/internal/btree.h:1433
#2 0x5d83215b8021 in absl::lts_20240722::container_internal::btree_container<absl::lts_20240722::container_internal::btree<absl::lts_20240722::container_internal::map_params<S2CellId, S2ShapeIndexCell*, std::less<S2CellId>, std::allocator<std::pair<S2CellId const, S2ShapeIndexCell*> >, 256, false> > >::end() const (/s2test+0x34021)
#3 0x5d83215b4edc in MutableS2ShapeIndex::Iterator::InitStale(MutableS2ShapeIndex const*, S2ShapeIndex::InitialPosition) (/s2test+0x30edc)
#4 0x5d83215ab9f0 in MutableS2ShapeIndex::Minimize() (/s2test+0x279f0)
#5 0x5d83215ac025 in MutableS2ShapeIndex::ReleaseAll() (/s2test+0x28025)
#6 0x5d83215ac082 in MutableS2ShapeIndex::Clear() (/s2test+0x28082)
#7 0x5d83215f7254 in S2Loop::ClearIndex() (/s2test+0x73254)
#8 0x5d83215f7286 in S2Loop::Init(absl::lts_20240722::Span<S2Point const>) (/s2test+0x73286)
#9 0x5d83215f7116 in S2Loop::S2Loop(absl::lts_20240722::Span<S2Point const>, S2Debug) (/s2test+0x73116)
#10 0x5d83215f7056 in S2Loop::S2Loop(absl::lts_20240722::Span<S2Point const>) (/s2test+0x73056)
#11 0x5d83215a1b0f in std::__detail::_MakeUniq<S2Loop>::__single_object std::make_unique<S2Loop, std::vector<S2Point, std::allocator<S2Point> >&>(std::vector<S2Point, std::allocator<S2Point> >&) /usr/include/c++/12/bits/unique_ptr.h:1065
#12 0x5d832159df0f in main /s2test.cpp:33
#13 0x7af609157249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#14 0x7af609157304 in __libc_start_main_impl ../csu/libc-start.c:360
#15 0x5d832159bfe0 in _start (/s2test+0x17fe0)

AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV INSTALL_PREFIX/include/absl/container/internal/btree.h:694 in absl::lts_20240722::container_internal::btree_node<absl::lts_20240722::container_internal::map_params<S2CellId, S2ShapeIndexCell, std::less, std::allocator<std::pair<S2CellId const, S2ShapeIndexCell> >, 256, false> >::finish() const ==38==ABORTING

I found a workaround by compiling both libraries with address sanitizer by using this flag -DCMAKE_CXX_FLAGS=-fsanitize=address and adding this attribute attribute((no_sanitize_address)) to the s2 line where the error occurs which is the following:

inline void attribute((no_sanitizeaddress)) MutableS2ShapeIndex::Iterator::InitStale( const MutableS2ShapeIndex* index, InitialPosition pos) { index = index; end = index->cellmap.end(); if (pos == BEGIN) { iter = index->cellmap.begin(); } else { iter = end; } Refresh(); }

I'm still a newbie when it comes to to this library, so feel free to point anything that seems off. Thank you!

— Reply to this email directly, view it on GitHub https://github.com/google/s2geometry/issues/382, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGEMKV3NG66FJ4SIBYRAILZYBY2ZAVCNFSM6AAAAABOW2BNXWVHI2DSMVQWIX3LMV43ASLTON2WKOZSGU2DGNJWGM3DKNY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

FAguGomez commented 2 months ago

I'm using gcc to compile them so I don't have -fsanitize=memory or -fsanitize-memory-track-origins available. But I've updated both s2 and abseil to the commits ca1f3416 and c6b27359 specifically of their main branches and it seems to be working properly. I'll wait for the next releases to update again. Thank you!