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

[Stefan Liebler <stli at linux dot vnet dot ibm dot com>]

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.




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"	\