-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The sonnetpatch utility fails on some binaries #16
Comments
It seems not to work on the wosdb executable. It extends it with a few thousand bytes but keeps the hunkheader unchanged. |
ACK, it's a bug 😄 . |
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? |
Sigh. It seems to work on an extremely simple executable (latest evenmore from aminet).
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. |
Any news on this? Or do you still need the binary (which is wosdb for example)? |
Oh, ok, I'll take wosdb binary as an example. Will take a look at this again. |
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 |
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
The "unaligned read" implies something might be wrong. Or it might not. |
Used the sonnetpatch from the lha file on Git 2015-12-05 13:35 GMT+01:00 Radosław Kujawa notifications@github.com:
|
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. |
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). |
I tried sonnetpatch on: http://aminet.net/package/util/libs/mpega-WarpUP |
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 |
As it is done manually: powerpc.library,0 changes to sonnet.library,0,0 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 |
N64 Emulator http://aminet.net/package/misc/emu/TrueReality |
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. |
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. |
That's really weird! The program should never touch the original file, provided you use it like: There are two separate descriptors, Is it possible to check with SnoopDos what's happening? I'm not very familiar with it. |
Just doing a sonnetpatch filename already messes up filename (appends zero bytes) |
That's certainly not supposed to happen. I'm analysing how |
Seems PosixLib disregards |
i'm guessing lseek? You are seeking past the end of the file? |
whoops, you were slightly faster |
lseek does indeed extend the file when newpos>fib_size |
I mailed Frank, we'll see what he answers. |
Also, the resulting file gets random protection bits preventing the file from being read, written to, executed or a combination of those. |
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. |
I'll need to add some debugging output for the code in sonnetpatch that is calling lseek. |
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:
O RLY?
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? |
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...). |
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? |
I didn't 😭 .
Yep, will do that. |
Any progress on this? The 'fast' option I mean? |
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. |
Maybe I'm reading that wrong as the {} are confusing in lseek :-P |
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 |
Nevermind, I'll be writing something from scratch using AmigaDOS commands. So It won't be portable |
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. |
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). |
This still needed with the move to system which patches during loading? |
Please use the linux binary or the powerpc.library instead of the sonnet.library. Support for the sonnet.library will be dropped soon(ish). |
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.
The text was updated successfully, but these errors were encountered: