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

Disable integer sign extensions #139

Merged

Conversation

rafal-ch
Copy link
Contributor

@rafal-ch rafal-ch commented May 28, 2024

This PR modifies the build script to build WASM without the unuspported opcodes. It also updates the pinned nightly to 2024-05-28 to show that this solution works with nightly toolchains based on Rust newer than 1.69.

Fixes: casper-network/casper-node#4574

Running:

make prepare
make test

should result in

test result: ok. 29 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.39s

Rationale:
This is caused by the sign extension feature in code generator (LLVM)

12:04:22 ~/Casper/cep18 (dev*) $ wasm2wat target/wasm32-unknown-unknown/release/cep18.wasm | grep i32.extend8_s
                i32.extend8_s

i32.extend8_s is the opcode number 192 (0xc0).

These are enabled by default in LLVM 16 which is used in Rust starting from 1.70.

$ rustc +1.69 --version --verbose
rustc 1.69.0 (84c898d65 2023-04-16)
LLVM version: 15.0.7
$ rustc +1.70 --version --verbose
rustc 1.70.0 (90c541806 2023-05-31)
LLVM version: 16.0.2

We can dodge that by setting the target-cpu=mvp flag, but since this affects std which comes as a prebuilt package, we also need to rebuild std ourselves.

Hence we need sources:
rustup component add rust-src

and appropriately modified build script.


Actually, it should be enough to set -C target-feature=-sign-ext, but this still produces the i32.extend8_s opcode (as can be seen in the compiler explorer here (not my link, though) - https://godbolt.org/z/Te5fczPhM
Could be an issue in the LLVM itself, we should keep an eye on this.

@gRoussac gRoussac requested a review from deuszex May 28, 2024 13:58
@deuszex
Copy link
Contributor

deuszex commented May 30, 2024

@ACStoneCL this might be worth a doc

Copy link
Contributor

@deuszex deuszex left a comment

Choose a reason for hiding this comment

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

Solving a years worth in 10 lines.

@deuszex deuszex merged commit d6348eb into casper-ecosystem:dev May 30, 2024
3 checks passed
@rafal-ch rafal-ch deleted the disable_integer_sign_extensions branch June 3, 2024 10:20
deuszex pushed a commit that referenced this pull request Jun 17, 2024
* Update to recent (post 1.69) nightly

* Build without LLVM sign extensions

* Satisfy clippy
deuszex pushed a commit that referenced this pull request Jun 17, 2024
* Update to recent (post 1.69) nightly

* Build without LLVM sign extensions

* Satisfy clippy
deuszex added a commit that referenced this pull request Aug 27, 2024
* WIP

* WIP

* Commit toolchain for ci/cd

* Disable integer sign extensions (#139)

* Update to recent (post 1.69) nightly

* Build without LLVM sign extensions

* Satisfy clippy

* Disable integer sign extensions (#139)

* Update to recent (post 1.69) nightly

* Build without LLVM sign extensions

* Satisfy clippy

* WIP

* 2.0.0-rc2

* 2.0.0-rc2 stable

* addressing comments, first round of fixture fights

* rc3

* fixture work

* fixedture

* Small review changes

* Small rename

* native eventing (#145)

* native eventing

* change_event_mode

* native eventing done

* Use DEFAULT_ACCOUNTS in tests (#146)

* disallow Key::AddressableEntity::SmartContract in favour of Key::Package, should have been this way to begin with

* more event tests

* fix fake positive event test

* remove unused dictinary location check

* failing test

* native expansive error topic

* fix allowance migration

* fix sec migration

* removed custom error to see source behind it

* fixed migration of contract packages

* Update utils.rs

* added compatibility mode

* added content from community-rc3 branch

* removed

* addressed commit responses + removed migration

---------

Co-authored-by: gRoussac <[email protected]>
Co-authored-by: Rafał Chabowski <[email protected]>
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.

Rust toolchain failure observed with CEP18 contract.
2 participants