RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated
Hi, following discussion "RFR: build pragma error with gcc 4.4.7"[1], I'm posting updated patch replacing deprecated readdir_r with readdir. Bug ID is JDK-8179887 [2]. webrev: http://cr.openjdk.java.net/~andrew/8179887/webrev/ I'm asking for the review and eventually sponsorship. Thanks! [1] - http://mail.openjdk.java.net/pipermail/build-dev/2018-March/021236.html [2] - https://bugs.openjdk.java.net/browse/JDK-8179887 -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated
Hi Michal, This was discussed in a couple of different threads. Please also see: http://mail.openjdk.java.net/pipermail/build-dev/2018-April/021530.html Thanks, David On 23/04/2018 5:51 PM, Michal Vala wrote: Hi, following discussion "RFR: build pragma error with gcc 4.4.7"[1], I'm posting updated patch replacing deprecated readdir_r with readdir. Bug ID is JDK-8179887 [2]. webrev: http://cr.openjdk.java.net/~andrew/8179887/webrev/ I'm asking for the review and eventually sponsorship. Thanks! [1] - http://mail.openjdk.java.net/pipermail/build-dev/2018-March/021236.html [2] - https://bugs.openjdk.java.net/browse/JDK-8179887
INCLUDE_SA/serviceability agent - support on s390x
Hello, as far as I know the serviceability agent is not supported on linux s390x . However (unlike on aix where it is not supported as well) , INCLUDE_SA=falseis not set in the central configure m4 files . Should we set it ( suggested diff below) ? Best regards, Matthias hg diff diff -r fcd5df7aa235 make/autoconf/jdk-options.m4 --- a/make/autoconf/jdk-options.m4 Wed Apr 18 11:19:32 2018 +0200 +++ b/make/autoconf/jdk-options.m4 Mon Apr 23 13:46:17 2018 +0200 @@ -238,6 +238,9 @@ if test "x$OPENJDK_TARGET_OS" = xaix ; then INCLUDE_SA=false fi + if test "x$OPENJDK_TARGET_CPU" = xs390x ; then +INCLUDE_SA=false + fi AC_SUBST(INCLUDE_SA) # Compress jars Best regards, Matthias
Re: INCLUDE_SA/serviceability agent - support on s390x
Makes sense to me. Looks good. /Erik On 2018-04-23 05:01, Baesken, Matthias wrote: Hello, as far as I know the serviceability agent is not supported on linux s390x . However (unlike on aix where it is not supported as well) , INCLUDE_SA=false is not set in the central configure m4 files . Should we set it ( suggested diff below) ? Best regards, Matthias hg diff diff -r fcd5df7aa235 make/autoconf/jdk-options.m4 --- a/make/autoconf/jdk-options.m4 Wed Apr 18 11:19:32 2018 +0200 +++ b/make/autoconf/jdk-options.m4 Mon Apr 23 13:46:17 2018 +0200 @@ -238,6 +238,9 @@ if test "x$OPENJDK_TARGET_OS" = xaix ; then INCLUDE_SA=false fi + if test "x$OPENJDK_TARGET_CPU" = xs390x ; then + INCLUDE_SA=false + fi AC_SUBST(INCLUDE_SA) # Compress jars Best regards, Matthias
Re: RFR: build pragma error with gcc 4.4.7
> On Apr 21, 2018, at 11:18 AM, Andrew Hughes wrote: > > On 19 March 2018 at 23:23, Kim Barrett wrote: >> There are also problems with the patch as provided. >> >> (1) Since PRAGMA_DIAG_PUSH/POP do nothing in the version of gcc this >> change is being made in support of, the warning would be disabled for >> all following code in any translation unit that includes this file. >> That doesn't seem good. > > No, but it's really the only solution on those compilers. We have such > usage already elsewhere e.g. > > // Silence -Wformat-security warning for fatal() > PRAGMA_DIAG_PUSH > PRAGMA_FORMAT_NONLITERAL_IGNORED > fatal(buf); > PRAGMA_DIAG_POP > return true; // silence compiler warnings > } > in src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp > > If there are other warnings, then they will picked up on newer compilers, > especially when building with -Werror. I don't think it's likely people are > doing development on older compilers, but rather that we have to use > them to build for older platforms. I would be a lot more comfortable if the possibly do-nothing push/pop and the associated code were in a .cpp file, rather than in a .hpp file where it could affect some open-ended and unexpected set of code. But I think this is moot if os::readdir can be changed to call ::readdir rather than ::readdir_r, as appears to be the case, possibly with some documentation about not sharing the DIR* among multiple threads, at least not without locking. That seemed to be what Michal was planning to do, but hasn’t gotten back to it yet.
Re: RFR: build pragma error with gcc 4.4.7
On 23 April 2018 at 20:19, Kim Barrett wrote: >> On Apr 21, 2018, at 11:18 AM, Andrew Hughes wrote: >> >> On 19 March 2018 at 23:23, Kim Barrett wrote: >>> There are also problems with the patch as provided. >>> >>> (1) Since PRAGMA_DIAG_PUSH/POP do nothing in the version of gcc this >>> change is being made in support of, the warning would be disabled for >>> all following code in any translation unit that includes this file. >>> That doesn't seem good. >> >> No, but it's really the only solution on those compilers. We have such >> usage already elsewhere e.g. >> >> // Silence -Wformat-security warning for fatal() >> PRAGMA_DIAG_PUSH >> PRAGMA_FORMAT_NONLITERAL_IGNORED >> fatal(buf); >> PRAGMA_DIAG_POP >> return true; // silence compiler warnings >> } >> in src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp >> >> If there are other warnings, then they will picked up on newer compilers, >> especially when building with -Werror. I don't think it's likely people are >> doing development on older compilers, but rather that we have to use >> them to build for older platforms. > > I would be a lot more comfortable if the possibly do-nothing push/pop and > the associated code were in a .cpp file, rather than in a .hpp file where it > could affect some open-ended and unexpected set of code. > Ah yes, sorry, I missed that this was a .hpp, while the others were .cpp. > But I think this is moot if os::readdir can be changed to call ::readdir > rather > than ::readdir_r, as appears to be the case, possibly with some documentation > about not sharing the DIR* among multiple threads, at least not without > locking. > I think so too for OpenJDK 11, but I'm reluctant to backport such a change to older JDKs. I guess if we want to continue to workaround the warning there, we'll need to move the function into the .cpp file. > That seemed to be what Michal was planning to do, but hasn’t gotten back to > it yet. Indeed. He has a patch that does that, that I've already reviewed. Just waiting for him to post it publicly :) > -- Andrew :) Senior Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) Web Site: http://fuseyism.com Twitter: https://twitter.com/gnu_andrew_java PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net) Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222
Re: RFR: build pragma error with gcc 4.4.7
On 24/04/2018 1:27 PM, Andrew Hughes wrote: On 23 April 2018 at 20:19, Kim Barrett wrote: On Apr 21, 2018, at 11:18 AM, Andrew Hughes wrote: On 19 March 2018 at 23:23, Kim Barrett wrote: There are also problems with the patch as provided. (1) Since PRAGMA_DIAG_PUSH/POP do nothing in the version of gcc this change is being made in support of, the warning would be disabled for all following code in any translation unit that includes this file. That doesn't seem good. No, but it's really the only solution on those compilers. We have such usage already elsewhere e.g. // Silence -Wformat-security warning for fatal() PRAGMA_DIAG_PUSH PRAGMA_FORMAT_NONLITERAL_IGNORED fatal(buf); PRAGMA_DIAG_POP return true; // silence compiler warnings } in src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp If there are other warnings, then they will picked up on newer compilers, especially when building with -Werror. I don't think it's likely people are doing development on older compilers, but rather that we have to use them to build for older platforms. I would be a lot more comfortable if the possibly do-nothing push/pop and the associated code were in a .cpp file, rather than in a .hpp file where it could affect some open-ended and unexpected set of code. Ah yes, sorry, I missed that this was a .hpp, while the others were .cpp. But I think this is moot if os::readdir can be changed to call ::readdir rather than ::readdir_r, as appears to be the case, possibly with some documentation about not sharing the DIR* among multiple threads, at least not without locking. I think so too for OpenJDK 11, but I'm reluctant to backport such a change to older JDKs. I guess if we want to continue to workaround the warning there, we'll need to move the function into the .cpp file. That seemed to be what Michal was planning to do, but hasn’t gotten back to it yet. Indeed. He has a patch that does that, that I've already reviewed. Just waiting for him to post it publicly :) He already has: RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated on both the mailing lists this email has gone to. David -
