Merge pull request #417 from Exiv2/safe_abs

Add overflow safe abs and enable UBSAN on travis
This commit is contained in:
D4N
2018-08-27 18:08:33 +02:00
committed by GitHub
12 changed files with 138 additions and 25 deletions
+17 -2
View File
@@ -6,15 +6,30 @@ matrix:
dist: trusty
sudo: required
compiler: gcc
env: COVERAGE=1 CMAKE_OPTIONS="-DCMAKE_BUILD_TYPE=Release -DEXIV2_ENABLE_VIDEO=ON -DEXIV2_ENABLE_WEBREADY=ON -DEXIV2_BUILD_UNIT_TESTS=ON -DBUILD_WITH_COVERAGE=ON" # All enabled + Coverage
env: COVERAGE=1 CMAKE_OPTIONS="-DCMAKE_BUILD_TYPE=Release -DEXIV2_ENABLE_VIDEO=ON -DEXIV2_ENABLE_WEBREADY=ON -DEXIV2_BUILD_UNIT_TESTS=ON -DBUILD_WITH_COVERAGE=ON -DEXIV2_TEAM_USE_SANITIZERS=ON" # All enabled + Coverage
- os: linux
dist: trusty
sudo: required
compiler: gcc-8
addons:
apt:
update: true
sources:
- sourceline: 'ppa:ubuntu-toolchain-r/test'
packages:
- g++-8
env: CC=gcc-8 CXX=g++-8 CMAKE_OPTIONS="-DCMAKE_BUILD_TYPE=Release -DEXIV2_ENABLE_VIDEO=ON -DEXIV2_ENABLE_WEBREADY=ON -DEXIV2_BUILD_UNIT_TESTS=ON -DEXIV2_TEAM_USE_SANITIZERS=ON"
- os: linux
dist: trusty
sudo: required
compiler: clang
- os: osx
osx_image: xcode9
compiler: clang
env: PYTHON=3.6.2 CMAKE_OPTIONS="-DCMAKE_BUILD_TYPE=Release -DEXIV2_ENABLE_VIDEO=ON -DEXIV2_ENABLE_WEBREADY=ON -DEXIV2_BUILD_UNIT_TESTS=ON" # All enabled
env: PYTHON=3.6.2 CMAKE_OPTIONS="-DCMAKE_BUILD_TYPE=Release -DEXIV2_ENABLE_VIDEO=ON -DEXIV2_ENABLE_WEBREADY=ON -DEXIV2_BUILD_UNIT_TESTS=ON -DEXIV2_TEAM_USE_SANITIZERS=ON" # All enabled
env:
#- CMAKE_OPTIONS="-DCMAKE_BUILD_TYPE=Release" # Default
+3 -1
View File
@@ -38,13 +38,15 @@ option( EXIV2_BUILD_DOC "Add 'doc' target to generate documentatio
# Only intended to be used by Exiv2 developers/contributors
option( EXIV2_TEAM_EXTRA_WARNINGS "Add more sanity checks using compiler flags" OFF )
option( EXIV2_TEAM_WARNINGS_AS_ERRORS "Treat warnings as errors" OFF )
option( EXIV2_TEAM_USE_SANITIZERS "Enable ASAN and UBSAN when available" OFF )
set(EXTRA_COMPILE_FLAGS " ")
mark_as_advanced(
EXIV2_TEAM_EXTRA_WARNINGS
EXIV2_TEAM_EXTRA_WARNINGS
EXIV2_TEAM_WARNINGS_AS_ERRORS
EXIV2_ENABLE_EXTERNAL_XMP
EXTRA_COMPILE_FLAGS
EXIV2_TEAM_USE_SANITIZERS
)
option( BUILD_WITH_CCACHE "Use ccache to speed up compilations" OFF )
+2 -5
View File
@@ -37,11 +37,8 @@ conan remote add conan-bincrafters https://api.bintray.com/conan/bincrafters/pub
mkdir -p ~/.conan/profiles
if [[ "$(uname -s)" == 'Linux' ]]; then
if [ ${CC} == "clang" ]; then
printf "[settings]\nos=Linux\narch=x86_64\ncompiler=clang\ncompiler.version=5.0\ncompiler.libcxx=libstdc++\nbuild_type=Release\n" > ~/.conan/profiles/release
else
printf "[settings]\nos=Linux\narch=x86_64\ncompiler=gcc\ncompiler.version=4.8\ncompiler.libcxx=libstdc++\nbuild_type=Release\n" > ~/.conan/profiles/release
fi
CC_VER=$(${CC} --version | head -1 | awk '{print $3}'| awk -F'.' '{ print $1"."$2 }')
printf "[settings]\nos=Linux\narch=x86_64\ncompiler=$CC\ncompiler.version=$CC_VER\ncompiler.libcxx=libstdc++\nbuild_type=Release\n" > ~/.conan/profiles/release
else
printf "[settings]\nos=Macos\narch=x86_64\ncompiler=apple-clang\ncompiler.version=9.0\ncompiler.libcxx=libc++\nbuild_type=Release\n" > ~/.conan/profiles/release
fi
+1 -6
View File
@@ -14,14 +14,9 @@ else
fi
mkdir build && cd build
conan install .. --build missing --profile release
cmake ${CMAKE_OPTIONS} -DCMAKE_INSTALL_PREFIX=install \
-DCMAKE_CXX_FLAGS="-fsanitize=address" \
-DCMAKE_C_FLAGS="-fsanitize=address" \
-DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address" \
-DCMAKE_MODULE_LINKER_FLAGS="-fsanitize=address" ..
cmake ${CMAKE_OPTIONS} -DCMAKE_INSTALL_PREFIX=install ..
make -j2 VERBOSE=1
#On most systems, you can set the TZ environment variable to set the timezone for a process. It's a POSIX feature.
+30
View File
@@ -40,6 +40,36 @@ if ( MINGW OR UNIX ) # MINGW, Linux, APPLE, CYGWIN
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
endif ()
if ( EXIV2_TEAM_USE_SANITIZERS )
# ASAN is available in gcc from 4.8 and UBSAN from 4.9
# ASAN is available in clang from 3.1 and UBSAN from 3.3
# UBSAN is not fatal by default, instead it only prints runtime errors to stderr
# => make it fatal with -fno-sanitize-recover (gcc) or -fno-sanitize-recover=all (clang)
# add -fno-omit-frame-pointer for better stack traces
if ( COMPILER_IS_GCC )
if ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 4.9 )
set(SANITIZER_FLAGS "-fno-omit-frame-pointer -fsanitize=address,undefined -fno-sanitize-recover")
elseif( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 4.8 )
set(SANITIZER_FLAGS "-fno-omit-frame-pointer -fsanitize=address")
endif()
elseif( COMPILER_IS_CLANG )
if ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 3.3 )
set(SANITIZER_FLAGS "-fno-omit-frame-pointer -fsanitize=address,undefined -fno-sanitize-recover=all")
elseif( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 3.1 )
set(SANITIZER_FLAGS "-fno-omit-frame-pointer -fsanitize=address")
endif()
endif()
# sorry, ASAN does not work on Windows
if ( NOT CYGWIN AND NOT MINGW )
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${SANITIZER_FLAGS}")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZER_FLAGS}")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${SANITIZER_FLAGS}")
set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} ${SANITIZER_FLAGS}")
endif()
endif()
if ( EXIV2_TEAM_EXTRA_WARNINGS )
# Note that this is intended to be used only by Exiv2 developers/contributors.
+3 -3
View File
@@ -59,11 +59,11 @@ namespace Exiv2
// free functions
/*!
@brief Return the value of environmental variable.
@param var The name of environmental variable.
@param[in] var The name of environmental variable. Must be a member of the enumeration @ref EnVar.
@return the value of environmental variable. If it's empty, the default value is returned.
@throws std::out_of_range when an unexpected EnVar is given as input.
*/
EXIV2API std::string getEnv(EnVar var);
EXIV2API std::string getEnv(int env_var);
/*!
@brief Encode the input url.
@@ -204,4 +204,4 @@ namespace Exiv2
} // namespace Exiv2
#endif // #ifndef FUTILS_HPP_
#endif // #ifndef FUTILS_HPP_
+8 -2
View File
@@ -38,6 +38,7 @@
#include <string>
#include <vector>
#include <iosfwd>
#include <limits>
#include <utility>
#include <algorithm>
#include <sstream>
@@ -555,8 +556,13 @@ namespace Exiv2 {
#ifdef _MSC_VER
#pragma warning( disable : 4146 )
#endif
if (n < zero)
n = -n;
if (n < zero) {
if (n == std::numeric_limits<IntType>::min()) {
n = std::numeric_limits<IntType>::max();
} else {
n = -n;
}
}
if (m < zero)
m = -m;
#ifdef _MSC_VER
+4 -3
View File
@@ -60,12 +60,13 @@ namespace Exiv2 {
const char* ENVARKEY[] = {"EXIV2_HTTP_POST", "EXIV2_TIMEOUT"}; //!< @brief request keys for http exiv2 handler and time-out
// *****************************************************************************
// free functions
std::string getEnv(EnVar var)
std::string getEnv(int env_var)
{
if (var < envHTTPPOST || var > envTIMEOUT) {
// this check is relying on undefined behavior and might not be effective
if (env_var < envHTTPPOST || env_var > envTIMEOUT) {
throw std::out_of_range("Unexpected env variable");
}
return getenv(ENVARKEY[var]) ? getenv(ENVARKEY[var]) : ENVARDEF[var];
return getenv(ENVARKEY[env_var]) ? getenv(ENVARKEY[env_var]) : ENVARDEF[env_var];
}
/// @brief Convert an integer value to its hex character.
+31
View File
@@ -302,6 +302,37 @@ namespace Safe
return res;
}
/*!
* @brief Calculates the absolute value of a number without producing
* negative values.
*
* The "standard" implementation of `abs(num)` (`num < 0 ? -num : num`)
* produces negative values when `num` is the smallest negative number. This
* is caused by `-1 * INTMAX = INTMIN + 1`, i.e. the real result of
* `abs(INTMIN)` overflows the integer type and results in `INTMIN` again
* (this is not guaranteed as it invokes undefined behavior).
*
* This function does not exhibit this behavior, it returns
* `std::numeric_limits<T>::max()` when the input is
* `std::numeric_limits<T>::min()`. The downside of this is that two
* negative values produce the same absolute value:
* `std::numeric_limits<T>::min()` and `std::numeric_limits<T>::min() + 1`.
*
* @tparam T a signed integer type
* @param[in] num The number which absolute value should be computed.
* @throws Never throws an exception.
* @return The absolute value of `num` or `std::numeric_limits<T>::max()`
* when `num == std::numeric_limits<T>::min()`.
*/
template <typename T>
typename Internal::enable_if<Internal::is_signed<T>::VALUE, T>::type abs(T num) throw()
{
if (num == std::numeric_limits<T>::min()) {
return std::numeric_limits<T>::max();
}
return num < 0 ? -num : num;
}
} // namespace Safe
#endif // SAFE_OP_HPP_
+9 -3
View File
@@ -29,6 +29,7 @@
#include "types.hpp"
#include "i18n.h" // for _exvGettext
#include "unused.h"
#include "safe_op.hpp"
// + standard includes
#ifdef EXV_UNICODE_PATH
@@ -46,6 +47,7 @@
#include <cassert>
#include <cstring>
#include <cmath>
#include <math.h>
// *****************************************************************************
namespace {
@@ -657,11 +659,15 @@ namespace Exiv2 {
Rational floatToRationalCast(float f)
{
if (isinf(f)) {
return Rational(f > 0 ? 1 : -1, 0);
}
// Beware: primitive conversion algorithm
int32_t den = 1000000;
if (std::labs(static_cast<long>(f)) > 2147) den = 10000;
if (std::labs(static_cast<long>(f)) > 214748) den = 100;
if (std::labs(static_cast<long>(f)) > 21474836) den = 1;
const long f_as_long = static_cast<long>(f);
if (Safe::abs(f_as_long) > 2147) den = 10000;
if (Safe::abs(f_as_long) > 214748) den = 100;
if (Safe::abs(f_as_long) > 21474836) den = 1;
const float rnd = f >= 0 ? 0.5f : -0.5f;
const int32_t nom = static_cast<int32_t>(f * den + rnd);
const int32_t g = gcd(nom, den);
+9
View File
@@ -181,3 +181,12 @@ TEST(safeAdd, checkSignedOverflow)
test_safe_add<long>();
test_safe_add<long long>();
}
TEST(safeAbs, checkValues)
{
static const int values[] = {-1, 1, std::numeric_limits<int>::max(), std::numeric_limits<int>::min() + 1};
for (size_t i = 0; i < sizeof(values) / sizeof(*values); ++i) {
ASSERT_EQ(Safe::abs(values[i]), abs(values[i]));
}
ASSERT_EQ(Safe::abs(std::numeric_limits<int>::min()), std::numeric_limits<int>::max());
}
+21
View File
@@ -1,5 +1,7 @@
#include <exiv2/types.hpp>
#include <math.h>
#include "gtestwrapper.h"
using namespace Exiv2;
@@ -37,3 +39,22 @@ TEST(DataBuf, allocatesDataWithNonEmptyConstructor)
ASSERT_NE(static_cast<byte *>(NULL), instance.pData_); /// \todo use nullptr once we move to c++11
ASSERT_EQ(5, instance.size_);
}
TEST(Rational, floatToRationalCast)
{
static const float floats[] = {0.5, 0.015, 0.0000625};
for (size_t i = 0; i < sizeof(floats) / sizeof(*floats); ++i) {
const Rational r = floatToRationalCast(floats[i]);
const float fraction = static_cast<float>(r.first) / static_cast<float>(r.second);
ASSERT_TRUE(fabs((floats[i] - fraction) / floats[i]) < 0.01f);
}
const Rational plus_inf = floatToRationalCast(INFINITY);
ASSERT_EQ(plus_inf.first, 1);
ASSERT_EQ(plus_inf.second, 0);
const Rational minus_inf = floatToRationalCast(-1 * INFINITY);
ASSERT_EQ(minus_inf.first, -1);
ASSERT_EQ(minus_inf.second, 0);
}