Tcl Source Code

View Ticket
Login
Ticket UUID: fc65ff1b663fd16e7582c34164acbd670514deed
Title: zipfs mkimg fails for statically built tclsh
Type: Bug Version: 8.7
Submitter: griffin Created on: 2021-08-30 17:45:45
Subsystem: 57. zlib Assigned To: jan.nijtmans
Priority: 5 Medium Severity: Important
Status: Closed Last Modified: 2021-11-04 17:59:42
Resolution: Fixed Closed By: griffin
    Closed on: 2021-11-04 17:59:42
Description:
When using a tclsh or wish built with --disabled-shared, 
the [zipfs mkimg] command fails to create an image correctly.  
Instead, it simply writes out a new zip archive. 

$ /home/brian/tcl_core/usr_default/bin/tclsh8.7 test.tcl
zipfs mkimg myApp.bin /home/brian/tcl_core/bugs/myApp /home/brian/tcl_core/bugs/myApp {}
---- image file type ----
file myApp.bin
myApp.bin: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=c5b14647e79dbaf01f67f4eea390f610eef1043e, for GNU/Linux 3.2.0, with debug_info, not stripped

---- Shared libraries? ----
ldd myApp.bin
	linux-vdso.so.1 (0x00007fff89bb9000)
	libtcl8.7.so => /home/brian/tcl_core/usr_default/lib/libtcl8.7.so (0x00007f1fbe997000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f1fbe78f000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f1fbe789000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f1fbe76d000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f1fbe74a000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f1fbe5fb000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f1fbec23000)


$ /home/brian/tcl_core/usr_special/bin/tclsh8.7 test.tcl
zipfs mkimg myApp.bin /home/brian/tcl_core/bugs/myApp /home/brian/tcl_core/bugs/myApp {}
---- image file type ----
file myApp.bin
myApp.bin: Zip archive data, at least v2.0 to extract

---- Shared libraries? ----
ldd myApp.bin
	not a dynamic executable
    while executing
"exec ldd $img"
    invoked from within
"puts [exec ldd $img]"
    (file "test.tcl" line 24)


$ ldd /home/brian/tcl_core/usr_special/bin/tclsh8.7
	linux-vdso.so.1 (0x00007fff24924000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f1bada59000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f1bada3d000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f1bada1a000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f1bad8cb000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f1bad6d9000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f1badcf4000)

$ ldd /home/brian/tcl_core/usr_default/bin/tclsh8.7
	linux-vdso.so.1 (0x00007ffd9fdf2000)
	libtcl8.7.so => /home/brian/tcl_core/usr_default/lib/libtcl8.7.so (0x00007f040046f000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f0400267000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f0400261000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f0400245000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f0400222000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f04000d3000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f04006fb000)
User Comments: griffin added on 2021-11-04 17:59:42:

Fixed with [338fe58e76c0c524565ff7f80d4326c1b24fda8fddda0ccd0d8074fdeaa79849]


griffin added on 2021-10-24 18:33:58:
Ok, I believe the bug to be truly fixed now, 
but would like a second opinion;
see branch bug_fc65ff1b66.  
I have only tested this on linux so far, but 
I have, in the past, confirmed that the issue 
is the same on all 3 platforms.

With this fix, it should be possible to use 
tclsh to install the archive in itself in 
the Makefile instead of using cat + zip -A.

griffin added on 2021-09-03 03:32:42:
Hi Jan, Thanks for reporting this regression failure.  
I'm not surprised there is a failure here, my patch is broken for sure.  
I'm still trying to understand the math to get the bad offset(s) corrected.

Hi Christian,  these files have diverged dramatically.  
Your (androwish) code doesn't just simply drop in, unfortunately.  
Sorting this out is going to be a lot of work.  For now, I will stick to 
fixing the one bug that I am aware of, and not try to introduce new 
problems.  
I think synchronizing androwish with tcl core is probably a bigger 
discussion the core team might want to consider, independent of 
this effort.

jan.nijtmans added on 2021-09-02 09:26:13:

Testcase is failing:

==== zipfs-4.2 zipfs lmkimg: making an image from an image FAILED
==== Contents of test case:

zipfs lmkimg $midImage [list $addFile test/ko.tcl] {} $baseImage zipfs lmkimg $targetImage [list $addFile test/ok.tcl] {} $midImage zipfs mount ziptest $targetImage try { list [glob -tails -directory //zipfs://ziptest/test *.tcl] [if {[file size $midImage] == [file size $targetImage]} { string cat equal } else { list mid=[file size $midImage] target=[file size $targetImage] }] } finally { zipfs unmount ziptest }

---- Result was: ok.tcl {mid=165 target=285} ---- Result should have been (exact matching): ok.tcl equal ==== zipfs-4.2 FAILED

zlib.test


chw added on 2021-09-01 20:53:01:
Brian, Jan,

I believe, that the zipfs.c in AndroWish does most of the things right
which currently seem to be broken. Unfortunately, the implementation in
Tcl core went very out of sync with mine in the meantime. Nevertheless,
maybe some of my thoughts can be back/forth/left/right/up/down ported.
See 

  https://www.androwish.org/home/file?name=jni/tcl/generic/zipfs.c&ci=tip

and check for yourselves if something can be taken over.

griffin added on 2021-09-01 14:48:09:
Hi Jan, thanks for testing this!  
It is intentional that the existing zip archive is removed; the documentation says so: 

"If the starting image has a ZIP archive already attached to it, 
it is removed from the copy in outfile before the new ZIP archive is added."  

The reason, I believe, is because it would defeat any password protection 
of the archive.  It's easy enough to extract the contents in various other 
ways when it is not protected.

I will investigate the offset issue.  There shouldn't be any warnings in the new image.
Thanks!

jan.nijtmans added on 2021-09-01 13:52:52:

Thank you for this work! I tested it, and it is an improvement although it doesn't fully work as expected.

After doing a "configure --disable-shared", let's look at the resulting tclsh:

$ unzip -l tclsh |head
Archive:  tclsh
  Length      Date    Time    Name
---------  ---------- -----   ----
  1. 09-01-2021 15:01 tcl_library/
  2. 08-27-2021 09:10 tcl_library/word.tcl
  3. 12-23-2020 15:04 tcl_library/tcltest/
  4. 08-27-2021 09:10 tcl_library/tcltest/tcltest.tcl
  5. 08-27-2021 09:10 tcl_library/tcltest/pkgIndex.tcl
  6. 08-27-2021 09:10 tcl_library/tm.tcl
  7. 08-27-2021 09:10 tcl_library/clock.tcl

Then, let's see what "zipfs mkimg" does:

$ ./tclsh
% zipfs mkimg tclsh /Users/jan/workspace/tcl8.7/unix/libtcl.vfs /Users/jan/workspace/tcl8.7/unix/libtcl.vfs
% exit
$ unzip -l tclsh |head
warning [tclsh]:  2770069 extra bytes at beginning or within zipfile
  (attempting to process anyway)
Archive:  tclsh
  Length      Date    Time    Name
---------  ---------- -----   ----
  1. 09-01-2021 14:57 tcl_library/auto.tcl
  2. 08-27-2021 09:10 tcl_library/clock.tcl
  3. 09-01-2021 14:57 tcl_library/cookiejar/cookiejar.tcl
  4. 08-27-2021 09:10 tcl_library/cookiejar/idna.tcl
  5. 08-27-2021 09:10 tcl_library/cookiejar/pkgIndex.tcl
  6. 08-27-2021 09:10 tcl_library/cookiejar/public_suffix_list.dat.gz
  7. 06-28-2021 17:33 tcl_library/encoding/ascii.enc

The warning tells us that all offsets in the file are relative to the zipfile start, not the file start. No problem, Tcl (and unzip) can handle that, but it would be better to produce a tclsh which can be handled by other tools without this warning ....

Then, I note that "zipfs mkimg" - after your change - just removes the old attached zip and adds a new one. That's better than what it did before, but it means that the content of $tcl_library needs to be packed together with the app, otherwise the app will not be functional. I would prefer "zipfs mkimg" to add/replace new entries, but keep the entries already present.

Yeah, I know, "zip" already does this, it's not trivial to build this into "zipfs mkimg" too. But until that's done, I prefer to use "zip" for manipulating the attached zipfile.

Anyway, +1 for merging this to core-8-branch.