Bug 440095

Summary: No support for "dc cvac" and "dc cvap" on Arm64
Product: [Developer tools] valgrind Reporter: kevinz <kevin.zhao>
Component: memcheckAssignee: Julian Seward <jseward>
Status: RESOLVED FIXED    
Severity: normal CC: assad.hashmi, mark, pjfloyd
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Other   
Latest Commit: Version Fixed In:
Attachments: Patch to enable support for dc cvap and dc cvac

Description kevinz 2021-07-21 03:23:48 UTC
We are working on enabling PMDK for Arm64, which is the persist memory development kit.

Running the test with valgrind, I experienced a valgrind error:
==256557== Memcheck, a memory error detector
==256557== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==256557== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==256557== Command: ./obj_basic_integration /mnt/mem0//test_obj_basic_integration5__________________PMDK_________/testfile1
==256557== Parent PID: 256503
==256557==
ARM64 front end: branch_etc
disInstr(arm64): unhandled instruction 0xD50B7A20
disInstr(arm64): 1101'0101 0000'1011 0111'1010 0010'0000
==256557== valgrind: Unrecognised instruction at address 0x493a148.
==256557==    at 0x493A148: arm_clean_va_to_poc (arm_cacheops.h:55)
==256557==    by 0x493A3BB: flush_poc_nolog (flush.h:27)
==256557==    by 0x493A3BB: flush_poc (init.c:33)
==256557==    by 0x4934C2F: pmem_deep_flush (pmem.c:214)
==256557==    by 0x48A8037: os_part_deep_common (os_deep_linux.c:136)
==256557==    by 0x48B672B: shutdown_state_checksum (shutdown_state.c:32)
==256557==    by 0x48B678B: shutdown_state_init (shutdown_state.c:47)
==256557==    by 0x48AFE23: util_header_create (set.c:2313)
==256557==    by 0x48B15A3: util_replica_init_headers_local (set.c:2777)
==256557==    by 0x48B17BB: util_replica_create_local (set.c:2823)
==256557==    by 0x48B2FEF: util_pool_create_uuids (set.c:3313)
==256557==    by 0x48B31C3: util_pool_create (set.c:3356)
==256557==    by 0x48D49DB: pmemobj_createU (obj.c:1351)
==256557== Your program just tried to execute an instruction that Valgrind
==256557== did not recognise.  There are two possible reasons for this.
==256557== 1. Your program has a bug and erroneously jumped to a non-code
==256557==    location.  If you are running Memcheck and you just saw a
==256557==    warning about a bad jump, it's probably your program's fault.
==256557== 2. The instruction is legitimate but Valgrind doesn't handle it,
==256557==    i.e. it's Valgrind's fault.  If you think this is the case or
==256557==    you are not sure, please let us know and we'll try to fix it.
==256557== Either way, Valgrind will now raise a SIGILL signal which will
==256557== probably kill your program.
==256557==
==256557== HEAP SUMMARY:
==256557==     in use at exit: 3,039 bytes in 39 blocks
==256557==   total heap usage: 1,220 allocs, 1,181 frees, 1,653,499 bytes allocated
==256557==
==256557== LEAK SUMMARY:
==256557==    definitely lost: 0 bytes in 0 blocks
==256557==    indirectly lost: 0 bytes in 0 blocks
==256557==      possibly lost: 0 bytes in 0 blocks
==256557==    still reachable: 3,039 bytes in 39 blocks
==256557==         suppressed: 0 bytes in 0 blocks
==256557== Reachable blocks (those to which a pointer was found) are not shown.
==256557== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==256557==
==256557== For lists of detected and suppressed errors, rerun with: -s
==256557== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

The one above will call: https://github.com/pmem/pmdk/blob/master/src/libpmem2/aarch64/arm_cacheops.h#L53, but Valgrind can not handle it.

Also, the other one as below with "dc cvap" also got the same consequence. The dc cvap was called at: https://github.com/pmem/pmdk/blob/master/src/libpmem2/aarch64/arm_cacheops.h#L59

