This is the mail archive of the
mailing list for the Java project.
Re: [PATCH] Java - allow compiling a compressed .jar/.zip archive
- To: Per Bothner <per at bothner dot com>
- Subject: Re: [PATCH] Java - allow compiling a compressed .jar/.zip archive
- From: Jeff Sturm <jeff dot sturm at commerceone dot com>
- Date: Wed, 24 Jan 2001 14:19:45 -0500
- CC: Jeff Sturm <jeff dot sturm at appnet dot com>, java-discuss at sources dot redhat dot com, gcc-patches at gcc dot gnu dot org
- Organization: Commerce One
- References: <email@example.com> <3A6D2BCF.E4FBC0AE@appnet.com> <firstname.lastname@example.org>
Per Bothner wrote:
> You're skeptical - but you actually don't have a reason to believe
> there is a bug.
Yes I do. It broke the Alpha build, and I've pinpointed the failure in gdb. (I
apologize if that wasn't clear in the first message.)
It fails in find_zip_file_start (zextract.c:266):
266: if (buffer != 'P' || strncmp (&buffer, LOCAL_HDR_SIG, 3))
267: return -1;
This is called from read_zip_archive, which unfortunately does not test for
errors in the return value, so the actual symptom occurs much later in
> If you look at the code you will find that it actually
> does take into account __alignof__(ZipDirectory).
Understood. However __alignof__ actually contributed to the problem, as did
increasing the size of ZipDirectory. Look at what read_zip_archive is doing.
Instead of allocating memory for each ZipDirectory it overlays them on top of
the central directory buffer. When the filename_length member is written (at
offset 32 in ZipDirectory), it can be anywhere from 33 to 40 in the header
because of structure alignment. That's far enough to clobber the file offset
some of the time, since it is read from offset 42 in the directory header. We
got lucky on x86, and before adding the zipf member to ZipDirectory, because
writing filename_length overwrote a portion of the central directory header we
> > Are compressed jar files an essential feature for 3.0?
> Not essential, but then Java isn't either. This patch adds valuable
> functionality to Java and cannot break non-Java.
Agreed, and I'm happy to have it. But if it breaks Alpha (and probably IA-64
too), is it worth it to commit this now, or put it off? I've spent time on this
I had hoped to spend investigating problems with the new ABI... <sigh>.
This struct/buffer overlapping in read_zip_archive is ugly and fragile, clever
as it may be. If ZipDirectory grows any larger it can start overwriting the
filename too. It seems to me that allocating ZipDirectory on the heap would be
the right thing to do, the memory overhead is negigible and the code cleanup
would be substantial. I can't do that now. The patch below may be good enough
for now (I can't test it until I get home, which is where my aging Alpha lives
2000-01-24 Jeff Sturm <email@example.com>
* zextract.c (read_zip_archive): Read file_offset before writing zipd
and consequently clobbering the header contents.
RCS file: /cvs/gcc/gcc/gcc/java/zextract.c,v
retrieving revision 1.11
diff -u -p -r1.11 zextract.c
--- zextract.c 2001/01/21 21:50:36 1.11
+++ zextract.c 2001/01/24 19:05:07
@@ -323,6 +323,7 @@ read_zip_archive (zipf)
long uncompressed_size = makelong (&dir_ptr[4+C_UNCOMPRESSED_SIZE]);
long filename_length = makeword (&dir_ptr[4+C_FILENAME_LENGTH]);
long extra_field_length = makeword (&dir_ptr[4+C_EXTRA_FIELD_LENGTH]);
+ long file_offset = makelong (&dir_ptr[4+C_RELATIVE_OFFSET_LOCAL_HEADER]);
@@ -337,8 +338,7 @@ read_zip_archive (zipf)
#define DIR_ALIGN sizeof(long)
- zipd->filestart = find_zip_file_start (zipf->fd,
+ zipd->filestart = find_zip_file_start (zipf->fd, file_offset);
zipd->filename_offset = CREC_SIZE+4 - dir_last_pad;
= zipd->filename_offset + zipd->filename_length + extra_field_length;