This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] Assert DECL_ABSTRACT_ORIGIN is different from the decl itself


On Mon, Nov 28, 2016 at 6:28 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi Jeff,
>
> On Mon, Nov 28, 2016 at 08:46:05AM -0700, Jeff Law wrote:
>> On 11/28/2016 07:27 AM, Martin Jambor wrote:
>> > Hi,
>> >
>> > one of a number of symptoms of an otherwise unrelated HSA bug I've
>> > been debugging today is gcc crashing or hanging in the C++ pretty
>> > printer when attempting to emit a warning because dump_decl() ended up
>> > in an infinite recursion calling itself on the DECL_ABSTRACT_ORIGIN of
>> > the decl it was looking at, which was however the same thing.  (It was
>> > set to itself on purpose in set_decl_origin_self as a part of final
>> > pass, the decl was being printed because it was itself an abstract
>> > origin of another one).
>> >
>> > If someone ever faces a similar problem, the following (untested)
>> > patch might save them a bit of time.  I have eventually decided not to
>> > make it a checking-only assert because it is on a cold path and
>> > because at release-build optimization levels, the tail-call is
>> > optimized to a jump and thus an infinite loop if the described
>> > situation happens, and I suppose an informative ICE is better tan that
>> > even for users.
>> >
>> > What do you think?  Would it be reasonable for trunk even now or
>> > should I queue it for the next stage1?
>> >
>> > Thanks,
>> >
>> > Martin
>> >
>> >
>> > gcc/cp/
>> >
>> > 2016-11-28  Martin Jambor  <mjambor@suse.cz>
>> >
>> >     * error.c (dump_decl): Add an assert that DECL_ABSTRACT_ORIGIN
>> >     is not the decl itself.
>> Given it's on an error/debug path it ought to be plenty safe for now. What's
>> more interesting is whether or not DECL_ABSTRACT_ORIGIN can legitimately
>> point to itself and if so, how is that happening.
>
> Well, I tried to explain it in my original email but I also wanted to
> be as brief as possible, so perhaps it is necessary to elaborate a bit:
>
> There is a function set_decl_origin_self() in dwarf2out.c that does
> just that, sets DECL_ABSTRACT_ORIGIN to the decl itself, and its
> comment makes it clear that is intended (according to git blame, the
> whole comment and much of the implementation come from 1992, though ;-)
> The function is called from the "final" pass through dwarf2out_decl(),
> and gen_decl_die().
>
> So, for one reason or another, this is the intended behavior.
> Apparently, after that one is not supposed to be printing the decl
> name of such a "finished" a function.  It is too bad however that this
> can happen if a "finished" function is itself an abstract origin of a
> different one, which is optimized and expanded only afterwards and you
> attempt to print its decl name, because it triggers printing the decl
> name of the finished function, in turn triggering the infinite
> recursion/loop.  I am quite surprised that we have not hit this
> earlier (e.g. with warnings in IPA-CP clones) but perhaps there is a
> reason.
>
> I will append the patch to some bootstrap and testing run and commit
> it afterwards if it passes.

Other users explicitely check for the self-reference when walking origins.

Richard.

> Thanks,
>
> Martin
>
>>
>> I don't think we have a checker for the basic tree datastructures, but maybe
>> we ought to?
>>
>> jeff
>>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]