diff mbox

[2/4] S390: Use own tbegin macro instead of __builtin_tbegin.

Message ID a0e05116-1077-8688-96b2-e177808fde3f@linux.vnet.ibm.com
State New
Headers show

Commit Message

Stefan Liebler Jan. 17, 2017, 3:28 p.m. UTC 
On 01/10/2017 05:34 PM, Torvald Riegel wrote:
  > I'm not sure that this is always a good thing to do.  It is nicer for
  > programmers if they can get an instruction pointer that points to where
  > the faulty access happened.  However:
  >
  > (1) This can mask failures present in an application, because the
  > fallback path is not guaranteed to operate on the same data.  It may
  > never run into this problem.  Can inconsistencies arise due to that?
  > For example, is a masked segfault still visible through performance
  > counters or something like that?
No. If an interruption is filtered, those informations are not stored.
  >
  > (2) This introduces a facility to probe memory for being accessible or
  > not, considering that you say it masks segfaults.  It seems that this
  > probing may not be visible to the same extent as possible if a signal
  > handler were installed.  Is this relevant from a security perspective?
  > It may not be given that perhaps the user can do that anyway through
  > direct usage of transactions, or perhaps because the user would have to
  > be able to check whether the program is currently executing in a
  > transaction or not.
  > It might be good to at least mention this in a comment in the code.
I've documented it in the comment.

> On Tue, 2016-12-06 at 14:51 +0100, Stefan Liebler wrote:
>> +/* These builtins are correct.  Use them.  */
>
> Is "correct" the right word here?  I suppose they are always correct,
> it's just that you want something different for glibc internally.
>
I'll correct it.

I've attached the diff here and will later make one patch with changelog 
for this and the other two patches.

Comments

Torvald Riegel Jan. 17, 2017, 5:44 p.m. UTC | #1 
On Tue, 2017-01-17 at 16:28 +0100, Stefan Liebler wrote:
> I've attached the diff here and will later make one patch with changelog 
> for this and the other two patches.

Both diffs for patch 2/4 are OK.
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/s390/htm.h b/sysdeps/unix/sysv/linux/s390/htm.h
index 32d5a88..3b0ab7f 100644
--- a/sysdeps/unix/sysv/linux/s390/htm.h
+++ b/sysdeps/unix/sysv/linux/s390/htm.h
@@ -123,7 +123,12 @@ 
 			      the normal lock path will execute it	\
 			      again and result in a core dump wich does	\
 			      now show at tbegin but the real executed	\
-			      instruction.  */				\
+			      instruction.				\
+			      However it is not guaranteed that this	\
+			      retry operate on the same data and thus	\
+			      may not end in an program-interruption.	\
+			      Note: This could also be used to probe	\
+			      memory for being accessible!  */		\
 			   "2: tbegin 0, 0xFF0E\n\t"			\
 			   /* Branch away in abort case (this is the	\
 			      prefered sequence.  See PoP in chapter 5	\
@@ -152,7 +157,8 @@ 
      __ret;								\
      })
 
-/* These builtins are correct.  Use them.  */
+/* These builtins are usable in context of glibc lock elision code without any
+   changes.  Use them.  */
 #define __libc_tend()							\
   ({ __asm__ __volatile__ (".machine push\n\t"				\
 			   ".machinemode \"zarch_nohighgprs\"\n\t"	\