RFR(S): 8252407: Build failure with gcc-8+ and asan

[Kim Barrett]

> 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.

------------------------------------------------------------------------------