The sonnetpatch utility fails on some binaries

[rkujawa]

Apparently AmigaOS 3 executable loader is not very strict about enforcing the format of binaries. I have several examples of programs that work just fine, but their hunk headers are broken in various ways. Most common offsense is wrong length of hunk specified in the hunk header. Sonnetpatch hunk parser needs to be extended not to fail on such programs.

[DvdBoon]

It seems not to work on the wosdb executable. It extends it with a few thousand bytes but keeps the hunkheader unchanged.

[rkujawa]

ACK, it's a bug :smile: .

[DvdBoon]

I think I haven't been able to produce any working binaries with this utility :-) Is there a way that I can help you debug it?

[rkujawa]

Sigh. It seems to work on an extremely simple executable (latest evenmore from aminet).

$ ./sonnetpatch -p0  ./evenmore ./evenmore-patched
0
HUNK_HEADER
    Hunk table size: 1
    First hunk to load: 0
    Last hunk to load: 0
HUNK_CODE (0x3e9) hunk number 0 at offset 0x18
    Size: 27426 Amiga longwords (109704 bytes)
    Flags: None (implied MEMF_PUBLIC)
    Relocation: HUNK_RELOC32 (0x3ec)
Patching hunk 0 size to c0006b22, ext. attr 1005

$ ./sonnetpatch ./evenmore-patched 
HUNK_HEADER
    Hunk table size: 1
    First hunk to load: 0
    Last hunk to load: 0
HUNK_CODE (0x3e9) hunk number 0 at offset 0x1c
    Size: 27426 Amiga longwords (109704 bytes)
    Flags: Extended mem attributes
        Extended memory attribute: 0x1005
    Relocation: HUNK_RELOC32 (0x3ec)

However, running it on anything more complex fails indeed. So I guess the patching routine is okay, just hunk header parser is having trouble identifying what to patch.

Please attach the binary you are trying to patch and the output of sonnetpatch.

[DvdBoon]

Any news on this? Or do you still need the binary (which is wosdb for example)?

[rkujawa]

Oh, ok, I'll take wosdb binary as an example. Will take a look at this again.

[DvdBoon]

Please do :-) Quake 2 was like 40 hunks I had to do by hand. I'd also like a feature/option to patch powerpc.library,0 to sonnet.library,0,0

[rkujawa]

Well, I took wosdb binary as an example and I can't immediately see anything wrong. The hunk headers were patched. Of course I can't test the binary... Please take a look:

http://c0ff33.net/drop/wosdb-patched-20151205-1329

Does it look ok for you? Here's the output of sonnetpatch ran on original wosdb binary:

$ ./sonnetpatch -p0,1,2,3,4,5,6 ~/Downloads/wosdb/wosdb wosdb-patched-20151205-1329
HUNK_HEADER
    Hunk table size: 7
    First hunk to load: 0
    Last hunk to load: 6
Unaligned read at 14f8c!
HUNK_CODE (0x3e9) hunk number 0 at offset 0x30
    Size: 156 Amiga longwords (624 bytes)
    Flags: None (implied MEMF_PUBLIC)
    Relocation: HUNK_DRELOC32 (0x3f7)
HUNK_CODE (0x3e9) hunk number 1 at offset 0x2a8
    Size: 16894 Amiga longwords (67576 bytes)
    Flags: None (implied MEMF_PUBLIC)
    Relocation: HUNK_RELOC32 (0x3ec)
HUNK_CODE (0x3e9) hunk number 2 at offset 0x10ac0
    Size: 7 Amiga longwords (28 bytes)
    Flags: None (implied MEMF_PUBLIC)
    Relocation: HUNK_DRELOC32 (0x3f7)
HUNK_DATA (0x3ea) hunk number 3 at offset 0x10b00
    Size: 729 Amiga longwords (2916 bytes)
    Flags: None (implied MEMF_PUBLIC)
    Relocation: HUNK_DRELOC32 (0x3f7)
HUNK_DATA (0x3ea) hunk number 4 at offset 0x1167c
    Size: 201 Amiga longwords (804 bytes)
    Flags: None (implied MEMF_PUBLIC)
    Relocation: HUNK_DRELOC32 (0x3f7)
HUNK_DATA (0x3ea) hunk number 5 at offset 0x11d88
    Size: 1164 Amiga longwords (4656 bytes)
    Flags: None (implied MEMF_PUBLIC)
