dosemu2 / comcom64

64bit command.com
https://github.com/dosemu2/comcom32/
GNU General Public License v3.0
32 stars 5 forks source link

ecm auto loadfix #60

Closed ecm-pushbx closed 2 years ago

ecm-pushbx commented 2 years ago

As requested in https://github.com/dosemu2/comcom32/issues/59#issuecomment-1179566948

I tested this with the vgalemmi.exe file included in the attachment of that report.

ecm-pushbx commented 2 years ago

This might be unable to actually match an EXEPACK file depending on where its entrypoint is. Examples to test against are welcome.

ecm-pushbx commented 2 years ago

Fixed an error (codesegment needs to be multiplied by 16) and a logic mistake (entrypoint is late in the file, not in the first 256 bytes) for the EXEPACK detection. I compressed a file using https://www.bamsoftware.com/software/exepack/ and checked that it is detected. (They use a custom depacker that doesn't need LOADFIX but I assume the format is compatible to that used by the original Microsoft EXEPACK.)

ecm-pushbx commented 2 years ago

Do you want me to add the additional parameter to perform_external_cmd now? I think my way works too but if you insist I will do it that way.

ecm-pushbx commented 2 years ago

I will force push the updates in separate commits now. I will squash those later when we have figured out whether to change the signature of perform_external_cmd, just adding them for review now.

stsp commented 2 years ago

I am not a big fan of a new parameter, either. The cleanest way would be to add some perform_exec() which would do that loadfix detection magic and then execute perform_external_cmd(). But if you think its too difficult, I can apply as is and then look into it myself.

ecm-pushbx commented 2 years ago

The cleanest way would be to add some perform_exec() which would do that loadfix detection magic and then execute perform_external_cmd(). But if you think its too difficult, I can apply as is and then look into it myself.

The problem with that is we need a lot of the PATH and filename extension handling that's currently in perform_external_cmd to have already happened to have a filename of the .COM or .EXE file to check. That's why I added the detection deep inside the function, not nearer to the callsite.

stsp commented 2 years ago

The problem with that is we need a lot of the PATH and filename extension handling that's currently in perform_external_cmd

Of course, but it can also be split into a separate function. Which is why I suggested a param instead - obviously not the best solution, but a least efforts one. But anyway, if you don't like doing a refactoring or add a new param as a work-around, then you can leave it to me.

ecm-pushbx commented 2 years ago

Put together the fixups into the original two commits now.

stsp commented 2 years ago

Thanks.

stsp commented 2 years ago

I've done the absolutely minimal simplifications. Seems to still work fine.

jschwartzenberg commented 2 years ago

This is really nice! Especially as some programs are ran through batch files, this makes things much easier.

Does this mean that the loadfix command is never ever needed anymore or might there be obscure programs that still require it but for which it's not autodetected? If there could still be such cases, I propose either documenting those (or enhancing the logic) when they are found?

stsp commented 2 years ago

This is what @ecm-pushbx was also asking. The detection may not be 100% reliable. The only way to proceed is to test things, and if something unsupported pops up - add it to detection logic.

stsp commented 2 years ago

Also the problem is that non-main executable can be compressed. If the program invokes it directly via 0x4b (and not via command /c) then we don't fix those. So indeed, some progs should be kept documented...

stsp commented 2 years ago

So the short answer is:

stsp commented 2 years ago

The hope is, as always for dosemu2, that we cover the majority of progs. Lets leave "all progs" to dosbox-X.

jschwartzenberg commented 2 years ago

Found one that still requires a manual loadfix: andretti.tar.gz

Without it: Screenshot_20220710_183907

Running loadfix ma after this starts the game successfully.

ecm-pushbx commented 2 years ago

The ma.com file loads load.exe with interrupt 21h service 4Bh. The shell cannot detect the need for loadfix here in the usual way we do it so far.

stsp commented 2 years ago

Why not to run load.exe by hands then?

jschwartzenberg commented 2 years ago

That gives: Screenshot_20220710_202512 Seems it needs some argument.

Another one that still needs loadfix: f15.tar.gz Screenshot_20220710_202708

jschwartzenberg commented 2 years ago

Another game that still behaves differently (gets further) with loadfix: moonston.tar.gz

jschwartzenberg commented 2 years ago

Another one that needs loadfix, otherwise: Screenshot_20220710_211027 pokets.tar.gz

jschwartzenberg commented 2 years ago

Found another one: ryder.tar.gz

@ecm-pushbx @stsp I don't know if these could be autodetected in some way as well or should I just add them all to the documentation? I should say it's quite rare to run into things that still need loadfix. The detection works for the vast majority of the cases!!

jschwartzenberg commented 2 years ago

Another one for which still needs an explicit loadfix: t_j.tar.gz

This one looks like it might use a slightly different version of the packer than the ones that are detected now.

jschwartzenberg commented 2 years ago

And another one: a10.tar.gz

stsp commented 2 years ago

Should now be fixed. What remained?

jschwartzenberg commented 2 years ago

From the zips I put above, only Tom&Jerry works without loadfix now. After testing, I found that the others still have the same symptoms as before.

stsp commented 2 years ago

Do they have a different exe file or another exepack variation?

jschwartzenberg commented 2 years ago

I would think some have another exepack variation such as f15, given the error message, but I don't know for sure how to check this?

stsp commented 2 years ago

Your screen-shot shows Packed file is corrupt, which is usual. What variation?

jschwartzenberg commented 2 years ago

What variation?

One that's not yet autodetected...

I can't really say more, maybe you could check?

stsp commented 2 years ago

f15.com is not an exe file so obviously it launches other files.

jschwartzenberg commented 2 years ago

Ah yeah you're right. Maybe it's similar to andretti above.

stsp commented 2 years ago

Should now be fixed for all progs.

jschwartzenberg commented 2 years ago

Except for ryder, they all indeed work now!! Great to see this!!

stsp commented 2 years ago

Does it still require loadfix?

jschwartzenberg commented 2 years ago

No it doesn't seem to load at all.

stsp commented 2 years ago

Regression? Likely fixed now.