Skip to content
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

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

Closed
kkebo opened this issue Oct 14, 2024 · 8 comments
Closed

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

kkebo opened this issue Oct 14, 2024 · 8 comments

Comments

@kkebo
Copy link
Contributor

kkebo commented Oct 14, 2024

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

  • a-Shell 1.15.6
  • iPadOS 18.1 Developer Beta 7
@kkebo
Copy link
Contributor Author

kkebo commented Oct 15, 2024

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Contributor Author

kkebo commented Oct 19, 2024

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
Copy link
Contributor Author

kkebo commented Oct 19, 2024

@kkebo
Copy link
Contributor Author

kkebo commented Oct 22, 2024

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
Copy link
Contributor Author

kkebo commented Oct 30, 2024

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

@kkebo
Copy link
Contributor Author

kkebo commented Oct 31, 2024

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

@kkebo kkebo closed this as completed Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants