search

BurntSushi / memchr

Optimized string search routines for Rust.
The Unlicense
799 stars 97 forks source link

`v2.7.3` breaks big endian targets #152

Closed CryZe closed 1 month ago

CryZe commented 1 month ago

The latest version 2.7.3 apparently broke on all big endian targets. It does not identify the positions correctly anymore. I'll look deeper into what might've caused it.

BurntSushi commented 1 month ago

Hmmm I thought powerpc64-unknown-linux-gnu and s390x-unknown-linux-gnu were both big endian targets. Did I get that wrong? (Because of that, I perhaps have overvalued confidence in CI passing.)

CryZe commented 1 month ago

That's indeed the targets that fail for me with 2.7.3: https://github.com/LiveSplit/livesplit-core/actions/runs/9507202428/job/26206138350

I can try to modify my CI further to get actual example values that fail.

I'm somewhat guessing your CI doesn't hit the code paths which according to what I see in the PR might be only for short strings.

CryZe commented 1 month ago

Alright, this fails:

assert_eq!(memchr::memchr(b':', b"1:23").unwrap(), 1);
assertion `left == right` failed
  left: 2
 right: 1
BurntSushi commented 1 month ago

OK, so the test suite exploded with dozens of failures as soon as I ran it locally on the powerpc64-unknown-linux-gnu target. Then I checked the CI configuration and to my horror, cargo was being used in some of the steps instead of cross (when it's enabled): https://github.com/BurntSushi/memchr/blob/ad08893ecd23a21dc2fdb27cdaf04f64f4834270/.github/workflows/ci.yml#L125

It should be using ${{ env.CARGO }} instead.

I'm working on fixing it. If I can't fix it quickly, I'll revert the PR. I have an idea as to where the problem is.