Hi Yasumasa,
I'm Okay with both A and B-1.
Personally, I like the simplicity of A but understand this issue is more
broad.
Thanks,
Serguei
On 10/11/19 20:22, Yasumasa Suenaga wrote:
Hi,
I updated Plan B-1 (#pragma) due to the comment from Chris [1].
A. Use memcpy()
http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.02/
B-1. Use #pragma
http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.B1-pragma.03/
I dropped Plan C (disable stringop-truncation in globally) from the
solutions
because Kim pointed this warning found out some actual bugs [2].
Thanks,
Yasumasa
[1]
https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029586.html
[2]
https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029590.html
On 2019/10/11 22:38, Yasumasa Suenaga wrote:
Hi,
Christos commented to B-1. Thanks!
clang defines __GNU_C__ , but stringop-truncation is not supported.
So I updated Plan B-1. It works fine on my Fedora30 box.
A. Use memcpy()
http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.02/
B-1. Use #pragma
http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.B1-pragma.02/
C. Set -Wno-stringop-truncation in globally
http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.C/
Of course I will push the change to submit repo before sending review
request.
Thanks,
Yasumasa
On 2019/10/11 15:55, Yasumasa Suenaga wrote:
Hi,
Thanks for a lot advises!
We have following solutions for this issue.
I'd like to send RFR again with much consented patch in early next
week.
A. Use memcpy()
http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.02/
B-1. Use #pragma
http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.B1-pragma/
C. Set -Wno-stringop-truncation in globally
http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.C/
If we fix with C, I will send RFR to hotspot-dev and build-dev.
Plan C also fixes other stringop-truncation problems such as
JDK-8220074.
Thus it affects all of JDK code, but it would be useful if
stringop-truncation
should be disabled in JDK build process.
Comments are welcome.
Yasumasa
On 2019/10/11 10:34, 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.
