public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libfortran/67535] New: write.c sanitizer detects null pointer passed to memcpy
@ 2015-09-10 4:15 zeccav at gmail dot com
2015-09-10 6:35 ` [Bug libfortran/67535] " kargl at gcc dot gnu.org
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: zeccav at gmail dot com @ 2015-09-10 4:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67535
Bug ID: 67535
Summary: write.c sanitizer detects null pointer passed to
memcpy
Product: gcc
Version: 5.2.0
Status: UNCONFIRMED
Severity: minor
Priority: P3
Component: libfortran
Assignee: unassigned at gcc dot gnu.org
Reporter: zeccav at gmail dot com
Target Milestone: ---
During "make check" a null pointer is sometimes passed to memcpy in
write.c:1877
memcpy (ext_name, base_name, base_name_len);
because base_name == NULL
but base_name_len == 0 so it should be harmless.
To make happy the sanitizer the statement should be
if(base_name_len) memcpy (ext_name, base_name, base_name_len);
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libfortran/67535] write.c sanitizer detects null pointer passed to memcpy
2015-09-10 4:15 [Bug libfortran/67535] New: write.c sanitizer detects null pointer passed to memcpy zeccav at gmail dot com
@ 2015-09-10 6:35 ` kargl at gcc dot gnu.org
2015-09-10 6:59 ` jakub at gcc dot gnu.org
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: kargl at gcc dot gnu.org @ 2015-09-10 6:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67535
kargl at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |kargl at gcc dot gnu.org
--- Comment #1 from kargl at gcc dot gnu.org ---
(In reply to Vittorio Zecca from comment #0)
> During "make check" a null pointer is sometimes passed to memcpy in
> write.c:1877
>
> memcpy (ext_name, base_name, base_name_len);
>
> because base_name == NULL
>
> but base_name_len == 0 so it should be harmless.
> To make happy the sanitizer the statement should be
>
> if(base_name_len) memcpy (ext_name, base_name, base_name_len);
What happens to performance? Simply making changes to
make sanitizer happy seems rather questionable. It's clear
from context that if base_name == NULL, then base_name_len
== 0, and the memcpy should be a NOP.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libfortran/67535] write.c sanitizer detects null pointer passed to memcpy
2015-09-10 4:15 [Bug libfortran/67535] New: write.c sanitizer detects null pointer passed to memcpy zeccav at gmail dot com
2015-09-10 6:35 ` [Bug libfortran/67535] " kargl at gcc dot gnu.org
@ 2015-09-10 6:59 ` jakub at gcc dot gnu.org
2015-09-10 9:00 ` zeccav at gmail dot com
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2015-09-10 6:59 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67535
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at gcc dot gnu.org
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
It is still undefined behavior, so e.g. gcc will happily remove NULL pointer
checks after it sees a pointer used in memcpy, etc.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libfortran/67535] write.c sanitizer detects null pointer passed to memcpy
2015-09-10 4:15 [Bug libfortran/67535] New: write.c sanitizer detects null pointer passed to memcpy zeccav at gmail dot com
2015-09-10 6:35 ` [Bug libfortran/67535] " kargl at gcc dot gnu.org
2015-09-10 6:59 ` jakub at gcc dot gnu.org
@ 2015-09-10 9:00 ` zeccav at gmail dot com
2015-09-10 13:51 ` sgk at troutmask dot apl.washington.edu
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: zeccav at gmail dot com @ 2015-09-10 9:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67535
--- Comment #3 from Vittorio Zecca <zeccav at gmail dot com> ---
(In reply to kargl from comment #1)
> What happens to performance? Simply making changes to
> make sanitizer happy seems rather questionable. It's clear
> from context that if base_name == NULL, then base_name_len
> == 0, and the memcpy should be a NOP.
How costly is that NOP compared to if(base_name_len)?
How costly is to let go an undefined behaviour?
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libfortran/67535] write.c sanitizer detects null pointer passed to memcpy
2015-09-10 4:15 [Bug libfortran/67535] New: write.c sanitizer detects null pointer passed to memcpy zeccav at gmail dot com
` (2 preceding siblings ...)
2015-09-10 9:00 ` zeccav at gmail dot com
@ 2015-09-10 13:51 ` sgk at troutmask dot apl.washington.edu
2015-09-10 14:08 ` jakub at gcc dot gnu.org
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: sgk at troutmask dot apl.washington.edu @ 2015-09-10 13:51 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67535
--- Comment #4 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Thu, Sep 10, 2015 at 09:00:06AM +0000, zeccav at gmail dot com wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67535
>
> --- Comment #3 from Vittorio Zecca <zeccav at gmail dot com> ---
> (In reply to kargl from comment #1)
>
> > What happens to performance? Simply making changes to
> > make sanitizer happy seems rather questionable. It's clear
> > from context that if base_name == NULL, then base_name_len
> > == 0, and the memcpy should be a NOP.
>
> How costly is that NOP compared to if(base_name_len)?
I don't know. Of course, I'm not the one purposing a
change.
>
> How costly is to let go an undefined behaviour?
>
It's undefined behavior to pass a NULL pointer into a function?
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libfortran/67535] write.c sanitizer detects null pointer passed to memcpy
2015-09-10 4:15 [Bug libfortran/67535] New: write.c sanitizer detects null pointer passed to memcpy zeccav at gmail dot com
` (3 preceding siblings ...)
2015-09-10 13:51 ` sgk at troutmask dot apl.washington.edu
@ 2015-09-10 14:08 ` jakub at gcc dot gnu.org
2015-09-11 8:33 ` zeccav at gmail dot com
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2015-09-10 14:08 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67535
--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Steve Kargl from comment #4)
> It's undefined behavior to pass a NULL pointer into a function?
To a function that does not allow it? Yes.
Citing the C standard:
7.21.2.1/2:
"The memcpy function copies n characters from the object pointed to by s2 into
the object pointed to by s1."
(NULL pointer does not point to any object).
7.1.4/1:
"If a function argument is
described as being an array, the pointer actually passed to the function shall
have a value
such that all address computations and accesses to objects (that would be valid
if the
pointer did point to the first element of such an array) are in fact valid."
7.21.1:
"Where an argument declared as size_t n specifies the length of the array for a
function, n can have the value zero on a call to that function. Unless
explicitly stated
otherwise in the description of a particular function in this subclause,
pointer arguments
on such a call shall still have valid values, as described in 7.1.4."
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libfortran/67535] write.c sanitizer detects null pointer passed to memcpy
2015-09-10 4:15 [Bug libfortran/67535] New: write.c sanitizer detects null pointer passed to memcpy zeccav at gmail dot com
` (4 preceding siblings ...)
2015-09-10 14:08 ` jakub at gcc dot gnu.org
@ 2015-09-11 8:33 ` zeccav at gmail dot com
2015-09-12 12:06 ` fxcoudert at gcc dot gnu.org
2015-09-12 12:07 ` fxcoudert at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: zeccav at gmail dot com @ 2015-09-11 8:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67535
--- Comment #6 from Vittorio Zecca <zeccav at gmail dot com> ---
The cost of adding "if(base_name_len)" is two x86-64 machine instructions
cmpl $0, -20(%rbp)
je .L2
Six instructions follow then
call memcpy
which is not exactly a NOP even with base_name_len==0
In my opinion two machine instructions to avoid a useless and undefined memcpy
are a bargain.
It is not undefined behaviour to pass a NULL pointer to an arbitrary function,
but it is undefined to pass it to memcpy.
And it seems to me that after
memcpy(dest,src,len)
the statement "if(src)" is always TRUE because the optimizer assumes src!=NULL
The following is from https://gcc.gnu.org/gcc-4.9/porting_to.html
Null pointer checks may be optimized away more aggressively
GCC might now optimize away the null pointer check in code like:
int copy (int* dest, int* src, size_t nbytes) {
memmove (dest, src, nbytes);
if (src != NULL)
return *src;
return 0;
}
The pointers passed to memmove (and similar functions in <string.h>) must be
non-null even when nbytes==0, so GCC can use that information to remove the
check after the memmove call. Calling copy(p, NULL, 0) can therefore deference
a null pointer and crash.
The example above needs to be fixed to avoid the invalid memmove call, for
example:
if (nbytes != 0)
memmove (dest, src, nbytes);
This optimization can also affect implicit null pointer checks such as the one
done by the C++ runtime for the delete[] operator.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libfortran/67535] write.c sanitizer detects null pointer passed to memcpy
2015-09-10 4:15 [Bug libfortran/67535] New: write.c sanitizer detects null pointer passed to memcpy zeccav at gmail dot com
` (5 preceding siblings ...)
2015-09-11 8:33 ` zeccav at gmail dot com
@ 2015-09-12 12:06 ` fxcoudert at gcc dot gnu.org
2015-09-12 12:07 ` fxcoudert at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: fxcoudert at gcc dot gnu.org @ 2015-09-12 12:06 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67535
--- Comment #7 from Francois-Xavier Coudert <fxcoudert at gcc dot gnu.org> ---
Author: fxcoudert
Date: Sat Sep 12 12:05:44 2015
New Revision: 227705
URL: https://gcc.gnu.org/viewcvs?rev=227705&root=gcc&view=rev
Log:
PR libfortran/67527
PR libfortran/67535
PR libfortran/67536
* io/io.h: Use unsigned values for 31-bit left shifts.
* io/unix.c (buf_read): Do not call memcpy() with NULL pointer arg.
* io/write.c (nml_write_obj): Likewise.
Modified:
trunk/libgfortran/ChangeLog
trunk/libgfortran/io/io.h
trunk/libgfortran/io/unix.c
trunk/libgfortran/io/write.c
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libfortran/67535] write.c sanitizer detects null pointer passed to memcpy
2015-09-10 4:15 [Bug libfortran/67535] New: write.c sanitizer detects null pointer passed to memcpy zeccav at gmail dot com
` (6 preceding siblings ...)
2015-09-12 12:06 ` fxcoudert at gcc dot gnu.org
@ 2015-09-12 12:07 ` fxcoudert at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: fxcoudert at gcc dot gnu.org @ 2015-09-12 12:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67535
Francois-Xavier Coudert <fxcoudert at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
CC| |fxcoudert at gcc dot gnu.org
Resolution|--- |FIXED
Assignee|unassigned at gcc dot gnu.org |fxcoudert at gcc dot gnu.org
Target Milestone|--- |6.0
--- Comment #8 from Francois-Xavier Coudert <fxcoudert at gcc dot gnu.org> ---
Fixed. Thanks for the report.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-09-12 12:07 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-10 4:15 [Bug libfortran/67535] New: write.c sanitizer detects null pointer passed to memcpy zeccav at gmail dot com
2015-09-10 6:35 ` [Bug libfortran/67535] " kargl at gcc dot gnu.org
2015-09-10 6:59 ` jakub at gcc dot gnu.org
2015-09-10 9:00 ` zeccav at gmail dot com
2015-09-10 13:51 ` sgk at troutmask dot apl.washington.edu
2015-09-10 14:08 ` jakub at gcc dot gnu.org
2015-09-11 8:33 ` zeccav at gmail dot com
2015-09-12 12:06 ` fxcoudert at gcc dot gnu.org
2015-09-12 12:07 ` fxcoudert at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).