On 25/07/18 14:47, Richard Biener wrote:
> On Wed, Jul 25, 2018 at 2:41 PM Richard Earnshaw (lists)
> <richard.earns...@arm.com> wrote:
>>
>> On 25/07/18 11:36, Richard Biener wrote:
>>> On Wed, Jul 25, 2018 at 11:49 AM Richard Earnshaw (lists)
>>> <richard.earns...@arm.com> wrote:
>>>>
>>>> On 24/07/18 18:26, Richard Biener wrote:
>>>>> On Mon, Jul 9, 2018 at 6:40 PM Richard Earnshaw
>>>>> <richard.earns...@arm.com> wrote:
>>>>>>
>>>>>>
>>>>>> This patch defines a new intrinsic function
>>>>>> __builtin_speculation_safe_value.  A generic default implementation is
>>>>>> defined which will attempt to use the backend pattern
>>>>>> "speculation_safe_barrier".  If this pattern is not defined, or if it
>>>>>> is not available, then the compiler will emit a warning, but
>>>>>> compilation will continue.
>>>>>>
>>>>>> Note that the test spec-barrier-1.c will currently fail on all
>>>>>> targets.  This is deliberate, the failure will go away when
>>>>>> appropriate action is taken for each target backend.
>>>>>
>>>>> So given this series is supposed to be backported I question
>>>>>
>>>>> +rtx
>>>>> +default_speculation_safe_value (machine_mode mode ATTRIBUTE_UNUSED,
>>>>> +                               rtx result, rtx val,
>>>>> +                               rtx failval ATTRIBUTE_UNUSED)
>>>>> +{
>>>>> +  emit_move_insn (result, val);
>>>>> +#ifdef HAVE_speculation_barrier
>>>>> +  /* Assume the target knows what it is doing: if it defines a
>>>>> +     speculation barrier, but it is not enabled, then assume that one
>>>>> +     isn't needed.  */
>>>>> +  if (HAVE_speculation_barrier)
>>>>> +    emit_insn (gen_speculation_barrier ());
>>>>> +
>>>>> +#else
>>>>> +  warning_at (input_location, 0,
>>>>> +             "this target does not define a speculation barrier; "
>>>>> +             "your program will still execute correctly, but speculation 
>>>>> "
>>>>> +             "will not be inhibited");
>>>>> +#endif
>>>>> +  return result;
>>>>>
>>>>> which makes all but aarch64 archs warn on __bultin_speculation_safe_value
>>>>> uses, even those that do not suffer from Spectre like all those embedded 
>>>>> targets
>>>>> where implementations usually do not speculate at all.
>>>>>
>>>>> In fact for those targets the builtin stays in the way of optimization on 
>>>>> GIMPLE
>>>>> as well so we should fold it away early if neither the target hook is
>>>>> implemented
>>>>> nor there is a speculation_barrier insn.
>>>>>
>>>>> So, please make resolve_overloaded_builtin return a no-op on such targets
>>>>> which means you can remove the above warning.  Maybe such targets
>>>>> shouldn't advertise / initialize the builtins at all?
>>>>
>>>> I disagree with your approach here.  Why would users not want to know
>>>> when the compiler is failing to implement a security feature when it
>>>> should?  As for targets that don't need something, they can easily
>>>> define the hook as described to suppress the warning.
>>>>
>>>> Or are you just suggesting moving the warning to resolve overloaded 
>>>> builtin.
>>>
>>> Well.  You could argue I say we shouldn't even support
>>> __builtin_sepeculation_safe_value
>>> for archs that do not need it or have it not implemented.  That way users 
>>> can
>>> decide:
>>>
>>> #if __HAVE_SPECULATION_SAFE_VALUE
>>>  ....
>>> #else
>>> #warning oops // or nothing
>>> #endif
>>>
>>
>> So how about removing the predefine of __HAVE_S_S_V when the builtin is
>> a nop, but then leaving the warning in if people try to use it anyway?
> 
> Little bit inconsistent but I guess I could live with that.  It still leaves
> the question open for how to declare you do not need speculation
> barriers at all then.
> 
>>>> Other ports will need to take action, but in general, it can be as
>>>> simple as, eg patch 2 or 3 do for the Arm and AArch64 backends - or
>>>> simpler still if nothing is needed for that architecture.
>>>
>>> Then that should be the default.  You might argue we'll only see
>>> __builtin_speculation_safe_value uses for things like Firefox which
>>> is unlikely built for AVR (just to make an example).  But people
>>> are going to test build just on x86 and if they build with -Werror
>>> this will break builds on all targets that didn't even get the chance
>>> to implement this feature.
>>>
>>>> There is a test which is intended to fail to targets that have not yet
>>>> been patched - I thought that was better than hard-failing the build,
>>>> especially given that we want to back-port.
>>>>
>>>> Port maintainers DO need to decide what to do about speculation, even if
>>>> it is explicitly that no mitigation is needed.
>>>
>>> Agreed.  But I didn't yet see a request for maintainers to decide that?
>>>
>>
>> consider it made, then :-)
> 
> I suspect that drew their attention ;)
> 
> So a different idea would be to produce patches implementing the hook for
> each target "empty", CC the target maintainers and hope they quickly
> ack if the target doesn't have a speculation problem.  Others then would
> get no patch (from you) and thus raise a warning?
> 
> Maybe at least do that for all primary and secondary targets given we do
> not want to regress diagnostic-wise (not get new _false_-positives) on
> the branch.
> 
>>>>>
>>>>> The builtins also have no attributes which mean they are assumed to be
>>>>> 1) calling back into the CU via exported functions, 2) possibly throwing
>>>>> exceptions, 3) affecting memory state.  I think you at least want
>>>>> to use ATTR_NOTHROW_LEAF_LIST.
>>>>>
>>>>> The builtins are not designed to be optimization or memory barriers as
>>>>> far as I can see and should thus be CONST as well.
>>>>>
>>>>
>>>> I think they should be barriers.  They do need to ensure that they can't
>>>> be moved past other operations that might depend on the speculation
>>>> state.  Consider, for example,
>>>
>>> That makes eliding them for targets that do not need mitigation even
>>> more important.
>>>
>>>>  ...
>>>>  t = untrusted_value;
>>>>  ...
>>>>  if (t + 5 < limit)
>>>>  {
>>>>    v = mem[__builtin_speculation_safe_value (untrusted_value)];
>>>>    ...
>>>>
>>>> The compiler must never lift the builtin outside the bounds check as
>>>> that is part of the speculation state.
>>>
>>> OK, so you are relying on the fact that with the current setup GCC has
>>> to assume the builtin has side-effects (GCC may not move it to a place that
>>> the original location is not post-dominated on).  It doesn't explain
>>> why you cannot set ECF_LEAF or why the builtin needs to be
>>> considered affecting the memory state.  That is, ECF_NOVOPS
>>> or ECF_LOOPING_CONST_OR_PURE (I don't think you can
>>> set that manually) would work here, both keep the builtin as
>>> having side-effects.
>>>
>>
>> I wish some of this builtin gloop were better documented; at present you
>> have to reverse engineer significant amounts of code just to decide
>> whether or not you even have to think about whether or not it's relevant...
>>
>>
>>> Btw, if you have an inline function with a pattern like above and
>>> you use it multiple times in a row GCC should be able to
>>> optimize this?  That is, optimizations like jump-threading also
>>> affect the speculation state by modifying the controling
>>> conditions.
>>
>> Ideally, if there's no control flow change, yes.  As soon as you insert
>> another branch (in or out) then you might have another speculation path
>> to consider.  Not sure how that can easily merging could be done, though.
> 
> The usual case would be
> 
>   if (cond)
>    ... _b_s_s_v (x);
> <code>
>   if (cond)
>     ... _b_s_s_v (x);
> 
> where jump-threading might decide to make that to
> 
>   if (cond)
>    {
>      ... _b_s_s_v (x);
>      <copy of code>
>      ... _b_s_s_v (x);
>    }
> 
> now we might even be able to CSE the 2nd _b_s_s_v (x)
> to the first?  That would mean using ECF_CONST|ECF_LOOPING_PURE_OR_CONST
> is the best (but we currently have no attribute for the latter).
> 
>>>
>>> You didn't answer my question about "what about C++"?
>>>
>>
>> It didn't need a response at this point.  It's a reasonable one, as are
>> some of your others...  I was focusing on the the comments that were
>> potentially contentious.
>>
>> BTW, this bit:
>>
>> +    case BUILT_IN_SPECULATION_SAFE_VALUE_N:
>> +      {
>> +       int n = speculation_safe_value_resolve_size (function, params);
>> +       tree new_function, first_param, result;
>> +       enum built_in_function fncode;
>> +
>> +       if (n == -1)
>> +         return error_mark_node;
>> +       else if (n == 0)
>> +         fncode = (enum built_in_function)((int)orig_code + 1);
>> +       else
>> +         fncode
>> +           = (enum built_in_function)((int)orig_code + exact_log2 (n) + 2);
>>
>>> resolve_size does that?  Why can that not return the built_in_function
>>> itself or BUILT_IN_NONE on error to make that clearer?
>>
>> is essentially a clone of some existing code that already does it this
>> way.  See BUILT_IN_SYNC_LOCK_RELEASE_N etc.  Admittedly, that hunk
>> handles multiple origins so would be harder to rewrite as you suggest;
>> it just seemed more appropriate to handle the cases similarly.
> 
> Yes, I realized you copied handling from that so I didn't look too closely...
> 
> These days we'd probably use an internal-function and spare us all
> the resolving completely (besides a test for validity) ;)
> 
> Richard.
> 
>> R.
>>
>>> Richard.
>>>
>>>>
>>>>
>>>>> BUILT_IN_SPECULATION_SAFE_VALUE_PTR is declared but
>>>>> nowhere generated?  Maybe
>>>>>
>>>>> +    case BUILT_IN_SPECULATION_SAFE_VALUE_N:
>>>>> +      {
>>>>> +       int n = speculation_safe_value_resolve_size (function, params);
>>>>> +       tree new_function, first_param, result;
>>>>> +       enum built_in_function fncode;
>>>>> +
>>>>> +       if (n == -1)
>>>>> +         return error_mark_node;
>>>>> +       else if (n == 0)
>>>>> +         fncode = (enum built_in_function)((int)orig_code + 1);
>>>>> +       else
>>>>> +         fncode
>>>>> +           = (enum built_in_function)((int)orig_code + exact_log2 (n) + 
>>>>> 2);
>>>>>
>>>>> resolve_size does that?  Why can that not return the built_in_function
>>>>> itself or BUILT_IN_NONE on error to make that clearer?
>>>>>
>>>>> Otherwise it looks reasonable but C FE maintainers should comment.
>>>>> I miss C++ testcases (or rather testcases should be in c-c++-common).
>>>>>
>>>>> Richard.
>>>>>
>>>>>> gcc:
>>>>>>         * builtin-types.def (BT_FN_PTR_PTR_VAR): New function type.
>>>>>>         (BT_FN_I1_I1_VAR, BT_FN_I2_I2_VAR, BT_FN_I4_I4_VAR): Likewise.
>>>>>>         (BT_FN_I8_I8_VAR, BT_FN_I16_I16_VAR): Likewise.
>>>>>>         * builtins.def (BUILT_IN_SPECULATION_SAFE_VALUE_N): New builtin.
>>>>>>         (BUILT_IN_SPECULATION_SAFE_VALUE_PTR): New internal builtin.
>>>>>>         (BUILT_IN_SPECULATION_SAFE_VALUE_1): Likewise.
>>>>>>         (BUILT_IN_SPECULATION_SAFE_VALUE_2): Likewise.
>>>>>>         (BUILT_IN_SPECULATION_SAFE_VALUE_4): Likewise.
>>>>>>         (BUILT_IN_SPECULATION_SAFE_VALUE_8): Likewise.
>>>>>>         (BUILT_IN_SPECULATION_SAFE_VALUE_16): Likewise.
>>>>>>         * builtins.c (expand_speculation_safe_value): New function.
>>>>>>         (expand_builtin): Call it.
>>>>>>         * doc/cpp.texi: Document predefine __HAVE_SPECULATION_SAFE_VALUE.
>>>>>>         * doc/extend.texi: Document __builtin_speculation_safe_value.
>>>>>>         * doc/md.texi: Document "speculation_barrier" pattern.
>>>>>>         * doc/tm.texi.in: Pull in TARGET_SPECULATION_SAFE_VALUE.
>>>>>>         * doc/tm.texi: Regenerated.
>>>>>>         * target.def (speculation_safe_value): New hook.
>>>>>>         * targhooks.c (default_speculation_safe_value): New function.
>>>>>>         * targhooks.h (default_speculation_safe_value): Add prototype.
>>>>>>
>>>>>> c-family:
>>>>>>         * c-common.c (speculation_safe_resolve_size): New function.
>>>>>>         (speculation_safe_resolve_params): New function.
>>>>>>         (speculation_safe_resolve_return): New function.
>>>>>>         (resolve_overloaded_builtin): Handle 
>>>>>> __builtin_speculation_safe_value.
>>>>>>         * c-cppbuiltin.c (c_cpp_builtins): Add pre-define for
>>>>>>         __HAVE_SPECULATION_SAFE_VALUE.
>>>>>>
>>>>>> testsuite:
>>>>>>         * gcc.dg/spec-barrier-1.c: New test.
>>>>>>         * gcc.dg/spec-barrier-2.c: New test.
>>>>>>         * gcc.dg/spec-barrier-3.c: New test.
>>>>>> ---
>>>>>>  gcc/builtin-types.def                 |   6 ++
>>>>>>  gcc/builtins.c                        |  57 ++++++++++++++
>>>>>>  gcc/builtins.def                      |  20 +++++
>>>>>>  gcc/c-family/c-common.c               | 143 
>>>>>> ++++++++++++++++++++++++++++++++++
>>>>>>  gcc/c-family/c-cppbuiltin.c           |   5 +-
>>>>>>  gcc/doc/cpp.texi                      |   4 +
>>>>>>  gcc/doc/extend.texi                   |  29 +++++++
>>>>>>  gcc/doc/md.texi                       |  15 ++++
>>>>>>  gcc/doc/tm.texi                       |  20 +++++
>>>>>>  gcc/doc/tm.texi.in                    |   2 +
>>>>>>  gcc/target.def                        |  23 ++++++
>>>>>>  gcc/targhooks.c                       |  27 +++++++
>>>>>>  gcc/targhooks.h                       |   2 +
>>>>>>  gcc/testsuite/gcc.dg/spec-barrier-1.c |  40 ++++++++++
>>>>>>  gcc/testsuite/gcc.dg/spec-barrier-2.c |  19 +++++
>>>>>>  gcc/testsuite/gcc.dg/spec-barrier-3.c |  13 ++++
>>>>>>  16 files changed, 424 insertions(+), 1 deletion(-)
>>>>>>  create mode 100644 gcc/testsuite/gcc.dg/spec-barrier-1.c
>>>>>>  create mode 100644 gcc/testsuite/gcc.dg/spec-barrier-2.c
>>>>>>  create mode 100644 gcc/testsuite/gcc.dg/spec-barrier-3.c
>>>>>>
>>>>
>>

Here's an updated version of this patch, based on these discussions.
Notable changes since last time:
- __HAVE_SPECULATION_SAFE_VALUE is now only defined if the target has
been updated for this feature.
- Warnings are only issued if the builtin is used when
__HAVE_SPECULATION_SAFE_VALUE is not defined (so the builtin will always
generate a workable program, it just might not be protected in this case).
- Some of the tests moved to c-c++-common to improve C++ testing.
- The builtin is elided early on targets that do not need, or do not
provide a specific means to restrict speculative execution.

A full bootstrap has completed, but tests are still running.

gcc:
        * builtin-types.def (BT_FN_PTR_PTR_VAR): New function type.
        (BT_FN_I1_I1_VAR, BT_FN_I2_I2_VAR, BT_FN_I4_I4_VAR): Likewise.
        (BT_FN_I8_I8_VAR, BT_FN_I16_I16_VAR): Likewise.
        * builtins.def (BUILT_IN_SPECULATION_SAFE_VALUE_N): New builtin.
        (BUILT_IN_SPECULATION_SAFE_VALUE_PTR): New internal builtin.
        (BUILT_IN_SPECULATION_SAFE_VALUE_1): Likewise.
        (BUILT_IN_SPECULATION_SAFE_VALUE_2): Likewise.
        (BUILT_IN_SPECULATION_SAFE_VALUE_4): Likewise.
        (BUILT_IN_SPECULATION_SAFE_VALUE_8): Likewise.
        (BUILT_IN_SPECULATION_SAFE_VALUE_16): Likewise.
        * builtins.c (expand_speculation_safe_value): New function.
        (expand_builtin): Call it.
        * doc/cpp.texi: Document predefine __HAVE_SPECULATION_SAFE_VALUE.
        * doc/extend.texi: Document __builtin_speculation_safe_value.
        * doc/md.texi: Document "speculation_barrier" pattern.
        * doc/tm.texi.in: Pull in TARGET_SPECULATION_SAFE_VALUE and
        TARGET_HAVE_SPECULATION_SAFE_VALUE.
        * doc/tm.texi: Regenerated.
        * target.def (have_speculation_safe_value, speculation_safe_value): New
        hooks.
        * targhooks.c (default_have_speculation_safe_value): New function.
        (default_speculation_safe_value): New function.
        * targhooks.h (default_have_speculation_safe_value): Add prototype.
        (default_speculation_safe_value): Add prototype.

c-family:
        * c-common.c (speculation_safe_resolve_call): New function.
        (speculation_safe_resolve_params): New function.
        (speculation_safe_resolve_return): New function.
        (resolve_overloaded_builtin): Handle __builtin_speculation_safe_value.
        * c-cppbuiltin.c (c_cpp_builtins): Add pre-define for
        __HAVE_SPECULATION_SAFE_VALUE.

testsuite:
        * c-c++-common/spec-barrier-1.c: New test.
        * c-c++-common/spec-barrier-2.c: New test.
        * gcc.dg/spec-barrier-3.c: New test.
diff --git a/gcc/builtin-types.def b/gcc/builtin-types.def
index b01095c..70fae35 100644
--- a/gcc/builtin-types.def
+++ b/gcc/builtin-types.def
@@ -763,6 +763,12 @@ DEF_FUNCTION_TYPE_VAR_1 (BT_FN_VOID_LONG_VAR,
 			 BT_VOID, BT_LONG)
 DEF_FUNCTION_TYPE_VAR_1 (BT_FN_VOID_ULL_VAR,
 			 BT_VOID, BT_ULONGLONG)
+DEF_FUNCTION_TYPE_VAR_1 (BT_FN_PTR_PTR_VAR, BT_PTR, BT_PTR)
+DEF_FUNCTION_TYPE_VAR_1 (BT_FN_I1_I1_VAR, BT_I1, BT_I1)
+DEF_FUNCTION_TYPE_VAR_1 (BT_FN_I2_I2_VAR, BT_I2, BT_I2)
+DEF_FUNCTION_TYPE_VAR_1 (BT_FN_I4_I4_VAR, BT_I4, BT_I4)
+DEF_FUNCTION_TYPE_VAR_1 (BT_FN_I8_I8_VAR, BT_I8, BT_I8)
+DEF_FUNCTION_TYPE_VAR_1 (BT_FN_I16_I16_VAR, BT_I16, BT_I16)
 
 DEF_FUNCTION_TYPE_VAR_2 (BT_FN_INT_FILEPTR_CONST_STRING_VAR,
 			 BT_INT, BT_FILEPTR, BT_CONST_STRING)
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 839a818..40183fb 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -6881,6 +6881,52 @@ inline_expand_builtin_string_cmp (tree exp, rtx target, bool is_memcmp)
 			    const_str_n, mode, is_memcmp);
 }
 
