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(stdlib): Remove memory corruption in File.fdReaddir #1573

Merged
merged 2 commits into from
Jan 5, 2023

Conversation

av8ta
Copy link
Contributor

@av8ta av8ta commented Jan 5, 2023

…of manually managing array memory

I found all sorts of errors when using File.fdReaddir. Sometimes it would hang without outputting data. Sometimes it would output data and then hang. Sometimes it would return cleanly with malformed data. Sometimes but not often it returned the right result!

Most frequent error:

RuntimeError: memory access out of bounds
    at wasm://wasm/0011eea6:wasm-function[55]:0x374f
    at wasm://wasm/0011eea6:wasm-function[60]:0x4068
    at wasm://wasm/0011eea6:wasm-function[72]:0x49b6
    at wasm://wasm/0011eea6:wasm-function[484]:0x2c159
    at wasm://wasm/0011eea6:wasm-function[492]:0x2ea9a
    at wasm://wasm/0011eea6:wasm-function[493]:0x2eea4
    at wasm://wasm/0011eea6:wasm-function[495]:0x2f496
    at wasm://wasm/0011eea6:wasm-function[496]:0x2f4d7
    at wasm://wasm/0011eea6:wasm-function[680]:0x47ade
    at wasm://wasm/0011eea6:wasm-function[682]:0x47b4e

Examples of malformed data returned:

// what's up with file x missing and the inode value?
Ok([> <record value>, {
  inode: <enum value>,
  filetype: RegularFile,
  path: "y"
}])

Ok([> <unknown heap tag type: 0x3f9898 | value: 0x3f97d0>, <enum value>, {
  inode: '',
  filetype: ": ",
  path: "z"
}])

While debugging I noticed that the when I printed dirent here let dirent = { inode, path, filetype } it always looked correct so I just changed the arr returned to stdlib array and appended to that. All the problems seem resolved :)

@ospencer
Copy link
Member

ospencer commented Jan 5, 2023

I see the bug in the original code—I'm going to adjust it to remove that bug. Hope you don't mind me pushing to your branch!

@ospencer ospencer changed the title fix(stdlib): build and return stdlib array in File.fdReaddir instead … fix(stdlib): Remove memory corruption in File.fdReaddir Jan 5, 2023
@av8ta
Copy link
Contributor Author

av8ta commented Jan 5, 2023

I see the bug in the original code—I'm going to adjust it to remove that bug. Hope you don't mind me pushing to your branch!

No problem!

@ospencer
Copy link
Member

ospencer commented Jan 5, 2023

@av8ta Could you try this out and verify it still works?

@ospencer ospencer self-assigned this Jan 5, 2023
@av8ta
Copy link
Contributor Author

av8ta commented Jan 5, 2023

My tests are all passing with this change :)

Copy link
Member

@ospencer ospencer left a comment

Choose a reason for hiding this comment

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

Thanks for the bug report and potential fix! Made it obvious what was wrong here 🙌

@av8ta
Copy link
Contributor Author

av8ta commented Jan 5, 2023

Thanks for the bug report and potential fix! Made it obvious what was wrong here raised_hands

I had a feeling you'd fix it more low-level!

@ospencer ospencer merged commit 060fc7b into grain-lang:main Jan 5, 2023
@av8ta av8ta deleted the av8ta/fdreaddir branch January 5, 2023 19:40
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.

3 participants