| Summary: | final_tidyup should mark helper registers as defined before jumping to the freeres_wrapper | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Mark Wielaard <mark> |
| Component: | general | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
Also tested on ppc64[be] and reviewed on irc by Julian (who noticed the issue originally). Committed. commit be7a73004583aab5d4c97cf55276ca58d5b3090b Author: Mark Wielaard <mark@klomp.org> Date: Wed Dec 12 14:15:28 2018 +0100 Mark helper regs defined in final_tidyup before freeres_wrapper call. In final_tidyup we setup the guest to call the freeres_wrapper, which will (possibly) call __gnu_cxx::__freeres() and/or __libc_freeres(). In a couple of cases (ppc64be, ppc64le and mips32) this involves setting up one or more helper registers. Since we setup these guest registers we should make sure to mark them as fully defined. Otherwise we might see spurious warnings about undefined value usage if the guest register happened to not be fully defined before. This fixes PR402006. |
In final_tidyup we setup the guest to call the freeres_wrapper, which will (possibly) call __gnu_cxx::__freeres() and/or __libc_freeres(). For ppc64le we do this with: # elif defined(VGP_ppc64le_linux) /* setting GPR2 but not really needed, GPR12 is needed */ VG_(threads)[tid].arch.vex.guest_GPR2 = freeres_wrapper; VG_(threads)[tid].arch.vex.guest_GPR12 = freeres_wrapper; # endif And then we setup the argument as: # elif defined(VGA_ppc64be) || defined(VGA_ppc64le) VG_(threads)[tid].arch.vex.guest_GPR3 = to_run; VG_TRACK(post_reg_write, Vg_CoreClientReq, tid, offsetof(VexGuestPPC64State, guest_GPR3), sizeof(VG_(threads)[tid].arch.vex.guest_GPR3)); Note how in the first hunk we are not marking GPR2 and GPR12 as defined/written. This might cause: ==21345== Use of uninitialised value of size 8 ==21345== at 0x4050794: _vgnU_freeres (vg_preloaded.c:68) ==21345== by 0x4114A03: __run_exit_handlers (in /usr/lib64/power9/libc-2.28.so) ==21345== by 0x4114A97: exit (in /usr/lib64/power9/libc-2.28.so) ==21345== by 0x40F417F: (below main) (in /usr/lib64/power9/libc-2.28.so) ==21345== ==21345== Use of uninitialised value of size 8 ==21345== at 0x40505E4: ??? (in /root/valgrind-install/lib/valgrind/vgpreload_core-ppc64le-linux.so) ==21345== by 0x4114A03: __run_exit_handlers (in /usr/lib64/power9/libc-2.28.so) ==21345== by 0x4114A97: exit (in /usr/lib64/power9/libc-2.28.so) ==21345== by 0x40F417F: (below main) (in /usr/lib64/power9/libc-2.28.so) ==21345== This code checks r12 (first to see if a weak function symbol is set, then inside the plt entry for __libc_freeres). So, the setup code should also mark the helper registers as defined/written for those architectures that use them: diff --git a/coregrind/m_main.c b/coregrind/m_main.c index 00702fc22..9052edf1e 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -2312,14 +2312,26 @@ static void final_tidyup(ThreadId tid) VG_(set_IP)(tid, freeres_wrapper); # if defined(VGP_ppc64be_linux) VG_(threads)[tid].arch.vex.guest_GPR2 = r2; + VG_TRACK(post_reg_write, Vg_CoreClientReq, tid, + offsetof(VexGuestPPC64State, guest_GPR2), + sizeof(VG_(threads)[tid].arch.vex.guest_GPR2)); # elif defined(VGP_ppc64le_linux) /* setting GPR2 but not really needed, GPR12 is needed */ VG_(threads)[tid].arch.vex.guest_GPR2 = freeres_wrapper; + VG_TRACK(post_reg_write, Vg_CoreClientReq, tid, + offsetof(VexGuestPPC64State, guest_GPR2), + sizeof(VG_(threads)[tid].arch.vex.guest_GPR2)); VG_(threads)[tid].arch.vex.guest_GPR12 = freeres_wrapper; + VG_TRACK(post_reg_write, Vg_CoreClientReq, tid, + offsetof(VexGuestPPC64State, guest_GPR12), + sizeof(VG_(threads)[tid].arch.vex.guest_GPR12)); # endif /* mips-linux note: we need to set t9 */ # if defined(VGP_mips32_linux) || defined(VGP_mips64_linux) VG_(threads)[tid].arch.vex.guest_r25 = freeres_wrapper; + VG_TRACK(post_reg_write, Vg_CoreClientReq, tid, + offsetof(VexGuestMIPS32State, guest_r25), + sizeof(VG_(threads)[tid].arch.vex.guest_r25)); # endif /* Pass a parameter to freeres_wrapper(). */ This is somewhat hard to test though because it really depends on whether or not those registers were fully defined right before. In most cases they are.