Bug 61294 - [4.9 Regression] erroneous memset used with constant zero length parameter warning
Summary: [4.9 Regression] erroneous memset used with constant zero length parameter wa...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.9.0
: P2 normal
Target Milestone: 5.0
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
: 16794 62033 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-05-23 15:29 UTC by Orion Poplawski
Modified: 2023-03-31 11:42 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work: 5.0
Known to fail:
Last reconfirmed: 2014-05-26 00:00:00


Attachments
preprocessed test case (153.56 KB, application/x-gzip)
2014-05-23 15:29 UTC, Orion Poplawski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Orion Poplawski 2014-05-23 15:29:18 UTC
Created attachment 32846 [details]
preprocessed test case

$ g++ -std=c++11 -O2 -c test.cxx
In function 'void* memset(void*, int, size_t)',
    inlined from 'static void vtkDistributedDataFilter::AddConstantUnsignedCharPointArray(vtkUnstructuredGrid*, const char*, unsigned char)' at test.cxx:31883:29:
test.cxx:15303:32: warning: call to '__warn_memset_zero_len' declared with attribute warning: memset used with constant zero length parameter; this could be due to transposed parameters
       __warn_memset_zero_len ();
                                ^

# g++ -v
Using built-in specs.
COLLECT_GCC=/usr/bin/g++
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/4.9.0/lto-wrapper
Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --enable-plugin --enable-initfini-array --disable-libgcj --with-isl=/builddir/build/BUILD/gcc-4.9.0-20140518/obj-x86_64-redhat-linux/isl-install --with-cloog=/builddir/build/BUILD/gcc-4.9.0-20140518/obj-x86_64-redhat-linux/cloog-install --enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
gcc version 4.9.0 20140518 (Red Hat 4.9.0-5) (GCC)

No warning with 4.8.2 (or without -O2)
Comment 1 Richard Biener 2014-05-26 12:42:44 UTC
Happens on an isolated path where GCC optimized

 if (len)
   ....

 memset (p, val, len);

to

 if (len == 0)
   {
  ....
   memset (p, val, 0);
   }
 else
   {
 .....
   memset (p, val, len);
   }

can happen and there is no good way around that, thus it's a false positive.

It's the

   if (grid->Points)

check that is false when the compiler can compute the 'len' parameter will
be zero.  Maybe that helps you deciding whether it really is a false positive
or not (well, yes, whether you are calling memset with len == 0, which
might be valid, but we warn about it).
Comment 2 Richard Biener 2014-05-26 12:45:14 UTC
That is,

inline vtkIdType vtkPointSet::GetNumberOfPoints()
{
  if (this->Points)
    {
    return this->Points->GetNumberOfPoints();
    }
  else
    {
    return 0;
    }
}

and


void vtkDistributedDataFilter::AddConstantUnsignedCharPointArray(
  vtkUnstructuredGrid *grid, const char *arrayName, unsigned char val)
{
  vtkIdType npoints = grid->GetNumberOfPoints();
  unsigned char *vals = new unsigned char [npoints];
  memset(vals, val, npoints);
}

may allocate zero points and initialize zero points due to that extra
"safety" check.  If you put a assert (npoints > 0) in there then
the warning will go away.
Comment 3 Orion Poplawski 2014-05-29 15:46:39 UTC
Hmm, now all of a sudden I don't seem to be able to reproduce this.  Thanks for your comments though.  I play around if it re-appears.  The problem was compounded by the use of -Wl,--fatal-warnings so it was preventing linkage.
Comment 4 Jason Merrill 2014-06-03 13:11:27 UTC
It's clearly a false positive; the warning is intended to catch calls where the user wrote a 0 directly in the argument list for memset (which suggests accidentally transposed arguments), not cases where some execution path might result in a 0 argument (which works fine).  The only question is what there is to fix either in GCC or GLIBC to avoid this false positive.

A simple way to work around this is to guard the memset with if (npoints > 0).

The false positive seems to come up fairly often:

https://sourceware.org/ml/binutils/2012-02/msg00073.html
https://bugzilla.redhat.com/show_bug.cgi?id=452219
https://www.nsnam.org/bugzilla/show_bug.cgi?id=1165

Changing component to middle-end.

Reduced C testcase:

typedef __SIZE_TYPE__ size_t;
extern void *malloc (size_t __size) __attribute__ ((__malloc__)) __attribute__ ((__warn_unused_result__));
extern void *memset (void *__s, int __c, size_t __n) __attribute__ ((__nonnull__ (1)));
extern void __warn_memset_zero_len (void) __attribute__((__warning__ ("memset used with constant zero length parameter; this could be due to transposed parameters")));
extern __inline __attribute__((__always_inline__)) __attribute__((__artificial__))
void *
  memset (void *__dest, int __ch, size_t __len)
{
  if (__builtin_constant_p (__len) && __len == 0
      && (!__builtin_constant_p (__ch) || __ch != 0))
    {
      __warn_memset_zero_len ();
      return __dest;
    }
  return __builtin___memset_chk (__dest, __ch, __len,
				 __builtin_object_size (__dest, 0));
}

