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 potential UB #524

Merged
merged 5 commits into from
Oct 20, 2022
Merged

Fix potential UB #524

merged 5 commits into from
Oct 20, 2022

Conversation

Robbepop
Copy link
Member

  • As reported by miri when testing for --doc tests.
  • We cannot use NonNull there since NonNull is missing the respective offset method, yet, as discussed and requested here: NonNull methods for pointer offset, add, sub rust-lang/rust#72429
  • This PR also adds a miri --doc CI test run since that is how we discovered this potential UB.

@paritytech-cicd-pr
Copy link

paritytech-cicd-pr commented Oct 18, 2022

BENCHMARKS

NATIVEWASMTIME
BENCHMARKMASTERPRDIFFMASTERPRDIFFWASMTIME OVERHEAD
execute/
bare_call_0
1.28ms 1.29ms ⚪ 0.79% 1.15ms 1.06ms 🔴 -8.02% 🟢 -17%
execute/
bare_call_0/typed
704.45µs 698.55µs ⚪ -0.73% 536.03µs 561.95µs 🔴 4.80% 🟢 -20%
execute/
bare_call_1
1.45ms 1.40ms 🔴 -3.07% 1.36ms 1.33ms 🔴 -2.12% 🟢 -5%
execute/
bare_call_16
2.86ms 2.94ms 🔴 2.82% 5.15ms 4.83ms 🔴 -5.30% 🟡 65%
execute/
bare_call_16/typed
1.29ms 1.30ms ⚪ 0.78% 2.49ms 2.52ms 🔴 1.44% 🟡 94%
execute/
bare_call_1/typed
733.81µs 722.60µs 🟢 -1.56% 694.19µs 730.18µs 🔴 5.18% 🟢 1%
execute/
bare_call_4
1.82ms 1.75ms ⚪ -1.08% 2.16ms 2.09ms 🔴 -3.40% 🟢 19%
execute/
bare_call_4/typed
894.73µs 890.69µs ⚪ -0.50% 995.54µs 1.04ms 🔴 4.30% 🟢 17%
execute/
count_until
931.07µs 887.63µs 🟢 -4.59% 2.38ms 2.27ms 🟢 -4.48% 🔴 156%
execute/
factorial_iterative
393.64ns 393.00ns ⚪ -0.08% 988.67ns 950.95ns 🟢 -3.90% 🔴 142%
execute/
factorial_recursive
748.41ns 784.81ns 🔴 4.85% 1.60µs 1.60µs ⚪ 0.34% 🔴 104%
execute/
fib_iterative
1.79ms 1.80ms ⚪ 0.29% 5.11ms 5.02ms 🟢 -1.68% 🔴 179%
execute/
fib_recursive
6.84ms 7.05ms 🔴 4.20% 14.24ms 14.47ms 🔴 1.65% 🔴 105%
execute/
global_bump
1.45ms 1.44ms ⚪ -0.95% 3.68ms 3.65ms ⚪ -0.93% 🔴 153%
execute/
host_calls
32.46µs 32.35µs ⚪ -0.07% 45.26µs 44.91µs ⚪ -0.98% 🟢 39%
execute/
memory_fill
1.53ms 1.54ms ⚪ 0.20% 4.33ms 4.14ms 🟢 -4.42% 🔴 169%
execute/
memory_sum
1.52ms 1.52ms ⚪ -0.51% 4.36ms 4.15ms 🟢 -4.76% 🔴 174%
execute/
memory_vec_add
3.02ms 3.04ms ⚪ 0.31% 9.00ms 8.76ms 🟢 -2.68% 🔴 188%
execute/
recursive_is_even
1.32ms 1.32ms ⚪ -0.65% 2.58ms 2.60ms ⚪ 0.50% 🟡 97%
execute/
recursive_ok
174.51µs 178.63µs 🔴 2.37% 358.79µs 359.74µs ⚪ 0.22% 🔴 101%
execute/
recursive_scan
217.44µs 221.14µs ⚪ 1.15% 462.69µs 464.86µs ⚪ 0.54% 🔴 110%
execute/
recursive_trap
17.22µs 18.42µs 🔴 6.98% 35.76µs 35.62µs ⚪ -0.43% 🟡 93%
execute/
regex_redux
651.29µs 697.38µs 🔴 6.90% 1.60ms 1.62ms ⚪ 1.18% 🔴 132%
execute/
rev_complement
586.24µs 600.11µs 🔴 2.30% 1.56ms 1.53ms 🟢 -1.35% 🔴 156%
execute/
tiny_keccak
456.66µs 452.82µs ⚪ -0.51% 1.45ms 1.41ms 🟢 -3.02% 🔴 211%
execute/
trunc_f2i
1.07ms 1.07ms ⚪ -0.19% 2.73ms 2.63ms 🟢 -3.58% 🔴 146%
instantiate/
wasm_kernel
66.21µs 67.62µs ⚪ 1.54% 127.35µs 126.39µs ⚪ -0.95% 🟡 87%
translate/
wasm_kernel
4.83ms 4.81ms ⚪ -0.40% 10.22ms 9.82ms 🟢 -3.97% 🔴 104%

Link to pipeline

@codecov-commenter
Copy link

Codecov Report

Merging #524 (c3c5017) into master (dab2565) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #524      +/-   ##
==========================================
- Coverage   79.38%   79.38%   -0.01%     
==========================================
  Files          74       74              
  Lines        6278     6276       -2     
==========================================
- Hits         4984     4982       -2     
  Misses       1294     1294              
Impacted Files Coverage Δ
crates/wasmi/src/engine/code_map.rs 81.57% <100.00%> (-0.93%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Robbepop Robbepop merged commit 52ad2ad into master Oct 20, 2022
@Robbepop Robbepop deleted the rf-fix-potential-ub branch October 20, 2022 08:19
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