+/* Expand a call to __builtin_speculation_safe_value_<N>.  MODE
+   represents the size of the first argument to that call, or VOIDmode
+   if the argument is a pointer.  IGNORE will be true if the result
+   isn't used.  */
+static rtx
+expand_speculation_safe_value (machine_mode mode, tree exp, rtx target,
+			       bool ignore)
+{
+  rtx val, failsafe;
+  unsigned nargs = call_expr_nargs (exp);
+
+  tree arg0 = CALL_EXPR_ARG (exp, 0);
+
+  if (mode == VOIDmode)
+    {
+      mode = TYPE_MODE (TREE_TYPE (arg0));
+      gcc_assert (GET_MODE_CLASS (mode) == MODE_INT);
+    }
+
+  val = expand_expr (arg0, NULL_RTX, mode, EXPAND_NORMAL);
+
+  /* An optional second argument can be used as a failsafe value on
+     some machines.  If it isn't present, then the failsafe value is
+     assumed to be 0.  */
+  if (nargs > 1)
+    {
+      tree arg1 = CALL_EXPR_ARG (exp, 1);
+      failsafe = expand_expr (arg1, NULL_RTX, mode, EXPAND_NORMAL);
+    }
+  else
+    failsafe = const0_rtx;
+
+  /* If the result isn't used, the behavior is undefined.  It would be
+     nice to emit a warning here, but path splitting means this might
+     happen with legitimate code.  So simply drop the builtin
+     expansion in that case; we've handled any side-effects above.  */
+  if (ignore)
+    return const0_rtx;
+
+  /* If we don't have a suitable target, create one to hold the result.  */
+  if (target == NULL)
+    target = gen_reg_rtx (mode);
+
+  return targetm.speculation_safe_value (mode, target, val, failsafe);
+}
+
 /* Expand an expression EXP that calls a built-in function,
    with result going to TARGET if that's convenient
    (and in mode MODE if that's convenient).
@@ -7992,6 +8038,17 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
     case BUILT_IN_GOACC_PARLEVEL_SIZE:
       return expand_builtin_goacc_parlevel_id_size (exp, target, ignore);
 
+    case BUILT_IN_SPECULATION_SAFE_VALUE_PTR:
+      return expand_speculation_safe_value (VOIDmode, exp, target, ignore);
+
+    case BUILT_IN_SPECULATION_SAFE_VALUE_1:
+    case BUILT_IN_SPECULATION_SAFE_VALUE_2:
+    case BUILT_IN_SPECULATION_SAFE_VALUE_4:
+    case BUILT_IN_SPECULATION_SAFE_VALUE_8:
+    case BUILT_IN_SPECULATION_SAFE_VALUE_16:
+      mode = get_builtin_sync_mode (fcode - BUILT_IN_SPECULATION_SAFE_VALUE_1);
+      return expand_speculation_safe_value (mode, exp, target, ignore);
+
     default:	/* just do library call, if unknown builtin */
       break;
     }
