-
Notifications
You must be signed in to change notification settings - Fork 202
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
Two minor correctness bugs found by Clang Analyzer #576
Comments
NWilson
added a commit
to NWilson/pcre2
that referenced
this issue
Dec 3, 2024
This was referenced Dec 3, 2024
NWilson
added a commit
to NWilson/pcre2
that referenced
this issue
Dec 4, 2024
NWilson
added a commit
to NWilson/pcre2
that referenced
this issue
Dec 6, 2024
NWilson
added a commit
that referenced
this issue
Dec 6, 2024
Found by Clang-Analyze in #576 This test fails without the code fix, which required correcting the way the pcre2grep tests are run with valgrind. The new test passes with the code changes made.
NWilson
added a commit
that referenced
this issue
Dec 7, 2024
One of these appears in Coverity's dashboard; the rest are from clang-scan. See #576
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I have a PR open which adds Clang's very good static analyzer to our reports.
This has flagged two issues, one of which appears to be a genuine bug:
end_of_line
function is naughty! It looks at the leading byte of a UTF-8 sequence to find out how many bytes to read, then reads them without checking whether the buffer contains enough bytes! It appears to be a read-past-the-end bug if the input ends in a truncated UTF-8 sequence.pcre2_regerror
has a NULL check forpreg
, and the function does support passing in a NULLpreg
. But further down in the function, it's used unconditionally inmessage_len(message, (int)preg->re_erroffset)
, and clang reckons that a NULL dereference (might) be possible there. I believe that this is bogus, however, and clang just isn't quite clever enough to work it out; but someone should probably double-check that reasoning.The text was updated successfully, but these errors were encountered: