Libguestfs - Nov 2021 - [PATCH nbdkit 2/2] common/include/checked-overflow.h: Provide fallback

On RHEL 7 (GCC 4.8.5) we don't have __builtin_add_overflow and similar
functions.  They were first added in GCC 5.  Provide a fallback path
for these older compilers.  Note that the fallback path does not
actually implement overflow detection.
---
 configure.ac                      | 6 ++++++
 common/include/checked-overflow.h | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/configure.ac b/configure.ac
index 8980fdd4..30330813 100644
--- a/configure.ac
+++ b/configure.ac
@@ -220,6 +220,12 @@ CFLAGS="$old_CFLAGS"
 AC_MSG_RESULT([$supports_std_c90])
 AM_CONDITIONAL([CAN_TEST_ANSI_C], [test "x$supports_std_c90" =
"xyes"])
 
+dnl Check for __builtin_*_overflow.  We need the dummy parameters
+dnl else detection doesn't work correctly for some reason.
+AC_CHECK_DECLS([__builtin_add_overflow(int, int, int *),
+                __builtin_mul_overflow(int, int, int *)],
+                [], [], [])
+
 dnl On Haiku we must use BSD-compatibility headers to get the endian
 dnl macros we use.
 AC_MSG_CHECKING(whether OS-dependent include paths are required)
diff --git a/common/include/checked-overflow.h
b/common/include/checked-overflow.h
index ddc4b487..c683b5a9 100644
--- a/common/include/checked-overflow.h
+++ b/common/include/checked-overflow.h
@@ -49,11 +49,19 @@
 /* Add two values.  *r = a + b
  * Returns true if overflow happened.
  */
+#if HAVE_DECL___BUILTIN_ADD_OVERFLOW
 #define ADD_OVERFLOW(a, b, r) __builtin_add_overflow((a), (b), (r))
+#else
+#define ADD_OVERFLOW(a, b, r) (*(r) = (a) + (b), 0)
+#endif
 
 /* Multiply two values.  *r = a * b
  * Returns true if overflow happened.
  */
+#if HAVE_DECL___BUILTIN_MUL_OVERFLOW
 #define MUL_OVERFLOW(a, b, r) __builtin_mul_overflow((a), (b), (r))
+#else
+#define MUL_OVERFLOW(a, b, r) (*(r) = (a) * (b), 0)
+#endif
 
 #endif /* NBDKIT_CHECKED_OVERFLOW_H */
-- 
2.32.0

On 11/15/21 13:14, Richard W.M. Jones wrote:
> On RHEL 7 (GCC 4.8.5) we don't have __builtin_add_overflow and similar
> functions.  They were first added in GCC 5.  Provide a fallback path
> for these older compilers.  Note that the fallback path does not
> actually implement overflow detection.
> ---
>  configure.ac                      | 6 ++++++
>  common/include/checked-overflow.h | 8 ++++++++
>  2 files changed, 14 insertions(+)
(1) I think that APIs that lie are wrong. I could contribute standard C
implementations if we really think nbdkit should continue building on
RHEL7. The difference with the built-ins should only be in performance,
not in functionality.

(2) Should nbdkit continue building on RHEL7? In the OCaml upgrade
thread
<https://listman.redhat.com/archives/libguestfs/2021-November/msg00095.html>
I thought we practically abandoned RHEL7 for the v2v projects.

(3) Red Hat Developer Toolset 10 (and 11 Beta) are available for RHEL7:

https://developers.redhat.com/products/developertoolset/overview

https://access.redhat.com/documentation/en-us/red_hat_developer_toolset/10/html/10.1_release_notes/system_requirements

https://access.redhat.com/documentation/en-us/red_hat_developer_toolset/11-beta/html/11.0_release_notes/system_requirements

providing gcc 10 and 11 respectively.

We could say that building nbdkit on RHEL7 requires devtoolset.

Thanks,
Laszlo
> 
> diff --git a/configure.ac b/configure.ac
> index 8980fdd4..30330813 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -220,6 +220,12 @@ CFLAGS="$old_CFLAGS"
>  AC_MSG_RESULT([$supports_std_c90])
>  AM_CONDITIONAL([CAN_TEST_ANSI_C], [test "x$supports_std_c90" =
"xyes"])
>  
> +dnl Check for __builtin_*_overflow.  We need the dummy parameters
> +dnl else detection doesn't work correctly for some reason.
> +AC_CHECK_DECLS([__builtin_add_overflow(int, int, int *),
> +                __builtin_mul_overflow(int, int, int *)],
> +                [], [], [])
> +
>  dnl On Haiku we must use BSD-compatibility headers to get the endian
>  dnl macros we use.
>  AC_MSG_CHECKING(whether OS-dependent include paths are required)
> diff --git a/common/include/checked-overflow.h
b/common/include/checked-overflow.h
> index ddc4b487..c683b5a9 100644
> --- a/common/include/checked-overflow.h
> +++ b/common/include/checked-overflow.h
> @@ -49,11 +49,19 @@
>  /* Add two values.  *r = a + b
>   * Returns true if overflow happened.
>   */
> +#if HAVE_DECL___BUILTIN_ADD_OVERFLOW
>  #define ADD_OVERFLOW(a, b, r) __builtin_add_overflow((a), (b), (r))
> +#else
> +#define ADD_OVERFLOW(a, b, r) (*(r) = (a) + (b), 0)
> +#endif
>  
>  /* Multiply two values.  *r = a * b
>   * Returns true if overflow happened.
>   */
> +#if HAVE_DECL___BUILTIN_MUL_OVERFLOW
>  #define MUL_OVERFLOW(a, b, r) __builtin_mul_overflow((a), (b), (r))
> +#else
> +#define MUL_OVERFLOW(a, b, r) (*(r) = (a) * (b), 0)
> +#endif
>  
>  #endif /* NBDKIT_CHECKED_OVERFLOW_H */
>