baseline: Fix incorrect exit after invalid jump

[chfast]

To handle invalid jump the implementation targets the byte just after
the official code length. For padded code this byte may be uninitialized
push data causing unpredictable behavior. The fix is to also init this
byte to OP_STOP.

The bug was detected in erigontech/silkworm#305.

Here two simple unit tests were added which are able to detect the core issue in MSVC/ASan/memcheck builds.

[yperbasis]

LGTM, but I would still do something like

std::fill_n(padded_code.get() + code_size, i + 1 - code_size, uint8_t{OP_STOP});

instead.
The overhead is probably low, while it's a guarantee against inconsistent behaviour across different platforms.

[codecov]

Codecov Report

Merging #370 (082d1d7) into release/0.8.0 (9570ee0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@              Coverage Diff               @@
##           release/0.8.0     #370   +/-   ##
==============================================
  Coverage          99.78%   99.78%           
==============================================
  Files                 30       30           
  Lines               4144     4153    +9     
==============================================
+ Hits                4135     4144    +9     
  Misses                 9        9           

Flag	Coverage Δ
consensus	91.51% <100.00%> (+<0.01%)	⬆️
unittests	99.78% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
lib/evmone/baseline.cpp	99.82% <100.00%> (+<0.01%)	⬆️
test/unittests/evm_test.cpp	99.70% <100.00%> (+<0.01%)	⬆️

[chfast]

LGTM, but I would still do something like

std::fill_n(padded_code.get() + code_size, i + 1 - code_size, uint8_t{OP_STOP});

instead.
The overhead is probably low, while it's a guarantee against inconsistent behaviour across different platforms.

Sounds reasonable, but I have slight preference not to be too defensive. The other bytes may only be used by a PUSH instruction and the very end and the values are not observable. If I'm wrong some fuzzing my discover this what is also good.

I also had some issues with std::fill because the compiler is not aware that the max length is 33 and it produces big unrolled loop or calls memset() or do both.

We also plan to use fixed-size padding with zeros because it allows single allocation instead of 2 (see TODO comment).

So I'm not sure the std::fill is worth right now, but I will follow your recommendation.

[yperbasis]

LGTM, but I would still do something like

std::fill_n(padded_code.get() + code_size, i + 1 - code_size, uint8_t{OP_STOP});

instead.
The overhead is probably low, while it's a guarantee against inconsistent behaviour across different platforms.

Sounds reasonable, but I have slight preference not to be too defensive. The other bytes may only be used by a PUSH instruction and the very end and the values are not observable. If I'm wrong some fuzzing my discover this what is also good.

I also had some issues with std::fill because the compiler is not aware that the max length is 33 and it produces big unrolled loop or calls memset() or do both.

We also plan to use fixed-size padding with zeros because it allows single allocation instead of 2 (see TODO comment).

So I'm not sure the std::fill is worth right now, but I will follow your recommendation.

Sure, fair enough, it's good as is now, especially given that you're going to revisit this code.