Re-enable rustc overflow checks
#2116
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Closes #2115
inkorpallet-contracts?Description
See #2115 for a detailed description of historical issues and limitations of the current solution.
There were 2 primary root causes for the historical issues:
rustcrelated to how library functions from#![no_builtins]annotated crates (most notably compiler intrinsics from thecompiler-builtinscrate) interacted with LTO (Link Time Optimization) (i.e. LTO would sometimes errorneously optimize out these functions or replace them with recursive calls)compiler-builtinscrate (most notably thememfeature for generating memory-related intrinsics likememcpy) in standard library crates (i.e.alloc,core,stde.t.c), meaning these features couldn't be explicitly enabled with the-Z build-std-featuresflag when compiling standard library crates using-Z build-stdflag.(1) was generally the more complicated issue, and was ultimately solved by
no-builtinsattribute to functions whenno_builtinsis applied at the crate level. rust-lang/rust#113716#![no_builtins]crates participation in LTO. rust-lang/rust#113923With the latter explicitly mentioning previously reported issues from ink! as resolved i.e:
-Z build-stdfor wasm rust-lang/rust#91158(2) was resolved by
Accordingly, with this PR, we now explicitly enable the
memfeature of thecompiler-builtinscrate ( via thecompiler-builtins-memstandard library feature).NOTE: This is functionally equivalent to the previous "work around" for
WebAssemblyi.e.As explained by the deprecation notice in
rlibcreadmeLastly, we prefer to enable the
memfeature explicitly via the-Zbuild-std-featuresflag, even if thecompiler-builtinscrate already attempts to automatically enable thememfeature for certain targets (e.g. targets with a-nonesuffix) via it's build script, see:memfeature. rust-lang/compiler-builtins#357Follow ups/ related TODOs
ink/integration-tests(chore: Add integration test for arithmetic overflow checks ink#2631)Checklist before requesting a review
CHANGELOG.md