Fix PR66206

Fix PR66206

[Bernd Schmidt]

[-- Attachment #1: Type: text/plain, Size: 183 bytes --]

This is a small problem found by a static analyzer, a function in 
bt-load can in theory return the address of a local variable.

Bootstrapped and tested on x86_64-linux, ok?


Bernd

[-- Attachment #2: btr.diff --]
[-- Type: text/x-patch, Size: 2411 bytes --]

	PR rtl-optimization/66206
	* bt-load.c (find_btr_use): Change first arg to be a pointer to an rtx.
	All callers changed.

Index: gcc/bt-load.c
===================================================================
--- gcc/bt-load.c	(revision 231653)
+++ gcc/bt-load.c	(working copy)
@@ -1,4 +1,3 @@
-
 /* Perform branch target register load optimizations.
    Copyright (C) 2001-2015 Free Software Foundation, Inc.
 
@@ -188,14 +187,14 @@ basic_block_freq (const_basic_block bb)
   return bb->frequency;
 }
 
-/* If X references (sets or reads) any branch target register, return one
-   such register.  If EXCLUDEP is set, disregard any references within
-   that location.  */
+/* If the rtx at *XP references (sets or reads) any branch target
+   register, return one such register.  If EXCLUDEP is set, disregard
+   any references within that location.  */
 static rtx *
-find_btr_use (rtx x, rtx *excludep = 0)
+find_btr_use (rtx *xp, rtx *excludep = 0)
 {
   subrtx_ptr_iterator::array_type array;
-  FOR_EACH_SUBRTX_PTR (iter, array, &x, NONCONST)
+  FOR_EACH_SUBRTX_PTR (iter, array, xp, NONCONST)
     {
       rtx *loc = *iter;
       if (loc == excludep)
@@ -232,7 +231,7 @@ insn_sets_btr_p (const rtx_insn *insn, i
       if (REG_P (dest)
 	  && TEST_HARD_REG_BIT (all_btrs, REGNO (dest)))
 	{
-	  gcc_assert (!find_btr_use (src));
+	  gcc_assert (!find_btr_use (&src));
 
 	  if (!check_const || CONSTANT_P (src))
 	    {
@@ -324,7 +323,7 @@ new_btr_user (basic_block bb, int insn_l
      to decide whether we can replace all target register
      uses easily.
    */
-  rtx *usep = find_btr_use (PATTERN (insn));
+  rtx *usep = find_btr_use (&PATTERN (insn));
   rtx use;
   btr_user *user = NULL;
 
@@ -335,7 +334,7 @@ new_btr_user (basic_block bb, int insn_l
       /* We want to ensure that USE is the only use of a target
 	 register in INSN, so that we know that to rewrite INSN to use
 	 a different target register, all we have to do is replace USE.  */
-      unambiguous_single_use = !find_btr_use (PATTERN (insn), usep);
+      unambiguous_single_use = !find_btr_use (&PATTERN (insn), usep);
       if (!unambiguous_single_use)
 	usep = NULL;
     }
@@ -511,7 +510,7 @@ compute_defs_uses_and_gen (btr_heap_t *a
 		}
 	      else
 		{
-		  if (find_btr_use (PATTERN (insn)))
+		  if (find_btr_use (&PATTERN (insn)))
 		    {
 		      btr_user *user = new_btr_user (bb, insn_luid, insn);
 

Re: Fix PR66206

[Andrew Pinski]

On Thu, Dec 17, 2015 at 5:00 PM, Bernd Schmidt <bernds_cb1@t-online.de> wrote:
> This is a small problem found by a static analyzer, a function in bt-load
> can in theory return the address of a local variable.
>
> Bootstrapped and tested on x86_64-linux, ok?

Except PATTERN (insn) will never be a REG.
The only case where the input can be a REG is:
gcc_assert (!find_btr_use (src));

And that is basically making sure the left hand side is not part of
all_btrs so the way to fix that would be simpler if you define a new
function called btr_use_p or something to that effect and return
true/false instead.

Also if you are touching this code, can you change the literal 0 to
NULL inside find_btr_use.

Thanks,
Andrew





>
>
> Bernd

Re: Fix PR66206

[Bernd Schmidt]

On 12/18/2015 02:15 AM, Andrew Pinski wrote:
>
> Except PATTERN (insn) will never be a REG.
> The only case where the input can be a REG is:
> gcc_assert (!find_btr_use (src));

Yeah, so we _are_ calling it with a REG. It's a minor issue that won't 
trigger in practice, but in order to close the PR we might as well fix 
it and this is the least invasive way.


Bernd

Re: Fix PR66206

[Eric Botcazou]

> This is a small problem found by a static analyzer, a function in
> bt-load can in theory return the address of a local variable.
> 
> Bootstrapped and tested on x86_64-linux, ok?

OK, thanks.

-- 
Eric Botcazou

Re: Fix PR66206

[Jeff Law]

On 12/17/2015 06:00 PM, Bernd Schmidt wrote:
> This is a small problem found by a static analyzer, a function in
> bt-load can in theory return the address of a local variable.
>
> Bootstrapped and tested on x86_64-linux, ok?
OK.

jeff