Re: [PATCH 03/25] Improve TARGET_MANGLE_DECL_ASSEMBLER_NAME.

[Julian Brown <julian@codesourcery.com>]

On Wed, 12 Sep 2018 17:31:58 +0100
Andrew Stubbs <ams@codesourcery.com> wrote:

> On 12/09/18 16:16, Richard Biener wrote:
> > I think the symptom GCN sees needs to be better understood - like
> > wheter it is generally OK to mangle things arbitrarily.  
> 
> The name mangling is a horrible workaround for a bug in the HSA
> runtime code (which we do not own, cannot fix, and would want to
> support old versions of anyway).  Basically it refuses to load any
> binary that has the same symbol as another, already loaded, binary,
> regardless of the symbol's linkage.  Worse, it also rejects any
> binary that has duplicate symbols within it, despite the fact that it
> already linked just fine.
> 
> Adding the extra lookups is enough to build GCN binaries, with
> mangled names, whereas the existing name mangling support was either
> more specialized or bit rotten (I don't know which).
> 
> It may well be that there's a better way to solve the problem, or at 
> least to do the lookups.
> 
> It may also be that there are some unintended consequences, such as 
> false name matches, but I don't know of any at present.
> 
> Julian, can you comment, please?

I did the local-symbol name mangling in two places:

- The ASM_FORMAT_PRIVATE_NAME macro (good for local statics)
- The TARGET_MANGLE_DECL_ASSEMBLER_NAME hook (for file-scope
  local/statics)

Possibly, this was an abuse of these hooks, but it's arguably wrong that
that e.g. handle_alias_pairs has the "assembler name" leak through into
the user's source code -- if it's expected that the hook could make
arbitrary transformations to the string. (The latter hook is only used
by PE code for x86 at present, by the look of it, and the default
handles only special-purpose mangling indicated by placing a '*' at the
front of the symbol.)

I couldn't find an existing place where the DECL_NAMEs for symbols were
indexed in a hash table, equivalent to the table for assembler names.
Aliases are made via pragmas, so it's not 100% clear to me what the
scoping/lookup rules are supposed to be for those anyway, nor what the
possibility or consequences might be of false matches.

(The "!target" case in maybe_apply_pending_pragma_weaks, if it doesn't
somehow make a false match, just slows down reporting of an error a
little, I think. Similarly in handle_alias_pairs.)

If we had a symtab_node::get_for_name () using a suitable hash table, I
think it'd probably be right to use that. Can that be done (easily), or
is there some equivalent way? Introducing a new hash table everywhere
for a bug workaround for a relatively obscure feature on a single
target seems unfortunate.

Thanks,

Julian