| Summary: | ppc stxsibx and stxsihx instructions write too much data | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Mark Wielaard <mark> |
| Component: | memcheck | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | cel, will_schmidt |
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
| Attachments: | Implement stxsibx and stxsihx as direct byte or half-word store | ||
Created attachment 134060 [details]
Implement stxsibx and stxsihx as direct byte or half-word store
This implements stxsibx and stxsihx as direct byte or half-word stores of 8 or 16 bytes without first reading the whole word at the effective address. This fixes the issue with memcheck/tests/str_tester and some other string functions and doesn't seem to produce any regressions.
The attached patch looks good to me. commit ab257bc49a6c8beefa794470446f917ec441f718 Author: Mark Wielaard <mark@klomp.org> Date: Tue Dec 15 11:49:58 2020 +0100 ppc stxsibx and stxsihx instructions write too much data stxsibx (Store VSX Scalar as Integer Byte Indexed X-form) is implemented by first reading a whole word, merging in the new byte, and then writing out the whole word. Causing memcheck to warn when the destination might have room for less than 8 bytes. The stxsihx (Store VSX Scalar as Integer Halfword Indexed X-form) instruction does something similar reading and then writing a full word instead of a half word. The code can be simplified (and made more correct) by storing the byte (or half-word) directly, IRStmt_Store seems fine to store byte or half word sized data, and so seems the ppc backend. https://bugs.kde.org/show_bug.cgi?id=430354 |
On a system with a power9 optimized glibc memcheck/tests/str_tester (and any other application using some of the glibc string functions) fail with: ==473431== Invalid write of size 8 ==473431== at 0x419C4CC: strndup (strndup.c:48) ==473431== by 0x1000B08F: test_strndup (str_tester.c:1297) ==473431== by 0x1000C043: main (str_tester.c:1528) ==473431== Address 0x4300046 is 6 bytes inside a block of size 7 alloc'd ==473431== at 0x40A428C: malloc (vg_replace_malloc.c:307) ==473431== by 0x419C4B3: strndup (strndup.c:43) ==473431== by 0x1000B08F: test_strndup (str_tester.c:1297) ==473431== by 0x1000C043: main (str_tester.c:1528) Looking with gdb finds: 0x000000000419c4cc in __GI___strndup (s=0x1000c418 "abcdef", n=<optimized out>) at strndup.c:48 48 new[len] = '\0'; Dump of assembler code for function __GI___strndup: 0x000000000419c480 <+0>: addis r2,r12,22 0x000000000419c484 <+4>: addi r2,r2,-20864 0x000000000419c488 <+8>: mflr r0 0x000000000419c48c <+12>: std r30,-16(r1) 0x000000000419c490 <+16>: std r31,-8(r1) 0x000000000419c494 <+20>: std r0,16(r1) 0x000000000419c498 <+24>: stdu r1,-48(r1) 0x000000000419c49c <+28>: mr r30,r3 0x000000000419c4a0 <+32>: bl 0x41ae390 <__strnlen_ppc> 0x000000000419c4a4 <+36>: nop 0x000000000419c4a8 <+40>: mr r31,r3 0x000000000419c4ac <+44>: addi r3,r3,1 0x000000000419c4b0 <+48>: bl 0x4118cc0 <00000025.plt_call.malloc> 0x000000000419c4b4 <+52>: ld r2,24(r1) 0x000000000419c4b8 <+56>: mr. r9,r3 0x000000000419c4bc <+60>: beq 0x419c4dc <__GI___strndup+92> 0x000000000419c4c0 <+64>: xxspltib vs0,0 0x000000000419c4c4 <+68>: mr r4,r30 0x000000000419c4c8 <+72>: mr r5,r31 => 0x000000000419c4cc <+76>: stxsibx vs0,r9,r31 0x000000000419c4d0 <+80>: bl 0x4118c80 <00000025.plt_call.__GI_memcpy> 0x000000000419c4d4 <+84>: ld r2,24(r1) 0x000000000419c4d8 <+88>: mr r9,r3 0x000000000419c4dc <+92>: addi r1,r1,48 0x000000000419c4e0 <+96>: mr r3,r9 0x000000000419c4e4 <+100>: ld r0,16(r1) 0x000000000419c4e8 <+104>: ld r30,-16(r1) 0x000000000419c4ec <+108>: ld r31,-8(r1) 0x000000000419c4f0 <+112>: mtlr r0 0x000000000419c4f4 <+116>: blr 0x000000000419c4f8 <+120>: .long 0x0 0x000000000419c4fc <+124>: .long 0x1000000 0x000000000419c500 <+128>: .long 0x280 End of assembler dump. So it is using stxsibx to write one byte at the end of the string. But stxsibx is implemented by first reading a whole word, merging in the new byte, and then writing out the whole word. Causing memcheck to warn the wrote is beyond the end of (malloced) string buffer. The the stxsihx (Store VSX Scalar as Integer Halfword Indexed X-form) instruction does something similar. It isn't immediately clear why the code isn't just storing the byte directly, IRStmt_Store seems fine to store byte or half word sized data, and so seems the ppc backend.