-
Notifications
You must be signed in to change notification settings - Fork 127
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
[Feature Request] support directly execution of αcτµαlly pδrταblε εxεcµταblε (Actually Portable Executable, Cosmopolitan, APE, cosmos) executable file format #424
Comments
Check for the signature of Actually Portable Executable binaries and treat them as executable. Adds 40-64 bytes. (GitHub issue #424)
Sure, why not? It's sufficiently amusing to be worth the cost.
Seems to work. Though if all you want is a Windows version of Available in the prerelease binaries (PRE-5375 or above). |
I don't know if these are enough to decide that this string is the APE magic also in future releases. I.e. that it's intended to keep this prefix for the forseeable future. It would have been better if it was documented someplace how it should be identified. Though in practice I think it's OK. Definitely at least for now. (regardless, I also bumped into this issue of busybox sh failing to invoke extension-less APE binaries, but didn't consider it important enough to spend time on) As for the implementation details, I think the original suggestion is cleaner, even if it costs two more bytes for the |
From https://justine.lol/ape.html :
As long as APE wants to support (MBR) BIOS bootup as well as raw VM execution, the magic is hard to change because it's ... pure magic. Also Cosmopolitan C's README recommends registering APE magic into binfmt_misc as an one-time effort. |
Well, that's shorter than the string which the code searches. by two chars: So should it be kept as is? or shorten to |
What was committed saves 16-32 bytes compared to the original suggestion.
Three characters: there's a single quote in there too. If six characters is enough for binfmt_misc it's probably good enough for us. This saves a further 24-32 bytes: diff --git a/win32/mingw.c b/win32/mingw.c
index 26b046f1a..39e2b86f7 100644
--- a/win32/mingw.c
+++ b/win32/mingw.c
@@ -475,7 +475,8 @@ static int has_exec_format(const char *name)
if (buf[0] == 'M' && buf[1] == 'Z') {
/* Actually Portable Executable */
/* See ape/ape.S at https://github.com/jart/cosmopolitan */
- if (n > 9 && memcmp(buf + 2, "qFpD='\n", 7) == 0)
+ offset = (buf[2] << 24) + (buf[3] << 16) + (buf[4] << 8) + buf[5];
+ if (n > 6 && offset == 0x71467044) // "qFpD"
return 1;
if (n > 0x3f) { |
Note that this works because EDIT: it's indeed fine, but if 0x80 was set it would have been UB. From C99 spec 6.5.7.4:
For completeness, I'd cast |
Surely, not fine? The top bit of a literal diff --git a/win32/mingw.c b/win32/mingw.c
index 26b046f1a..98254fdbe 100644
--- a/win32/mingw.c
+++ b/win32/mingw.c
@@ -473,16 +473,20 @@ static int has_exec_format(const char *name)
* the magic from the file command.
*/
if (buf[0] == 'M' && buf[1] == 'Z') {
+/* Convert four unsigned bytes to an unsigned int (little-endian) */
+#define LE4(b, o) (((unsigned)b[o+3] << 24) + (b[o+2] << 16) + \
+ (b[o+1] << 8) + b[o])
+
/* Actually Portable Executable */
/* See ape/ape.S at https://github.com/jart/cosmopolitan */
- if (n > 9 && memcmp(buf + 2, "qFpD='\n", 7) == 0)
+ const unsigned char *qFpD = (unsigned char *)"qFpD";
+ if (n > 6 && LE4(buf, 2) == LE4(qFpD, 0))
return 1;
if (n > 0x3f) {
offset = (buf[0x19] << 8) + buf[0x18];
if (offset > 0x3f) {
- offset = (buf[0x3f] << 24) + (buf[0x3e] << 16) +
- (buf[0x3d] << 8) + buf[0x3c];
+ offset = LE4(buf, 0x3c);
if (offset < sizeof(buf)-100) {
if (memcmp(buf+offset, "PE\0\0", 4) == 0) {
sig = (buf[offset+25] << 8) + buf[offset+24]; This still saves 24-32 bytes and should avoid undefined behaviour. Previously I was using a big-endian Using an integer comparison for |
Indeed, I somehow felt it's the literal
My take is that I wouldn't care about bloat at that level. This can be down to compiler optimizations of the specific compiler you tested with etc. Especially because, for me, at least for windows code which doesn't go upstream, the priority should be code clarity and efficiency where needed, which should typically get the code near the lower bound, though not necessarily the absolute lowest. Because Windows is not a router. We don't care all that much if the application is 600k or 6M (though obviously we'd take the former over the latter if given the choice), and code clarity makes it much more hackable - quite unlike upstream, and for me the ability to return to the code later is more important than saving few bytes. This is subjective obviously, and doesn't matter too much in this case, but I'd just use |
Detecting Actually Portable Executable binaries used a longer signature than seems necessary. Six characters should be enough for anyone. When right shifting a byte by 24 bits, cast it to unsigned to avoid undefined behaviour. Saves 24-32 bytes. (GitHub issue #424)
I've pushed my most recent patch. There are prereleases (PRE-5377 or above). |
I'm not entirely sure how many bytes the current code checks, but I think it's 6, at which case that would be 2 less than at the offical spec which was published recently - https://github.com/jart/cosmopolitan/blob/master/ape/specification.md . A 9th byte newline is recommended to exist at the file, but not required. |
Yes, it's six bytes. That's how many the Cosmopolitan README suggests are supplied to Using six rather than eight increases the possibility of a false positive. But the consequences aren't that severe: some random file might be considered executable and a user might try to run it. If it is executable it'll run, if not it won't. Even checking eight bytes might still result in a false positive if you're very unlucky. |
@rmyorston It say: |
I can't reproduce the error. I've run the executable in my virtual machines with 64-bit Windows 8.1, 10 and 11. In each case it works. |
This is why error messages should include the module name which generated this message.... otherwise it's hard to tell who prints this message (which also seems incorrect, because 0xffffffffffffffff == 0xffffffffffffffff ...) |
I use the 32-but busybox pre-release (latest as of today) on 64-bit Windows 10 |
Nope, that works for me too. |
I can reproduce it. It happens when running it without the @ale5000-git next time please post the exact line which you invoke to make it easier to (try and) reproduce it. This is in cmd.exe with latest pre-release-32
|
OK, now I can reproduce it.
I saw that and took a different message from it. This issue was originally about:
So I renamed the file to To get the error the file has to be |
The error message is in the |
I didn't think that calling it without extension could make a difference but I have now reported the problem on their repo. |
For reference: jart/cosmopolitan#1306 |
αcτµαlly pδrταblε εxεcµταblε (Actually Portable Executable, APE) (https://justine.lol/ape.html) executable file format, along with Cosmopolitan C libc library (https://justine.lol/cosmopolitan/), is cool.
A quick patch to make busybox-w32 support directly executing APE format executables, without changing their file extension to
.exe
:To test:
Download pre-built binaries from superconfigure GitHub repository's release page, or from https://cosmo.zip/pub/cosmos/bin/ .
Then try to execute one — like, bash — in busybox-w32 ash:
The text was updated successfully, but these errors were encountered: