Re: [PATCH 2.5/4 v2] GAS: Make new fake labels when cloning a symbol

["Maciej W. Rozycki" <macro at codesourcery dot com>]

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