[PR,debug/53682] avoid crash in cselib promote_debug_loc

When promote_debug_loc was first introduced, it would never be called
with a NULL loc list.  However, because of the strategy of temporarily
resetting loc lists before recursion introduced a few months ago in
alias.c, the earlier assumption no longer holds.

This patch adusts promote_debug_loc to deal with this case.

Ok to install?

On Wed, Jun 20, 2012 at 12:39:29AM -0300, Alexandre Oliva wrote:
> When promote_debug_loc was first introduced, it would never be called
> with a NULL loc list.  However, because of the strategy of temporarily
> resetting loc lists before recursion introduced a few months ago in
> alias.c, the earlier assumption no longer holds.
> 
> This patch adusts promote_debug_loc to deal with this case.

The thing I'm worried about is what will happen with -g0 in that case.
If the loc list is temporarily reset, it will be restored again,
won't that mean that for -g0 we'll then have a loc that is in the
corresponding -g compilation referenced by a DEBUG_INSNs only (and thus
non-promoted)?

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/53682
> 	* cselib.c (promote_debug_loc): Don't crash on NULL argument.
> 
> Index: gcc/cselib.c
> ===================================================================
> --- gcc/cselib.c.orig	2012-06-17 22:52:27.740087279 -0300
> +++ gcc/cselib.c	2012-06-18 08:55:32.948832112 -0300
> @@ -322,7 +322,7 @@ new_elt_loc_list (cselib_val *val, rtx l
>  static inline void
>  promote_debug_loc (struct elt_loc_list *l)
>  {
> -  if (l->setting_insn && DEBUG_INSN_P (l->setting_insn)
> +  if (l && l->setting_insn && DEBUG_INSN_P (l->setting_insn)
>        && (!cselib_current_insn || !DEBUG_INSN_P (cselib_current_insn)))
>      {
>        n_debug_values--;

	Jakub

On Jun 20, 2012, Jakub Jelinek <jakub@redhat.com> wrote:

> On Wed, Jun 20, 2012 at 12:39:29AM -0300, Alexandre Oliva wrote:
>> When promote_debug_loc was first introduced, it would never be called
>> with a NULL loc list.  However, because of the strategy of temporarily
>> resetting loc lists before recursion introduced a few months ago in
>> alias.c, the earlier assumption no longer holds.

>> This patch adusts promote_debug_loc to deal with this case.

> The thing I'm worried about is what will happen with -g0 in that case.
> If the loc list is temporarily reset, it will be restored again,
> won't that mean that for -g0 we'll then have a loc that is in the
> corresponding -g compilation referenced by a DEBUG_INSNs only (and thus
> non-promoted)?

I don't see how.  If we get to a NULL loc list, it means it's not the
first time we visit that node (it was visited upstack), so if it needed
promotion, it would have already been promoted then.

On Wed, Jun 20, 2012 at 08:43:53PM -0300, Alexandre Oliva wrote:
> On Jun 20, 2012, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> > On Wed, Jun 20, 2012 at 12:39:29AM -0300, Alexandre Oliva wrote:
> >> When promote_debug_loc was first introduced, it would never be called
> >> with a NULL loc list.  However, because of the strategy of temporarily
> >> resetting loc lists before recursion introduced a few months ago in
> >> alias.c, the earlier assumption no longer holds.
> 
> >> This patch adusts promote_debug_loc to deal with this case.
> 
> > The thing I'm worried about is what will happen with -g0 in that case.
> > If the loc list is temporarily reset, it will be restored again,
> > won't that mean that for -g0 we'll then have a loc that is in the
> > corresponding -g compilation referenced by a DEBUG_INSNs only (and thus
> > non-promoted)?
> 
> I don't see how.  If we get to a NULL loc list, it means it's not the
> first time we visit that node (it was visited upstack), so if it needed
> promotion, it would have already been promoted then.

The patch is ok then.

	Jakub

On Jun 20, 2012, Alexandre Oliva <aoliva@redhat.com> wrote:

> When promote_debug_loc was first introduced, it would never be called
> with a NULL loc list.  However, because of the strategy of temporarily
> resetting loc lists before recursion introduced a few months ago in
> alias.c, the earlier assumption no longer holds.

> This patch adusts promote_debug_loc to deal with this case.

> Ok to install?

Unless there are objections in the next 24 hours, I'm going to install
this in the 4.7 branch too.  Regstrapped on x86_64-linux-gnu and
i686-linux-gnu.

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>

> 	PR debug/53682
> 	* cselib.c (promote_debug_loc): Don't crash on NULL argument.

On Mon, Jun 25, 2012 at 07:32:04AM -0300, Alexandre Oliva wrote:
> Unless there are objections in the next 24 hours, I'm going to install
> this in the 4.7 branch too.  Regstrapped on x86_64-linux-gnu and
> i686-linux-gnu.

Just install it right away.  Thanks.

> > for  gcc/ChangeLog
> > from  Alexandre Oliva  <aoliva@redhat.com>
> 
> > 	PR debug/53682
> > 	* cselib.c (promote_debug_loc): Don't crash on NULL argument.

	Jakub