This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 2.5/4 v2] GAS: Make new fake labels when cloning a symbol
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: Catherine Moore <clm at codesourcery dot com>, binutils at sourceware dot org
- Date: Mon, 1 Nov 2010 12:23:15 +0000 (GMT)
- Subject: Re: [PATCH 2.5/4 v2] GAS: Make new fake labels when cloning a symbol
- References: <alpine.DEB.1.10.1007241721120.29495@tp.orcam.me.uk> <alpine.DEB.1.10.1010291442440.25860@tp.orcam.me.uk> <87aalwngmr.fsf@firetop.home>
On Sat, 30 Oct 2010, Richard Sandiford wrote:
> Looks good. However,
>
> gcc (Debian 4.4.4-8) 4.4.5 20100728 (prerelease)
>
> complains about so-called aliasing violations on these calls:
>
> > + S_SET_NAME (&dot_symbol, ".");
> > + S_SET_FORWARD_REF (&dot_symbol);
>
> It's inlining the calls and warning about the local case, which obviously
> doesn't apply here. I know it's not usually good practice to recode to
> avoid bogus warnings, but let's use:
>
> dot_symbol.bsym->name = ".";
> dot_symbol.sy_forward_ref = 1;
>
> anyway.
Hmm, TBH I didn't like the way these structs are type-punned from the
very first moment I realised what was going on here, but GCC 4.3.2 doesn't
seem as picky, so I let it be. The complaint about the risk of alias
violations with these structs is IMO legitimate; if not here, then
elsewhere. And since this is 2.22 material anyway, how about we do this
properly and convert symbolS to make it a union of the two cases: local
and external? I mean:
union symbol
{
struct local_symbol l;
struct external_symbol e;
};
typedef union symbol symbolS;
where "struct local_symbol" would remain unchanged and the current "struct
symbol" would become "struct external_symbol".
I realise syntactically that's going to be an extensive change, but
mostly if not entirely mechanical and will improve the overall quality of
code involved. And if anytime, freshly after a release branch has been
forked off is I think the best moment to make such changes. What do you
think?
> Could you also add a gcc_assert to symbol_clone to make sure that we
> don't accidentally clone dot_symbol in other cases?
Sure.
> OK with those changes once 2.21 has branched, thanks.
OK, fair enough, all of this is indeed subtle. I'll wait and commit
these changes in due course. Thanks for your review.
Maciej