Hi all,
I pushed the change: https://hg.openjdk.java.net/jdk/jdk/rev/9f5b92d5a1b2
I hope PRAGMA_STRINGOP_TRUNCATION_IGNORED which introduced in this commit will
be used when similar issues occur in future.
Thanks for all your help! ありがとう！
Yasumasa
On 2019/10/17 13:54, Ioi Lam wrote:
On
On 10/16/19 9:25 PM, Yasumasa Suenaga wrote:
On 2019/10/17 13:21, David Holmes wrote:
On 17/10/2019 2:20 pm, Yasumasa Suenaga wrote:
On 2019/10/17 12:49, David Holmes wrote:
On 17/10/2019 12:45 pm, Yasumasa Suenaga wrote:
On 2019/10/17 11:39, David Holmes wrote:
On 17/10/2019 12:19 pm,
On 2019/10/17 13:21, David Holmes wrote:
On 17/10/2019 2:20 pm, Yasumasa Suenaga wrote:
On 2019/10/17 12:49, David Holmes wrote:
On 17/10/2019 12:45 pm, Yasumasa Suenaga wrote:
On 2019/10/17 11:39, David Holmes wrote:
On 17/10/2019 12:19 pm, Yasumasa Suenaga wrote:
On 2019/10/17 9:34, David
On 17/10/2019 2:20 pm, Yasumasa Suenaga wrote:
On 2019/10/17 12:49, David Holmes wrote:
On 17/10/2019 12:45 pm, Yasumasa Suenaga wrote:
On 2019/10/17 11:39, David Holmes wrote:
On 17/10/2019 12:19 pm, Yasumasa Suenaga wrote:
On 2019/10/17 9:34, David Holmes wrote:
On 17/10/2019 10:07 am,
On 2019/10/17 12:49, David Holmes wrote:
On 17/10/2019 12:45 pm, Yasumasa Suenaga wrote:
On 2019/10/17 11:39, David Holmes wrote:
On 17/10/2019 12:19 pm, Yasumasa Suenaga wrote:
On 2019/10/17 9:34, David Holmes wrote:
On 17/10/2019 10:07 am, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
On 17/10/2019 12:45 pm, Yasumasa Suenaga wrote:
On 2019/10/17 11:39, David Holmes wrote:
On 17/10/2019 12:19 pm, Yasumasa Suenaga wrote:
On 2019/10/17 9:34, David Holmes wrote:
On 17/10/2019 10:07 am, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
It looks good.
One tip:
+ // This code
On 2019/10/17 11:39, David Holmes wrote:
On 17/10/2019 12:19 pm, Yasumasa Suenaga wrote:
On 2019/10/17 9:34, David Holmes wrote:
On 17/10/2019 10:07 am, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
It looks good.
One tip:
+ // This code would be warned as "stringop-truncation" by GCC 8 or
On 17/10/2019 12:19 pm, Yasumasa Suenaga wrote:
On 2019/10/17 9:34, David Holmes wrote:
On 17/10/2019 10:07 am, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
It looks good.
One tip:
+ // This code would be warned as "stringop-truncation" by GCC 8 or
later.
+PRAGMA_DIAG_PUSH
On 2019/10/17 9:34, David Holmes wrote:
On 17/10/2019 10:07 am, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
It looks good.
One tip:
+ // This code would be warned as "stringop-truncation" by GCC 8 or later.
+PRAGMA_DIAG_PUSH
+PRAGMA_STRINGOP_TRUNCATION_IGNORED
strncpy(buf, str,
On 17/10/2019 10:07 am, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
It looks good.
One tip:
+ // This code would be warned as "stringop-truncation" by GCC 8 or later.
+PRAGMA_DIAG_PUSH
+PRAGMA_STRINGOP_TRUNCATION_IGNORED
strncpy(buf, str, len);
+PRAGMA_DIAG_POP
I'd suggest to
Hi Serguei,
On 2019/10/17 9:07, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
It looks good.
One tip:
+ // This code would be warned as "stringop-truncation" by GCC 8 or later.
+PRAGMA_DIAG_PUSH
+PRAGMA_STRINGOP_TRUNCATION_IGNORED
strncpy(buf, str, len);
+PRAGMA_DIAG_POP
I'd
Looks good.
Chris
On 10/16/19 4:25 PM, Yasumasa Suenaga wrote:
(Re-send email because I could not send original email to
serviceability-dev and hotspot-compiler-dev)
Hi,
We discussed the fix for JDK-8232084 in [1], and I think we should fix
it with #pragma.
I uploaded new webrev. Could
Hi Yasumasa,
It looks good.
One tip:
+ // This code would be warned as "stringop-truncation" by GCC 8 or later.
+PRAGMA_DIAG_PUSH
+PRAGMA_STRINGOP_TRUNCATION_IGNORED
strncpy(buf, str, len);
+PRAGMA_DIAG_POP
I'd suggest to place the
Hi,
We discussed the fix for JDK-8232084 in [1], and I think we should fix it with
#pragma.
I uploaded new webrev. Could you review it?
JBS: https://bugs.openjdk.java.net/browse/JDK-8232084
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.03/
This change has passed the
(Re-send email because I could not send original email to serviceability-dev
and hotspot-compiler-dev)
Hi,
We discussed the fix for JDK-8232084 in [1], and I think we should fix it with
#pragma.
I uploaded new webrev. Could you review it?
JBS:
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()
On 2019-10-11 15: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()
> On Oct 11, 2019, at 11:22 PM, 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
>
They both look good to me, so I assume we go with B-1 since that's what
Kim prefers.
thanks,
Chris
On 10/11/19 8:22 PM, Yasumasa Suenaga wrote:
Hi,
I updated Plan B-1 (#pragma) due to the comment from Chris [1].
A. Use memcpy()
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
On 2019/10/12 7:30, Kim Barrett wrote:
On Oct 11, 2019, at 3:29 PM, Chris Plummer wrote:
Second, can the macros be used just before and after the strncpy reference
rather than around the whole function? It would clarify both that the strncpy
use has these macros in place and that the macros
> On Oct 11, 2019, at 6:18 PM, Kim Barrett wrote:
> Right now it seems to me that -Wstringop-truncation is seriously
> buggy, both in the false positive and false negative directions. This
> seems to be true at least through 9.x for some recent x. And all of
> the occurrences of this warning in
> On Oct 11, 2019, at 3:29 PM, Chris Plummer wrote:
> Second, can the macros be used just before and after the strncpy reference
> rather than around the whole function? It would clarify both that the strncpy
> use has these macros in place and that the macros are there for the strncpy.
> As
> On Oct 11, 2019, at 3:29 PM, Chris Plummer wrote:
>
> Hi Yasumasa,
>
> A couple of comments on (B). First there is a typo in "stringop-truncatino".
> Second, can the macros be used just before and after the strncpy reference
> rather than around the whole function? It would clarify both
Hi Yasumasa,
A couple of comments on (B). First there is a typo in
"stringop-truncatino". Second, can the macros be used just before and
after the strncpy reference rather than around the whole function? It
would clarify both that the strncpy use has these macros in place and
that the macros
Thanks!
That's seems to be a nicer way to fix it, but remember that both gcc and clang
define __GNUC__ so I think this patch should be more restrictive:
http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.B1-pragma/src/hotspot/share/utilities/compilerWarnings_gcc.hpp.udiff.html
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 wrote:
>
> Yes I prefer:
>
> D. Use #pragma ... at the callsite to disable
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
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]
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.
Compilers are supposed to ignore pragmas they don't understand.
Complaining
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
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
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
Hi Yasumasa,
On 10/10/19 21:33, Yasumasa Suenaga wrote:
Hi Serguei,
On 2019/10/11 12:38, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
If this problem is local then I'd prefer variant A as this solution
is local too.
If this problem is observed in other places then variant C would be
Hi Serguei,
On 2019/10/11 12:38, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
If this problem is local then I'd prefer variant A as this solution is local
too.
If this problem is observed in other places then variant C would be better.
This problem might occur in other places in the
Hi Yasumasa,
If this problem is local then I'd prefer variant A as this solution is
local too.
If this problem is observed in other places then variant C would be better.
Thanks,
Serguei
On 10/10/19 18:34, Yasumasa Suenaga wrote:
Hi,
I want to get conclusion of this discussion.
I
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
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
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,
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()
> On Oct 10, 2019, at 4:54 PM, Kim Barrett wrote:
> This is a
> relatively recent warning option (introduced in gcc8, and included in
> -Wall)
Maybe it was added to -Wall in gcc9, since we aren’t seeing these warnings with
gcc8?
> On Oct 10, 2019, at 3:03 AM, David Holmes 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
I thought it was because gcc sees that "len" is always passed in as
strlen(str). It therefore knows without any doubt that you are doing
strncpy() on a nul terminated string, and are copying all but the nul.
However, I missed one case where parse_value() is called and len not
only is not
We have strncpy all over the VM. Why is this the only one that's causing
problems with gcc?
We have one instance in the same file:
void StringArrayArgument::add(const char* str, size_t len) {
if (str != NULL) {
char* ptr = NEW_C_HEAP_ARRAY(char, len+1, mtInternal);
strncpy(ptr, str,
On 10/10/19 1:07 AM, Yasumasa Suenaga wrote:
On 2019/10/10 16:42, David Holmes wrote:
On 10/10/2019 5:25 pm, serguei.spit...@oracle.com wrote:
On 10/9/19 23:39, Chris Plummer wrote:
I think the +1 gets rid of the warning, but it also means there is
no need for the following line that sets
On 10/10/19 12:42 AM, David Holmes wrote:
On 10/10/2019 5:25 pm, serguei.spit...@oracle.com wrote:
On 10/9/19 23:39, Chris Plummer wrote:
I think the +1 gets rid of the warning, but it also means there is
no need for the following line that sets buf[len].
Yes, it was my guess.
Then I agree
I saw the discussion in some OpenJDK mail list #pragma is uncomfortable
because it disables to find implicit bug - but I could not find out them.
It also includes -Wno-* option in GCC.
Yasumasa
On 2019/10/10 17:11, Yasumasa Suenaga wrote:
On 2019/10/10 17:06, serguei.spit...@oracle.com
On 2019/10/10 17:06, serguei.spit...@oracle.com wrote:
On 10/10/19 00:42, David Holmes wrote:
On 10/10/2019 5:25 pm, serguei.spit...@oracle.com wrote:
On 10/9/19 23:39, Chris Plummer wrote:
I think the +1 gets rid of the warning, but it also means there is no need for
the following line that
On 2019/10/10 16:42, David Holmes wrote:
On 10/10/2019 5:25 pm, serguei.spit...@oracle.com wrote:
On 10/9/19 23:39, Chris Plummer wrote:
I think the +1 gets rid of the warning, but it also means there is no need for
the following line that sets buf[len].
Yes, it was my guess.
Then I agree
On 10/10/19 00:42, David Holmes wrote:
On
10/10/2019 5:25 pm, serguei.spit...@oracle.com wrote:
On 10/9/19 23:39, Chris Plummer wrote:
I think the +1 gets rid of the warning,
but it also means there is no need for the following line
On 10/10/2019 5:25 pm, serguei.spit...@oracle.com wrote:
On 10/9/19 23:39, Chris Plummer wrote:
I think the +1 gets rid of the warning, but it also means there is no
need for the following line that sets buf[len].
Yes, it was my guess.
Then I agree this statement must be removed: buf[len] =
On 2019/10/10 16:25, serguei.spit...@oracle.com wrote:
On 10/9/19 23:39, Chris Plummer wrote:
I think the +1 gets rid of the warning, but it also means there is no need for
the following line that sets buf[len].
Yes, it was my guess.
Then I agree this statement must be removed: buf[len] =
On 10/9/19 23:39, Chris Plummer wrote:
I think the +1 gets rid of the warning, but it also means there is no
need for the following line that sets buf[len].
Yes, it was my guess.
Then I agree this statement must be removed: buf[len] = '\0';
Thanks,
Serguei
Chris
On 10/9/19 11:31 PM,
On 10/10/19 12:03 AM, David Holmes wrote:
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:
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:
On 2019/10/10 15:50, 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:
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
I think the +1 gets rid of the warning, but it also means there is no
need for the following line that sets buf[len].
Chris
On 10/9/19 11:31 PM, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
The fix in src/hotspot/cpu/x86/macroAssembler_x86.hpp looks Okay.
But I don't understand why you
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
Hi Yasumasa,
The fix in src/hotspot/cpu/x86/macroAssembler_x86.hpp looks Okay.
But I don't understand why you need this fix in
src/hotspot/share/services/diagnosticArgument.cpp ?:
char* buf = NEW_RESOURCE_ARRAY(char, len + 1);
-
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 .
61 matches
Mail list logo
