holzschu / a-shell

A terminal for iOS, with multiple windows
BSD 3-Clause "New" or "Revised" License
2.72k stars 117 forks source link

`wasm3`'s `readdir()` doesn't return a directory as a directory #836

Open kkebo opened 1 month ago

kkebo commented 1 month ago

Description

readdir() returns dirent and it's a directory if its d_type equals to DT_DIR.

https://www.man7.org/linux/man-pages/man3/readdir.3.html

Although wasm returns a directory as a directory (DT_DIR), wasm3 returns a directory as a regular file (DT_REG).

Steps to reproduce

  1. Create readdir.c

    #include <stdio.h>
    #include <dirent.h>
    
    int main(int argc, const char *argv[]) {
        DIR *dp = opendir(argv[1]);
        if (!dp) {
            return 1;
        }
        struct dirent *ent;
        while ((ent = readdir(dp))) {
            printf("%s\n", ent->d_name);
            if (ent->d_type == DT_DIR) {
                printf("is a directory (%d)\n", ent->d_type);
            } else {
                printf("is not a directory (%d)\n", ent->d_type);
            }
        }
        closedir(dp);
        return 0;
    }
  2. Run clang readdir.c on a-Shell
  3. Run mkdir test on a-Shell
  4. Run touch test/a on a-Shell
  5. Run touch test/b on a-Shell
  6. Run mkdir test/c on a-Shell
  7. Run wasm a.out test and wasm3 a.out test on a-Shell
    $ wasm a.out test
    a
    is not a directory (4)
    c
    is a directory (3)
    b
    is not a directory (4)
    $ wasm3 a.out test
    .
    is not a directory (4)
    ..
    is not a directory (4)
    a
    is not a directory (8)
    c
    is not a directory (4)
    b
    is not a directory (8)

Expected behavior

wasm's behavior is my expected behavior.

Environment

kkebo commented 1 month ago

When I ran the same C code with the prebuilt wasm3 that can be available here, it outputted the correct results. So, although there are differences between Linux and iOS, I don't think the root cause is wasm3 itself.

$ clang -target wasm32-wasi --sysroot=$HOME/wasi-libc/sysroot -nodefaultlibs -lc -o readdir.wasm readdir.c
$ ./wasm3-aarch64-linux-musl readdir.wasm test
a
is not a directory (4)
b
is not a directory (4)
c
is a directory (3)
$ uname -a
Linux Brown-rhinoceros-beetle 6.11.0-400.asahi.fc40.aarch64+16k #1 SMP PREEMPT_DYNAMIC Fri Sep 27 02:59:31 UTC 2024 aarch64 GNU/Linux

By the way, wasi-libc's DT_DIR is usually 3 (__WASI_FILETYPE_DIRECTORY), but if the __wasilibc_unmodified_upstream flag is defined, it will be 4. So I'm wondering if wasm3 included in a-Shell might have been compiled with wrong flags or wrong header files.

holzschu commented 1 month ago

That is an interesting and challenging issue. Wasm3 connection with the system is in m3_api_wasi.c, and for readdir in m3_wasi_generic_fd_readdir(). That function calls readdir() and sets the type of the return entry to the type it has read:

entry.d_type = file_entry->d_type;

From your tests, I see that for directories, d_type is set to 4, and for files it is set to 8. I think the issue is that DT_DIR and DT_REG have different values in iOS and is Wasm. That should be fixable.

holzschu commented 4 weeks ago

Yes, that was the issue. Now the only difference between wasm and wasm3 is on . and ..: since wasm3 uses readdir(), it shows them in the output, while wasm uses FileManager().contentsOfDirectory(atPath: arguments[2]), which doesn't have them.

kkebo commented 4 weeks ago

Thank you for fixing it!

Now the only difference between wasm and wasm3 is on . and ..: since wasm3 uses readdir(), it shows them in the output, while wasm uses FileManager().contentsOfDirectory(atPath: arguments[2]), which doesn't have them.

By the way, the issue above doesn't matter for me, but I think the correct behavior is wasm3's.

kkebo commented 4 weeks ago

If you use readdir() in Swift, the following code may be helpful.

https://github.com/swiftlang/swift-foundation/blob/f77911a0e83248612f305ed3a23098a6c355f495/Sources/FoundationEssentials/FileManager/FileOperations%2BEnumeration.swift#L346

kkebo commented 3 weeks ago

Thanks for the fix and publishing it to TestFlight! However, I wasn't able to test it due to the new issue #838. I've confirmed that the issue has been fixed with readdir.c.

kkebo commented 2 weeks ago

I've confirmed that readdir() has been fixed on a-Shell 1.15.7 (432 and 433). Thank you.

kkebo commented 2 weeks ago

I'll close this issue when the fixed version is released to App Store.