[root@fedora obj_basic_integration]# cat memcheck5.log
==261736== Memcheck, a memory error detector
==261736== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==261736== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==261736== Command: ./obj_basic_integration /mnt/mem0//test_obj_basic_integration5__________________PMDK_________/testfile1
==261736== Parent PID: 261682
==261736==
ARM64 front end: branch_etc
disInstr(arm64): unhandled instruction 0xD50B7C20
disInstr(arm64): 1101'0101 0000'1011 0111'1100 0010'0000
==261736== valgrind: Unrecognised instruction at address 0x493a164.
==261736==    at 0x493A164: arm_clean_va_to_pop (arm_cacheops.h:61)
==261736==    by 0x493A453: flush_pop_nolog (flush.h:45)
==261736==    by 0x493A453: flush_pop (init.c:49)
==261736==    by 0x4934C2F: pmem_deep_flush (pmem.c:214)
==261736==    by 0x48A8037: os_part_deep_common (os_deep_linux.c:136)
==261736==    by 0x48B672B: shutdown_state_checksum (shutdown_state.c:32)
==261736==    by 0x48B678B: shutdown_state_init (shutdown_state.c:47)
==261736==    by 0x48AFE23: util_header_create (set.c:2313)
==261736==    by 0x48B15A3: util_replica_init_headers_local (set.c:2777)
==261736==    by 0x48B17BB: util_replica_create_local (set.c:2823)
==261736==    by 0x48B2FEF: util_pool_create_uuids (set.c:3313)
==261736==    by 0x48B31C3: util_pool_create (set.c:3356)
==261736==    by 0x48D49DB: pmemobj_createU (obj.c:1351)
==261736== Your program just tried to execute an instruction that Valgrind
==261736== did not recognise.  There are two possible reasons for this.
==261736== 1. Your program has a bug and erroneously jumped to a non-code
==261736==    location.  If you are running Memcheck and you just saw a
==261736==    warning about a bad jump, it's probably your program's fault.
==261736== 2. The instruction is legitimate but Valgrind doesn't handle it,
==261736==    i.e. it's Valgrind's fault.  If you think this is the case or
==261736==    you are not sure, please let us know and we'll try to fix it.
==261736== Either way, Valgrind will now raise a SIGILL signal which will
==261736== probably kill your program.
==261736==
==261736== HEAP SUMMARY:
==261736==     in use at exit: 3,039 bytes in 39 blocks
==261736==   total heap usage: 1,220 allocs, 1,181 frees, 1,653,499 bytes allocated
==261736==
==261736== LEAK SUMMARY:
==261736==    definitely lost: 0 bytes in 0 blocks
==261736==    indirectly lost: 0 bytes in 0 blocks
==261736==      possibly lost: 0 bytes in 0 blocks
==261736==    still reachable: 3,039 bytes in 39 blocks
==261736==         suppressed: 0 bytes in 0 blocks
==261736== Reachable blocks (those to which a pointer was found) are not shown.
==261736== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==261736==
==261736== For lists of detected and suppressed errors, rerun with: -s
==261736== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

OBSERVED RESULT


EXPECTED RESULT


Linux/KDE Plasma: Fedora 34, linux 5.12
Comment 1 Julian Seward 2021-07-21 12:53:36 UTC
Try copying the scheme used for 'dc cvau' in guest_arm64_toIR.c.
That might just work for the cases you care about too.
Comment 2 kevinz 2021-07-21 14:12:20 UTC
Hi Julian,

Thanks, I will work on this
Comment 3 kevinz 2021-07-23 03:44:06 UTC
Created attachment 140268 [details]
Patch to enable support for dc cvap and dc cvac
Comment 4 kevinz 2021-07-23 03:44:22 UTC
Tested locally on Arm v8.2 machines, works well
Comment 5 kevinz 2021-07-26 09:12:30 UTC
@Julian,

Could you point me how to submit patches to Valgrind?
Thanks!
Comment 6 Mark Wielaard 2021-07-26 19:15:32 UTC
(In reply to kevinz from comment #5)
> Could you point me how to submit patches to Valgrind?
> Thanks!

Best is to attach a patch to this bug.
Comment 7 kevinz 2021-07-27 02:26:27 UTC
Yes, already attached. Please help to check and review. Thanks a lot!
Comment 8 kevinz 2021-08-09 04:07:52 UTC
ping @Julian,

Could you help to take a look at the attached patch?
Thanks!
Comment 9 Paul Floyd 2024-05-11 12:40:04 UTC
commit 40505e5056a7e434e291526d6662943e31274fc1 (HEAD -> master, origin/master, origin/HEAD)
Author: Kevin Zhao <kevin.zhao@linaro.org>
Date:   Thu Jul 22 16:00:21 2021 +0800

    arm64 front end: add support for 'dc cvac', 'dc cvap', handling it the same as 'dc cvau'.
    
    patch modified to process DBP component of ID_AA64ISAR1_EL1
    
    Signed-off-by: Kevin Zhao <kevin.zhao@linaro.org>