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

Fix readdir for posix #3339

Merged
merged 2 commits into from
Apr 22, 2024
Merged

Conversation

komasayuki
Copy link
Contributor

@komasayuki komasayuki commented Apr 22, 2024

This PR fixes a readir for posix. readdir is not working correctly in rust.

The current WAMR's readdir implementation for posix is,
If readdir returns 0, it will exit with an error.

Posix readdir returns 0 at the end of the directory. To handle this correctly, if readdir returns 0, it should only raise an error if errno has changed.

You can reproduce it with the following rust code.

use std::fs;

fn main() {
    let entries  = fs::read_dir(".").unwrap();
    for entry in entries {
        println!("read_dir:{:?}", entry);
    }
}
# Ubuntu 22
% iwasm --dir=. rust_wasm_readdir.wasm
read_dir:Err(Os { code: 52, kind: Unsupported, message: "Function not implemented" }

It seems that readdir is implemented differently in C and Rust, and a similar implementation in C did not cause this problem.

This problem didn't occur on Mac + iwasm. But this occured on Linux + iwasm.

Copy link
Contributor

@wenyongh wenyongh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@TianlongLiang
Copy link
Contributor

LGTM

@wenyongh wenyongh merged commit 67dc2ae into bytecodealliance:main Apr 22, 2024
386 checks passed
wenyongh added a commit that referenced this pull request Apr 28, 2024
Apply patches which were fixed in branch main to branch `release/1.3.x`:
- aot debug: Fix a few NULL dereferences on errors (#3273)
- aot debug: Fix a NULL dereference (#3274)
- aot debug: Process lldb_function_to_function_dbi only for C (#3278)
- Revert "lldb_function_to_function_dbi: A hack to avoid crashing on C++ methods (#3190)" (#3281)
- Fix warnings/issues reported in Windows and by CodeQL/Coverity (#3275)
- thread mgr: Free aux stack only when it was allocated (#3282)
- interp: Restore context from prev_frame after tail calling a native f
unction (#3283)
- Sync simd opcode definitions spec (#3290)
- Add more checks in wasm loader (#3300)
- Fix posix_fadvise error handling (#3323)
- Fix readdir for posix (#3339)
- wasm loader: Fix checks for opcode ref.func and opcode else (#3340)
- Enhance wasm loader checks for opcode br_table (#3352)
- CI: Use macos-13 instead of macos-latest (#3366)
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 1, 2024
This PR fixes a readir for posix. readdir is not working correctly in rust.

The current WAMR's readdir implementation for posix is, if readdir returns 0,
it will exit with an error. But posix readdir returns 0 at the end of the directory.
To handle this correctly, if readdir returns 0, it should only raise an error if
errno has changed. We can reproduce it with the following rust code:

```rust
use std::fs;

fn main() {
    let entries  = fs::read_dir(".").unwrap();
    for entry in entries {
        println!("read_dir:{:?}", entry);
    }
}
```
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 2, 2024
This PR fixes a readir for posix. readdir is not working correctly in rust.

The current WAMR's readdir implementation for posix is, if readdir returns 0,
it will exit with an error. But posix readdir returns 0 at the end of the directory.
To handle this correctly, if readdir returns 0, it should only raise an error if
errno has changed. We can reproduce it with the following rust code:

```rust
use std::fs;

fn main() {
    let entries  = fs::read_dir(".").unwrap();
    for entry in entries {
        println!("read_dir:{:?}", entry);
    }
}
```

Signed-off-by: victoryang00 <victoryang00@ucsc.edu>
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
This PR fixes a readir for posix. readdir is not working correctly in rust.

The current WAMR's readdir implementation for posix is, if readdir returns 0,
it will exit with an error. But posix readdir returns 0 at the end of the directory.
To handle this correctly, if readdir returns 0, it should only raise an error if
errno has changed. We can reproduce it with the following rust code:

```rust
use std::fs;

fn main() {
    let entries  = fs::read_dir(".").unwrap();
    for entry in entries {
        println!("read_dir:{:?}", entry);
    }
}
```
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
This PR fixes a readir for posix. readdir is not working correctly in rust.

The current WAMR's readdir implementation for posix is, if readdir returns 0,
it will exit with an error. But posix readdir returns 0 at the end of the directory.
To handle this correctly, if readdir returns 0, it should only raise an error if
errno has changed. We can reproduce it with the following rust code:

```rust
use std::fs;

fn main() {
    let entries  = fs::read_dir(".").unwrap();
    for entry in entries {
        println!("read_dir:{:?}", entry);
    }
}
```

Signed-off-by: victoryang00 <victoryang00@ucsc.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants