| Summary: | commit to fix vki_siginfo_t definition created numerous regression errors on PPC64 | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Carl Love <cel> |
| Component: | vex | Assignee: | Julian Seward <jseward> |
| Status: | CLOSED FIXED | ||
| Severity: | normal | CC: | mark |
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
| Attachments: | PPC64 alginment fix for struct rt_sigframe | ||
|
Description
Carl Love
2020-02-11 15:35:34 UTC
Would like to get this issue fixed before the next Valgrind release. I will look at the commit further to help determine the root cause of the breakage. This is also https://bugs.kde.org/show_bug.cgi?id=416760 The original bug fixed was https://bugs.kde.org/show_bug.cgi?id=405201 The following patch seems to clean up most of the issues:
Fix for regression error
---
coregrind/m_sigframe/sigframe-ppc64-linux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/coregrind/m_sigframe/sigframe-ppc64-linux.c b/coregrind/m_sigframe/
sigframe-ppc64-linux.c
index 0406f3c..b54c4e0 100644
--- a/coregrind/m_sigframe/sigframe-ppc64-linux.c
+++ b/coregrind/m_sigframe/sigframe-ppc64-linux.c
@@ -112,7 +112,7 @@ struct rt_sigframe {
vki_siginfo_t info;
struct vg_sig_private priv;
UChar abigap[288]; // unused
-};
+} __attribute__ ((aligned (16)));
#define SET_SIGNAL_LR(zztst, zzval) \
do { tst->arch.vex.guest_LR = (zzval); \
--
2.7.4
With this patch the number of regression errors is reduced to:
== 649 tests, 6 stderr failures, 3 stdout failures, 0 stderrB failures, 2 stdou\
tB failures, 2 post failures ==
gdbserver_tests/nlgone_abrt (stdoutB)
gdbserver_tests/nlpasssigalrm (stdoutB)
memcheck/tests/bug340392 (stderr)
memcheck/tests/leak_cpp_interior (stderr)
memcheck/tests/linux/rfcomm (stderr)
memcheck/tests/vcpu_fnfns (stdout)
memcheck/tests/vcpu_fnfns (stderr)
massif/tests/new-cpp (post)
massif/tests/overloaded-new (post)
none/tests/ppc64/test_isa_2_06_part3 (stdout)
none/tests/ppc64/test_isa_2_06_part3 (stderr)
none/tests/ppc64/test_isa_2_07_part2 (stdout)
none/tests/ppc64/test_isa_2_07_part2 (stderr)
Continuing to look at what additional changes are needed. But it looks like a start.
Rolled back to Eugene's commit 3bac39a10abf292d332bb20ab58c6dd5c28f9108 and applied my patch. That does fix all the regression error at that point. == 647 tests, 3 stderr failures, 0 stdout failures, 0 stderrB failures, 2 stdou\ tB failures, 2 post failures == gdbserver_tests/nlgone_abrt (stdoutB) gdbserver_tests/nlpasssigalrm (stdoutB) memcheck/tests/bug340392 (stderr) memcheck/tests/leak_cpp_interior (stderr) memcheck/tests/linux/rfcomm (stderr) massif/tests/new-cpp (post) massif/tests/overloaded-new (post) So, it looks like there is another commit that breaks a few things later. Looking.... Looks like the few additional errors that the fix given for this bug is for an unrelated commit after Eugene's changes. If we can get some testing of the fix on other platforms that would be helpful to decide if this is a good fix or not. I am a little concerned how universally the __attribute__ ((aligned (16))) works. Thanks. Created attachment 126280 [details]
PPC64 alginment fix for struct rt_sigframe
PPC64 alginment fix for struct rt_sigframe
The attached patch fixes the alignment for the rt_sigframe structure on PPC64. This fixes numerous regresion errors. On power 7, 8 and 9, it fixes 31 stderr failures, 10 stdout failures.
The patch was committed to Valgrind mainline commit 6f8920fd8f9381cf75f5dd0c3c65ac5d86c0a537 Author: Carl Love <carll@us.ibm.com> Date: Fri Feb 21 17:22:26 2020 -0600 PPC64, fix for alignment of the rt_sigframe data structure. The PPC64 implementation checks that the data structure is aligned. The changes in commit listed below breaks the alignment. This patch adds an explicit alignment directive to ensure the data structure is allocated with the required alignment. This fixes 31 stderr failures, 10 stdout failures on the Power 7, Power 8 and Power 9 platforms. commit 3bac39a10abf292d332bb20ab58c6dd5c28f9108 Author: Eugene Syromyatnikov <evgsyr@gmail.com> Date: Fri Mar 8 04:07:00 2019 +0100 Nightly regression tests errors were reduced as expected. No unexpected issues/errors were seen. Closing. |