int i;
inline int f()
{
  if (i)
    return i;
  else
    return 0;
}

void g(unsigned char val)
{
  int len = f();
  void *p = malloc (len);
  memset (p, val, len);
}
Comment 5 Jakub Jelinek 2014-06-25 11:37:00 UTC
I wonder if we just shouldn't move this warning to GCC and in glibc remove it if sufficiently new GCC version is used.
GCC could then warn about this in the FEs, only in the cases where user likely made a mistake (i.e. when third argument is literal 0 (rather than e.g. an expression folded to 0) and second argument is not literal 0.
Comment 6 Jakub Jelinek 2014-07-14 07:37:10 UTC
Author: jakub
Date: Mon Jul 14 07:36:39 2014
New Revision: 212510

URL: https://gcc.gnu.org/viewcvs?rev=212510&root=gcc&view=rev
Log:
	PR middle-end/61294
gcc/c-family/
	* c.opt (Wmemset-transposed-args): New warning.
gcc/c/
	* c-parser.c (c_parser_expr_list): Add new argument literal_zero_mask.
	If non-NULL, call c_parser_check_literal_zero.
	(c_parser_check_literal_zero): New function.
	(c_parser_postfix_expression_after_primary): Adjust
	c_parser_expr_list caller, handle -Wmemset-transposed-args.
gcc/cp/
	* cp-tree.h (LITERAL_ZERO_P): Define.
	* parser.c (cp_parser_parenthesized_expression_list): Add
	want_literal_zero_p argument, if true, for literal zeros
	insert INTEGER_CSTs with LITERAL_ZERO_P flag set.
	(cp_parser_postfix_expression): Adjust
	cp_parser_parenthesized_expression_list caller, handle
	-Wmemset-transposed-args.
	(literal_zeros): New variable.
gcc/
	* doc/invoke.texi (-Wmemset-transposed-args): Document.
gcc/testsuite/
	* c-c++-common/Wmemset-transposed-args1.c: New test.
	* c-c++-common/Wmemset-transposed-args2.c: New test.
	* g++.dg/warn/Wmemset-transposed-args-1.C: New test.

Added:
    trunk/gcc/testsuite/c-c++-common/Wmemset-transposed-args1.c
    trunk/gcc/testsuite/c-c++-common/Wmemset-transposed-args2.c
    trunk/gcc/testsuite/g++.dg/warn/Wmemset-transposed-args-1.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c.opt
    trunk/gcc/c/ChangeLog
    trunk/gcc/c/c-parser.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/parser.c
    trunk/gcc/doc/invoke.texi
    trunk/gcc/testsuite/ChangeLog
Comment 7 Jakub Jelinek 2014-07-16 13:29:06 UTC
GCC 4.9.1 has been released.
Comment 8 Andrew Pinski 2014-08-06 15:30:34 UTC
*** Bug 62033 has been marked as a duplicate of this bug. ***
Comment 9 Jakub Jelinek 2014-10-30 10:39:06 UTC
GCC 4.9.2 has been released.
Comment 10 Richard Biener 2014-11-24 16:44:59 UTC
doesn't exactly work with GCC 5 now but requires an updated GLIBC AFAICS.
Comment 11 Jakub Jelinek 2014-11-24 16:50:45 UTC
Carlos/Siddhesh, can you please adjust the glibc headers, so that for __GNUC_PREREQ (5, 0) it doesn't contain the __warn_memset_zero_len stuff, as GCC will handle warning about it?
Comment 12 Carlos O'Donell 2014-11-24 19:26:57 UTC
(In reply to Jakub Jelinek from comment #11)
> Carlos/Siddhesh, can you please adjust the glibc headers, so that for
> __GNUC_PREREQ (5, 0) it doesn't contain the __warn_memset_zero_len stuff, as
> GCC will handle warning about it?

Siddhesh will have a look at fixing this in master.
Comment 13 Siddhesh Poyarekar 2014-11-27 05:47:09 UTC
Fixed in glibc:

commit 1721f0a406e52f976f9daf6f59acf42c1dbd33ff
Author: Siddhesh Poyarekar <siddhesh@redhat.com>
Date:   Thu Nov 27 11:15:45 2014 +0530

    Don't use __warn_memset_zero_len for gcc-5.0 or newer
    
    gcc now warns when the arguments to memset may have been accidentally
    transposed (i.e. length set to zero instead of the byte), so we don't
    need that bit of the code in glibc headers anymore.
    
    Tested on x86_64.  Coe generated by gcc 4.8 is identical with or
    without the patch.  I also tested gcc master, which does not result in
    any new failures.  It does fail quite a few FORTIFY_SOURCE tests, but
    those failures are not due to this patch.
Comment 14 Jakub Jelinek 2015-06-26 19:57:02 UTC
GCC 4.9.3 has been released.
Comment 15 Richard Biener 2016-08-03 10:52:45 UTC
Fixed in GCC 5+
Comment 16 Andrew Pinski 2023-03-31 11:42:53 UTC
*** Bug 16794 has been marked as a duplicate of this bug. ***