math icon indicating copy to clipboard operation
math copied to clipboard

Fix for buffer overflow in fp_traits.hpp on aarch64 / mingw

Open joostn opened this issue 1 month ago • 4 comments

Hi,

I'm targeting aarch64-w64-mingw32 (probably I'm the first to try) and get this error:

/home/joostn/devel/wxwprojects_ptgui13/build/windows_mingw/arm64/RelWithDebInfo/thirdparty/boost-boost-1.90.0-prefix/include/boost/math/special_functions/detail/fp_traits.hpp:452:9: error: 'memcpy' will always overflow; destination buffer has size 0, but size argument is 4 [-Werror,-Wfortify-source]
  452 |         std::memcpy(reinterpret_cast<unsigned char*>(&x) + offset_, &a, 4);
      |         ^
1 error generated.

In include/boost/math/special_functions/detail/fp_traits.hpp you have this:

#if defined(BOOST_NO_INT64_T) || defined(BOOST_NO_INCLASS_MEMBER_INITIALIZATION)\
   || defined(BOOST_BORLANDC) || defined(__CODEGEAR__) || (defined(__APPLE__) && defined(__aarch64__)) || defined(_MSC_VER)

but AFAIK aarch64-w64-mingw32 also has 64 bit long double, so it should also take that path. This would fix it:

   || defined(BOOST_BORLANDC) || defined(__CODEGEAR__) || (defined(__APPLE__) && defined(__aarch64__)) || defined(_MSC_VER) || (defined(__MINGW32__) && defined(_WIN32))

I'm not sure if there are other places where this should be fixed, or if there are side effects, so I don't dare to submit a pull request.

joostn avatar Jan 10 '26 12:01 joostn

Hi @joostn, thanks for this good find.

We can handle this for sure. But we should discuss a few details in order to find the best way to handle this issue.

It looks like you are getting an error via -Werror regarding a suspected buffer overflow. Some similar warnings we encountered in the past have proven to be false negative, bogus warnings.

So we need to see if the warning can safely be disabled for this compiler and code sequence. if so, then we simply disable the warning within a PP-block in the header itself.

So just to be sure, without -Werror, is this just a warning? Then we can look at the warning and try to determin if it is real/bogus. Then potentially disable.

ckormanyos avatar Jan 10 '26 19:01 ckormanyos

And yes, the hard-coded number 4 does look like it might simply be wrong for this architecture. We need to catch that if this is the case. Then that would be a real warning/error and not a bogus one.

ckormanyos avatar Jan 10 '26 19:01 ckormanyos

And yes, the hard-coded number 4 does look like it might simply be wrong for this architecture. We need to catch that if this is the case. Then that would be a real warning/error and not a bogus one.

It's based on the assumption that the final #else is always a 128-bit long double, but this DR proves that not to be true, so maybe we should add a bit more error checking... I'll file a PR.

jzmaddock avatar Jan 11 '26 12:01 jzmaddock

I tested it for the 5 clang toolchains I have at hand:

#if defined(__APPLE__)
  #if defined(__aarch64__)
    static_assert(sizeof(long double) == 8);
  #elif defined(__x86_64__)
    static_assert(sizeof(long double) == 16);
  #else
    #error "Unsupported architecture"
  #endif
#elif (defined(_WIN32) && defined(_MSC_VER) && defined __clang__)
  #if defined(__aarch64__)
    static_assert(sizeof(long double) == 8);
  #elif defined(__x86_64__)
    static_assert(sizeof(long double) == 8);
  #else
    #error "Unsupported architecture"
  #endif
#elif (defined(_WIN32) && defined(__MINGW32__) && defined __clang__)
  #if defined(__aarch64__)
    static_assert(sizeof(long double) == 8);
  #elif defined(__x86_64__)
    static_assert(sizeof(long double) == 16);
  #else
    #error "Unsupported architecture"
  #endif
#elif (defined(__linux__) && defined __clang__)
  #if defined(__x86_64__)
    static_assert(sizeof(long double) == 16);
  #else
    #error "Unsupported architecture"
  #endif
#else
  #error "untested"
#endif

I think it would be good to add static assertions to fp_traits.hpp, mismatches would be caught at compile time

joostn avatar Jan 11 '26 14:01 joostn

I'd like to support embedded arm-none-eabi also, even if this is not an OFFICIALLY supported compiler, it's nice to have in Math.

ckormanyos avatar Jan 24 '26 19:01 ckormanyos

See also #1352

ckormanyos avatar Jan 24 '26 19:01 ckormanyos