| Summary: | s390x: helgrind/tests/tc11_XCHG.c expects code to be at a low address space | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Tulio Magno Quites Machado Filho <tuliom> |
| Component: | helgrind | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | arnez, mark |
| Priority: | NOR | ||
| Version: | 3.22 GIT | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: | Suggested fix for this issue | ||
I don't fully understand the s390x assembly, but this fix looks correct to me. With the patch the code becomes: 1001286: 58 00 20 00 l %r0,0(%r2) 100128a: ba 01 20 00 cs %r0,%r1,0(%r2) 100128e: a7 74 ff fc jne 1001286 <child_fn+0x26> Andreas, do you approve of this fix? (In reply to Mark Wielaard from comment #1) > I don't fully understand the s390x assembly, but this fix looks correct to > me. > With the patch the code becomes: > > 1001286: 58 00 20 00 l %r0,0(%r2) > 100128a: ba 01 20 00 cs %r0,%r1,0(%r2) > 100128e: a7 74 ff fc jne 1001286 <child_fn+0x26> > > Andreas, do you approve of this fix? Wow, interesting find. Yes, the fix looks good to me, thanks! commit 5a97c06080078aab8adfcc8985aecce7bfa5a738 Author: Tulio Magno Quites Machado Filho <tuliom@redhat.com> Date: Wed Jun 14 11:28:38 2023 -0300 s390x: Replace absolute jump for a relative one The bne instruction expects an absolute target address and it isn't best-suited for implementing a short range jump, such as the one in XCHG_M_R(). Replace it with jne which expects a relative address that can be correctly computed a link time. Interestingly, the jump is almost never taken. If it would, this would crash the test. However, linkers may complain when relacating the target address used in bne. |
Created attachment 159659 [details] Suggested fix for this issue SUMMARY The bne instruction used in the s390x implementation of XCHG_M_R(_addr,_lval) expects an absolute address and shouldn't be used in this case. I'm attaching a patch fixing this issue. STEPS TO REPRODUCE 1. ./configure && make && make check 2. Disassemble the test with: objdump -d ./helgrind/tests/tc11_XCHG | less OBSERVED RESULT Look for the child_fn function and see how bne (address 0x100128e) may jump to nowhere (address 646): 0000000001001260 <child_fn>: ... 1001286: 58 00 20 00 l %r0,0(%r2) 100128a: ba 01 20 00 cs %r0,%r1,0(%r2) 100128e: 47 70 02 86 bne 646 EXPECTED RESULT That instruction should jump to 2 instructions before at address 0x1001286. SOFTWARE/OS VERSIONS Windows: macOS: Linux/KDE Plasma: (available in About System) KDE Plasma Version: KDE Frameworks Version: Qt Version: ADDITIONAL INFORMATION