Re: Silence gcc-8 warnings

[Alan Modra <amodra at gmail dot com>]

On Mon, Apr 23, 2018 at 05:12:18PM +0000, Michael Matz wrote:
> Hi,
> 
> On Tue, 24 Apr 2018, Alan Modra wrote:
> 
> > On Tue, Apr 24, 2018 at 12:41:31AM +0930, Alan Modra wrote:
> > > Yeah, really curious.  I failed to mention that gcc build was on
> > > hppa-linux, whereas the one that didn't show any error was on
> > > x86_64-linux.  I have a little sleuthing to do to figure out what made
> > > the difference.  It doesn't seem likely that it was any of the more
> > > recent gcc patches.
> > 
> > Oh wow, I don't see the first error on x86_64-linux (for
> > swap_linux_prpsinfo32_ugid32_out) when the preprocessed source looks
> > like the following.  However, take out the file/line directives and
> > the problem appears!
> 
> The elf-linux-core.h header is regarded as system header with the #line 
> directives, and that disables the warning.

Horrible.  The line directives are *not* there for hppa-linux.  So
whether you get the warning or not apparently depends on a
#define strncpy(dest, src, n) __builtin_strncpy (dest, src, n)
appearing in a system include.

This gives a warning on x86_64-linux:

#include <string.h>
#define strncpy(dest, src, n) __builtin_strncpy (dest, src, n)
struct internal {char pr_fname[16 + 1];};
struct external {char pr_fname[16];};

struct external *
f1 (const struct internal *from)
{
  static struct external data;
  strncpy (data.pr_fname, from->pr_fname, sizeof (data.pr_fname));
  return &data;
}

Remove the #define and there's no warning!

>  I've looked at this somewhat 
> further, and actually the warning is correct (!), but subtle:
> 
> > static inline void
> > swap_linux_prpsinfo32_ugid32_out
> >   (bfd *obfd,
> >    const struct elf_internal_linux_prpsinfo *from,
> >    struct elf_external_linux_prpsinfo32_ugid32 *to)
> 
> So, FROM has different type from TO (xxx vs xxx_ugid32):
> from (from elf-bfd.h):
>   struct elf_internal_linux_prpsinfo
>   {
>     ...
>     char pr_fname[16 + 1];
>     char pr_psargs[80 + 1];
>   };
> and to (from elf-linux-core.h):
> struct elf_external_linux_prpsinfo32_ugid32
>   {
>     ...
>     char pr_fname[16];
>     char pr_psargs[80];
>   };
> 
> So, the strncpy was:
> 
>  __builtin_strncpy (to->pr_fname, from->pr_fname, sizeof (to->pr_fname))
> 
> sizeof(to->prfname) is 16, but sizeof(from->prfname) is 17, so it is 
> indeed conceivable that the from string has 16 characters plus null 
> terminator, which would not fit terminated into to->pr_fname, and this is 
> what is warned about.  I think this is a genuine bug in bfd (even though 
> possibly a harmless one with non-fuzzed prpsinfo structs in core files).

No, it is not a bug.  We have internal and external structs, and a
field in the external struct that does not need to be NULL
terminated.  For convenience, the internal struct field is one larger
so that the internal representation is always NULL terminated.  When
copying from internal to external representation there is no
possibility that we lose anything but the NULL.

What's more, we do want strncpy behaviour rather than memcpy, since
for defensive programming we don't want to copy possibly uninitialized
data past a string terminator in the internal representation, and do
want to fill the external representation with zeros.

-- 
Alan Modra
Australia Development Lab, IBM