> On Sep 1, 2020, at 9:59 AM, Eric Liu <eric.c....@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. ------------------------------------------------------------------------------
