Discussion:
[PATCH] dfa: port to gcc -fsanitize=undefined
(too old to reply)
Paul Eggert
2017-01-16 02:09:05 UTC
Permalink
Raw Message
* lib/dfa.c (copy): Don’t pass NULL with size 0 to memcpy,
as this runs afoul of gcc -fsanitize=undefined.
---
ChangeLog | 6 ++++++
lib/dfa.c | 3 ++-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index f13c784..e0b836b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2017-01-15 Paul Eggert <***@cs.ucla.edu>
+
+ dfa: port to gcc -fsanitize=undefined
+ * lib/dfa.c (copy): Don’t pass NULL with size 0 to memcpy,
+ as this runs afoul of gcc -fsanitize=undefined.
+
2017-01-14 Paul Eggert <***@cs.ucla.edu>

strftime: %z is -00 if unknown
diff --git a/lib/dfa.c b/lib/dfa.c
index 5df27ea..f6c3017 100644
--- a/lib/dfa.c
+++ b/lib/dfa.c
@@ -1999,8 +1999,9 @@ copy (position_set const *src, position_set *dst)
dst->elems = xpalloc (NULL, &dst->alloc, src->nelem - dst->alloc, -1,
sizeof *dst->elems);
}
- memcpy (dst->elems, src->elems, src->nelem * sizeof *dst->elems);
dst->nelem = src->nelem;
+ if (src->nelem != 0)
+ memcpy (dst->elems, src->elems, src->nelem * sizeof *dst->elems);
}

static void
--
2.9.3
Eric Blake
2017-01-16 16:29:34 UTC
Permalink
Raw Message
* lib/dfa.c (copy): Don’t pass NULL with size 0 to memcpy,
as this runs afoul of gcc -fsanitize=undefined.
It's lame that gcc warns on that usage; I'm half-tempted to propose a
POSIX bug that various memory functions (memcpy being one of them)
should sanely behave on a length of 0 regardless of the pointer argument
(that is, require that implementations not dereference the pointer when
length is 0). But even if POSIX accepts such an improvement, we're
still stuck working around existing compilers/system headers that warn.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Paul Eggert
2017-01-16 18:26:37 UTC
Permalink
Raw Message
Post by Eric Blake
Post by Paul Eggert
* lib/dfa.c (copy): Don’t pass NULL with size 0 to memcpy,
as this runs afoul of gcc -fsanitize=undefined.
It's lame that gcc warns on that usage
I agree; I wish it didn't, as the usage is benign on all real-world platforms
other than those set up to debug "violations". It's reasonable to warn about
memcpy (x, y, 0) when x or y is garbage, but not when x or y is null.
Kamil Dudka
2017-01-16 18:37:18 UTC
Permalink
Raw Message
Post by Eric Blake
Post by Paul Eggert
* lib/dfa.c (copy): Don’t pass NULL with size 0 to memcpy,
as this runs afoul of gcc -fsanitize=undefined.
It's lame that gcc warns on that usage; I'm half-tempted to propose a
POSIX bug that various memory functions (memcpy being one of them)
should sanely behave on a length of 0 regardless of the pointer argument
(that is, require that implementations not dereference the pointer when
length is 0). But even if POSIX accepts such an improvement, we're
still stuck working around existing compilers/system headers that warn.
Note this is not the first time this topic is discussed on bug-gnulib:

https://lists.gnu.org/archive/html/bug-gnulib/2009-05/msg00100.html

It can cause a real crash in certain execution environments.

Kamil
Paul Eggert
2017-01-16 19:57:03 UTC
Permalink
Raw Message
Post by Kamil Dudka
It can cause a real crash in certain execution environments.
Which ones? I'm not interested in environments like -fsanitize=undefined, which
is designed to catch violations of the standard. I want to know of a real
execution environment where memcpy (0, 0, 0) does something bad, and why it does so.
Kamil Dudka
2017-01-16 22:00:02 UTC
Permalink
Raw Message
Post by Paul Eggert
Post by Kamil Dudka
It can cause a real crash in certain execution environments.
Which ones? I'm not interested in environments like -fsanitize=undefined,
which is designed to catch violations of the standard. I want to know of a
real execution environment where memcpy (0, 0, 0) does something bad, and
why it does so.
Have you actually looked at the discussion I referenced?

It was about memchr (0, 'a', 0) causing SIGSEGV without -fsanitize=undefined:

https://lists.gnu.org/archive/html/bug-gnulib/2009-05/msg00081.html

I am not sure about memcpy (0, 0, 0) in particular but, in principle, I see
no difference between those two cases.

Kamil
Eric Blake
2017-01-16 22:20:20 UTC
Permalink
Raw Message
Post by Kamil Dudka
Post by Paul Eggert
Post by Kamil Dudka
It can cause a real crash in certain execution environments.
Which ones? I'm not interested in environments like -fsanitize=undefined,
which is designed to catch violations of the standard. I want to know of a
real execution environment where memcpy (0, 0, 0) does something bad, and
why it does so.
Have you actually looked at the discussion I referenced?
https://lists.gnu.org/archive/html/bug-gnulib/2009-05/msg00081.html
The thread went on to point out that it was a glibc bug for doing so,
and that quality-of-implementation meant that glibc was patched to no
longer have that bug.

In other words, strengthening POSIX to guarantee something (that the
pointer is never dereferenced, therefore undefined behavior never
triggered, on zero size) beyond what the C standard leaves unspecified
has actual cases where implementations either already get it right or
are willing to fix code to get it right.
Post by Kamil Dudka
I am not sure about memcpy (0, 0, 0) in particular but, in principle, I see
no difference between those two cases.
And the principle is that POSIX is allowed to make guarantees where the
C standard left things unspecified, particularly if those guarantees are
already something that many coders are already relying on because they
don't know any better.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Eric Blake
2017-01-16 22:31:08 UTC
Permalink
Raw Message
Post by Eric Blake
And the principle is that POSIX is allowed to make guarantees where the
C standard left things unspecified, particularly if those guarantees are
already something that many coders are already relying on because they
don't know any better.
My case study for that is the fact that storage returned by calloc() and
interpreted as a struct containing pointers is NOT required by the C
standard to treat those fields as null pointers, but POSIX was willing
to add that as an additional requirement:
http://austingroupbugs.net/view.php?id=940
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Paul Eggert
2017-01-16 23:15:04 UTC
Permalink
Raw Message
Post by Eric Blake
POSIX is allowed to make guarantees where the
C standard left things unspecified, particularly if those guarantees are
already something that many coders are already relying on because they
don't know any better.
Yes, Gnulib code makes several such assumptions, documented in the Gnulib
manual. See:

https://www.gnu.org/software/gnulib/manual/html_node/Portability-guidelines.html

For example, Gnulib code assumes that adding zero to a null pointer results in a
null pointer. Luckily for us, -fsanitize=undefined doesn't try to catch these
other violations of the standard, something that would waste even more of
everybody's time.

Perhaps we should add memcpy (0, 0, 0) to that section of the Gnulib manual, if
only to document our irritation at -fsanitize=undefined crying wolf here.
Loading...