diff --git a/gcc/builtins.def b/gcc/builtins.def
index aacbd51..6e8af2a 100644
--- a/gcc/builtins.def
+++ b/gcc/builtins.def
@@ -1003,6 +1003,28 @@ DEF_BUILTIN (BUILT_IN_EMUTLS_REGISTER_COMMON,
 	     true, true, true, ATTR_NOTHROW_LEAF_LIST, false,
 	     !targetm.have_tls)
 
+/* Suppressing speculation.  Users are expected to use the first (N)
+   variant, which will be translated internally into one of the other
+   types.  */
+
+DEF_GCC_BUILTIN (BUILT_IN_SPECULATION_SAFE_VALUE_N, "speculation_safe_value",
+		 BT_FN_VOID_VAR, ATTR_NOTHROW_LEAF_LIST)
+
+DEF_GCC_BUILTIN (BUILT_IN_SPECULATION_SAFE_VALUE_PTR,
+		 "speculation_safe_value_ptr", BT_FN_PTR_PTR_VAR,
+		 ATTR_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN (BUILT_IN_SPECULATION_SAFE_VALUE_1, "speculation_safe_value_1",
+		 BT_FN_I1_I1_VAR, ATTR_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN (BUILT_IN_SPECULATION_SAFE_VALUE_2, "speculation_safe_value_2",
+		 BT_FN_I2_I2_VAR, ATTR_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN (BUILT_IN_SPECULATION_SAFE_VALUE_4, "speculation_safe_value_4",
+		 BT_FN_I4_I4_VAR, ATTR_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN (BUILT_IN_SPECULATION_SAFE_VALUE_8, "speculation_safe_value_8",
+		 BT_FN_I8_I8_VAR, ATTR_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN (BUILT_IN_SPECULATION_SAFE_VALUE_16,
+		 "speculation_safe_value_16", BT_FN_I16_I16_VAR,
+		 ATTR_NOTHROW_LEAF_LIST)
+
 /* Exception support.  */
 DEF_BUILTIN_STUB (BUILT_IN_UNWIND_RESUME, "__builtin_unwind_resume")
 DEF_BUILTIN_STUB (BUILT_IN_CXA_END_CLEANUP, "__builtin_cxa_end_cleanup")
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index f5e1111..e368fd2 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -6457,6 +6457,122 @@ builtin_type_for_size (int size, bool unsignedp)
   return type ? type : error_mark_node;
 }
 
+/* Work out the size of the first argument of a call to
+   __builtin_speculation_safe_value.  Only pointers and integral types
+   are permitted.  Return -1 if the argument type is not supported or
+   the size is too large; 0 if the argument type is a pointer or the
+   size if it is integral.  */
+static enum built_in_function
+speculation_safe_value_resolve_call (tree function, vec<tree, va_gc> *params)
+{
+  /* Type of the argument.  */
+  tree type;
+  int size;
+
+  if (vec_safe_is_empty (params))
+    {
+      error ("too few arguments to function %qE", function);
+      return BUILT_IN_NONE;
+    }
+
+  type = TREE_TYPE ((*params)[0]);
+  if (TREE_CODE (type) == ARRAY_TYPE && c_dialect_cxx ())
+    {
+      /* Force array-to-pointer decay for C++.   */
+      (*params)[0] = default_conversion ((*params)[0]);
+      type = TREE_TYPE ((*params)[0]);
+    }
+
+  if (POINTER_TYPE_P (type))
+    return BUILT_IN_SPECULATION_SAFE_VALUE_PTR;
+
+  if (!INTEGRAL_TYPE_P (type))
+    goto incompatible;
+
+  if (!COMPLETE_TYPE_P (type))
+    goto incompatible;
+
+  size = tree_to_uhwi (TYPE_SIZE_UNIT (type));
+  if (size == 1 || size == 2 || size == 4 || size == 8 || size == 16)
+    return ((enum built_in_function)
+	    ((int) BUILT_IN_SPECULATION_SAFE_VALUE_1 + exact_log2 (size)));
+
+ incompatible:
+  /* Issue the diagnostic only if the argument is valid, otherwise
+     it would be redundant at best and could be misleading.  */
+  if (type != error_mark_node)
+    error ("operand type %qT is incompatible with argument %d of %qE",
+	   type, 1, function);
+
+  return BUILT_IN_NONE;
+}
+
+/* Validate and coerce PARAMS, the arguments to ORIG_FUNCTION to fit
+   the prototype for FUNCTION.  The first argument is mandatory, a second
+   argument, if present, must be type compatible with the first.  */
+static bool
+speculation_safe_value_resolve_params (location_t loc, tree orig_function,
+				       vec<tree, va_gc> *params)
+{
+  tree val;
+
+  if (params->length () == 0)
+    {
+      error_at (loc, "too few arguments to function %qE", orig_function);
+      return false;
+    }
+
+  else if (params->length () > 2)
+    {
+      error_at (loc, "too many arguments to function %qE", orig_function);
+      return false;
+    }
+
+  val = (*params)[0];
+  if (TREE_CODE (TREE_TYPE (val)) == ARRAY_TYPE)
+    val = default_conversion (val);
+  if (!(TREE_CODE (TREE_TYPE (val)) == POINTER_TYPE
+	|| TREE_CODE (TREE_TYPE (val)) == INTEGER_TYPE))
+    {
+      error_at (loc,
+		"expecting argument of type pointer or of type integer "
+		"for argument 1");
+      return false;
+    }
+  (*params)[0] = val;
+
+  if (params->length () == 2)
+    {
+      tree val2 = (*params)[1];
+      if (TREE_CODE (TREE_TYPE (val2)) == ARRAY_TYPE)
+	val2 = default_conversion (val2);
+      if (!(TREE_TYPE (val) == TREE_TYPE (val2)
+	    || useless_type_conversion_p (TREE_TYPE (val), TREE_TYPE (val2))))
+	{
+	  error_at (loc, "both arguments must be compatible");
+	  return false;
+	}
+      (*params)[1] = val2;
+    }
+
+  return true;
+}
+
+/* Cast the result of the builtin back to the type of the first argument,
+   preserving any qualifiers that it might have.  */
+static tree
+speculation_safe_value_resolve_return (tree first_param, tree result)
+{
+  tree ptype = TREE_TYPE (first_param);
+  tree rtype = TREE_TYPE (result);
+  ptype = TYPE_MAIN_VARIANT (ptype);
+
+  if (tree_int_cst_equal (TYPE_SIZE (ptype), TYPE_SIZE (rtype)))
+    return convert (ptype, result);
+
+  return result;
+}
+
 /* A helper function for resolve_overloaded_builtin in resolving the
    overloaded __sync_ builtins.  Returns a positive power of 2 if the
    first operand of PARAMS is a pointer to a supported data type.
@@ -7111,6 +7227,54 @@ resolve_overloaded_builtin (location_t loc, tree function,
   /* Handle BUILT_IN_NORMAL here.  */
   switch (orig_code)
     {
+    case BUILT_IN_SPECULATION_SAFE_VALUE_N:
+      {
+	tree new_function, first_param, result;
+	enum built_in_function fncode
+	  = speculation_safe_value_resolve_call (function, params);;
+
+	first_param = (*params)[0];
+	if (fncode == BUILT_IN_NONE
+	    || !speculation_safe_value_resolve_params (loc, function, params))
+	  return error_mark_node;
+
+	if (targetm.have_speculation_safe_value (true))
+	  {
+	    new_function = builtin_decl_explicit (fncode);
+	    result = build_function_call_vec (loc, vNULL, new_function, params,
+					      NULL);
+
+	    if (result == error_mark_node)
+	      return result;
+
+	    return speculation_safe_value_resolve_return (first_param, result);
+	  }
+	else
+	  {
+	    /* This target doesn't have, or doesn't need, active mitigation
+	       against incorrect speculative execution.  Simply return the
+	       first parameter to the builtin.  */
+	    if (!targetm.have_speculation_safe_value (false))
+	      /* The user has invoked __builtin_speculation_safe_value
+		 even though __HAVE_SPECULATION_SAFE_VALUE is not
+		 defined: emit a warning.  */
+	      warning_at (input_location, 0,
+			  "this target does not define a speculation barrier; "
+			  "your program will still execute correctly, "
+			  "but incorrect speculation may not be be "
+			  "restricted");
+
+	    /* If the optional second argument is present, handle any side
+	       effects now.  */
+	    if (params->length () == 2
+		&& TREE_SIDE_EFFECTS ((*params)[1]))
+	      return build2 (COMPOUND_EXPR, TREE_TYPE (first_param),
+			     (*params)[1], first_param);
+
+	    return first_param;
+	  }
+      }
+
     case BUILT_IN_ATOMIC_EXCHANGE:
     case BUILT_IN_ATOMIC_COMPARE_EXCHANGE:
     case BUILT_IN_ATOMIC_LOAD:
diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index bdb5691..4fcf3a6 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -1361,7 +1361,12 @@ c_cpp_builtins (cpp_reader *pfile)
     cpp_define (pfile, "__WCHAR_UNSIGNED__");
 
   cpp_atomic_builtins (pfile);
-    
+
+  /* Show support for __builtin_speculation_safe_value () if the target
+     has been updated to fully support it.  */
+  if (targetm.have_speculation_safe_value (false))
+    cpp_define (pfile, "__HAVE_SPECULATION_SAFE_VALUE");
+
 #ifdef DWARF2_UNWIND_INFO
   if (dwarf2out_do_cfi_asm ())
     cpp_define (pfile, "__GCC_HAVE_DWARF2_CFI_ASM");
diff --git a/gcc/doc/cpp.texi b/gcc/doc/cpp.texi
index 3f7a8fc..efad2c8 100644
--- a/gcc/doc/cpp.texi
+++ b/gcc/doc/cpp.texi
@@ -2381,6 +2381,10 @@ If GCC cannot determine the current date, it will emit a warning message
 These macros are defined when the target processor supports atomic compare
 and swap operations on operands 1, 2, 4, 8 or 16 bytes in length, respectively.
 
+@item __HAVE_SPECULATION_SAFE_VALUE
+This macro is defined with the value 1 to show that this version of GCC
+supports @code{__builtin_speculation_safe_value}.
+
 @item __GCC_HAVE_DWARF2_CFI_ASM
 This macro is defined when the compiler is emitting DWARF CFI directives
 to the assembler.  When this is defined, it is possible to emit those same
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 954e8a1..0ba1931 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -10948,6 +10948,7 @@ is called and the @var{flag} argument passed to it.
 @findex __builtin_powi
 @findex __builtin_powif
 @findex __builtin_powil
+@findex __builtin_speculation_safe_value
 @findex _Exit
 @findex _exit
 @findex abort
@@ -11592,6 +11593,96 @@ check its compatibility with @var{size}.
 
 @end deftypefn
 
+@deftypefn {Built-in Function} @var{type} __builtin_speculation_safe_value (@var{type} val, @var{type} failval)
+
+This built-in function can be used to help mitigate against unsafe
+speculative execution.  @var{type} may be any integral type or any
+pointer type.
+
+@enumerate
+@item
+If the CPU is not speculatively executing the code, then @var{val}
+is returned.
+@item
+If the CPU is executing speculatively then either:
+@itemize
+@item
+The function may cause execution to pause until it is known that the
+code is no-longer being executed speculatively (in which case
+@var{val} can be returned, as above); or
+@item
+The function may use target-dependent speculation tracking state to cause
+@var{failval} to be returned when it is known that speculative
+execution has incorrectly predicted a conditional branch operation.
+@end itemize
+@end enumerate
+
+The second argument, @var{failval}, is optional and defaults to zero
+if omitted.
+
+GCC defines the preprocessor macro
+@code{__HAVE_BUILTIN_SPECULATION_SAFE_VALUE} for targets that have been
+updated to support this builtin.
+
+The built-in function can be used where a variable appears to be used in a
+safe way, but the CPU, due to speculative execution may temporarily ignore
+the bounds checks.  Consider, for example, the following function:
+
+@smallexample
+int array[500];
+int f (unsigned untrusted_index)
+@{
+  if (untrusted_index < 500)
+    return array[untrusted_index];
+  return 0;
+@}
+@end smallexample
+
+If the function is called repeatedly with @code{untrusted_index} less
+than the limit of 500, then a branch predictor will learn that the
+block of code that returns a value stored in @code{array} will be
+executed.  If the function is subsequently called with an
+out-of-range value it will still try to execute that block of code
+first until the CPU determines that the prediction was incorrect
+(the CPU will unwind any incorrect operations at that point).
+However, depending on how the result of the function is used, it might be
+possible to leave traces in the cache that can reveal what was stored
+at the out-of-bounds location.  The built-in function can be used to
+provide some protection against leaking data in this way by changing
+the code to:
+
+@smallexample
+int array[500];
+int f (unsigned untrusted_index)
+@{
+  if (untrusted_index < 500)
+    return array[__builtin_speculation_safe_value (untrusted_index)];
+  return 0;
+@}
+@end smallexample
+
+The built-in function will either cause execution to stall until the
+conditional branch has been fully resolved, or it may permit
+speculative execution to continue, but using 0 instead of
+@code{untrusted_value} if that exceeds the limit.
+
+If accessing any memory location is potentially unsafe when speculative
+execution is incorrect, then the code can be rewritten as
+
+@smallexample
+int array[500];
+int f (unsigned untrusted_index)
+@{
+  if (untrusted_index < 500)
+    return *__builtin_speculation_safe_value (&array[untrusted_index], NULL);
+  return 0;
+@}
+@end smallexample
+
+which will cause a @code{NULL} pointer to be used for the unsafe case.
+
+@end deftypefn
+
 @deftypefn {Built-in Function} int __builtin_types_compatible_p (@var{type1}, @var{type2})
 
 You can use the built-in function @code{__builtin_types_compatible_p} to
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 734bc76..00c1239 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -7032,6 +7032,21 @@ should be defined to an instruction that orders both loads and stores
 before the instruction with respect to loads and stores after the instruction.
 This pattern has no operands.
 
+@cindex @code{speculation_barrier} instruction pattern
+@item @samp{speculation_barrier}
+If the target can support speculative execution, then this pattern should
+be defined to an instruction that will block subsequent execution until
+any prior speculation conditions has been resolved.  The pattern must also
+ensure that the compiler cannot move memory operations past the barrier,
+so it needs to be an UNSPEC_VOLATILE pattern.  The pattern has no
+operands.
+
+If this pattern is not defined then the default expansion of
+@code{__builtin_speculation_safe_value} will emit a warning.  You can
+suppress this warning by defining this pattern with a final condition
+of @code{0} (zero), which tells the compiler that a speculation
+barrier is not needed for this target.
+
 @cindex @code{sync_compare_and_swap@var{mode}} instruction pattern
 @item @samp{sync_compare_and_swap@var{mode}}
 This pattern, if defined, emits code for an atomic compare-and-swap
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index ff6d514..ee3f55c 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -11948,6 +11948,36 @@ maintainer is familiar with.
 
 @end defmac
 
+@deftypefn {Target Hook} bool TARGET_HAVE_SPECULATION_SAFE_VALUE (bool @var{active})
+This hook is used to determine the level of target support for
+ @code{__builtin_speculation_safe_value}.  If called with an argument
+ of false, it returns true if the target has been modified to support
+ this builtin.  If called with an argument of true, it returns true
+ if the target requires active mitigation execution might be speculative.
+ 
+ The default implementation returns true for the first case if the target
+ defines a pattern named @code{speculation_barrier}; for the second case
+ and if the pattern is enabled for the current compilation.
+@end deftypefn
+
+@deftypefn {Target Hook} rtx TARGET_SPECULATION_SAFE_VALUE (machine_mode @var{mode}, rtx @var{result}, rtx @var{val}, rtx @var{failval})
+This target hook can be used to generate a target-specific code
+ sequence that implements the @code{__builtin_speculation_safe_value}
+ built-in function.  The function must always return @var{val} in
+ @var{result} in mode @var{mode} when the cpu is not executing
+ speculatively, but must never return that when speculating until it
+ is known that the speculation will not be unwound.  The hook supports
+ two primary mechanisms for implementing the requirements.  The first
+ is to emit a speculation barrier which forces the processor to wait
+ until all prior speculative operations have been resolved; the second
+ is to use a target-specific mechanism that can track the speculation
+ state and to return @var{failval} if it can determine that
+ speculation must be unwound at a later time.
+ 
+ The default implementation simply copies @var{val} to @var{result} and
+ emits a @code{speculation_barrier} instruction if that is defined.
+@end deftypefn
+
 @deftypefn {Target Hook} void TARGET_RUN_TARGET_SELFTESTS (void)
 If selftests are enabled, run any selftests for this target.
 @end deftypefn
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 2f97151..94ad868 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -8109,4 +8109,8 @@ maintainer is familiar with.
 
 @end defmac
 
+@hook TARGET_HAVE_SPECULATION_SAFE_VALUE
+
+@hook TARGET_SPECULATION_SAFE_VALUE
+
 @hook TARGET_RUN_TARGET_SELFTESTS
diff --git a/gcc/target.def b/gcc/target.def
index ff89e72..3a9de78 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -4196,6 +4196,40 @@ DEFHOOK
  hook_bool_void_true)
 
 DEFHOOK
+(have_speculation_safe_value,
+"This hook is used to determine the level of target support for\n\
+ @code{__builtin_speculation_safe_value}.  If called with an argument\n\
+ of false, it returns true if the target has been modified to support\n\
+ this builtin.  If called with an argument of true, it returns true\n\
+ if the target requires active mitigation execution might be speculative.\n\
+ \n\
+ The default implementation returns true for the first case if the target\n\
+ defines a pattern named @code{speculation_barrier}; for the second case\n\
+ and if the pattern is enabled for the current compilation.",
+bool, (bool active), default_have_speculation_safe_value)
+
+DEFHOOK
+(speculation_safe_value,
+"This target hook can be used to generate a target-specific code\n\
+ sequence that implements the @code{__builtin_speculation_safe_value}\n\
+ built-in function.  The function must always return @var{val} in\n\
+ @var{result} in mode @var{mode} when the cpu is not executing\n\
+ speculatively, but must never return that when speculating until it\n\
+ is known that the speculation will not be unwound.  The hook supports\n\
+ two primary mechanisms for implementing the requirements.  The first\n\
+ is to emit a speculation barrier which forces the processor to wait\n\
+ until all prior speculative operations have been resolved; the second\n\
+ is to use a target-specific mechanism that can track the speculation\n\
+ state and to return @var{failval} if it can determine that\n\
+ speculation must be unwound at a later time.\n\
+ \n\
+ The default implementation simply copies @var{val} to @var{result} and\n\
+ emits a @code{speculation_barrier} instruction if that is defined.",
+rtx, (machine_mode mode, rtx result, rtx val, rtx failval),
+ default_speculation_safe_value)
+ 
+
+DEFHOOK
 (can_use_doloop_p,
  "Return true if it is possible to use low-overhead loops (@code{doloop_end}\n\
 and @code{doloop_begin}) for a particular loop.  @var{iterations} gives the\n\
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 9b06d7a..06de1e3 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -2314,4 +2314,36 @@ default_preferred_else_value (unsigned, tree type, unsigned, tree *)
   return build_zero_cst (type);
 }
 
+/* Default implementation of TARGET_HAVE_SPECULATION_SAFE_VALUE.  */
+bool
+default_have_speculation_safe_value (bool active)
+{
+#ifdef HAVE_speculation_barrier
+  return active ? HAVE_speculation_barrier : true;
+#else
+  return false;
+#endif
+}
+
+/* Default implementation of the speculation-safe-load builtin.  This
+   implementation simply copies val to result and generates a
+   speculation_barrier insn, if such a pattern is defined.  */
+rtx
+default_speculation_safe_value (machine_mode mode ATTRIBUTE_UNUSED,
+				rtx result, rtx val,
+				rtx failval ATTRIBUTE_UNUSED)
+{
+  emit_move_insn (result, val);
+
+#ifdef HAVE_speculation_barrier
+  /* Assume the target knows what it is doing: if it defines a
+     speculation barrier, but it is not enabled, then assume that one
+     isn't needed.  */
+  if (HAVE_speculation_barrier)
+    emit_insn (gen_speculation_barrier ());
+#endif
+
+  return result;
+}
+
 #include "gt-targhooks.h"
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 8d234cf..74ffe5f 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -285,4 +285,7 @@ extern bool default_stack_clash_protection_final_dynamic_probe (rtx);
 extern void default_select_early_remat_modes (sbitmap);
 extern tree default_preferred_else_value (unsigned, tree, unsigned, tree *);
 
+extern bool default_have_speculation_safe_value (bool);
+extern rtx default_speculation_safe_value (machine_mode, rtx, rtx, rtx);
+
 #endif /* GCC_TARGHOOKS_H */
diff --git a/gcc/testsuite/c-c++-common/spec-barrier-1.c b/gcc/testsuite/c-c++-common/spec-barrier-1.c
new file mode 100644
index 0000000..e4b44f2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/spec-barrier-1.c
@@ -0,0 +1,38 @@
+/* { dg-do run } */
+/* { dg-options "-O" } */
+
+/* Test that __builtin_speculation_safe_value returns the correct value.  */
+/* This test will cause an unfiltered warning to be emitted on targets
+   that have not implemented support for speculative execution
+   barriers.  They should fix that rather than disabling this
+   test.  */
+char a = 1;
+short b = 2;
+int c = 3;
+long d = 4;
+long long e = 5;
+int *f = (int*) &c;
+#ifdef __SIZEOF_INT128__
+__int128 g = 9;
+#endif
+
+int main ()
+{
+  if (__builtin_speculation_safe_value (a) != 1)
+    __builtin_abort ();
+  if (__builtin_speculation_safe_value (b) != 2)
+    __builtin_abort ();
+  if (__builtin_speculation_safe_value (c) != 3)
+    __builtin_abort ();
+  if (__builtin_speculation_safe_value (d) != 4)
+    __builtin_abort ();
+  if (__builtin_speculation_safe_value (e) != 5)
+    __builtin_abort ();
+  if (__builtin_speculation_safe_value (f) != &c)
+    __builtin_abort ();
+#ifdef __SIZEOF_INT128__
+  if (__builtin_speculation_safe_value (g) != 9)
+    __builtin_abort ();
+#endif
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/spec-barrier-2.c b/gcc/testsuite/c-c++-common/spec-barrier-2.c
new file mode 100644
index 0000000..b09567e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/spec-barrier-2.c
@@ -0,0 +1,17 @@
+/* { dg-do run } */
+
+/* Even on targets that don't need the optional failval parameter,
+   side-effects on the operand should still be calculated.  */
+
+int x = 3;
+volatile int y = 9;
+
+int main ()
+{
+  int z = __builtin_speculation_safe_value (x, y++);
+  if (z != 3 || y != 10)
+    __builtin_abort ();
+  return 0;
+}
+
+/* { dg-prune-output "this target does not define a speculation barrier;" } */
diff --git a/gcc/testsuite/gcc.dg/spec-barrier-3.c b/gcc/testsuite/gcc.dg/spec-barrier-3.c
new file mode 100644
index 0000000..3ed4d39
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spec-barrier-3.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-Wpedantic" } */
+
+/* __builtin_speculation_safe_value returns a value with the same type
+   as its first argument.  There should be a warning if that isn't
+   type-compatible with the use.  */
+int *
+f (int x)
+{
+  return __builtin_speculation_safe_value (x);  /* { dg-warning "returning 'int' from a function with return type 'int \\*' makes pointer from integer without a cast" } */
+}
+
+/* { dg-prune-output "this target does not define a speculation barrier;" } */

Reply via email to