Libguestfs - Sep 2020 - Re: [PATCH nbdkit] server/public.c: Uninline nbdkit_strdup_intern to avoid compiler warning.

I'm not sure if this is a GCC bug or a bug in our code, but
the attached workaround fixes it for me.

Rich.

Previously with GCC 10.2 this code produced:

In function ‘nbdkit_strndup_intern’,
    inlined from ‘nbdkit_strdup_intern’ at public.c:839:10:
public.c:827:10: error: ‘strndup’ specified bound 18446744073709551615 exceeds
maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  827 |   copy = strndup (str, n);
      |          ^~~~~~~~~~~~~~~~
---
 server/public.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/server/public.c b/server/public.c
index e871fdeb..270f81a4 100644
--- a/server/public.c
+++ b/server/public.c
@@ -836,7 +836,21 @@ nbdkit_strndup_intern (const char *str, size_t n)
 const char *
 nbdkit_strdup_intern (const char *str)
 {
-  return nbdkit_strndup_intern (str, SIZE_MAX);
+  char *copy;
+
+  if (str == NULL) {
+    nbdkit_error ("nbdkit_strdup_intern: no string given");
+    errno = EINVAL;
+    return NULL;
+  }
+
+  copy = strdup (str);
+  if (copy == NULL) {
+    nbdkit_error ("strdup: %m");
+    return NULL;
+  }
+
+  return add_intern (copy);
 }
 
 const char *
-- 
2.27.0

A simple reproducer is:

----------------------------------------------------------------------
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <string.h>

const char *
copyn (const char *str, size_t n)
{
  return strndup (str, n);
}

const char *
copy (const char *str)
{
  return copyn (str, SIZE_MAX);
}
----------------------------------------------------------------------

$ gcc -O2 -Wall -c test.c
In function ‘copyn’,
    inlined from ‘copy’ at test.c:15:10:
test.c:9:10: warning: ‘strndup’ specified bound 18446744073709551615 exceeds
maximum object size 9223372036854775807 [-Wstringop-overflow=]
    9 |   return strndup (str, n);
      |          ^~~~~~~~~~~~~~~~

$ gcc --version
gcc (GCC) 10.2.1 20200826 (Red Hat 10.2.1-3)
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

I'm still not clear if this is a GCC problem or a code problem,
although Dan pointed out on the list that SIZE_MAX is the largest
size_t, so I guess our code seems OK ...

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

On Thu, Sep 03, 2020 at 10:47:13AM +0100, Richard W.M. Jones
wrote:
> 
> A simple reproducer is:
> 
> ----------------------------------------------------------------------
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
> #include <string.h>
> 
> const char *
> copyn (const char *str, size_t n)
> {
>   return strndup (str, n);
> }
> 
> const char *
> copy (const char *str)
> {
>   return copyn (str, SIZE_MAX);
> }
> ----------------------------------------------------------------------
> 
> $ gcc -O2 -Wall -c test.c
> In function ‘copyn’,
>     inlined from ‘copy’ at test.c:15:10:
> test.c:9:10: warning: ‘strndup’ specified bound 18446744073709551615
exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
>     9 |   return strndup (str, n);
>       |          ^~~~~~~~~~~~~~~~
> 
> $ gcc --version
> gcc (GCC) 10.2.1 20200826 (Red Hat 10.2.1-3)
> Copyright (C) 2020 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> I'm still not clear if this is a GCC problem or a code problem,
> although Dan pointed out on the list that SIZE_MAX is the largest
> size_t, so I guess our code seems OK ...
The error message seems to indicate that the compiler believes a pointer
cannot reference an object larger than SSIZE_MAX and thus doesn't like
the use of SIZE_MAX.

This is quite strange though, given that malloc() and other C functions
pretty much all use  size_t for object sizes, not ssize_t.

I think we should try reporting to the GCC maintainers for an opinion
on whether its a compiler bug or not

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On 9/3/20 4:41 AM, Richard W.M. Jones wrote:
> Previously with GCC 10.2 this code produced:
> 
> In function ‘nbdkit_strndup_intern’,
>      inlined from ‘nbdkit_strdup_intern’ at public.c:839:10:
> public.c:827:10: error: ‘strndup’ specified bound 18446744073709551615
exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
>    827 |   copy = strndup (str, n);
>        |          ^~~~~~~~~~~~~~~~
> ---
>   server/public.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
I find the gcc warning annoying, but this workaround is fine (and 
potentially even a bit more efficient, because it uses fewer function 
calls).

ACK, regardless of whether gcc also agrees with your bug report.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org