| Summary: | ppc64 --track-origins=yes causes failures because of bad cmov addHRegUse | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Mark Wielaard <mark> |
| Component: | vex | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | cel, will_schmidt |
| Priority: | NOR | ||
| Version First Reported In: | 3.18.1 | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
Mark, thanks for letting us know about the issue. Let us know if there is anything we can do to help verify the fix. Off hand I have nothing to add to the fix. (In reply to Carl Love from comment #1) > Mark, thanks for letting us know about the issue. Let us know if there is > anything we can do to help verify the fix. Off hand I have nothing to add > to the fix. Thanks. Pushed as: commit fb6a77ed78876083e8ba4c2f92384db5c2e41be8 (HEAD -> master, origin/master, origin/HEAD) Author: Mark Wielaard <mark@klomp.org> Date: Tue Feb 8 16:36:08 2022 +0100 ppc64 --track-origins=yes failure because of bad cmov addHRegUse For Pin_CMov getRegUsage_PPCInstr called addHRegUse for the dst register with HRmWrite, but since this is a conditional move the register could be both read and written (read + write = modify). This matches the dst of Pin_FpCMov and Pin_AvCMov. In a very rare case, and only with --track-origins=yes, this could cause bad code generation. This is slightly amazing, this code is from 2005 and as far as I know we never seen an issue with --track-origins=yes on power before. And I have been unable to come up simple reproducer. https://bugs.kde.org/show_bug.cgi?id=449672 I have ran various tests and seen no regressions with this change. The bad code generation is also gone. The issue is pretty hard to reproduce and I haven't been able to produce a simple reproducer. If you find any issues with this code please add to this bug. |
We had an issue with some code running under memcheck with --track-origins=yes which caused a crash, it ran fine without any issues under memcheck without --track-origins=yes. Unfortunately the issue was a nontrivial test for an security issue (which I cannot disclose as is) and it only happens deep inside glibc and only when glibc is build with GCC 11.2.1. Julian suggested to look at the Pin_CMov reg usage and we found this: diff --git a/VEX/priv/host_ppc_defs.c b/VEX/priv/host_ppc_defs.c index 3ae0f6e08..4222b4786 100644 --- a/VEX/priv/host_ppc_defs.c +++ b/VEX/priv/host_ppc_defs.c @@ -2590,7 +2590,7 @@ void getRegUsage_PPCInstr ( HRegUsage* u, const PPCInstr* i, Bool mode64 ) return; case Pin_CMov: addRegUsage_PPCRI(u, i->Pin.CMov.src); - addHRegUse(u, HRmWrite, i->Pin.CMov.dst); + addHRegUse(u, HRmModify, i->Pin.CMov.dst); return; case Pin_Load: addRegUsage_PPCAMode(u, i->Pin.Load.src); And that resolved it. Since this is a conditional move the register could be both read and written (read + write = modify). This matches the dst of Pin_FpCMov and Pin_AvCMov. This is slightly amazing, this code is from 2005 and as far as I know we never seen an issue with --track-origins=yes on power before. And I have been unable to come up with a simpler reproducer.