Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
On 07/03/2013 02:57, Martin Buchholz wrote: Pushed to jdk8/tl/jdk. I recommend this be backported to jdk7u. I agree that it would be good to backport. It doesn't happen automatically so you'll need to mail jdk7u-dev asking for approved to push it to jdk7u/jdk7u-dev. -Alan
hg: jdk8/tl/jdk: 8008759: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
Changeset: 14e49a70729a Author:martin Date: 2013-03-06 17:43 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/14e49a70729a 8008759: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so Summary: Define FILES_m to force use of linker script Reviewed-by: sherman, alanb, ohair ! make/java/zip/Makefile ! src/share/native/java/util/zip/Inflater.c
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
Pushed to jdk8/tl/jdk. I recommend this be backported to jdk7u.
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
On 28/02/2013 16:11, Martin Buchholz wrote: On Thu, Feb 28, 2013 at 6:03 AM, Alan Bateman alan.bate...@oracle.com mailto:alan.bate...@oracle.com wrote: The update to make/java/zip/Makefile looks good to me, we should have done it a long time ago. I assume you are pushing ahead on this because you want to push it to jdk7u-dev (as it's not interesting to jdk8 now because of the new build). Yes I'm hoping you push this fix to jdk7u. Also, I'm not sure that the new build system will not try to be bug-for-bug compatible with the old one and will reintroduce the problem. The old build is on life-support, I don't know how long it will before it will terminally break or be removed. In any case, I wouldn't expect anyone to decide to change it to not use the map files. The exciting new exception detail change looks okay to me too although it is hard to read. It wasn't immediately obvious to me why stddef.h was needed and we'd need to make sure that is okay on all platforms. It's hard to find something more standard than stddef.h. It's hard to find something as non-standard as Windows. My point was we just need to double check that this builds okay on Windows, which is does. So I think you are all set on this one. -Alan.
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
On Mar 1, 2013, at 6:44 AM, Alan Bateman wrote: It's hard to find something more standard than stddef.h. It's hard to find something as non-standard as Windows. My point was we just need to double check that this builds okay on Windows, which is does. The only issue I have seen with adding includes, even standard ones like these, is that they tend to drag in so many more hidden include files, and there could be a chance at a compiler error. But that's fairly easy to diagnose, either it builds or it doesn't. What makes it a bit dangerous is that if it creates warning errors that are significant, we probably won't easily see them because we are not using -Werror across the board. :^( Having said that, my job is to be paranoid, and this does border on paranoia, so I say add the stddef.h and damn the torpedos, full steam ahead. -kto
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
On Thu, Feb 28, 2013 at 6:03 AM, Alan Bateman alan.bate...@oracle.comwrote: The update to make/java/zip/Makefile looks good to me, we should have done it a long time ago. I assume you are pushing ahead on this because you want to push it to jdk7u-dev (as it's not interesting to jdk8 now because of the new build). Yes I'm hoping you push this fix to jdk7u. Also, I'm not sure that the new build system will not try to be bug-for-bug compatible with the old one and will reintroduce the problem. The exciting new exception detail change looks okay to me too although it is hard to read. It wasn't immediately obvious to me why stddef.h was needed and we'd need to make sure that is okay on all platforms. It's hard to find something more standard than stddef.h. Include What You Use == always #include stddef.h in any source file that uses NULL.
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
I have another iteration of this change http://cr.openjdk.java.net/~martin/webrevs/openjdk8/hide-zlib/ that adds exciting new exception detail message for the InternalError I was scratching my head about earlier. -msg = strm-msg; +msg = ((strm-msg != NULL) ? strm-msg : + (ret == Z_VERSION_ERROR) ? + zlib returned Z_VERSION_ERROR: + compile time and runtime zlib implementations differ : + (ret == Z_STREAM_ERROR) ? + inflateInit2 returned Z_STREAM_ERROR : + unknown error initializing zlib library);
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
On Feb 23, 2013, at 12:12 PM, Alan Bateman wrote: On 23/02/2013 18:06, Martin Buchholz wrote: I am actually encountering this in openjdk7 with the old build system. I can repro the problem in openjdk8 with the old build system, but not the new one. I don't know if you consider this a bug with the new build system - it's supposed to generate the same bits, after all???!! I'm not sure why - I'd guess something to do with VARIANT or FILES_m It's highly dubious whether we should have this extremely subtle and error-prone distinction between release and fastdebug builds at all. Thanks for confirming it's old build only, that was my reading too but without checking. Anyway, I do consider it a bug (in the old build), and really hard to track down too until you see both libz and the JDK's libzip loaded. Kelly might have some insight the product vs. fastdebug difference. I have filed many issues over the years asking that mapfiles (or version scripts) be used to limit the visibility of extern symbols in Solaris and Linux. Most developers fail to understand the importance of this. I have also always wanted the fastdebug libraries and even debug build libraries to be 'plugplay' so that they exported the exact same externs and same prototypes, and could be plopped into a product build to isolate problems in just one single library. Unfortunately, some teams have added additional externs to the debug versions and that would imply the need for a different mapfile, so I suspect the easy path was to just not use a mapfile with the debug builds. It is rare that this is an issue, but obviously this is a case where it was. The policy should be that ALL shared libraries be scoped and only expose the extern symbols needed. However, with the old builds being this way all along, I see no need to go into the old builds to fix this. We have so much work to do as it is. -kto -Alan
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
Kelly, Thanks. I think we agree. I think we have consensus that my change is an improvement and should be committed. Can I haz approval? I'm not brave enough to try to change the default for all mapfiles, although I would support Kelly or anyone else who tries. Martin On Mon, Feb 25, 2013 at 9:18 AM, Kelly O'Hair kelly.oh...@oracle.comwrote: On Feb 23, 2013, at 12:12 PM, Alan Bateman wrote: On 23/02/2013 18:06, Martin Buchholz wrote: I am actually encountering this in openjdk7 with the old build system. I can repro the problem in openjdk8 with the old build system, but not the new one. I don't know if you consider this a bug with the new build system - it's supposed to generate the same bits, after all???!! I'm not sure why - I'd guess something to do with VARIANT or FILES_m It's highly dubious whether we should have this extremely subtle and error-prone distinction between release and fastdebug builds at all. Thanks for confirming it's old build only, that was my reading too but without checking. Anyway, I do consider it a bug (in the old build), and really hard to track down too until you see both libz and the JDK's libzip loaded. Kelly might have some insight the product vs. fastdebug difference. I have filed many issues over the years asking that mapfiles (or version scripts) be used to limit the visibility of extern symbols in Solaris and Linux. Most developers fail to understand the importance of this. I have also always wanted the fastdebug libraries and even debug build libraries to be 'plugplay' so that they exported the exact same externs and same prototypes, and could be plopped into a product build to isolate problems in just one single library. Unfortunately, some teams have added additional externs to the debug versions and that would imply the need for a different mapfile, so I suspect the easy path was to just not use a mapfile with the debug builds. It is rare that this is an issue, but obviously this is a case where it was. The policy should be that ALL shared libraries be scoped and only expose the extern symbols needed. However, with the old builds being this way all along, I see no need to go into the old builds to fix this. We have so much work to do as it is. -kto -Alan
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
On 22/02/2013 22:03, Martin Buchholz wrote: Hi Alan, Xueming, build-ers, I'd like you to do a code review. I've finally figured out why fastdebug jdk occasionally gives InternalError in the zip code. Exception in thread main java.lang.InternalError at java.util.zip.Inflater.init(Native Method) at java.util.zip.Inflater.init(Inflater.java:101) at java.util.zip.ZipFile.getInflater(ZipFile.java:448) It's because: - jdk changed structure size of z_stream struct - making jdk zlib incompatible with stock zlib - as a result of which, it is imperative to keep jdk zlib sequestered from system zlib - so need to not export zlib symbols from libzip.so - so need to tell makefiles to use linker script unconditionally http://cr.openjdk.java.net/~martin/webrevs/openjdk8/hide-zlib/ http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk8/hide-zlib/ I see Sherman has created a bug for this but it turns out that someone else running with fastdebug ran into issues too: 8006319: fastdebug libzip.so is linked with global symbols, allowing symbols to be overridden I see you are proposing to update make/java/zip/Makefile so that is the old build. Looking at makefiles/CompileNativeLibraries.gmk (new build) then it looks to me that it is using the map file. I haven't checking the symbols in a fast debug build to double check so I'm curious if you've seen this with a recent (new) build. -Alan.
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
I am actually encountering this in openjdk7 with the old build system. I can repro the problem in openjdk8 with the old build system, but not the new one. I don't know if you consider this a bug with the new build system - it's supposed to generate the same bits, after all???!! I'm not sure why - I'd guess something to do with VARIANT or FILES_m It's highly dubious whether we should have this extremely subtle and error-prone distinction between release and fastdebug builds at all. Consider fixing this more fundamentally in Mapfile-vers.gmk, or at least document clearly why mapfiles are only on in OPT, and what the consequences might be. ifeq ($(PLATFORM), linux) ifeq ($(VARIANT), OPT) # OPT build MUST have a mapfile? ifndef FILES_m FILES_m =mapfile-vers endif endif # VARIANT Here's a repro recipe on Ubuntu Linux, running javac on any old source file: env LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libz.so ~/ws/openjdk8/build/linux-amd64-fastdebug/j2sdk-image/bin/javac Main.java An exception has occurred in the compiler (1.8.0-internal). Please file a bug at the Java Developer Connection (http://java.sun.com/webapps/bugreport) after checking the Bug Parade for duplicates. Include your program and the following diagnostic in your report. Thank you. java.lang.InternalError at java.util.zip.Inflater.init(Native Method) at java.util.zip.Inflater.init(Inflater.java:103) at java.util.zip.ZipFile.getInflater(ZipFile.java:448) at java.util.zip.ZipFile.getInputStream(ZipFile.java:367) at java.util.jar.JarFile.getBytes(JarFile.java:379) at java.util.jar.JarFile.getManifestFromReference(JarFile.java:178) at java.util.jar.JarFile.getManifest(JarFile.java:165) at com.sun.tools.javac.file.FSInfo.getJarClassPath(FSInfo.java:69) at com.sun.tools.javac.file.CacheFSInfo.getJarClassPath(CacheFSInfo.java:94) at com.sun.tools.javac.file.Locations$Path.addJarClassPath(Locations.java:304) at com.sun.tools.javac.file.Locations$Path.addFile(Locations.java:295) at com.sun.tools.javac.file.Locations$Path.addDirectory(Locations.java:217) at com.sun.tools.javac.file.Locations$Path.addDirectories(Locations.java:192) at com.sun.tools.javac.file.Locations$BootClassPathLocationHandler.computePath(Locations.java:630) at com.sun.tools.javac.file.Locations$BootClassPathLocationHandler.lazy(Locations.java:642) at com.sun.tools.javac.file.Locations$BootClassPathLocationHandler.getLocation(Locations.java:576) at com.sun.tools.javac.file.Locations.getLocation(Locations.java:677) at com.sun.tools.javac.file.JavacFileManager.getLocation(JavacFileManager.java:803) at com.sun.tools.javac.file.JavacFileManager.list(JavacFileManager.java:617) at com.sun.tools.javac.jvm.ClassReader.fillIn(ClassReader.java:2699) at com.sun.tools.javac.jvm.ClassReader.complete(ClassReader.java:2395) at com.sun.tools.javac.code.Symbol.complete(Symbol.java:434) at com.sun.tools.javac.comp.Enter.visitTopLevel(Enter.java:302) at com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:519) at com.sun.tools.javac.comp.Enter.classEnter(Enter.java:262) at com.sun.tools.javac.comp.Enter.classEnter(Enter.java:276) at com.sun.tools.javac.comp.Enter.complete(Enter.java:488) at com.sun.tools.javac.comp.Enter.main(Enter.java:473) at com.sun.tools.javac.main.JavaCompiler.enterTrees(JavaCompiler.java:990) at com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:865) at com.sun.tools.javac.main.Main.compile(Main.java:517) at com.sun.tools.javac.main.Main.compile(Main.java:376) at com.sun.tools.javac.main.Main.compile(Main.java:365) at com.sun.tools.javac.main.Main.compile(Main.java:356) at com.sun.tools.javac.Main.compile(Main.java:76) at com.sun.tools.javac.Main.main(Main.java:61) On Sat, Feb 23, 2013 at 3:51 AM, Alan Bateman alan.bate...@oracle.comwrote: On 22/02/2013 22:03, Martin Buchholz wrote: Hi Alan, Xueming, build-ers, I'd like you to do a code review. I've finally figured out why fastdebug jdk occasionally gives InternalError in the zip code. Exception in thread main java.lang.InternalError at java.util.zip.Inflater.init(Native Method) at java.util.zip.Inflater.init(Inflater.java:101) at java.util.zip.ZipFile.getInflater(ZipFile.java:448) It's because: - jdk changed structure size of z_stream struct - making jdk zlib incompatible with stock zlib - as a result of which, it is imperative to keep jdk zlib sequestered from system zlib - so need to not export zlib symbols from libzip.so - so need to tell makefiles to use linker script unconditionally http://cr.openjdk.java.net/~martin/webrevs/openjdk8/hide-zlib/ I see Sherman has created a bug for this but it turns out that someone else running with fastdebug ran into issues too: 8006319: fastdebug libzip.so is linked with global symbols, allowing symbols to be overridden I see you are proposing to update make/java/zip/Makefile so that is the old build. Looking at makefiles/CompileNativeLibraries.gmk (new build) then it looks to me that it is
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
On 23/02/2013 18:06, Martin Buchholz wrote: I am actually encountering this in openjdk7 with the old build system. I can repro the problem in openjdk8 with the old build system, but not the new one. I don't know if you consider this a bug with the new build system - it's supposed to generate the same bits, after all???!! I'm not sure why - I'd guess something to do with VARIANT or FILES_m It's highly dubious whether we should have this extremely subtle and error-prone distinction between release and fastdebug builds at all. Thanks for confirming it's old build only, that was my reading too but without checking. Anyway, I do consider it a bug (in the old build), and really hard to track down too until you see both libz and the JDK's libzip loaded. Kelly might have some insight the product vs. fastdebug difference. -Alan
Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
Hi Alan, Xueming, build-ers, I'd like you to do a code review. I've finally figured out why fastdebug jdk occasionally gives InternalError in the zip code. Exception in thread main java.lang.InternalError at java.util.zip.Inflater.init(Native Method) at java.util.zip.Inflater.init(Inflater.java:101) at java.util.zip.ZipFile.getInflater(ZipFile.java:448) It's because: - jdk changed structure size of z_stream struct - making jdk zlib incompatible with stock zlib - as a result of which, it is imperative to keep jdk zlib sequestered from system zlib - so need to not export zlib symbols from libzip.so - so need to tell makefiles to use linker script unconditionally http://cr.openjdk.java.net/~martin/webrevs/openjdk8/hide-zlib/
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
Here is the bugid you will need. 8008759: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so On 02/22/2013 02:03 PM, Martin Buchholz wrote: Hi Alan, Xueming, build-ers, I'd like you to do a code review. I've finally figured out why fastdebug jdk occasionally gives InternalError in the zip code. Exception in thread main java.lang.InternalError at java.util.zip.Inflater.init(Native Method) at java.util.zip.Inflater.init(Inflater.java:101) at java.util.zip.ZipFile.getInflater(ZipFile.java:448) It's because: - jdk changed structure size of z_stream struct - making jdk zlib incompatible with stock zlib - as a result of which, it is imperative to keep jdk zlib sequestered from system zlib - so need to not export zlib symbols from libzip.so - so need to tell makefiles to use linker script unconditionally http://cr.openjdk.java.net/~martin/webrevs/openjdk8/hide-zlib/ http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk8/hide-zlib/
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
On Fri, Feb 22, 2013 at 2:15 PM, Xueming Shen xueming.s...@oracle.comwrote: ** Here is the bugid you will need. 8008759: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so Thanks! webrev updated.
