ABI incompatibility between POWER and Z HTM builtins and intrinsics

Bug #1320292 reported by Peter Bergner on 2014-05-16
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
gcc-4.8 (Ubuntu)
Undecided
Unassigned
Trusty
Undecided
Unassigned
Utopic
Undecided
Unassigned
gcc-4.9 (Ubuntu)
Undecided
Unassigned
Trusty
Undecided
Unassigned
Utopic
Undecided
Unassigned
gccgo-4.9 (Ubuntu)
Undecided
Unassigned
Trusty
Undecided
Unassigned
Utopic
Undecided
Unassigned

Bug Description

The IBM XL team defined a set of HTM intrinsic functions that were supposed
to be API compatible across the XL and GCC compilers on both Power and S390.
PR61193 describes an issue where the functions that begin a transaction
are incompatible. The Power intrinsics return non-zero on success, while
the S390 intrinsics return zero on success.

After discussing this with the XL compiler team, the S390 GCC team and
the Power GCC team, we have decided to leave the incompatibility between
Power and S390. However, the XL and GCC compilers will be compatible
with each other when targeting the same processor target. To mitigate
the incompatibility somewhat, we have decided to add a macro to the
powerpc*-linux's htmintrin.h file that defines what the "successful"
return status value of the __TM_simple_begin() and __TM_begin() intrinsic
function is. This macro is already defined in the S390's htmintrin.h
header file and is used by the S390 to determine whether the transaction
was successfully started or not. By adding the same macro on Power, we
allow users to write common code between Power and S390, even though our
successful return status values are different.

For example, the following code can be used on Power and S390, even
though the actual value returned by __TM_simple_begin() is different.

  if ((tx_state = __TM_simple_begin ()) == _HTM_TBEGIN_STARTED)
    {
      /* Transaction State Initiated. */
      ...
    }
  else
    {
      /* Transaction State Failed. */
      ...
    }

David approved this offline, so I'm committing this to GCC mainline as revision 210486, as well as the 4.9 (210487) and 4.8 (210488) branches.

Peter

 PR target/61193
 * config/rs6000/htmxlintrin.h (_HTM_TBEGIN_STARTED): New define.
 (__TM_simple_begin): Use it.
 (__TM_begin): Likewise.

