fix(BytesOp::Find): fix slice index overflow#82
Conversation
…he lenth of ByteStr is at most u16::MAX
src/isa/exec.rs
Outdated
| let mut count = 0usize; | ||
| for i in 0..r1.len() { | ||
| if r1[i..len] == r2[..len] { | ||
| for i in 0..(r1_len + 1).saturating_sub(r2_len) { |
There was a problem hiding this comment.
r1_len + 1 can also panic (in debug) or overflow (in release)
There was a problem hiding this comment.
r1_len is at most u16::MAX (because it is ByteStr) stored as usize. so it will not overflow on +1
There was a problem hiding this comment.
but I have to admit this is error prone. fixing it
6293
force-pushed
the
fix-fits-usize
branch
2 times, most recently
from
a2f1e68 to
9ce67a5
Compare
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #82 +/- ##
======================================
Coverage 34.2% 34.2%
======================================
Files 19 19
Lines 4544 4541 -3
======================================
Hits 1554 1554
+ Misses 2990 2987 -3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| }; | ||
| f().unwrap_or_else(|| { | ||
| regs.st0 = false; | ||
| regs.set(RegA::A16, Reg32::Reg0, MaybeNumber::none()); |
There was a problem hiding this comment.
Why are we removing this if the spec below says
If the first or the second string is `None`, sets `st0` to `false` and `a16[0]` to `None`.
There was a problem hiding this comment.
this is the spec for Find. The code here is for Rev. I thought this was accidentally copy-pasted from Find above
There was a problem hiding this comment.
Yet still the Rev must reset the destination to the uninitialized state:
/// If the source string register is uninitialized, resets destination to the uninitialized
So I assume the above line should stay and be changed to
| regs.set(RegA::A16, Reg32::Reg0, MaybeNumber::none()); | |
| regs.s16[reg2.as_usize()] = None; |
| }; | ||
| f().unwrap_or_else(|| { | ||
| regs.st0 = false; | ||
| regs.set(RegA::A16, Reg32::Reg0, MaybeNumber::none()); |
There was a problem hiding this comment.
Yet still the Rev must reset the destination to the uninitialized state:
/// If the source string register is uninitialized, resets destination to the uninitialized
So I assume the above line should stay and be changed to
| regs.set(RegA::A16, Reg32::Reg0, MaybeNumber::none()); | |
| regs.s16[reg2.as_usize()] = None; |
3 participants
No description provided.