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

[Kim Barrett]

> On Sep 7, 2020, at 10:56 AM, Eric Liu <eric.c.liu at arm.com> wrote:
> I have tested 4 cases for those warnings:
> a) Without my patch, without asan, gcc-8 and gcc-10 are OK.
> b) Without my patch, with asan, gcc-8 has warned, gcc-10 is OK.
> c) With my patch, without asan, gcc-8 and gcc-10 are OK.

That’s *very* strange, since I get warnings in NetworkInterface getIndex and getFlags
with gcc10.2 + your patch.

> d) With my patch, with asan, gcc-8 is OK, gcc-10 has warned.
> 
> Those warnings can be reported by both gcc-8 and gcc-10 only when asan enabled. Without
> asan, no warning would be found even through some of them doesn't align with the documentation.

>> 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.
> 
> Yse, I followed the documentation and gcc-8 does not warn about this, but
> gcc-10 does (both with asan enabled). 
> 
>> 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;
> 
> After removing the memset, gcc-10(10.1.0) still warns about it, gcc-8 and gcc-9 also.

I’m confused.  Above you said that with your patch + asan gcc-8 did not warn.
Maybe what you are saying is that *just* removing the memset, and not applying
your patch, for this case with asan warns with all of gcc-8,9,10?

> I'm working on a new patch that to make both gcc-8 and gcc-10 happy.

Good luck!

I think I’m coming around to Florian’s “never use strncpy” position, not so much because
it’s the wrong function to use (though sometimes it probably is, or is being used wrongly),
but because fighting with -Wstringop-truncation is just not worth the aggravation.