Index: gcc/config/rs6000/htmxlintrin.h
===================================================================
--- gcc/config/rs6000/htmxlintrin.h (revision 210485)
+++ gcc/config/rs6000/htmxlintrin.h (working copy)
@@ -46,12 +46,17 @@ extern "C" {

 typedef char TM_buff_type[16];

+/* Compatibility macro with s390. This macro can be used to determine
+ whether a transaction was successfully started from the __TM_begin()
+ and __TM_simple_begin() intrinsic functions below. */
+#define _HTM_TBEGIN_STARTED 1
+
 extern __inline long
 __attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
 __TM_simple_begin (void)
 {
   if (__builtin_expect (__builtin_tbegin (0), 1))
- return 1;
+ return _HTM_TBEGIN_STARTED;
   return 0;
 }

@@ -61,7 +66,7 @@ __TM_begin (void* const TM_buff)
 {
   *_TEXASRL_PTR (TM_buff) = 0;
   if (__builtin_expect (__builtin_tbegin (0), 1))
- return 1;
+ return _HTM_TBEGIN_STARTED;
 #ifdef __powerpc64__
   *_TEXASR_PTR (TM_buff) = __builtin_get_texasr ();
 #else

CVE References

The attachment "Patch to mitigate API incompatibility between Power and S390" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

tags: added: patch
Matthias Klose (doko) wrote :

fixed in gcc-4.8 4.8.2-23ubuntu1 in utopic

affects: gcc-defaults (Ubuntu) → gcc-4.8 (Ubuntu)
Changed in gccgo-4.9 (Ubuntu Utopic):
status: New → Invalid
Changed in gcc-4.9 (Ubuntu Trusty):
status: New → Invalid
Changed in gcc-4.8 (Ubuntu Utopic):
status: New → Fix Released
Matthias Klose (doko) wrote :

fixed in gcc-4.9 in utopic

Changed in gcc-4.9 (Ubuntu Utopic):
status: New → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gccgo-4.9 - 4.9.1-0ubuntu1

---------------
gccgo-4.9 (4.9.1-0ubuntu1) trusty-proposed; urgency=medium

  * Upload the final GCC 4.9.1 release.
  * Merge changes from gcc-4.9 4.9.0-2ubuntu1, including:
    - Fix PR go/60931, garbage collector issue with non 4kB system page size.
      LP: #1304754.
    - Fix wrong-code issue in the little endian vector API (ppc64el).
      LP: #1311128.
    - Fix ABI incompatibility between POWER and Z HTM builtins and intrinsics.
      LP: #1320292.
    - Fix an ICE with invalid code. PR c++/61046. LP: #1313102.
    - gccgo: Don't overwrite memory if an archive has a bad file name.
  * Include the cc1 binary into the gccgo-4.9 package.
  * Do not build-depend on sdt-systemtap for the trusty upload.
  * Warn about ppc ELFv2 ABI issues, which will change in GCC 4.10.
 -- Matthias Klose <email address hidden> Thu, 17 Jul 2014 15:51:15 +0200

Changed in gccgo-4.9 (Ubuntu Trusty):
status: New → Fix Released

The verification of the Stable Release Update for gccgo-4.9 has completed successfully and the package has now been 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.

Hello Peter, or anyone else affected,

Accepted gcc-4.8 into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/gcc-4.8/4.8.4-2ubuntu1~14.04 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 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, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

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

Changed in gcc-4.8 (Ubuntu Trusty):
status: New → Fix Committed
tags: added: verification-needed
Peter Bergner (pbergner) wrote :

I can verify that this is fixed.

Peter Bergner (pbergner) wrote :

Sorry, verified with "gcc version 4.9.1 (Ubuntu 4.9.1-16ubuntu6)".

tags: added: verification-done
removed: verification-needed
Peter Bergner (pbergner) wrote :

Verified in gcc version 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04) too.

Launchpad Janitor (janitor) wrote :
Download full text (19.4 KiB)

This bug was fixed in the package gcc-4.8 - 4.8.4-2ubuntu1~14.04

---------------
gcc-4.8 (4.8.4-2ubuntu1~14.04) trusty-proposed; urgency=medium

  * SRU LP: #1311866.
  * Fix PR tree-optimization/63341 (wrong code, rs6000).
  * Allow to turn off -Wformat using Wno-format. LP: #1401836.
  * Fix PR target/60693 (x86, ice on valid code). LP: #1378737.
  * Fix PR tree-optimization/61964 (wrong code). LP: #1347147.
  * Fix GCC miscompilation with boost::asio::io_service::work. LP: #1338693.
  * Fix PR target/61208 (POWER, wrong code). LP: #1322287.
  * Fix ABI incompatibility between POWER and Z HTM builtins and intrinsics.
    LP: #1320292.
  * Fix PR c++/61046 (ice on invalid code). LP: #1313102.
  * Fix wrong-code issue in the little endian vector API (ppc64el).
    LP: #1311128.
  * Fix PR tree-optimization/59358 (wrong code). LP: #1395019.
  * Fix ice on ARM32. LP: #1268893.
  * Don't apply the backport for PR61841 for trusty, causing link failures.
  * Fix wrong code for vector doubleword extract (POWER). LP: #1437467.

gcc-4.8 (4.8.4-2ubuntu1) vivid; urgency=medium

  * Merge with Debian; remaining changes:
    - Build from the upstream source.

gcc-4.8 (4.8.4-2) unstable; urgency=medium

  * Update to SVN 20150426 (r222448) from the gcc-4_8-branch.
    - Fix PR libstdc++/60966, PR c/61553, PR middle-end/63704,
      PR target/61413 (ARM), PR target/64358 (RS6000), PR target/64479 (SH),
      PR target/64409 (x86), PR rtl-optimization/64037, PR c++/64487,
      PR c++/64251, PR c++/64297, PR fortran/63733, PR fortran/64244,
      PR c/64766, PR target/64882, PR rtl-optimization/61058,
      PR middle-end/43631, PR tree-optimization/64563, PR target/64513,
      PR middle-end/57748, PR middle-end/57748, PR target/64795,
      PR fortran/64528, PR fortran/56867, PR fortran/57023, PR c/57653,
      PR tree-optimization/63844 (OpenMP), PR middle-end/64199 (ice on valid),
      PR tree-optimization/64493 (ice on valid), PR tree-optimization/64495
      (wrong code), PR tree-optimization/56273 (diagnostics),
      PR tree-optimization/59124 (diagnostic), PR tree-optimization/64277
      (diagnostic), PR lto/65015, PR target/65163 (SH), PR target/64113 (ALPHA,
      link failure), PR rtl-optimization/64557, PR rtl-optimization/63475
      (ALPHA, wrong code), PR rtl-optimization/63483 (ALPHA, wrong code),
      PR target/64452 (AVR), PR target/64387 (x86, ice on valid),
      PR target/64979 (wrong code), PR target/64580 (rs6000),
      PR fortran/63744 (rejects valid), PR lto/65193 (ice on valid),
      PR tree-optimization/61634 (ice on valid), PR target/65196 (AVR),
      PR tree-optimization/63593 (ice on valid),
      PR tree-optimization/65063 (wrong code), PR target/65286 (rs6000),
      PR 65138/target (rs6000), PR target/53988 (SH), PR target/59593 (ARM),
      PR target/64453 (ARM), PR middle-end/65409 (ice on valid),
      PR tree-optimization/65388, PR fortran/65024 (ice),
      PR fortran/60898 (ice on valid), PR fortran/61138, PR libgfortran/60956,
      PR libstdc++/65279, PR libstdc++/65543, PR target/65849, PR target/65456,
      PR target/65787, PR c++/65727, PR c++/65721, PR fortran/56674,
      PR fortran/58813, PR fortran/590...

Changed in gcc-4.8 (Ubuntu Trusty):
status: Fix Committed → Fix Released
Steve Langasek (vorlon) wrote :

Hello Peter, or anyone else affected,

Accepted gccgo-4.9 into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/gccgo-4.9/4.9.3-0ubuntu4 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 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, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

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

tags: removed: verification-done
tags: added: verification-needed
Steve Langasek (vorlon) wrote :

This change was already included in the previous sru of gccgo-4.9, 4.9.1-0ubuntu1; no re-verification is required.

tags: added: verification-done
removed: verification-needed
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers