[PATCH] Avoid UB in the Ada FE

[PATCH] Avoid UB in the Ada FE

[Jakub Jelinek]

Hi!

I'm seeing almost 750 of runtime errors like:
../../gcc/ada/gcc-interface/trans.c:6992:20: runtime error: load of value 240, which is not a valid value for type 'bool'
(with random values in place of the 240 above) during bootstrap-ubsan.

The problem is that atomic_access_required_p only initializes what the
last argument points to if it returns true.  Usually it is used like
            else if (atomic_access_required_p (gnat_actual, &sync))
              gnu_result = build_atomic_store (gnu_actual, gnu_result, sync);
so it is fine, but in this particular snippet we have:
          bool atomic_access
            = !outer_atomic_access
              && atomic_access_required_p (Name (gnat_node), &sync);
          gnu_result
            = Call_to_gnu (Expression (gnat_node), &gnu_result_type, gnu_lhs,
                           outer_atomic_access, atomic_access, sync);
i.e. we unconditionally load a bool value that is only conditionally
initialized, and pass it to another function (which uses it conditionally
only, but the UB is already the load of the uninitialized value).

Fixed thusly, ok for trunk if it passes bootstrap/regtest?

Another option would be to change atomic_access_required_p to add
*sync = false;
before the first return, or to initialize bool sync = false; at the
definition.

2017-06-22  Jakub Jelinek  <jakub@redhat.com>

	* gcc-interface/trans.c (gnat_to_gnu): Initialize sync to false to
	avoid UB.

--- gcc/ada/gcc-interface/trans.c.jj	2017-06-21 16:53:37.000000000 +0200
+++ gcc/ada/gcc-interface/trans.c	2017-06-22 12:19:45.458928009 +0200
@@ -6985,6 +6985,7 @@ gnat_to_gnu (Node_Id gnat_node)
 	{
 	  bool outer_atomic_access
 	    = outer_atomic_access_required_p (Name (gnat_node));
+	  sync = false;
 	  bool atomic_access
 	    = !outer_atomic_access
 	      && atomic_access_required_p (Name (gnat_node), &sync);


	Jakub

Re: [PATCH] Avoid UB in the Ada FE

[Eric Botcazou]

> Another option would be to change atomic_access_required_p to add
> *sync = false;
> before the first return, or to initialize bool sync = false; at the
> definition.

Yes, let's do the initialization at the definition (no need to retest).

-- 
Eric Botcazou