| Summary: | memccpy false positives | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Mark Wielaard <mark> |
| Component: | memcheck | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | pjfloyd |
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
Gah. Should also check the return values. I changed the condition - the ternary expression is always true. Extended the testcase as well. Forgot to add an attribution for Mark though. commit 805c020c6e5161966e6eb0099ebe937a510cea9e (HEAD -> master, origin/master, origin/HEAD) Author: Paul Floyd <pjfloyd@wanadoo.fr> Date: Wed May 1 09:24:14 2024 +0200 Bug 486293 - memccpy false positives |
The length of the src area is calculated wrongly in the memccpy replace function. This causes a false positive overlap warning when the c character is found early in the src memory area. Fix this by making sure the srclen is calculated correctly (only covers the area of bytes actually copied). And add a new testcase in memcheck/tests/memccpy2.c that fails (shows a false positive) with the old implementation, but passes with the new implementation of the memccpy replacement function. diff --git a/memcheck/tests/memccpy2.c b/memcheck/tests/memccpy2.c index a5a1dfc9f0af..4a54e04c0ae6 100644 --- a/memcheck/tests/memccpy2.c +++ b/memcheck/tests/memccpy2.c @@ -9,5 +9,7 @@ int main(void) memccpy(astring+10, astring, '#', len-10); sprintf(astring, "this is a string # with something to seek"); memccpy(astring, astring+10, '#', len); + sprintf(astring, "this is a string # with something to seek"); + memccpy(astring+10, astring, ' ', len-10); /* space is early, no overlap */ } diff --git a/shared/vg_replace_strmem.c b/shared/vg_replace_strmem.c index 737abbf67898..036763efcf0c 100644 --- a/shared/vg_replace_strmem.c +++ b/shared/vg_replace_strmem.c @@ -2364,9 +2364,9 @@ static inline void my_exit ( int x ) \ while (i-- > 0) \ if ((*d++ = *s++) == x) { \ - SizeT srclen = (i < len) ? i : len; \ + SizeT srclen = (i < len) ? len - i : len; \ RECORD_COPY(srclen); \ - if (is_overlap(dst, src, srclen, srclen)) \ + if (is_overlap(dst, src, len, srclen)) \ RECORD_OVERLAP_ERROR("memccpy", dst, src, len); \ return d; \ } \