On 10/10/2019 4:50 pm, Chris Plummer wrote:
On 10/9/19 11:36 PM, David Holmes wrote:
Hi Yasumasa,
On 10/10/2019 3:55 pm, Yasumasa Suenaga wrote:
Hi all,
Please review this change:
JBS: https://bugs.openjdk.java.net/browse/JDK-8232084
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.00/
I tried to build OpenJDK on Fedora 30 x64 with GCC 9.2.1, but it was
failed in macroAssembler_x86.hpp and diagnosticArgument.cpp .
The macroAssembler fix is fine - we need to ensure there is a NULL
check for variables that are passed to %s.
The strncpy fix in diagnosticArgument.cpp I'm less clear about. I
don't understand what the warning is warning about. It seems to be
concerned about possible truncation, but IIUC truncation is fine, in
fact that's exactly why we use strncpy to ensure we don't overflow the
buffer if the value we are copying is too long! Is this a case of gcc
trying to be too helpful ?? I found this article:
https://developers.redhat.com/blog/2018/05/24/detecting-string-truncation-with-gcc-8/
which suggests that pattern we have of explicitly setting the NUL
after the strncpy should be fine. I can't see anything wrong with the
existing code. That said the modified code is also not incorrect.
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.
Cheers,
David
Chris
Thanks,
David
-----
(Please see JBS for details)
This change has passed tests on submit repo.
Thanks,
Yasumasa (ysuenaga)
