Re: [PATCH] Fix for lazy decl_rtl bug in dwarf2out.c

[Daniel Berlin <dan at cgsoftware dot com>]

Mark Mitchell <mark@codesourcery.com> writes:

> >>>>> "Daniel" == Daniel Berlin <dan@cgsoftware.com> writes:
> 
>     Daniel> So should I change it to check for the case it isn't set,
>     Daniel> and abort, instead?
> 
> Maybe.
> 
>     Daniel> Why should we ever be generating a declaration die for a
>     Daniel> LABEL_DECL if it wasn't output?
> 
> I dunno.  It could be saying `there is no address for this label',
> which could enable the debugger to do better than `no such label' when
> the user tries to set a breakpoint there.  Instead, it could say
> `label optimized away' or some such.


I checked this, and for user labels, DECL_RTL is always set, even if
the label is optimized away. I can't find a way to generate a
LABEL_DECL with no RTL set, but i'm sure if  someone did, they'd get
an assembler error. 

User labels aren't ever optimized away, we'd abort if they were, a few
lines down.   


In fact, with a user label at -O0, you end up with a DIE with the
label, and it's low_pc, in the right place in the DIE tree (IE child
of the subprogram it's in)
at -O3, you end up with a DIE with the
label name, no low_pc, at the CU scope.
In the right place, you have a label die with the right low_pc, and an
abstract_origin of the CU scope label.  

Look, ma, it's an inlined label!

I tried to see if i could force it to generate the inline label where
the procedure got inlined to, and discovered a bug in flow.c:

int inlineme(volatile int *a)
{
        fred:
                *a=5;
               inlineme(a);
}
int main(void)
{
        int a;
        dan:
          inlineme(&a);
          inlineme(&a);
         printf("%d\n", a);
}

Remove the call to inlineme from inlineme, and flow stops breaking
trying to make edges.
(Of course, it's an infinite loop, but i only cared about the debug
info.)
But anyway,  2.95.2 generates the right DIE's from this code (IE the inlined label
in the inlined subroutine has the correct abstract origin and low_pc).

>From all this analysis, all i can say is that if DECL_RTL isn't set at
that point in gen_label_die, we should abort.  If we don't, we'll end
up with a label that only exists in the debug info, and get an
assembler error, and a confused user, since the assembler error will
go away if you turn off debug info.

> output that
> 
>     Daniel> If it is output, why would it ever not be set?
> 
> It wouldn't be.
> 
> Yes, I think it's OK to add the abort.  But, it might also be that we
> should just say `look, no RTL, let's emit a DIE that indicates
> that'.
But it should always have RTL, (or be DECL_ABSTRACT, which is taken
care of in the "if" that this is the else to) or so the code below it claims.

No matter what heinous stuff i throw at it, I can't produce a case
where DECL_RTL isn't set at this point (I added an
abort() if it wasn't set, and ran the entire libstdc++, and gcc
testsuite, at -O3 -gdwarf-2 without hitting it. In addition to my own
evil label tests),  tells me eventually someone will find a way to
make it happen,  and be really confused about the assembler error.
So we probably should add an abort, if only on the mainline, so if it
does ever occur, we know either it's a bug somewhere else, or we know
what this case looks like, so we can figure out what to generate here,
because AFAICT, every case we want to handle is already taken care of,
and DECL_RTL is set.

> 
> --
> Mark Mitchell                   mark@codesourcery.com
> CodeSourcery, LLC               http://www.codesourcery.com

-- 
When I was five years old I was on a merry go round.  There was
a gunshot nearby.  The horses stampeded.  There I was running
down the street on a purple wooden horse.