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

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 */
>

On Mon, Nov 15, 2021 at 02:11:01PM +0100, Laszlo Ersek
wrote:
> (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.
If I sound schizophrenic on RHEL 7 that'll be because I'm in two minds
still about supporting it.

In this particular case I was trying to test the OCaml changes on the
oldest version of OCaml I have easy access to and found that there
were various problems compiling on RHEL 7.  I fixed the other
problems, but this fix was a bit more controversial.
> (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.
This doesn't sound very convenient, but there's a stronger reason to
keep stuff working with GCC 3/4 and that's because OpenBSD sticks with
GCC 4.2 as it was the last GPLv2+ version.  I think the reason OpenBSD
does this is wrong, but that's their choice.  (Of course they have
Clang).
> (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.
I agree.  If you want to have a go at standard C versions that would
be great.  Even unsigned addition looks very hard.

I found this lengthy paper on the subject:
https://www.cs.utah.edu/~regehr/papers/overflow12.pdf

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW