| Summary: | arm64 core dump support unimplemented | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Alexandra Hajkova <ahajkova> |
| Component: | general | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | ahajkova, marcin, mark |
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
| Attachments: |
patch
new version of the patch patch |
||
|
Description
Alexandra Hajkova
2019-03-21 13:33:09 UTC
STEPS TO REPRODUCE 1. ulimit -c unlimited 2. valgrind sleep 60 & 3. kill -s SIGABRT %1 OBSERVED RESULT valgrind: m_coredump/coredump-elf.c:495 (fill_fpu): Assertion 'Unimplemented functionality' failed. EXPECTED RESULT Process terminating with default action of signal 6 (SIGABRT): dumping core My proposed solution: https://github.com/sasshka/valgrind/commit/bc6040d4368c8abf653d48de6fe021d82f345e2f Hi Sasha, I saw you have a new commit that also fills in the floating point structure. https://github.com/sasshka/valgrind/commit/8903bfd77791723a46505530b993118c7abf1755 Could you please attach (git format-patch HEAD^) the patch to this bug as you would like to commit it? That makes it easier to review. It looks like the only question is whether pstate is setup correctly. I am not sure myself. In other code (synth_ucontext in coregrind/m_sigframe/sigframe-arm64-linux.c) we don't even try... Maybe the vgdb code does, but I cannot find where. Now that you have a bug number could you also add it to your commit message and in the NEWS file? Could you run make regtests before and after your patch to check for tests that now pass (or fail)? Thanks, Mark Created attachment 119022 [details]
patch
Comment on attachment 119022 [details] patch From 66eddce06e9edc54d268e174d51c23f1044cc070 Mon Sep 17 00:00:00 2001 From: Alexandra Hajkova <ahajkova@redhat.com> Date: Wed, 20 Mar 2019 10:10:44 +0100 Subject: [PATCH] Support arm64 core dump Fixes BZ #405722. Implements coredump-elf.c fill_prstatus() and fill_fpu () for VGP_arm64_linux. --- NEWS | 1 + coregrind/m_coredump/coredump-elf.c | 71 +++++++++++++++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index bf90093..77cc540 100644 --- a/NEWS +++ b/NEWS @@ -122,6 +122,7 @@ where XXXXXX is the bug number as listed below. 405733 PPC64, xvcvdpsp should write 32-bit result to upper and lower 32-bits of the 64-bit destination field. 405734 PPC64, vrlwnm, vrlwmi, vrldrm, vrldmi do not work properly when me < mb +405722 Support arm64 core dump n-i-bz add syswrap for PTRACE_GET|SET_THREAD_AREA on amd64. n-i-bz Fix callgrind_annotate non deterministic order for equal total diff --git a/coregrind/m_coredump/coredump-elf.c b/coregrind/m_coredump/coredump-elf.c index 2d36b26..5f71e3a 100644 --- a/coregrind/m_coredump/coredump-elf.c +++ b/coregrind/m_coredump/coredump-elf.c @@ -380,8 +380,40 @@ static void fill_prstatus(const ThreadState *tst, regs->ARM_cpsr = LibVEX_GuestARM_get_cpsr( &arch->vex ); #elif defined(VGP_arm64_linux) - (void)arch; - I_die_here; + regs->regs[0] = arch->vex.guest_X0; + regs->regs[1] = arch->vex.guest_X1; + regs->regs[2] = arch->vex.guest_X2; + regs->regs[3] = arch->vex.guest_X3; + regs->regs[4] = arch->vex.guest_X4; + regs->regs[5] = arch->vex.guest_X5; + regs->regs[6] = arch->vex.guest_X6; + regs->regs[7] = arch->vex.guest_X7; + regs->regs[8] = arch->vex.guest_X8; + regs->regs[9] = arch->vex.guest_X9; + regs->regs[10] = arch->vex.guest_X10; + regs->regs[11] = arch->vex.guest_X11; + regs->regs[12] = arch->vex.guest_X12; + regs->regs[13] = arch->vex.guest_X13; + regs->regs[14] = arch->vex.guest_X14; + regs->regs[15] = arch->vex.guest_X15; + regs->regs[16] = arch->vex.guest_X16; + regs->regs[17] = arch->vex.guest_X17; + regs->regs[18] = arch->vex.guest_X18; + regs->regs[19] = arch->vex.guest_X19; + regs->regs[20] = arch->vex.guest_X20; + regs->regs[21] = arch->vex.guest_X21; + regs->regs[22] = arch->vex.guest_X22; + regs->regs[23] = arch->vex.guest_X23; + regs->regs[24] = arch->vex.guest_X24; + regs->regs[25] = arch->vex.guest_X25; + regs->regs[26] = arch->vex.guest_X26; + regs->regs[27] = arch->vex.guest_X27; + regs->regs[28] = arch->vex.guest_X28; + regs->regs[29] = arch->vex.guest_X29; + regs->regs[30] = arch->vex.guest_X30; + regs->sp = arch->vex.guest_XSP; + regs->pc = arch->vex.guest_PC; + regs->pstate = LibVEX_GuestARM64_get_nzcv( &arch->vex ); /* is this correct? */ #elif defined(VGP_s390x_linux) # define DO(n) regs->gprs[n] = arch->vex.guest_r##n @@ -492,7 +524,40 @@ static void fill_fpu(const ThreadState *tst, vki_elf_fpregset_t *fpu) // umm ... #elif defined(VGP_arm64_linux) - I_die_here; + fpu->vregs[0] = arch->vex.guest_Q0; + fpu->vregs[1] = arch->vex.guest_Q1; + fpu->vregs[2] = arch->vex.guest_Q2; + fpu->vregs[3] = arch->vex.guest_Q3; + fpu->vregs[4] = arch->vex.guest_Q4; + fpu->vregs[5] = arch->vex.guest_Q5; + fpu->vregs[6] = arch->vex.guest_Q6; + fpu->vregs[7] = arch->vex.guest_Q7; + fpu->vregs[8] = arch->vex.guest_Q8; + fpu->vregs[9] = arch->vex.guest_Q9; + fpu->vregs[10] = arch->vex.guest_Q10; + fpu->vregs[11] = arch->vex.guest_Q11; + fpu->vregs[12] = arch->vex.guest_Q12; + fpu->vregs[13] = arch->vex.guest_Q13; + fpu->vregs[14] = arch->vex.guest_Q14; + fpu->vregs[15] = arch->vex.guest_Q15; + fpu->vregs[16] = arch->vex.guest_Q16; + fpu->vregs[17] = arch->vex.guest_Q17; + fpu->vregs[18] = arch->vex.guest_Q18; + fpu->vregs[19] = arch->vex.guest_Q19; + fpu->vregs[20] = arch->vex.guest_Q20; + fpu->vregs[21] = arch->vex.guest_Q21; + fpu->vregs[22] = arch->vex.guest_Q22; + fpu->vregs[23] = arch->vex.guest_Q23; + fpu->vregs[24] = arch->vex.guest_Q24; + fpu->vregs[25] = arch->vex.guest_Q25; + fpu->vregs[26] = arch->vex.guest_Q26; + fpu->vregs[27] = arch->vex.guest_Q27; + fpu->vregs[28] = arch->vex.guest_Q28; + fpu->vregs[29] = arch->vex.guest_Q29; + fpu->vregs[30] = arch->vex.guest_Q30; + fpu->vregs[31] = arch->vex.guest_Q31; + fpu->fpsr = arch->vex.guest_QCFLAG; + fpu->fpcr = arch->vex.guest_FPCR; #elif defined(VGP_s390x_linux) /* NOTE: The 16 FP registers map to the first 16 VSX registers. */ -- 1.8.3.1 Created attachment 119023 [details]
new version of the patch
(In reply to Mark Wielaard from comment #2) > Hi Sasha, > > I saw you have a new commit that also fills in the floating point structure. > https://github.com/sasshka/valgrind/commit/ > 8903bfd77791723a46505530b993118c7abf1755 > > Could you please attach (git format-patch HEAD^) the patch to this bug as > you would like to commit it? That makes it easier to review. > > It looks like the only question is whether pstate is setup correctly. > I am not sure myself. In other code (synth_ucontext in > coregrind/m_sigframe/sigframe-arm64-linux.c) we don't even try... Maybe the > vgdb code does, but I cannot find where. > > Now that you have a bug number could you also add it to your commit message > and in the NEWS file? > > Could you run make regtests before and after your patch to check for tests > that now pass (or fail)? > > Thanks, > > Mark when running 1) ulimit -c unlimited 2) make nonexp-regtest check on top of master: == 594 tests, 27 stderr failures, 2 stdout failures, 0 stderrB failures, 0 stdoutB failures, 1 post failure == memcheck/tests/arm64-linux/scalar (stderr) memcheck/tests/badjump (stderr) memcheck/tests/deep-backtrace (stderr) memcheck/tests/dw4 (stderr) memcheck/tests/gone_abrt_xml (stderr) memcheck/tests/leak-segv-jmp (stderr) memcheck/tests/supp_unknown (stderr) memcheck/tests/varinfo1 (stderr) memcheck/tests/varinfo2 (stderr) memcheck/tests/varinfo3 (stderr) memcheck/tests/varinfo4 (stderr) memcheck/tests/varinfo5 (stderr) memcheck/tests/varinfo6 (stderr) memcheck/tests/varinforestrict (stderr) helgrind/tests/hg05_race2 (stderr) helgrind/tests/tc20_verifywrap (stderr) helgrind/tests/tc22_exit_w_lock (stderr) drd/tests/pth_cancel_locked (stderr) drd/tests/tc04_free_lock (stderr) drd/tests/tc09_bad_unlock (stderr) drd/tests/tc22_exit_w_lock (stderr) massif/tests/thresholds_0_0 (stderr) massif/tests/thresholds_0_0 (post) none/tests/allexec32 (stdout) none/tests/allexec32 (stderr) none/tests/async-sigs (stderr) none/tests/linux/blockfault (stderr) none/tests/linux/stack-overflow (stderr) none/tests/pth_cancel1 (stdout) none/tests/pth_cancel1 (stderr) on the top of the branch with my core dump patch: https://github.com/sasshka/valgrind/commit/1a5b6418a92db52c16a994dccafa110b84d68c43 == 594 tests, 17 stderr failures, 1 stdout failure, 0 stderrB failures, 0 stdoutB failures, 0 post failures == memcheck/tests/arm64-linux/scalar (stderr) memcheck/tests/dw4 (stderr) memcheck/tests/leak-segv-jmp (stderr) memcheck/tests/supp_unknown (stderr) memcheck/tests/varinfo1 (stderr) memcheck/tests/varinfo2 (stderr) memcheck/tests/varinfo3 (stderr) memcheck/tests/varinfo4 (stderr) memcheck/tests/varinfo5 (stderr) memcheck/tests/varinfo6 (stderr) memcheck/tests/varinforestrict (stderr) helgrind/tests/hg05_race2 (stderr) helgrind/tests/tc20_verifywrap (stderr) drd/tests/pth_cancel_locked (stderr) drd/tests/tc04_free_lock (stderr) drd/tests/tc09_bad_unlock (stderr) none/tests/pth_cancel1 (stdout) none/tests/pth_cancel1 (stderr) I updated my patch:
1)added my commit to the NEWS file
2)used cast *(const __uint128_t*)arch->vex.guest_Q0 to silence
m_coredump/coredump-elf.c:527:20: warning: assignment makes integer
from pointer without a cast [enabled by default]
Created attachment 119184 [details]
patch
Thanks for the updates and showing which test cases were fixed by this. The casting to __uint128_t looks correct casts look right because the types align correctly. (Strangely the type itself is nowhere documented, even though gcc accepts it and everybody uses it). Pushed as: commit 965876e22b863bca1cbe7db9578e648397a705a7 Author: Alexandra Hajkova <ahajkova@redhat.com> Date: Wed Mar 20 10:10:44 2019 +0100 Support arm64 core dump Fixes BZ #405722. Implements coredump-elf.c fill_prstatus() and fill_fpu () for VGP_arm64_linux. *** Bug 371439 has been marked as a duplicate of this bug. *** |