HUNK_BSS (0x3eb) hunk number 6 at offset 0x13050
    Size: 1996 Amiga longwords (7984 bytes)
    Flags: None (implied MEMF_PUBLIC)
Patching hunk 0 size to c000009c, ext. attr 2005
Patching hunk 1 size to c00041fe, ext. attr 2005
Patching hunk 2 size to c0000007, ext. attr 2005
Patching hunk 3 size to c00002d9, ext. attr 2005
Patching hunk 4 size to c00000c9, ext. attr 2005
Patching hunk 5 size to c000048c, ext. attr 2005
Patching hunk 6 size to c00007cc, ext. attr 2005

The "unaligned read" implies something might be wrong. Or it might not.

[DvdBoon]

Used the sonnetpatch from the lha file on Git

2015-12-05 13:35 GMT+01:00 Radosław Kujawa notifications@github.com:

Well, I took wosdb binary as an example and I can't immediately see anything wrong. The hunk headers were patched. Of course I can't test the binary... Please take a look:

http://c0ff33.net/drop/wosdb-patched-20151205-1329

Does it look ok for you? Here's the output of sonnetpatch ran on original wosdb binary:

$ ./sonnetpatch -p0,1,2,3,4,5,6 ~/Downloads/wosdb/wosdb foo HUNK_HEADER Hunk table size: 7 First hunk to load: 0 Last hunk to load: 6 Unaligned read at 14f8c! HUNK_CODE (0x3e9) hunk number 0 at offset 0x30 Size: 156 Amiga longwords (624 bytes) Flags: None (implied MEMF_PUBLIC) Relocation: HUNK_DRELOC32 (0x3f7) HUNK_CODE (0x3e9) hunk number 1 at offset 0x2a8 Size: 16894 Amiga longwords (67576 bytes) Flags: None (implied MEMF_PUBLIC) Relocation: HUNK_RELOC32 (0x3ec) HUNK_CODE (0x3e9) hunk number 2 at offset 0x10ac0 Size: 7 Amiga longwords (28 bytes) Flags: None (implied MEMF_PUBLIC) Relocation: HUNK_DRELOC32 (0x3f7) HUNK_DATA (0x3ea) hunk number 3 at offset 0x10b00 Size: 729 Amiga longwords (2916 bytes) Flags: None (implied MEMF_PUBLIC) Relocation: HUNK_DRELOC32 (0x3f7) HUNK_DATA (0x3ea) hunk number 4 at offset 0x1167c Size: 201 Amiga longwords (804 bytes) Flags: None (implied MEMF_PUBLIC) Relocation: HUNK_DRELOC32 (0x3f7) HUNK_DATA (0x3ea) hunk number 5 at offset 0x11d88 Size: 1164 Amiga longwords (4656 bytes) Flags: None (implied MEMF_PUBLIC) HUNK_BSS (0x3eb) hunk number 6 at offset 0x13050 Size: 1996 Amiga longwords (7984 bytes) Flags: None (implied MEMF_PUBLIC) Patching hunk 0 size to c000009c, ext. attr 2005 Patching hunk 1 size to c00041fe, ext. attr 2005 Patching hunk 2 size to c0000007, ext. attr 2005 Patching hunk 3 size to c00002d9, ext. attr 2005 Patching hunk 4 size to c00000c9, ext. attr 2005 Patching hunk 5 size to c000048c, ext. attr 2005 Patching hunk 6 size to c00007cc, ext. attr 2005

The "unaligned read" implies something might be wrong. Or it might not.

— Reply to this email directly or view it on GitHub https://github.com/Sakura-IT/SonnetAmiga/issues/16#issuecomment-162178404 .

[rkujawa]

It's possible that sonnetpach is bugged more on AmigaOS than on UNIX-likes ;). Still, it would be useful if you tested patched wosdb binary I linked in previous comment.

[DvdBoon]

