From 6f885b5bdb2d220eb49e7d0fabcd520f56f2b419 Mon Sep 17 00:00:00 2001 From: "Daniel J. Hofmann" Date: Wed, 16 Nov 2016 11:33:59 +0100 Subject: [PATCH] Squashed 'third_party/libosmium/' changes from 40c4a48..d5ecf4d d5ecf4d Release v2.10.2 7c04564 Update embedded protozero to version 1.4.4. e209d81 Write code for 64bit systems, so it compiles on 32bit w/o warning. 640217c Fix buffer overflow. 8b4620f Release v2.10.1 38bf3ab Update protozero to 1.4.3. f81b3c6 Fix IdSet on 32 bit. 5ff4753 Workaround so the test works on 32bit systems. 7542694 Include our endian.hpp before using the endianness test macros. git-subtree-dir: third_party/libosmium git-subtree-split: d5ecf4df90e2995c816886d2a002c3d3de7062ee --- CHANGELOG.md | 27 ++++++- CMakeLists.txt | 2 +- include/osmium/builder/osm_object_builder.hpp | 6 +- include/osmium/index/id_set.hpp | 11 ++- include/osmium/osm/location.hpp | 4 +- include/osmium/version.hpp | 4 +- include/protozero/byteswap.hpp | 79 +++++++++++-------- include/protozero/iterators.hpp | 20 +---- include/protozero/pbf_message.hpp | 4 +- include/protozero/pbf_reader.hpp | 45 ++++++----- include/protozero/pbf_writer.hpp | 9 +-- include/protozero/types.hpp | 18 ++--- include/protozero/version.hpp | 4 +- test/t/geom/test_ogr_wkb.cpp | 5 +- test/t/geom/test_wkb.cpp | 2 + test/t/index/test_id_set.cpp | 16 ++-- test/t/osm/test_location.cpp | 6 +- 17 files changed, 149 insertions(+), 113 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25327c8e0..ef5c65675 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,29 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +## [2.10.2] - 2016-11-16 + +### Changed + +- Updated embedded protozero to 1.4.4. + +### Fixed + +- Buffer overflow in osmium::Buffer. + + +## [2.10.1] - 2016-11-15 + +### Changed + +- Updated embedded protozero to 1.4.3. + +### Fixed + +- Made IdSet work on 32bit systems. +- Fixed endianness check for WKB tests. + + ## [2.10.0] - 2016-11-11 ### Added @@ -457,7 +480,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). Doxygen (up to version 1.8.8). This version contains a workaround to fix this. -[unreleased]: https://github.com/osmcode/libosmium/compare/v2.10.0...HEAD +[unreleased]: https://github.com/osmcode/libosmium/compare/v2.10.2...HEAD +[2.10.2]: https://github.com/osmcode/libosmium/compare/v2.10.1...v2.10.2 +[2.10.1]: https://github.com/osmcode/libosmium/compare/v2.10.0...v2.10.1 [2.10.0]: https://github.com/osmcode/libosmium/compare/v2.9.0...v2.10.0 [2.9.0]: https://github.com/osmcode/libosmium/compare/v2.8.0...v2.9.0 [2.8.0]: https://github.com/osmcode/libosmium/compare/v2.7.2...v2.8.0 diff --git a/CMakeLists.txt b/CMakeLists.txt index 21cf98e11..6f10b0ceb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,7 +25,7 @@ project(libosmium) set(LIBOSMIUM_VERSION_MAJOR 2) set(LIBOSMIUM_VERSION_MINOR 10) -set(LIBOSMIUM_VERSION_PATCH 0) +set(LIBOSMIUM_VERSION_PATCH 2) set(LIBOSMIUM_VERSION "${LIBOSMIUM_VERSION_MAJOR}.${LIBOSMIUM_VERSION_MINOR}.${LIBOSMIUM_VERSION_PATCH}") diff --git a/include/osmium/builder/osm_object_builder.hpp b/include/osmium/builder/osm_object_builder.hpp index b1c7220a7..0a8fa6f96 100644 --- a/include/osmium/builder/osm_object_builder.hpp +++ b/include/osmium/builder/osm_object_builder.hpp @@ -413,11 +413,10 @@ namespace osmium { const auto available_space = min_size_for_user - sizeof(string_size_type) - 1; if (length > available_space) { const auto space_needed = osmium::memory::padded_length(length - available_space); - reserve_space(space_needed); + std::fill_n(reserve_space(space_needed), space_needed, 0); add_size(static_cast(space_needed)); } std::copy_n(user, length, object().data() + size_of_object); - std::fill_n(object().data() + size_of_object + length, osmium::memory::padded_length(length + 1) - length, 0); object().set_user_size(length + 1); return static_cast(*this); @@ -612,11 +611,10 @@ namespace osmium { const auto available_space = min_size_for_user - 1; if (length > available_space) { const auto space_needed = osmium::memory::padded_length(length - available_space); - reserve_space(space_needed); + std::fill_n(reserve_space(space_needed), space_needed, 0); add_size(static_cast(space_needed)); } std::copy_n(user, length, object().data() + sizeof(Changeset)); - std::fill_n(object().data() + sizeof(Changeset) + length, osmium::memory::padded_length(length + 1) - length, 0); object().set_user_size(length + 1); return *this; diff --git a/include/osmium/index/id_set.hpp b/include/osmium/index/id_set.hpp index 4a894a0ba..b9a9f9d7a 100644 --- a/include/osmium/index/id_set.hpp +++ b/include/osmium/index/id_set.hpp @@ -91,13 +91,16 @@ namespace osmium { template class IdSetDenseIterator { + static_assert(std::is_unsigned::value, "Needs unsigned type"); + static_assert(sizeof(T) >= 4, "Needs at least 32bit type"); + const IdSetDense* m_set; T m_value; T m_last; void next() noexcept { while (m_value != m_last && !m_set->get(m_value)) { - const auto cid = IdSetDense::chunk_id(m_value); + const T cid = IdSetDense::chunk_id(m_value); assert(cid < m_set->m_data.size()); if (!m_set->m_data[cid]) { m_value = (cid + 1) << (IdSetDense::chunk_bits + 3); @@ -179,7 +182,7 @@ namespace osmium { constexpr static const size_t chunk_size = 1 << chunk_bits; std::vector> m_data; - size_t m_size = 0; + T m_size = 0; static size_t chunk_id(T id) noexcept { return id >> (chunk_bits + 3); @@ -194,7 +197,7 @@ namespace osmium { } T last() const noexcept { - return m_data.size() * chunk_size * 8; + return static_cast(m_data.size()) * chunk_size * 8; } unsigned char& get_element(T id) { @@ -285,7 +288,7 @@ namespace osmium { /** * The number of Ids stored in the set. */ - size_t size() const noexcept { + T size() const noexcept { return m_size; } diff --git a/include/osmium/osm/location.hpp b/include/osmium/osm/location.hpp index d20871759..bc03a2ef4 100644 --- a/include/osmium/osm/location.hpp +++ b/include/osmium/osm/location.hpp @@ -512,9 +512,9 @@ namespace osmium { template <> inline size_t hash<8>(const osmium::Location& location) noexcept { - size_t h = location.x(); + uint64_t h = location.x(); h <<= 32; - return h ^ location.y(); + return static_cast(h ^ location.y()); } } // namespace detail diff --git a/include/osmium/version.hpp b/include/osmium/version.hpp index 09c576288..bae10dd3e 100644 --- a/include/osmium/version.hpp +++ b/include/osmium/version.hpp @@ -35,8 +35,8 @@ DEALINGS IN THE SOFTWARE. #define LIBOSMIUM_VERSION_MAJOR 2 #define LIBOSMIUM_VERSION_MINOR 10 -#define LIBOSMIUM_VERSION_PATCH 0 +#define LIBOSMIUM_VERSION_PATCH 2 -#define LIBOSMIUM_VERSION_STRING "2.10.0" +#define LIBOSMIUM_VERSION_STRING "2.10.2" #endif // OSMIUM_VERSION_HPP diff --git a/include/protozero/byteswap.hpp b/include/protozero/byteswap.hpp index 06ba6ded9..3afb34840 100644 --- a/include/protozero/byteswap.hpp +++ b/include/protozero/byteswap.hpp @@ -22,50 +22,63 @@ documentation. #include namespace protozero { +namespace detail { -/** - * Swap N byte value between endianness formats. This template function must - * be specialized to actually work. - */ -template -inline void byteswap(const char* /*data*/, char* /*result*/) noexcept { - static_assert(N == 1, "Can only swap 4 or 8 byte values"); -} - -/** - * Swap 4 byte value (int32_t, uint32_t, float) between endianness formats. - */ -template <> -inline void byteswap<4>(const char* data, char* result) noexcept { +inline uint32_t byteswap_impl(uint32_t value) noexcept { #ifdef PROTOZERO_USE_BUILTIN_BSWAP - *reinterpret_cast(result) = __builtin_bswap32(*reinterpret_cast(data)); + return __builtin_bswap32(value); #else - result[3] = data[0]; - result[2] = data[1]; - result[1] = data[2]; - result[0] = data[3]; + return ((value & 0xff000000) >> 24) | + ((value & 0x00ff0000) >> 8) | + ((value & 0x0000ff00) << 8) | + ((value & 0x000000ff) << 24); #endif } -/** - * Swap 8 byte value (int64_t, uint64_t, double) between endianness formats. - */ -template <> -inline void byteswap<8>(const char* data, char* result) noexcept { +inline uint64_t byteswap_impl(uint64_t value) noexcept { #ifdef PROTOZERO_USE_BUILTIN_BSWAP - *reinterpret_cast(result) = __builtin_bswap64(*reinterpret_cast(data)); + return __builtin_bswap64(value); #else - result[7] = data[0]; - result[6] = data[1]; - result[5] = data[2]; - result[4] = data[3]; - result[3] = data[4]; - result[2] = data[5]; - result[1] = data[6]; - result[0] = data[7]; + return ((value & 0xff00000000000000ULL) >> 56) | + ((value & 0x00ff000000000000ULL) >> 40) | + ((value & 0x0000ff0000000000ULL) >> 24) | + ((value & 0x000000ff00000000ULL) >> 8) | + ((value & 0x00000000ff000000ULL) << 8) | + ((value & 0x0000000000ff0000ULL) << 24) | + ((value & 0x000000000000ff00ULL) << 40) | + ((value & 0x00000000000000ffULL) << 56); #endif } +inline void byteswap_inplace(uint32_t* ptr) noexcept { + *ptr = byteswap_impl(*ptr); +} + +inline void byteswap_inplace(uint64_t* ptr) noexcept { + *ptr = byteswap_impl(*ptr); +} + +inline void byteswap_inplace(int32_t* ptr) noexcept { + auto bptr = reinterpret_cast(ptr); + *bptr = byteswap_impl(*bptr); +} + +inline void byteswap_inplace(int64_t* ptr) noexcept { + auto bptr = reinterpret_cast(ptr); + *bptr = byteswap_impl(*bptr); +} + +inline void byteswap_inplace(float* ptr) noexcept { + auto bptr = reinterpret_cast(ptr); + *bptr = byteswap_impl(*bptr); +} + +inline void byteswap_inplace(double* ptr) noexcept { + auto bptr = reinterpret_cast(ptr); + *bptr = byteswap_impl(*bptr); +} + +} // end namespace detail } // end namespace protozero #endif // PROTOZERO_BYTESWAP_HPP diff --git a/include/protozero/iterators.hpp b/include/protozero/iterators.hpp index 00ba91935..40259a9fc 100644 --- a/include/protozero/iterators.hpp +++ b/include/protozero/iterators.hpp @@ -29,21 +29,6 @@ documentation. namespace protozero { -namespace detail { - - // Copy N bytes from src to dest on little endian machines, on big - // endian swap the bytes in the process. - template - inline void copy_or_byteswap(const char* src, void* dest) noexcept { -#if PROTOZERO_BYTE_ORDER == PROTOZERO_LITTLE_ENDIAN - std::memcpy(dest, src, N); -#else - byteswap(src, reinterpret_cast(dest)); -#endif - } - -} // end namespace detail - /** * A range of iterators based on std::pair. Created from beginning and * end iterators. Used as a return type from some pbf_reader methods @@ -213,7 +198,10 @@ public: value_type operator*() const { value_type result; - detail::copy_or_byteswap(m_data , &result); + std::memcpy(&result, m_data, sizeof(value_type)); +#if PROTOZERO_BYTE_ORDER != PROTOZERO_LITTLE_ENDIAN + detail::byteswap_inplace(&result); +#endif return result; } diff --git a/include/protozero/pbf_message.hpp b/include/protozero/pbf_message.hpp index 055773408..f66604a94 100644 --- a/include/protozero/pbf_message.hpp +++ b/include/protozero/pbf_message.hpp @@ -79,8 +79,8 @@ public: return pbf_reader::next(); } - bool next(T tag) { - return pbf_reader::next(pbf_tag_type(tag)); + bool next(T next_tag) { + return pbf_reader::next(pbf_tag_type(next_tag)); } T tag() const noexcept { diff --git a/include/protozero/pbf_reader.hpp b/include/protozero/pbf_reader.hpp index 2f4054d2d..69f2d7287 100644 --- a/include/protozero/pbf_reader.hpp +++ b/include/protozero/pbf_reader.hpp @@ -75,7 +75,10 @@ class pbf_reader { T get_fixed() { T result; skip_bytes(sizeof(T)); - detail::copy_or_byteswap(m_data - sizeof(T), &result); + std::memcpy(&result, m_data - sizeof(T), sizeof(T)); +#if PROTOZERO_BYTE_ORDER != PROTOZERO_LITTLE_ENDIAN + detail::byteswap_inplace(&result); +#endif return result; } @@ -134,7 +137,7 @@ public: /** * Construct a pbf_reader message from a data_view. The pointer from the * data_view will be stored inside the pbf_reader object, no data is - * copied. So you must* make sure the view stays valid as long as the + * copied. So you must make sure the view stays valid as long as the * pbf_reader object is used. * * The buffer must contain a complete protobuf message. @@ -149,25 +152,27 @@ public: } /** - * Construct a pbf_reader message from a data pointer and a length. The pointer - * will be stored inside the pbf_reader object, no data is copied. So you must - * make sure the buffer stays valid as long as the pbf_reader object is used. + * Construct a pbf_reader message from a data pointer and a length. The + * pointer will be stored inside the pbf_reader object, no data is copied. + * So you must make sure the buffer stays valid as long as the pbf_reader + * object is used. * * The buffer must contain a complete protobuf message. * * @post There is no current field. */ - pbf_reader(const char* data, std::size_t length) noexcept + pbf_reader(const char* data, std::size_t size) noexcept : m_data(data), - m_end(data + length), + m_end(data + size), m_wire_type(pbf_wire_type::unknown), m_tag(0) { } /** - * Construct a pbf_reader message from a data pointer and a length. The pointer - * will be stored inside the pbf_reader object, no data is copied. So you must - * make sure the buffer stays valid as long as the pbf_reader object is used. + * Construct a pbf_reader message from a data pointer and a length. The + * pointer will be stored inside the pbf_reader object, no data is copied. + * So you must make sure the buffer stays valid as long as the pbf_reader + * object is used. * * The buffer must contain a complete protobuf message. * @@ -181,10 +186,10 @@ public: } /** - * Construct a pbf_reader message from a std::string. A pointer to the string - * internals will be stored inside the pbf_reader object, no data is copied. - * So you must make sure the string is unchanged as long as the pbf_reader - * object is used. + * Construct a pbf_reader message from a std::string. A pointer to the + * string internals will be stored inside the pbf_reader object, no data + * is copied. So you must make sure the string is unchanged as long as the + * pbf_reader object is used. * * The string must contain a complete protobuf message. * @@ -231,8 +236,9 @@ public: } /** - * In a boolean context the pbf_reader class evaluates to `true` if there are - * still fields available and to `false` if the last field has been read. + * In a boolean context the pbf_reader class evaluates to `true` if there + * are still fields available and to `false` if the last field has been + * read. */ operator bool() const noexcept { return m_data < m_end; @@ -276,7 +282,8 @@ public: // tags 0 and 19000 to 19999 are not allowed as per // https://developers.google.com/protocol-buffers/docs/proto - protozero_assert(((m_tag > 0 && m_tag < 19000) || (m_tag > 19999 && m_tag <= ((1 << 29) - 1))) && "tag out of range"); + protozero_assert(((m_tag > 0 && m_tag < 19000) || + (m_tag > 19999 && m_tag <= ((1 << 29) - 1))) && "tag out of range"); m_wire_type = pbf_wire_type(value & 0x07); switch (m_wire_type) { @@ -317,9 +324,9 @@ public: * @pre There must be no current field. * @post If it returns `true` there is a current field now with the given tag. */ - bool next(pbf_tag_type tag) { + bool next(pbf_tag_type next_tag) { while (next()) { - if (m_tag == tag) { + if (m_tag == next_tag) { return true; } else { skip(); diff --git a/include/protozero/pbf_writer.hpp b/include/protozero/pbf_writer.hpp index bce1c3ffc..77cc9d025 100644 --- a/include/protozero/pbf_writer.hpp +++ b/include/protozero/pbf_writer.hpp @@ -90,13 +90,10 @@ class pbf_writer { void add_fixed(T value) { protozero_assert(m_pos == 0 && "you can't add fields to a parent pbf_writer if there is an existing pbf_writer for a submessage"); protozero_assert(m_data); -#if PROTOZERO_BYTE_ORDER == PROTOZERO_LITTLE_ENDIAN - m_data->append(reinterpret_cast(&value), sizeof(T)); -#else - const auto size = m_data->size(); - m_data->resize(size + sizeof(T)); - byteswap(reinterpret_cast(&value), const_cast(m_data->data() + size)); +#if PROTOZERO_BYTE_ORDER != PROTOZERO_LITTLE_ENDIAN + detail::byteswap_inplace(&value); #endif + m_data->append(reinterpret_cast(&value), sizeof(T)); } template diff --git a/include/protozero/types.hpp b/include/protozero/types.hpp index 8b046387e..431c2074c 100644 --- a/include/protozero/types.hpp +++ b/include/protozero/types.hpp @@ -78,12 +78,12 @@ public: /** * Create data_view from pointer and size. * - * @param data Pointer to the data. - * @param size Length of the data. + * @param ptr Pointer to the data. + * @param length Length of the data. */ - constexpr data_view(const char* data, std::size_t size) noexcept - : m_data(data), - m_size(size) { + constexpr data_view(const char* ptr, std::size_t length) noexcept + : m_data(ptr), + m_size(length) { } /** @@ -99,11 +99,11 @@ public: /** * Create data_view from zero-terminated string. * - * @param data Pointer to the data. + * @param ptr Pointer to the data. */ - data_view(const char* data) noexcept - : m_data(data), - m_size(std::strlen(data)) { + data_view(const char* ptr) noexcept + : m_data(ptr), + m_size(std::strlen(ptr)) { } /** diff --git a/include/protozero/version.hpp b/include/protozero/version.hpp index 127e64972..c3e91a36b 100644 --- a/include/protozero/version.hpp +++ b/include/protozero/version.hpp @@ -23,13 +23,13 @@ documentation. #define PROTOZERO_VERSION_MINOR 4 /// The patch number -#define PROTOZERO_VERSION_PATCH 2 +#define PROTOZERO_VERSION_PATCH 4 /// The complete version number #define PROTOZERO_VERSION_CODE (PROTOZERO_VERSION_MAJOR * 10000 + PROTOZERO_VERSION_MINOR * 100 + PROTOZERO_VERSION_PATCH) /// Version number as string -#define PROTOZERO_VERSION_STRING "1.4.2" +#define PROTOZERO_VERSION_STRING "1.4.4" #endif // PROTOZERO_VERSION_HPP diff --git a/test/t/geom/test_ogr_wkb.cpp b/test/t/geom/test_ogr_wkb.cpp index 548d14389..47d1cdd77 100644 --- a/test/t/geom/test_ogr_wkb.cpp +++ b/test/t/geom/test_ogr_wkb.cpp @@ -1,6 +1,4 @@ -#if __BYTE_ORDER == __LITTLE_ENDIAN - #include "catch.hpp" #include @@ -9,6 +7,9 @@ #include #include +#include + +#if __BYTE_ORDER == __LITTLE_ENDIAN #include "area_helper.hpp" #include "wnl_helper.hpp" diff --git a/test/t/geom/test_wkb.cpp b/test/t/geom/test_wkb.cpp index 66dd42e51..e46c5deb8 100644 --- a/test/t/geom/test_wkb.cpp +++ b/test/t/geom/test_wkb.cpp @@ -2,6 +2,8 @@ #include #include +#include + #include "wnl_helper.hpp" #if __BYTE_ORDER == __LITTLE_ENDIAN diff --git a/test/t/index/test_id_set.cpp b/test/t/index/test_id_set.cpp index 4c2444717..ec8280a38 100644 --- a/test/t/index/test_id_set.cpp +++ b/test/t/index/test_id_set.cpp @@ -51,9 +51,9 @@ TEST_CASE("Iterating over IdSetDense") { s.set(35); s.set(35); s.set(20); - s.set(1LL << 33); + s.set(1ULL << 33); s.set(21); - s.set((1LL << 27) + 13); + s.set((1ULL << 27) + 13); REQUIRE(s.size() == 6); @@ -71,10 +71,10 @@ TEST_CASE("Iterating over IdSetDense") { REQUIRE(*it == 35); ++it; REQUIRE(it != s.end()); - REQUIRE(*it == (1LL << 27) + 13); + REQUIRE(*it == (1ULL << 27) + 13); ++it; REQUIRE(it != s.end()); - REQUIRE(*it == 1LL << 33); + REQUIRE(*it == 1ULL << 33); ++it; REQUIRE(it == s.end()); } @@ -133,9 +133,9 @@ TEST_CASE("Iterating over IdSetSmall") { s.set(35); s.set(35); s.set(20); - s.set(1LL << 33); + s.set(1ULL << 33); s.set(21); - s.set((1LL << 27) + 13); + s.set((1ULL << 27) + 13); // needs to be called before size() and iterator will work properly s.sort_unique(); @@ -156,10 +156,10 @@ TEST_CASE("Iterating over IdSetSmall") { REQUIRE(*it == 35); ++it; REQUIRE(it != s.end()); - REQUIRE(*it == (1LL << 27) + 13); + REQUIRE(*it == (1ULL << 27) + 13); ++it; REQUIRE(it != s.end()); - REQUIRE(*it == 1LL << 33); + REQUIRE(*it == 1ULL << 33); ++it; REQUIRE(it == s.end()); } diff --git a/test/t/osm/test_location.cpp b/test/t/osm/test_location.cpp index 7abf77962..6aba91dbe 100644 --- a/test/t/osm/test_location.cpp +++ b/test/t/osm/test_location.cpp @@ -156,8 +156,10 @@ TEST_CASE("Location hash") { if (sizeof(size_t) == 8) { REQUIRE(std::hash{}({0, 0}) == 0); REQUIRE(std::hash{}({0, 1}) == 1); - REQUIRE(std::hash{}({1, 0}) == 0x100000000); - REQUIRE(std::hash{}({1, 1}) == 0x100000001); + const int64_t a = std::hash{}({1, 0}); + REQUIRE(a == 0x100000000); + const int64_t b = std::hash{}({1, 1}); + REQUIRE(b == 0x100000001); } else { REQUIRE(std::hash{}({0, 0}) == 0); REQUIRE(std::hash{}({0, 1}) == 1);