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

Take alignment into consideration during malloc #3463

Merged
merged 4 commits into from
Jun 6, 2023

Conversation

daxpedda
Copy link
Collaborator

@daxpedda daxpedda commented Jun 5, 2023

Unfortunately I had to hardcode an alignment of 4 because we didn't implement something to detect the target architecture when using wasm-bindgen-cli.

This is still missing adjustments on __wbindgen_realloc and __wbindgen_free.

This is fairly hacky, unfortunately I simply don't know enough about the wasm-bindgen codebase. Feel free to suggest how it could be done right.

Fixes #3461.

@daxpedda daxpedda marked this pull request as draft June 5, 2023 23:13
@daxpedda daxpedda force-pushed the alloc-align branch 2 times, most recently from b2afed6 to da0cf24 Compare June 5, 2023 23:54
@daxpedda daxpedda marked this pull request as ready for review June 6, 2023 00:03
Copy link
Collaborator

@Liamolucko Liamolucko left a comment

Choose a reason for hiding this comment

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

Looks good for the most part!

crates/threads-xform/src/lib.rs Outdated Show resolved Hide resolved
crates/threads-xform/src/lib.rs Outdated Show resolved Hide resolved
crates/threads-xform/src/lib.rs Outdated Show resolved Hide resolved
crates/cli-support/src/js/binding.rs Outdated Show resolved Hide resolved
crates/cli-support/src/js/binding.rs Outdated Show resolved Hide resolved
crates/cli-support/src/js/binding.rs Outdated Show resolved Hide resolved
crates/cli-support/src/js/mod.rs Outdated Show resolved Hide resolved
crates/cli-support/src/js/mod.rs Outdated Show resolved Hide resolved
crates/cli-support/src/js/binding.rs Outdated Show resolved Hide resolved
Co-Authored-By: Liam Murphy <43807659+Liamolucko@users.noreply.github.com>
Co-Authored-By: Liam Murphy <43807659+Liamolucko@users.noreply.github.com>
crates/cli-support/src/js/binding.rs Outdated Show resolved Hide resolved
crates/cli-support/src/wit/outgoing.rs Show resolved Hide resolved
crates/cli-support/src/wit/standard.rs Outdated Show resolved Hide resolved
@daxpedda daxpedda requested a review from Liamolucko June 6, 2023 11:56
Copy link
Collaborator

@Liamolucko Liamolucko left a comment

Choose a reason for hiding this comment

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

This is pretty much good to merge, I just noticed that the bit of the guide you updated is still using an alignment of 4 for strings.

guide/src/contributing/design/exporting-rust.md Outdated Show resolved Hide resolved
guide/src/contributing/design/exporting-rust.md Outdated Show resolved Hide resolved
Co-Authored-By: Liam Murphy <43807659+Liamolucko@users.noreply.github.com>
@daxpedda daxpedda force-pushed the alloc-align branch 2 times, most recently from 28b3c12 to 750eccc Compare June 6, 2023 12:10
@Liamolucko Liamolucko merged commit a2ab2d5 into rustwasm:main Jun 6, 2023
Wumpf pushed a commit to rerun-io/rerun that referenced this pull request Jun 13, 2023
It contains this important fix for Rust 1.70:
rustwasm/wasm-bindgen#3463

I tested the web viewer, and it works.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2406

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/eebb149/docs
Examples preview: https://rerun.io/preview/eebb149/examples
<!-- pr-link-docs:end -->
emilk added a commit to rerun-io/rerun that referenced this pull request Jun 15, 2023
It contains this important fix for Rust 1.70:
rustwasm/wasm-bindgen#3463

I tested the web viewer, and it works.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2406

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/eebb149/docs
Examples preview: https://rerun.io/preview/eebb149/examples
<!-- pr-link-docs:end -->
crapStone added a commit to Calciumdibromid/CaBr2 that referenced this pull request Jun 29, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [js-sys](https://rustwasm.github.io/wasm-bindgen/) ([source](https://github.com/rustwasm/wasm-bindgen)) | dependencies | patch | `=0.3.63` -> `=0.3.64` |
| [wasm-bindgen](https://rustwasm.github.io/) ([source](https://github.com/rustwasm/wasm-bindgen)) | dependencies | patch | `=0.2.86` -> `=0.2.87` |
| [wasm-bindgen-futures](https://rustwasm.github.io/wasm-bindgen/) ([source](https://github.com/rustwasm/wasm-bindgen)) | dependencies | patch | `=0.4.36` -> `=0.4.37` |

---

### Release Notes

<details>
<summary>rustwasm/wasm-bindgen (wasm-bindgen)</summary>

### [`v0.2.87`](https://github.com/rustwasm/wasm-bindgen/blob/HEAD/CHANGELOG.md#&#8203;0287-httpsgithubcomrustwasmwasm-bindgencompare02860287)

[Compare Source](rustwasm/wasm-bindgen@0.2.86...0.2.87)

Released 2023-06-12.

##### Added

-   Implemented `IntoIterator` for `Array`.
    [#&#8203;3477](rustwasm/wasm-bindgen#3477)

##### Changed

-   Deprecate `HtmlMenuItemElement` and parts of `HtmlMenuElement`.
    [#&#8203;3448](rustwasm/wasm-bindgen#3448)

-   Stabilize `ResizeObserver`.
    [#&#8203;3459](rustwasm/wasm-bindgen#3459)

##### Fixed

-   Take alignment into consideration during (de/re)allocation.
    [#&#8203;3463](rustwasm/wasm-bindgen#3463)

***

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xNDQuMyIsInVwZGF0ZWRJblZlciI6IjM1LjE0NC4zIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Co-authored-by: crapStone <crapstone01@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1947
Reviewed-by: crapStone <crapstone01@gmail.com>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
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.

Exporting bigint slices panics
2 participants