[Sema] Mark target of __attribute__((alias("target"))) used for C

[nickdesaulniers]

Summary

Prevents -Wunneeded-internal-delcaration warnings when the target has no
other references. This occurs frequently in device drivers in the Linux
kernel.

Sema would need to invoke the demangler on the target, since in C++ the
target name is mangled:

int f() { return 42; }
int g() attribute((alias("_Z1fv")));

Sema does not have the ability to demangle names at this time.

https://bugs.llvm.org/show_bug.cgi?id=39088
https://github.com/ClangBuiltLinux/linux/issues/232

[rsmith]

I don't like having a solution that doesn't work for C++ and can't reasonably be extended to a solution for C++ (we really don't want to be demangling the alias target here and trying to match it against the program, and ideally we don't want to delay the warning in question to codegen time), but this does reduce warning false-positives so it seems hard to object to. (We'll still need to tell people to mark declarations as __attribute__((used)) when they're used in this way if their names are mangled.)

On balance I'm OK with this, but adding @rjmccall in case he has a different view or sees a better approach.

[rjmccall]

I really dislike this particular idiom.

This is probably the most reasonable short-term solution to this problem. I do wonder if we should just shift to making Sema track a symbol-to-GD map, though, and maybe make Sema entirely responsible for pushing entities to IRGen to emit.

test/Sema/alias-unused.c

3 ↗

(On Diff #172877)

Is this meant to be static?

[nickdesaulniers]

test/Sema/alias-unused.c

3 ↗

(On Diff #172877)

It doesn't make a difference for this test. Would you like me to add it before submitting?

[rjmccall]

test/Sema/alias-unused.c

3 ↗

(On Diff #172877)

It's what you're trying to test, right? f() is not otherwise an internal declaration, so of course the warning won't fire for it.

[nickdesaulniers]

• add static keyword to test case

[nickdesaulniers]

test/Sema/alias-unused.c

3 ↗

(On Diff #172877)

sorry, you're right. (I was testing with my locally patched version of clang, godbolt to the rescue to test "pre-my-change-clang." Done.

[rjmccall]

Don't worry, that's a familiar mistake. :)

LGTM.

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

static void handleAliasAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   } else {
     const auto *VD = cast<VarDecl>(D);
     if (VD->isThisDeclarationADefinition() && VD->isExternallyVisible()) {
       S.Diag(AL.getLoc(), diag::err_alias_is_definition) << VD << 0;
       return;
     }
   }
 
-  // FIXME: check if target symbol exists in current file
+  // Mark target used to prevent unneeded-internal-declaration warnings.
+  if (!S.LangOpts.CPlusPlus) {
+    // FIXME: demangle Str for C++, as the attribute refers to the mangled
+    // linkage name, not the pre-mangled identifier.
+    const DeclarationNameInfo target(&S.Context.Idents.get(Str), AL.getLoc());
+    LookupResult LR(S, target, Sema::LookupOrdinaryName);
+    if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+      for (NamedDecl *ND : LR)
+        ND->markUsed(S.Context);
+  }
 
   D->addAttr(::new (S.Context) AliasAttr(AL.getRange(), S.Context, Str,
                                          AL.getAttributeSpellingListIndex()));
 }
 
 static void handleTLSModelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   StringRef Model;
   SourceLocation LiteralLoc;

cfe/trunk/test/Sema/alias-unused.c

+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x c -verify %s
+// expected-no-diagnostics
+static int f() { return 42; }
+int g() __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD };
+extern typeof(foo) bar __attribute__((unused, alias("foo")));