-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
MatchData::find
's EMPTY
may still be dangling
#42
Comments
I can't make sense of the failure modes you're seeing, but what's the fix for this? My read of rust-lang/rust#123936 seems to suggest that switching from I guess to some extent this is a bug in PCRE2. We hand it a pointer and a length, and even though the length is zero, PCRE2 still dereferences it. Since Fedora has the newer PCRE2, I would guess that's why you don't see the bad behavior on Fedora? IDK, I'm happy to try and fix things on my end to the extent possible, I'm just not sure what. I suppose we could pass |
Yeah, that's what I figure too, but I don't know enough about that library to really track it down. I'm especially not keen to dig into its JIT details.
I think that should work, since provenance of that pointer will cover real memory while it's passed to FFI, length aside. |
To work around likely bugs in (older versions of) PCRE2. Namely, at one point, PCRE2 would dereference the haystack pointer even when the length was zero. This was reported in #10 and we worked around this in #11 by passing a pointer to a const `&[]`, with the (erroneous) presumption that this would be a valid pointer to dereference. In retrospect though, this was a little silly, because you should never be dereferencing a pointer to an empty slice. It's not valid. Alas, at that time, Rust did actually hand you a valid pointer that could be dereferenced. But [this PR][rust-pull] changed that. And thus, we're back to where we started: handing buggy versions of PCRE2 a zero length haystack with a dangling pointer. So we fix this once and for all by passing a slice of length 1, but with a haystack length of 0, to the PCRE2 search routine when searching an empty haystack. This will guarantee the provision of a dereferencable pointer should PCRE2 decide to dereference it. Fixes #42 [rust-pull]: rust-lang/rust#123936
OK, should be fixed in |
Thanks! I confirmed in a local build that 0.2.9 does fix the ripgrep tests. I'll report back if anything else comes up in our full EPEL build environment. |
FYI, our PCRE2 maintainer found the upstream fix -- links for cross-reference: |
#11 added an
EMPTY
constant to avoid the dangling pointer of an emptyVec
, but there's no guarantee that this is actually a dereferenceable pointer either -- and it is not since rust-lang/rust#123936, Rust 1.79.We're seeing this cause segfaults in ripgrep integrations tests on EPEL 9 aarch64:
https://koji.fedoraproject.org/koji/taskinfo?taskID=120840654
aarch64
build.log
excerptThis was compiled against the RHEL 9's libpcre2, version 10.40. When debugging, I found calls to
pcre2_match_8
with thesubject
pointer 1,length
0. Since it only crashed on aarch64, I'm guessing there's something in the aarch64 JIT that wasn't handling this properly. However, when I copy this exactrg
binary to a current Fedora system with newer pcre2, it passes.#10 noted that upstream pcre2 also made fixes, although the vcs link is now dead, but that would have long predated 10.40 in April 2022. So I'm not sure what further fix makes it work in Fedora.
The text was updated successfully, but these errors were encountered: