[20.04] Valgrind: PPC sync instruction L field should only be 2 bits in ISA 3.0

[bugproxy]

SRU Description

[Impact]
Valgrind mishandles the L field of the sync instruction.
More details are available at: https://bugs.kde.org/show_bug.cgi?id=422677
Single line patch available online with commit fb6f7abcbc92506d302fb18a2c5fc853d2929248

[Test Case]
On a PPC64le Hardware
# sudo apt-get install gcc valgrind
#cat test-sync.c
define __SYNC(l) ".long (0x7c0004AC | ((" #l ") << 21))"

int
main ()
{
        asm volatile(__SYNC(4) : : : "memory");
}

# gcc test-sync.c && valgrind --tool=none ./a.out
This will report this error unpatched part of the valgrind output

dis_memsync(ppc)(sync/lwsync,flag_L)
disInstr(ppc): unhandled instruction: 0x7C8004AC
                 primary 31(0x1F), secondary 1196(0x4AC)

# This won't report this error once using the updated version

[Regression Potential]
The regression potential is very low since this bug has been submitted upstream and is already available in Groovy. Package build and run successfully. Not anticipating regression

End SRU Description

---Problem Description---
This is a bug report for focal.

Valgrind, including version 3.16, mishandles the L field of the sync instruction.
More details are available at: https://bugs.kde.org/show_bug.cgi?id=422677

This is a request to backport the following Valgrind patch from upstream:

commit fb6f7abcbc92506d302fb18a2c5fc853d2929248
Author: Carl Love <email address hidden>
Date: Tue Jun 9 10:42:03 2020 -0500

    Power PC Fix extraction of the L field for sync instruction

    The L field is currently a two bit[22:21] field in ISA 3.0. The size of the
    L field has changed over time.

    Currently the ISA 3.0 Valgrind sync instruction support code sets the
    flag_L for the instruction L field to a five bit value that includes bits
    that are marked reserved the sync instruction. This patch fixes the issue for ISA 3.0
    to only setting flag_L the specified two bits.

    Valgrind bugzilla: https://bugs.kde.org/show_bug.cgi?id=422677

Contact Information = Tulio <email address hidden>

---uname output---
N/A

Machine Type = Reproducible on all POWER8 and POWER9 servers

---Debugger---
A debugger is not configured

---Steps to Reproduce---
 $ cat test-sync.c
#define __SYNC(l) ".long (0x7c0004AC | ((" #l ") << 21))"

int
main ()
{
        asm volatile(__SYNC(4) : : : "memory");
}
$ gcc test-sync.c && valgrind --tool=none ./a.out
==150073== Nulgrind, the minimal Valgrind tool
==150073== Copyright (C) 2002-2017, and GNU GPL'd, by Nicholas Nethercote.
==150073== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==150073== Command: ./a.out
==150073==
dis_memsync(ppc)(sync/lwsync,flag_L)
disInstr(ppc): unhandled instruction: 0x7C8004AC
                 primary 31(0x1F), secondary 1196(0x4AC)
==150073== valgrind: Unrecognised instruction at address 0x180788.
==150073== at 0x180788: main (in /home/tuliom/tmp/a.out)
==150073== Your program just tried to execute an instruction that Valgrind
==150073== did not recognise.
...

Userspace tool common name: Valgrind

The userspace tool has the following bit modes: 64-bit

Userspace deb: valgrind

Userspace tool obtained from project website: na

[Frank Heimes (fheimes)]

Just as a side note: this is a follow on ticket of LP 1884143

[Matthieu Clemenceau (mclemenceau)]

debdiff ready for sponsorship and SRU for Focal

[Matthieu Clemenceau (mclemenceau)]

debdiff ready for sponsorship and SRU for Bionic

[Matthieu Clemenceau (mclemenceau)]

Both patches are available and ready for SRU on Focal and Bionic.
After being tested individually from a ppa ppa:mclemenceau/distro-work on ppc64le instances

[Michael Hudson-Doyle (mwhudson)]

I've sponsored both patches.

[Brian Murray (brian-murray)]

Hello bugproxy, or anyone else affected,

Accepted valgrind into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/valgrind/1:3.15.0-1ubuntu9.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Brian Murray (brian-murray)]

Hello bugproxy, or anyone else affected,

Accepted valgrind into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/valgrind/1:3.13.0-2ubuntu2.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2020-09-04 12:03 EDT-------
I've just confirmed on focal that valgrind 1:3.15.0-1ubuntu9.1 fixes this issue.

I've also confirmed on bionic that valgrind 1:3.13.0-2ubuntu also fixes this issue.

I used the following program to validate that valgrind doesn't crash anymore:

cat test-sync.c
#define __SYNC(l) ".long (0x7c0004AC | ((" #l ") << 21))"

int
main ()
{
asm volatile(__SYNC(4) : : : "memory");
}

Thank you!

[Matthieu Clemenceau (mclemenceau)]

Verification done for both focal and bionic.
valgrind from focal-proposed and bionic-proposed fixes LP:1887974

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2020-09-04 12:09 EDT-------
(In reply to comment #15)
> I've also confirmed on bionic that valgrind 1:3.13.0-2ubuntu also fixes this
> issue.

Part of my paste got lost here. I meant to say 1:3.13.0-2ubuntu2.3

[Launchpad Janitor (janitor)]

This bug was fixed in the package valgrind - 1:3.13.0-2ubuntu2.3

---------------
valgrind (1:3.13.0-2ubuntu2.3) bionic; urgency=medium

  * The L field is currently a two bit[22:21] field in ISA 3.0. (LP: #1887974)

 -- Matthieu Clemenceau <email address hidden> Thu, 20 Aug 2020 12:07:13 -0500

[Brian Murray (brian-murray)]

The verification of the Stable Release Update for valgrind has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package valgrind - 1:3.15.0-1ubuntu9.1

---------------
valgrind (1:3.15.0-1ubuntu9.1) focal; urgency=medium

  * The L field is currently a two bit[22:21] field in ISA 3.0. (LP: #1887974)

 -- Matthieu Clemenceau <email address hidden> Thu, 20 Aug 2020 11:50:58 -0500