Skip to content

Commit 1820423

Browse files
rustyrussellendothermicdev
authored andcommitted
common: fix memcpy error in Fischer-Yates shuffle.
Reported by Grubles on ARM64: ``` VALGRIND=1 valgrind -q --error-exitcode=7 --track-origins=yes --leak-check=full --show-reachable=yes --errors-for-leak-kinds=all common/test/run-tal_arr_randomize > /dev/null ==151138== Source and destination overlap in memcpy(0x4d69f08, 0x4d69f08, 8) ==151138== at 0x48CB68C: __GI_memcpy (vg_replace_strmem.c:1147) ==151138== by 0x41B50B: tal_arr_randomize_ (pseudorand.c:84) ==151138== by 0x41BB07: main (run-tal_arr_randomize.c:166) ==151138== make: *** [Makefile:750: unittest/common/test/run-tal_arr_randomize] Error 7 ``` It is correct: you can't overlap src and dst in memcpy. It *probably* works in this case, but it's undefined! Fixes: #7030 Signed-off-by: Rusty Russell <[email protected]>
1 parent d554206 commit 1820423

File tree

1 file changed

+3
-0
lines changed

1 file changed

+3
-0
lines changed

common/pseudorand.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ void tal_arr_randomize_(void *arr, size_t elemsize)
8080
size_t j = i + pseudorand(n - i);
8181
char tmp[elemsize];
8282

83+
/* Technically, memcpy in place is undefined (src and dest overlap). */
84+
if (j == i)
85+
continue;
8386
memcpy(tmp, carr + i * elemsize, elemsize);
8487
memcpy(carr + i * elemsize, carr + j * elemsize, elemsize);
8588
memcpy(carr + j * elemsize, tmp, elemsize);

0 commit comments

Comments
 (0)