I think Kim has already been clear that he is not a fan of option A:
I'm not a fan of replacing correct and idiomatic uses of strncpy with
strcpy or memcpy. I've suggested in the past that we should turn off
this warning while it is so buggy.
Dan
On 10/10/19 10:02 PM, Yasumasa Suenaga wrote:
Thanks Chris!
If others agree with A, I will push webrev.02 .
Yasumasa
On 2019/10/11 10:50, Chris Plummer wrote:
These solutions all have their merits and their warts, but we have to
pick something. I'm ok with with A or C. For B I'd rather you instead
used the #pragma at the warning site. If you go with (C), the review
should go out to all of hotspot-dev and build-dev.
Chris
On 10/10/19 6:34 PM, Yasumasa Suenaga wrote:
Hi,
I want to get conclusion of this discussion.
I understand the fix of macroAssembler_x86.hpp is ok, but we have
not yet had conclusion
how we should fix diagnosticArgument.cpp .
I think we can fix diagnosticArgument.cpp as following:
A. Use memcpy()
http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.02/
B. Add -Wno-stringop-truncation to
make/hotspot/lib/JvmOverrideFiles.gmk
This option will be added diagnosticArgument.cpp only.
C. Set -Wno-stringop-truncation in globally
make/hotspot/lib/CompileJvm.gmk
I prefer to fix like A because it affects minimally.
Some issues might be found out by stringop-truncation in future.
Thanks,
Yasumasa
On 2019/10/11 5:54, Kim Barrett wrote:
On Oct 10, 2019, at 3:03 AM, David Holmes
<david.hol...@oracle.com> wrote:
On 10/10/2019 4:50 pm, Chris Plummer wrote:
From JBS:
/home/ysuenaga/OpenJDK/jdk/src/hotspot/share/services/diagnosticArgument.cpp:154:14:
warning: 'char* strncpy(char*, const char*, size_t)' output
truncated before terminating nul copying as many bytes from a
string as its length [-Wstringop-truncation]
154 | strncpy(buf, str, len);
| ~~~~~~~^~~~~~~~~~~~~~~
I assume this means that in all cases the "len" value is seen to
be derived from strlen, and therefore strncpy is always copying
one byte short of \0, and this is most likely not what the user
wants. I seem to
Yes but we then explicitly set the NULL at buf[len] which is the
expected/required pattern for this.
recall another recent similar fix that was done by switching to
using memcpy instead.
Here's a discussion of interest, also suggesting memcpy:
https://stackoverflow.com/questions/50198319/gcc-8-wstringop-truncation-what-is-the-good-practice
Seems to me that strncpy and memcpy are semantically equivalent
here so all this does is avoid gcc's over zealous warnings. I'm
inclined to use the:
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wstringop-truncation"
solution.
YMMV.
We've run into and discussed problems with -Wstringop-truncation
before. (See discussions of JDK-8214777 and JDK-8223186.) This is a
relatively recent warning option (introduced in gcc8, and included in
-Wall), and seems to have a considerable bug tail:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88781
A metabug for -Wstringop-truncation, currently with 16 open and 10
resolved associated bugs.
I'm not a fan of replacing correct and idiomatic uses of strncpy with
strcpy or memcpy. I've suggested in the past that we should turn off
this warning while it is so buggy.
