Write padding values to ensure pcre2_serialize_encode() outputs defined values #826
Conversation
| #ifdef SUPPORT_VALGRIND | ||
| VALGRIND_MAKE_MEM_NOACCESS(data - 2, 2); | ||
| #endif |
There was a problem hiding this comment.
even if the data is unconditionally initialized it should be made inaccessible for reads, since it is not valid, so this part of the code is still needed IMHO.
There was a problem hiding this comment.
On the other hand, the test then fails. The data can't be no-access, because the serialisation code reads it. But if it is uninitialised (for valgrind) then we can't use valgrind to assert that all the memory has been set.
That's a nasty trade off. I wonder if there's a way to benefit both ways...?
There was a problem hiding this comment.
Technically; the fact that the serialization code is reading it is a bug that was introduced when this feature was added in 10.45, and therefore we should fix that as well by making sure that when the code is serialized all holes are skipped instead.
There was a problem hiding this comment.
Or you could just define the holes as something and specify that they always contain zeros or some other value.
There was a problem hiding this comment.
I feel we overcomplicate this. Just write any value there.
There was a problem hiding this comment.
I agree with Philip and Zoltan. My PR was just writing a value in. That's all we need.
Fixes low-severity valgrind error reported in GHSA-q7rw-r7qq-2hx6.