At a first glance the hunk header looks ok. I have not had the time to test it. But judging at the difference in output you posted and the picture I posted (which I don't see anymore?), there is some problem between 32 and 64 bit parameters (but i am not an expert).

[togflops]

I tried sonnetpatch on: http://aminet.net/package/util/libs/mpega-WarpUP Seemed to work but Amplifier could not load it; 10.Ram Disk:mpega_library/libs> sonnetpatch -p0,1,2,3,4,5 mpega.library mpega.library2 0 1 2 3 4 5 HUNK_HEADER Hunk table size: 6 First hunk to load: 0 Last hunk to load: 5 Couldn't get description for 3ea (sub) Unaligned read at 19f1800000003! Unaligned read at 19f1800000003! HUNK_CODE (0x3e9) hunk number 0 at offset 0x2cffffffff Size: 616 Amiga longwords (2464 bytes) Flags: None (implied MEMF_PUBLIC) Relocation: HUNK_RELOC32 (0x3ec) HUNK_CODE (0x3e9) hunk number 1 at offset 0x9d4ffffffff Size: 20199 Amiga longwords (80796 bytes) Flags: None (implied MEMF_PUBLIC) Relocation: HUNK_RELOC32 (0x3ec) HUNK_CODE (0x3e9) hunk number 2 at offset 0x1475cffffffff Size: 34 Amiga longwords (136 bytes) Flags: None (implied MEMF_PUBLIC)e HUNK_DATA (0x3ea) hunk number 3 at offset 0x14804ffffffff Size: 106 Amiga longwords (424 bytes) Flags: None (implied MEMF_PUBLIC) HUNK_DATA (0x3ea) hunk number 4 at offset 0x149b8ffffffff Size: 4357 Amiga longwords (17428 bytes) Flags: None (implied MEMF_PUBLIC) Relocation: HUNK_RELOC32 (0x3ec) HUNK_BSS (0x3eb) hunk number 5 at offset 0x18eacffffffff Size: 973 Amiga longwords (3892 bytes) Flags: None (implied MEMF_PUBLIC) Relocation: HUNK_RELOC32 (0x3ec) Patching hunk 0 size to c0000268, ext. attr 2005 Patching hunk 1 size to c0004ee7, ext. attr 2005 Patching hunk 2 size to c0000022, ext. attr 2005 Patching hunk 3 size to c000006a, ext. attr 2005 Patching hunk 4 size to c0001105, ext. attr 2005 Patching hunk 5 size to c00003cd, ext. attr 2005

[rkujawa]

The output does not looks correct, the offsets are wrong, looks like a manifestation of issue #16 to me. Besides, even if patching of hunk header worked, there's a reference to powerpc.library at offset 0x3FA in the file. Changing it to sonnet.library would probably also require updating offsets in hunk header (or padding the hunk?), due to simple fact that these strings are different length. Hmm.

[DvdBoon]

As it is done manually:

powerpc.library,0 changes to sonnet.library,0,0 powerpc.library(xxx...),10,0 changes to sonnet.library(xxx...),10,0,0 powepc.library(xxx...),10,x changes to sonnet.library(xxx...),32,10,x

Something like that. Only the top one is important (opening the library) the rest are strings.

It would be more easy to just replace powerpc.library with sonnet.library,32 in strings (any line where powerpc.iibrary is not followed by a '0'.)

No padding or updating the hunkheader needed as the sonnet string is smaller by one byte than the powerpc.library

[togflops]

N64 Emulator http://aminet.net/package/misc/emu/TrueReality
Maybe this is too ambitious with the state of Sonnetpatch and it requires StormMesa but that has both PPC & 68K versions. But it's only three hunks. Fails with the usual bad loadfile hunk

Truerealpatch.txt

[DvdBoon]

Ok, so I am stupid. I looked at the output of the sonnetpatch utility without arguments and it said [-p hnum...] hunkfile. Didn't took me too long to figure out I had to leave out the space between p and hnum, but I only just found out that you need to specify an output file. LOL. Thought it would just overwrite.

[DvdBoon]

Ok, tried with lha_wos: using -p0,1,2,3,4,5

Bug: Original file gets appended with 4556 zero bytes (127352 -> 131908). Original file is overwritten with this. Then the file is patched. Patching seems correct, but the file being patched is already messed up due to the previous step. Expected length was 127372 after patching. now it was 131928.

[rkujawa]

That's really weird! The program should never touch the original file, provided you use it like: sonnetpatch -p hunks original-file patched-file

There are two separate descriptors, ifd which is used for input file, ofd which is used for output file. Additionally, ifd is opened with O_RDONLY. Maybe the bug is in PosixLib then? I could never reproduce this when running sonnetpatch natively on Linux or OS X.

Is it possible to check with SnoopDos what's happening? I'm not very familiar with it.

[DvdBoon]

Just doing a sonnetpatch filename already messes up filename (appends zero bytes)

[rkujawa]

That's certainly not supposed to happen. I'm analysing how open, lseek and read are implemented in PosixLib.

[rkujawa]

Seems PosixLib disregards O_RDONLY and there's a possible bug in its implementation of lseek. I need to consult with the author.

[DvdBoon]

i'm guessing lseek? You are seeking past the end of the file?

[DvdBoon]

whoops, you were slightly faster

[DvdBoon]

lseek does indeed extend the file when newpos>fib_size

[rkujawa]

I mailed Frank, we'll see what he answers.

[DvdBoon]

Also, the resulting file gets random protection bits preventing the file from being read, written to, executed or a combination of those.

[DvdBoon]

Tried the new lseek for posixlib and the utility now fails on the last hunk. As it is C i'm not sure what is wrong. Maybe lseek now fails when seeking past the end of a write protected file instead of just going to the end of the file? Also, don't know if I applied the lseek patch correctly.

[rkujawa]

I'll need to add some debugging output for the code in sonnetpatch that is calling lseek.

[rkujawa]

I found out what originally caused extending of files on AmigaOS. It all comes back to the first post in this issue. Hunk headers lie blatantly. And often.

And example can be found even in original wosdb executable from Frank's archive:

HUNK_BSS (0x3eb) hunk number 6 at offset 0x13050
    Size: 1996 Amiga longwords (7984 bytes)
    Flags: None (implied MEMF_PUBLIC)

O RLY?

00013050  00 00 03 f2 00 00 03 eb  00 00 07 cc 00 00 03 f2
00013060

The size information in above example was obtained from hunk header at the beginning of a file. However, the actual hunk is way smaller. As one can see, the hunk is really only one longword.

Any suggestions how to treat these situations?

[rkujawa]

Apparently, I'm be a noob and don't know how LoadSeg() works. Since our newly compiled Sonnet-aware wosdb executable has the same exact HUNK_BSS section (smaller than what header states).

My guess is that the lenght in hunk header merely informs the OS how much memory should be allocated for a given section. However, the section in file might be smaller and no one will complain about this (besides sonnetpatch of course...).

[DvdBoon]

BSS is treated as ds.l ;-) Thought you knew.

Could you do a -simple (or -fast or something) option which like loads the whole binary in, adds (numbers of hunks x 4 bytes) change only the header and write the file back?

[rkujawa]

BSS is treated as ds.l ;-) Thought you knew.

I didn't :sob: .

Could you do a -simple (or -fast or something) option which like loads the whole binary in, adds (numbers of hunks x 4 bytes) change only the header and write the file back?

Yep, will do that.

[DvdBoon]

Any progress on this? The 'fast' option I mean?

[DvdBoon]

The latest compiled version fails with either 'Syntax error while tokenizing hunk list' with -p and 'Unable to open file: Bad address' when using -p0,1,2,....

Also (and I'm not that into C as you know) the patch Frank provided fails when newpos>fib.Size and O_ACCMODE = O_RDONLY. Is this correct behaviour for what we want? It only goes to OFFSET_END when the file is NOT read only.

[DvdBoon]

Maybe I'm reading that wrong as the {} are confusing in lseek :-P

[DvdBoon]

Maybe moving the Seek(fp->file,0,OFFSET_END); would solve the problem, but as I said, sonnetpatch is not working anymore for me on AmigaOS with the old or new posixlib

[DvdBoon]

Nevermind, I'll be writing something from scratch using AmigaDOS commands. So It won't be portable

[rkujawa]

Sorry, I've really had limited amount of time in last few months. If you write a replacement, commit it and I'll plug it into the build system via Makefile.

[DvdBoon]

I'm actually thinking to bite the bullet and implement a fake powerpc.library, some loadseg patch and something to mark the executables as being ppc executables (maybe unused protection bits or something).

[DvdBoon]

This still needed with the move to system which patches during loading?

[DvdBoon]

Please use the linux binary or the powerpc.library instead of the sonnet.library. Support for the sonnet.library will be dropped soon(ish).