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.
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. >>>>> >>> >>>
