diff mbox

Fix UB in ira-costs.c (find_costs_and_classes)

Message ID 20170620072736.GD2123@tucnak
State New
Headers show

Commit Message

Jakub Jelinek June 20, 2017, 7:27 a.m. UTC
Hi!

bootstrap-ubsan revealed many
../../gcc/ira-costs.c:1747:20: runtime error: member access within null pointer of type 'cost_classes *[107]'
issues.  The problem is that cost_classes_ptr is sometimes NULL, but
in those cases we have early exit:
          if (! allocno_p)
            {
              if (regno_reg_rtx[i] == NULL_RTX)
                continue;	// <----- HERE
              memcpy (temp_costs, COSTS (costs, i), struct_costs_size);
              i_mem_cost = temp_costs->mem_cost;
            }
          else
            {
              if (ira_regno_allocno_map[i] == NULL)
                continue;	// <----- or HERE
...
            }
Still, cost_classes_ptr->classes where classes is an array is UB when
cost_classes_ptr is NULL, so this patch moves it after the if (...) continue;
in both branches (because it is needed both later in the else ...
and after the whole if.

Bootstrapped/regtested on x86_64-linux and i686-linux (with
bootstrap-ubsan), ok for trunk?

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

	* ira-costs.c (find_costs_and_classes): Initialize cost_classes later
	to make sure not to dereference a NULL cost_classes_ptr pointer.


	Jakub

Comments

Vladimir Makarov June 20, 2017, 6:50 p.m. UTC | #1
On 06/20/2017 03:27 AM, Jakub Jelinek wrote:
> Hi!
>
> bootstrap-ubsan revealed many
> ../../gcc/ira-costs.c:1747:20: runtime error: member access within null pointer of type 'cost_classes *[107]'
> issues.  The problem is that cost_classes_ptr is sometimes NULL, but
> in those cases we have early exit:
>            if (! allocno_p)
>              {
>                if (regno_reg_rtx[i] == NULL_RTX)
>                  continue;	// <----- HERE
>                memcpy (temp_costs, COSTS (costs, i), struct_costs_size);
>                i_mem_cost = temp_costs->mem_cost;
>              }
>            else
>              {
>                if (ira_regno_allocno_map[i] == NULL)
>                  continue;	// <----- or HERE
> ...
>              }
> Still, cost_classes_ptr->classes where classes is an array is UB when
> cost_classes_ptr is NULL, so this patch moves it after the if (...) continue;
> in both branches (because it is needed both later in the else ...
> and after the whole if.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux (with
> bootstrap-ubsan), ok for trunk?
Sure.

Jakub, thank you for addressing the issue.
> 2017-06-20  Jakub Jelinek  <jakub@redhat.com>
>
> 	* ira-costs.c (find_costs_and_classes): Initialize cost_classes later
> 	to make sure not to dereference a NULL cost_classes_ptr pointer.
>
diff mbox

Patch

--- gcc/ira-costs.c.jj	2017-06-19 22:56:35.000000000 +0200
+++ gcc/ira-costs.c	2017-06-20 00:27:38.032572231 +0200
@@ -1744,7 +1744,7 @@  find_costs_and_classes (FILE *dump_file)
 	  int best_cost, allocno_cost;
 	  enum reg_class best, alt_class;
 	  cost_classes_t cost_classes_ptr = regno_cost_classes[i];
-	  enum reg_class *cost_classes = cost_classes_ptr->classes;
+	  enum reg_class *cost_classes;
 	  int *i_costs = temp_costs->cost;
 	  int i_mem_cost;
 	  int equiv_savings = regno_equiv_gains[i];
@@ -1755,6 +1755,7 @@  find_costs_and_classes (FILE *dump_file)
 		continue;
 	      memcpy (temp_costs, COSTS (costs, i), struct_costs_size);
 	      i_mem_cost = temp_costs->mem_cost;
+	      cost_classes = cost_classes_ptr->classes;
 	    }
 	  else
 	    {
@@ -1762,6 +1763,7 @@  find_costs_and_classes (FILE *dump_file)
 		continue;
 	      memset (temp_costs, 0, struct_costs_size);
 	      i_mem_cost = 0;
+	      cost_classes = cost_classes_ptr->classes;
 	      /* Find cost of all allocnos with the same regno.  */
 	      for (a = ira_regno_allocno_map[i];
 		   a != NULL;