libmongocrypt icon indicating copy to clipboard operation
libmongocrypt copied to clipboard

MONGOCRYPT-535 Support libdfp as a Decimal128 abstraction

Open vector-of-bool opened this issue 2 years ago • 1 comments
trafficstars

This changeset allows building using a system-installation of libdfp as an alternative to IntelDFP.

Change Summary

Package/CMake Changes

  • Setting MONGOCRYPT_DFP_DIR to USE-SYSTEM will search for both IntelDFP and libdfp. If IntelDFP is found, it will be used; otherwise if libdfp is found, it will be used; otherwise we generate an error.
  • CMake code has been cleaned up to say "dfp" instead of "intel_dfp" throughout.
  • The _mongocrypt::dfp is an INTERFACE target alike what was previously defined, and is used to pivot the selection of a DFP backend.
  • The _mongocrypt::system-dfp is an IMPORTED target that points to the system-installed DFP library. _mongocrypt::dfp links to this library iff USE-SYSTEM was specified.
  • At import-time, the _mongocrypt::system-dfp library needs to be redefined if we were built with USE-SYSTEM, and a minimum amount of find_library logic was inserted into mongocrypt-config.cmake to find the same library that was used for the build. This is similar to the prior version, but it now needs to consider whether we are built with IntelDFP or libdfp. We export the filename of the found library into the mongocrypt-config.cmake so that we can find the correct matching library later on.
  • The .pc for the shared library has the link to the DFP lib file removed since either: 1) We linked against a static DFP lib, and the mongocrypt.so already contains the required TUs, or 2) we built against a dynamic DFP lib, and the mongocrypt.so will already have the reference to the proper SONAME. This makes the explicit linking unecessary in either case. The explicit link may fail if the generated .pc contains the path to a dev-only symlink (this is the case on e.g. Fedora Rawhide where libdfp.so is a symilnk to libdfp.so.1, but the libdfp runtime package only includes libdfp.so.1). Alternative solution: Have libmongocrypt-dev depend on the libdfp-dev packages, but that feels like a heavier burden on users.
  • A few miscellaneous tweaks from uncovering some testing issues on some platforms (add_test can be finicky)
  • Test the RPM build using system's libdfp-devel instead of the bundled IntelDFP. Also tentatively: Use libdfp-dev instead of libintelrdfp-dev as the build-dep for the libmongocrypt deb. This can be rolled back if we don't want to make that switch.

Code Changes

  • We detect libdfp by #includeing math.h, and checking for _DFP_MATH_H. If found, it means that libdfp has intercepted the stdlib headers and the stdlib symbols may be replaced by the libdfp shims. This switches mc-dec128.h to use libdfp as the backend instead of IntelDFP.
  • If libdfp is not detected, continue to use IntelDFP.
  • Some dec128 classifying functions were renamed to use the stdlib-like names (e.g. is_infisinf).
  • The core of "implementation switching" is done with two function-like macros: MC_IF_LIBDFP(...), which expands its arguments if libdfp is in use, and MC_IF_IntelDFP(...) which expands its arguments if libdfp is not in use.
  • Significant: libdfp uses the C stdlib <fenv.h> facilities, which are awful and painful and Widely Regarded as a Bad Move. (Which is why the IntelDFP version opts for the explicit-env arguments for rounding and exceptions).
    • The fenv facilities are abstracted using MC_HOLD_FENV(Rounding, FlagsPtr), which is a control-statement-like function-macro that wraps a code block and saves/restores the floating-point environment, while also setting the appropriate rounding mode and firing SIGFPE in the case of unhandled div-by-zero.
  • The to/from string functions were moved out-of-line into a separate TU, since #include-order can affect the behavior of libdfp on the required functions, so we need to ensure it comes first in the TU.
  • The migration to libdfp uncovered some misbehaving test cases that were "just working" due to bad macro expansions. These cases have been fixed.

vector-of-bool avatar Feb 09 '23 21:02 vector-of-bool

Alternative solution: Have libmongocrypt-dev depend on the libdfp-dev packages, but that feels like a heavier burden on users.

This is actually the correct solution. For instance, in the C driver, the libmongoc-dev package expresses the following dependencies:

Depends: libmongoc-1.0-0 (= ${binary:Version}),
         libbson-dev (= ${binary:Version}),
         libmongocrypt-dev,
         libssl-dev,
         zlib1g-dev,
         libsnappy-dev,
         libsasl2-dev,
         libzstd-dev,

In general, we need that sort of thing to ensure access to a .so (the unversioned symlink), the .a (for libraries that support static linking), and also helper scripts (like .pc scripts for pkg-config and .cmake target files) to ensure appropriate compiler options/definitions are set and transitive link dependencies are resolved.

rcsanchez97 avatar Feb 10 '23 17:02 rcsanchez97