Hi,
I proposed 3 solutions in [1].
Plan B-1 is #pragma, but HotSpot provides valid macro to compiler.
I added stringop-truncation to it [2], and it would work each compilers.
Yasumasa
[1]
https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029580.html
[2] http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.B1-pragma/
On 2019/10/11 21:51, David Holmes wrote:
On 11/10/2019 10:47 pm, Christos Zoulas wrote:
At NetBSD we decided to change the Makefiles because we compile with both
clang and gcc
and clang does not have those warnings and it complains about the pragmas.
<sigh> Compilers are supposed to ignore pragmas they don't understand.
Complaining about an unknown pragma defeats the whole purpose of having pragmas.
I suppose to prevent problems with clang we must disable the warning in the
makefile.
David
christos
On Oct 11, 2019, at 2:54 AM, David Holmes <david.hol...@oracle.com> wrote:
Yes I prefer:
D. Use #pragma ... at the callsite to disable this particular diagnostic.
over changing the makefiles.
David
On 11/10/2019 3:37 pm, Ioi Lam wrote:
For diagnosticArgument.cpp, I would prefer adding inline #pragma at only the
sites affected (as suggested by David Holmes), with a comment about why this is
necessary.
But if others decide to go with other solutions, that's fine with me, too, but
please remember to add a comment :-)
Thanks
- Ioi
On 10/10/19 6:50 PM, 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.
