On Mon, Nov 28, 2016 at 6:28 PM, Martin Jambor <mjam...@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 <mjam...@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 >>
