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

heap_get_oob only on aarch64 #3764

Closed
kwonoj opened this issue Apr 12, 2023 · 19 comments
Closed

heap_get_oob only on aarch64 #3764

kwonoj opened this issue Apr 12, 2023 · 19 comments
Assignees
Labels
priority-medium Medium priority issue
Milestone

Comments

@kwonoj
Copy link
Contributor

kwonoj commented Apr 12, 2023

Describe the bug

SWC (https://github.com/swc-project/swc) recently tried another attempt to upgrade wasmer from 2 -> 3. Previously it cause all of the execution fails without clear reason why. Now it seems working on some cases, however only on aarch64 (M1 mac, for example) seeing heap_get_oob when execute wasm plugins.

For example, this changes (which upgrades wasmer to 3) kwonoj/swc-plugin-coverage-instrument#208 passes CI correctly, but running it on aarch64 machine emits

    1: RuntimeError: out of bounds memory access
           at rkyv::impls::core::primitive::_::<impl rkyv::Deserialize<u32,D> for u32>::deserialize::h530d545be03c9d79 (<module>[17175]:0x5dc86d)
...
          at swc_common::plugin::serialized::deserialize_from_ptr::h1c80414e4fb59669 (<module>[15399]:0x551e72)
           at swc_plugin_proxy::memory_interop::read_returned_result_from_host::read_returned_result_from_host::{{closure}}::hee8205ceba5e2c83 (<module>[15149]:0x54162c)
           at core::option::Option<T>::map::h2991d34946fd4bf3 (<module>[15151]:0x541821)
           at swc_plugin_proxy::memory_interop::read_returned_result_from_host::read_returned_result_from_host::h536e12fc3e0c9ff0 (<module>[15142]:0x53fb75)
...
           at swc_plugin_coverage::process::h5a1dbe072b3a6e2c (<module>[3105]:0x184850)
           at __transform_plugin_process_impl (<module>[3110]:0x18564b)
           at __transform_plugin_process_impl.command_export (<module>[27581]:0x965950)
    2: heap_get_oob
    at Compiler.transformSync (/home

We do not have clear isolated repro case yet, since bit unsure where should I look into further. Would like to ask some helps to dig this further. Is there any differences how memory work between different host? or something configurable?

@syrusakbary
Copy link
Member

Thanks for opening the bug @kwonoj.
@ptitSeb can you take a look on this?

@ptitSeb
Copy link
Contributor

ptitSeb commented Apr 13, 2023

I tried to reproduce the error on a mac m1, but I'm not sure what I should clone.

I cloned https://github.com/kwonoj/swc-plugin-coverage-instrument and tried cargo test but test went fine, and I see no trace of wasmer on that repo.

@kwonoj
Copy link
Contributor Author

kwonoj commented Apr 13, 2023

@ptitSeb did you do npm test?

@ptitSeb
Copy link
Contributor

ptitSeb commented Apr 13, 2023

ah no. That was the missing step I guess.

@ptitSeb
Copy link
Contributor

ptitSeb commented Apr 13, 2023

Ok, I reproduce now. I'll analyse what is happening...

@ptitSeb
Copy link
Contributor

ptitSeb commented Apr 14, 2023

@kwonoj how can I reproduce the issue using the swc repo directly? I need to be able to use dev. version of wasmer.

As a first analysis, I see the error seems to be inside a wait/notify opcode. The support for this opcode as been improved with the current dev. version so I would need to test with current dev. file before I can release the 3.2 of wasmer.

@kwonoj
Copy link
Contributor Author

kwonoj commented Apr 14, 2023

@ptitSeb You'll need couple of steps:

  1. Update all of the references to the swc_core in the plugin (i.e https://github.com/kwonoj/swc-plugin-coverage-instrument/blob/f50951a795ed2a5a464652f2230180090a8baace/packages/swc-plugin-coverage/Cargo.toml#L16) to point local path, or either patch, etcs

  2. Go to cloned swc repo, do the same for the bindings (https://github.com/swc-project/swc/blob/main/bindings/binding_core_node/Cargo.toml#L51-L63)

  3. yarn build to create new bindings

  4. Instead of using published version https://github.com/kwonoj/swc-plugin-coverage-instrument/blob/f50951a795ed2a5a464652f2230180090a8baace/spec/util/verifier.ts#L1, point to local build at 3 (const transformSync = require('...path/to/swc_repo_root'). Do not need to point specific file, root of the swc repo can resolve it.

@ptitSeb
Copy link
Contributor

ptitSeb commented Apr 14, 2023

Ok, that's quite some steps! Thanks for the details, I'll try all this later.

@ptitSeb
Copy link
Contributor

ptitSeb commented Apr 17, 2023

Mmmm, I checkout swc at dd56100585 and changed sec-plugin-coverage-instrument to point to the sec repo but when I npm test I get some errors
image
So there must be something I do wrong. The swc-core is at version 0.75.11 as expected at that revision.

@kwonoj
Copy link
Contributor Author

kwonoj commented Apr 17, 2023

I'm not sure, I tried the same commit and it compiles : https://github.com/kwonoj/swc-plugin-coverage-instrument/pull/210/files

@ptitSeb ptitSeb self-assigned this Apr 18, 2023
@ptitSeb ptitSeb added the priority-medium Medium priority issue label Apr 18, 2023
@ptitSeb ptitSeb added this to the v3.x milestone Apr 18, 2023
@syrusakbary
Copy link
Member

Hey @kwonoj can you try 3.2.0? We did some fixes that we believe might have solved the issue

@kwonoj
Copy link
Contributor Author

kwonoj commented Apr 19, 2023

@syrusakbary Unfortunately still seeing same after we bump up wasmer@3.2.

@ptitSeb ptitSeb modified the milestones: v3.x, v3.2.1, v3.3 Apr 20, 2023
@syrusakbary
Copy link
Member

I'm working with @ptitSeb to find out the culprit, will keep posted here the updates

@ptitSeb
Copy link
Contributor

ptitSeb commented Apr 26, 2023

@kwonoj I'm sorry but I still cannot get a build of the plugin with a local swc repo.

I use latest version of swc-plugin-coverage-instrument and swc, modified the 3 Cargo.toml to point the swc-core from the local swc, changed spec/utils/verifier.ts to also point to local swc but when I use npm test, the swc-coverage-custom-transform plugin always fail to build with many errors like the trait swc_common::errors::SourcesMapper is not implemented for S . Could you provide a step by step way to setup everything to be able build/test everything with local copy wasmer & swc?

@ptitSeb
Copy link
Contributor

ptitSeb commented Apr 28, 2023

@kwonoj I don't reproduce the issue anymore on my side. Did you fix it upstream?

@ptitSeb
Copy link
Contributor

ptitSeb commented May 2, 2023

Closing the ticket. Fell free to reopen it (or create a new one) if something get wrong again @kwonoj

@ptitSeb ptitSeb closed this as completed May 2, 2023
@kwonoj
Copy link
Contributor Author

kwonoj commented May 2, 2023

@ptitSeb Just saw this, this is still reproducible by running release build of plugin.

@kwonoj
Copy link
Contributor Author

kwonoj commented May 2, 2023

release build of plugin
Interestingly enabling lto seems affecting this.

@ptitSeb
Copy link
Contributor

ptitSeb commented May 2, 2023

Oh, lto? Mmm, so that maybe a compiler issue then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-medium Medium priority issue
Projects
None yet
Development

No branches or pull requests

3 participants