RFR(S): 8252407: Build failure with gcc-8+ and asan
Kim Barrett
kim.barrett at oracle.com
Sun Sep 6 11:35:40 UTC 2020
> On Sep 1, 2020, at 9:59 AM, Eric Liu <eric.c.liu at arm.com> wrote:
> I just tested this patch by GCC (10.1.0) and it would really re-trigger those warnings :(
> I have not noticed the history before, but it's really hard to imagine that GCC would
> have different behaviors.
Can you be (very) specific about this. Do all of these changes cause gcc10 to warn? Or
do only some of them. If only some, specifically which ones? I have a conjecture about
some of them (see below). (I should probably try this myself; I know we have done some
testing with gcc10, but I don’t remember where to find that devkit.)
------------------------------------------------------------------------------
src/java.base/unix/native/libnet/NetworkInterface.c
232 strncpy(searchName, name_utf, IFNAMESIZE - 1);
A better solution here would be to eliminate the strncpy entirely.
The strncpy is used to make a copy of a string so the copy can be
searched for a colon, and if one is found, terminate the string there.
But there's no need to make such a copy. The string being copied is
the local temporary string name_utf. We could instead use name_utf
everywhere searchName is currently used, including clobbering the
first colon to NUL. We'd have to undo that before the later loop at
line 249, but we have the information needed to do so.
------------------------------------------------------------------------------
src/java.base/unix/native/libnet/NetworkInterface.c
1298 memset((char *)&if2, 0, sizeof(if2));
1299 strncpy(if2.ifr_name, name, sizeof(if2.ifr_name) - 1);
1300 if2.ifr_name[sizeof(if2.ifr_name) - 1] = 0;
(in getIndex)
So gcc8 does not warn about this, but gcc10 does? That would be a
regression in gcc10 (and should be reported as such), because the
documentation for -Wstringop-truncate is clear that the above is the
proper idiom for avoiding the warning.
Regardless, the memset on 1298 is useless. The code from before
JDK-8250863 was the memset then the strncpy with
sizeof(if2.ifr_name)-1 as the bound, which worked because the memset
would have zeroed the last element.
The change for JDK-8250863 did not follow the documented idiom though.
It would be interesting to find out if removal of the memset changes
anything. It's barely conceivable that it's confusing the heuristics
used to decide whether -Wstringop-truncation should trigger. For
example, the compiler might decide that the subsequent assignment of
the last element is unnecessary because of the memset and optimize the
assignment away, removing the idiomatic warning suppression.
If gcc10 still warns about the above after removing the memset then I
see little recourse but to use PRAGMA_STRINGOP_TRUNCATION_IGNORED here.
Similarly for the strncpy in getFlags:
1362 memset((char *)&if2, 0, sizeof(if2));
1363 strncpy(if2.ifr_name, ifname, sizeof(if2.ifr_name) - 1);
1364 if2.ifr_name[sizeof(if2.ifr_name) - 1] = 0;
------------------------------------------------------------------------------
src/java.base/unix/native/libnet/NetworkInterface.c
937 strncpy(name, if_name, IFNAMESIZE - 1);
938 name[IFNAMESIZE - 1] = '\0';
gcc10 presumably did not complain about the old version here, and this
was not touched as part of JDK-8250863. Does gcc10 complain about
this new version? If so, then I see little recoarse but to use
PRAGMA_STRINGOP_TRUNCATION_IGNORED here, because a gcc10 warning here
is contrary to the documentation.
------------------------------------------------------------------------------
src/hotspot/share/compiler/compileBroker.hpp
64 PRAGMA_DIAG_PUSH
65 PRAGMA_STRINGOP_TRUNCATION_IGNORED
66 // This code can incorrectly cause a "stringop-truncation" warning with gcc
67 strncpy(_current_method, method, (size_t)cmname_buffer_length-1);
68 PRAGMA_DIAG_POP
69 _current_method[cmname_buffer_length-1] = '\0';
I think I'd prefer the PRAGMA_DIAG_POP moved one line further down, so
that the push/pop surrounds the entire idiomactic usage that is
supposed to prevent the warning. This seems to be a gcc8 bug.
gcc10 doesn't warn about this (and shouldn't). It would be
interesting to know if it too warns with --enable-asan.
------------------------------------------------------------------------------
src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c
est/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/events/EM04/em04t001/em04t001.cpp
These changes look good, as they follow the documented idiom for
suppressing -Wstringop-truncation. Does gcc10 warn after these changes?
------------------------------------------------------------------------------
test/jdk/java/nio/channels/spi/SelectorProvider/inheritedChannel/libInheritedChannel.c
(In Java_UnixDomainSocket_bind0)
234 memset(&addr, 0, sizeof(addr));
235 addr.sun_family = AF_UNIX;
236 strncpy(addr.sun_path, nameUtf, length - 1);
237 addr.sun_path[length - 1] = '\0';
(In Java_UnixDomainSocket_connect0)
267 memset(&addr, 0, sizeof(addr));
268 addr.sun_family = AF_UNIX;
269 strncpy(addr.sun_path, nameUtf, length - 1);
270 addr.sun_path[length - 1] = '\0';
These changes look good, as they follow the documented idiom for
suppressing -Wstringop-truncation.
Does gcc10 warn after these changes? Per the discussion above about
getIndex in NetworkInterface.c, does removing the memsets change that?
The memsets seem entirely unnecessary, since we're filling in all
(both) members.
------------------------------------------------------------------------------
More information about the security-dev
mailing list