[RFC] Alias should not point to declarations

A long time ago (before r97733) we used to model the weakref attribute
by outputting a new declaration and a weak alias to it. This was
fairly buggy and we now implement weakref directly in clang, with the
same logic an assembler uses to implement .weakref (which is what gcc
prints).

One thing that was left from that old implementation is that we still
have alias to declarations and they are a very different concept from
alias to definitions.

Alias to definitions are just another symbol pointing to the same
position in the object file. For example, given

@bar = alias void ()* @foo
define void @foo() {
  ret void
}

Both ELF end COFF just get two symbols (foo and bar or _foo and _bar)
pointing to the same section and offset.

Alias to declarations are a lot fuzzier. They are still considered
definitions according to GlobalValue::isDeclaration(), but the IR

@bar = alias void ()* @foo
declare void @foo()

produces an empty ELF file and COFF files with two undefined symbols,
but no connection between them. There is no good answer, since neither
COFF nor ELF have an alias directive to pass the connection down to
the linker.

The only possible use right now seems to be what we used to do for
weakref. Given

@bar = hidden alias void ()* @foo
declare void @foo()
define void @zed() {
       call void @foo()
       call void @bar()
       ret void
}

llc with the pic relocation model will produce a R_X86_64_PLT32 and a
R_X86_64_PC32, both pointing to foo. That is, the alias acts as an
alternative declaration.

The supported way of doing this is to have a proper declaration in the
file using the symbol:

declare hidden void @bar()
declare void @foo()
define void @zed() {
       call void @foo()
       call void @bar()
       ret void
}

and an alias on the file defining foo:

@bar = hidden alias void ()* @foo
define void @foo() {
       ret void
}

With all that in mind, the attached patch changes the verifier to
reject aliases to declarations and updates that language reference.
The patch also updates the testcases. They seem to be almost all from
the old weakref implementation.

Cheers,
Rafael

t.patch (19.9 KB)

This makes sense to me, but I'm not an expert on ELF aliases.

If you're working in this area, there is something that has been bugging me for years about GlobalAlias: because we require the type of the alias and aliasee to match, we have to allow constant exprs in the aliasee (see GlobalAlias::getAliasedGlobal).

This representation is bad for several reasons. I think it would be much better to change GlobalAlias to allow the aliasee to have a different type than the alias itself, and then require the aliasee to be an actual GlobalValue, disallowing constexprs completely.

-Chris

Hi Rafael,

With all that in mind, the attached patch changes the verifier to
reject aliases to declarations and updates that language reference.

MachO has an R_INDR (== "indirect") symbol flag/type that (from my
understanding) exactly reflects this. The linker is supposed to record
the alias and define both symbols when the referee is defined. In
fact, I've been working on exposing this to C code very recently so
I'd obviously quite like it to stick around.

Currently I've not submitted a patch because, even though it's
documented, ld64 doesn't have support just yet so I can't actually
test things properly.

I think the only benefit is going to be to compile-time and
source-structure, but it would still be a shame to lose it when LLVM
is so close to having the ideal model right now.

Cheers.

Tim.

This is llvm.org/pr10367, right?

Cheers,
Rafael

Yeah, it totally is! :slight_smile:

-Chris

We chatted a bit or IRC and it does look like N_INDR has the
behaviour of llvm aliases. An alias to a declaration is a definition
for example. It is documented here:
https://developer.apple.com/library/mac/documentation/DeveloperTools/Conceptual/MachORuntime/Reference/reference.html

I will try to make the ELF/COFF restrictions explicit with errors in
codegen instead.

Cheers,
Rafael

We chatted a bit or IRC and it does look like N_INDR has the
behaviour of llvm aliases. An alias to a declaration is a definition
for example. It is documented here:
https://developer.apple.com/library/mac/documentation/DeveloperTools/Conceptual/MachORuntime/Reference/reference.html

We discussed this a bit more on IRC today. Fixing pr10367 is more
important now that there is work under way to support comdats, which
would use aliases to implement RTTI for the msvc abi.

It is much easier to design a feature based on what existing tools
(linkers in this case) support and N_INDR is not currently
implemented. On the other hand, adding support for it once it is
implemented should be relatively easy.

The attached patch then effectively just replaces a report_fatal_error
with a proper verifier check that represent what can currently be
implemented.

Tim, can you confirm that it is OK?

Thanks,
Rafael

t.patch (14.7 KB)

Tim, can you confirm that it is OK?

Yep. I'll try to make sure nothing makes N_INDR support impossible,
but there's no particular reason to pretend it's already there as far
as I can see.

Cheers.

Tim.