Warn about code that uses variables and functions with internal linkage

without defining them.  This should be an error, but I'm paranoid about
"uses" that end up not actually requiring a definition.  I'll revisit later.

Also, teach IR generation to not set internal linkage on variable
declarations, just for safety's sake.  Doing so produces an invalid module
if the variable is not ultimately defined.

Also, fix several places in the test suite where we were using internal
functions without definitions.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@126016 91177308-0d34-0410-b5e6-96231b3b80d8

16 files changed, 220 insertions, 42 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index d38d14c856..4560f6e9a3 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1997,6 +1997,11 @@ def err_definition_of_implicitly_declared_member : Error<
 def err_redefinition_extern_inline : Error<
   "redefinition of a 'extern inline' function %0 is not supported in "
   "%select{C99 mode|C++}1">;
+
+// This should eventually be an error.
+def warn_undefined_internal : Warning<
+  "%select{function|variable}0 %q1 has internal linkage but is not defined">;
+def note_used_here : Note<"used here">;
   
 def warn_redefinition_of_typedef : Warning<
   "redefinition of typedef %0 is invalid in C">,
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 4e3173ab09..a835bb219e 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -600,6 +600,10 @@ public:
   // argument locations.
   llvm::DenseMap<ParmVarDecl *,SourceLocation> UnparsedDefaultArgLocs;
 
+  /// UndefinedInternals - all the used, undefined objects with
+  /// internal linkage in this translation unit.
+  llvm::DenseMap<NamedDecl*, SourceLocation> UndefinedInternals;
+
   typedef std::pair<ObjCMethodList, ObjCMethodList> GlobalMethods;
   typedef llvm::DenseMap<Selector, GlobalMethods> GlobalMethodPool;
 
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index d5e9dbf4fc..7d4b461f5a 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -637,14 +637,26 @@ void NamedDecl::ClearLinkageCache() {
   if (const CXXRecordDecl *record = dyn_cast<CXXRecordDecl>(this))
     clearLinkageForClass(record);
 
-  if (const ClassTemplateDecl *temp = dyn_cast<ClassTemplateDecl>(this)) {
+  if (ClassTemplateDecl *temp =
+        dyn_cast<ClassTemplateDecl>(const_cast<NamedDecl*>(this))) {
     // Clear linkage for the template pattern.
     CXXRecordDecl *record = temp->getTemplatedDecl();
     record->HasCachedLinkage = 0;
     clearLinkageForClass(record);
 
-    // ...do we need to clear linkage for specializations, too?
+    // We need to clear linkage for specializations, too.
+    for (ClassTemplateDecl::spec_iterator
+           i = temp->spec_begin(), e = temp->spec_end(); i != e; ++i)
+      i->ClearLinkageCache();
   }
+
+  // Clear cached linkage for function template decls, too.
+  if (FunctionTemplateDecl *temp =
+        dyn_cast<FunctionTemplateDecl>(const_cast<NamedDecl*>(this)))
+    for (FunctionTemplateDecl::spec_iterator
+           i = temp->spec_begin(), e = temp->spec_end(); i != e; ++i)
+      i->ClearLinkageCache();
+    
 }
 
 Linkage NamedDecl::getLinkage() const {
diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
index 5a45df24fe..9e5d7cf112 100644
--- a/lib/CodeGen/CodeGenModule.cpp
+++ b/lib/CodeGen/CodeGenModule.cpp
@@ -983,7 +983,7 @@ CodeGenModule::GetOrCreateLLVMGlobal(llvm::StringRef MangledName,
     // Set linkage and visibility in case we never see a definition.
     NamedDecl::LinkageInfo LV = D->getLinkageAndVisibility();
     if (LV.linkage() != ExternalLinkage) {
-      GV->setLinkage(llvm::GlobalValue::InternalLinkage);
+      // Don't set internal linkage on declarations.
     } else {
       if (D->hasAttr<DLLImportAttr>())
         GV->setLinkage(llvm::GlobalValue::DLLImportLinkage);
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 7eb1c57412..23a3c24804 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -271,6 +271,65 @@ static bool ShouldRemoveFromUnused(Sema *SemaRef, const DeclaratorDecl *D) {
   return false;
 }
 
+namespace {
+  struct UndefinedInternal {
+    NamedDecl *decl;
+    FullSourceLoc useLoc;
+
+    UndefinedInternal(NamedDecl *decl, FullSourceLoc useLoc)
+      : decl(decl), useLoc(useLoc) {}
+  };
+
+  bool operator<(const UndefinedInternal &l, const UndefinedInternal &r) {
+    return l.useLoc.isBeforeInTranslationUnitThan(r.useLoc);
+  }
+}
+
+/// checkUndefinedInternals - Check for undefined objects with internal linkage.
+static void checkUndefinedInternals(Sema &S) {
+  if (S.UndefinedInternals.empty()) return;
+
+  // Collect all the still-undefined entities with internal linkage.
+  llvm::SmallVector<UndefinedInternal, 16> undefined;
+  for (llvm::DenseMap<NamedDecl*,SourceLocation>::iterator
+         i = S.UndefinedInternals.begin(), e = S.UndefinedInternals.end();
+       i != e; ++i) {
+    NamedDecl *decl = i->first;
+
+    // Ignore attributes that have become invalid.
+    if (decl->isInvalidDecl()) continue;
+
+    // __attribute__((weakref)) is basically a definition.
+    if (decl->hasAttr<WeakRefAttr>()) continue;
+
+    if (FunctionDecl *fn = dyn_cast<FunctionDecl>(decl)) {
+      if (fn->isPure() || fn->hasBody())
+        continue;
+    } else {
+      if (cast<VarDecl>(decl)->hasDefinition() != VarDecl::DeclarationOnly)
+        continue;
+    }
+
+    // We build a FullSourceLoc so that we can sort with array_pod_sort.
+    FullSourceLoc loc(i->second, S.Context.getSourceManager());
+    undefined.push_back(UndefinedInternal(decl, loc));
+  }
+
+  if (undefined.empty()) return;
+
+  // Sort (in order of use site) so that we're not (as) dependent on
+  // the iteration order through an llvm::DenseMap.
+  llvm::array_pod_sort(undefined.begin(), undefined.end());
+
+  for (llvm::SmallVectorImpl<UndefinedInternal>::iterator
+         i = undefined.begin(), e = undefined.end(); i != e; ++i) {
+    NamedDecl *decl = i->decl;
+    S.Diag(decl->getLocation(), diag::warn_undefined_internal)
+      << isa<VarDecl>(decl) << decl;
+    S.Diag(i->useLoc, diag::note_used_here);
+  }
+}
+
 /// ActOnEndOfTranslationUnit - This is called at the very end of the
 /// translation unit when EOF is reached and all but the top-level scope is
 /// popped.
@@ -403,6 +462,8 @@ void Sema::ActOnEndOfTranslationUnit() {
               << DiagD->getDeclName();
       }
     }
+
+    checkUndefinedInternals(*this);
   }
 
   TUScope = 0;
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 792eb8af98..df49ad5c9a 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -9284,6 +9284,9 @@ void Sema::MarkDeclarationReferenced(SourceLocation Loc, Decl *D) {
       MarkVTableUsed(Loc, MethodDecl->getParent());
   }
   if (FunctionDecl *Function = dyn_cast<FunctionDecl>(D)) {
+    // Recursive functions should be marked when used from another function.
+    if (CurContext == Function) return;
+
     // Implicit instantiation of function templates and member functions of
     // class templates.
     if (Function->isImplicitlyInstantiable()) {
@@ -9312,19 +9315,23 @@ void Sema::MarkDeclarationReferenced(SourceLocation Loc, Decl *D) {
         else
           PendingInstantiations.push_back(std::make_pair(Function, Loc));
       }
-    } else // Walk redefinitions, as some of them may be instantiable.
+    } else {
+      // Walk redefinitions, as some of them may be instantiable.
       for (FunctionDecl::redecl_iterator i(Function->redecls_begin()),
            e(Function->redecls_end()); i != e; ++i) {
         if (!i->isUsed(false) && i->isImplicitlyInstantiable())
           MarkDeclarationReferenced(Loc, *i);
       }
+    }
 
-    // FIXME: keep track of references to static functions
-
-    // Recursive functions should be marked when used from another function.
-    if (CurContext != Function)
-      Function->setUsed(true);
+    // Keep track of used but undefined functions.
+    if (!Function->isPure() && !Function->hasBody() &&
+        Function->getLinkage() != ExternalLinkage) {
+      SourceLocation &old = UndefinedInternals[Function->getCanonicalDecl()];
+      if (old.isInvalid()) old = Loc;
+    }
 
+    Function->setUsed(true);
     return;
   }
 
@@ -9341,7 +9348,12 @@ void Sema::MarkDeclarationReferenced(SourceLocation Loc, Decl *D) {
       }
     }
 
-    // FIXME: keep track of references to static data?
+    // Keep track of used but undefined variables.
+    if (Var->hasDefinition() == VarDecl::DeclarationOnly
+        && Var->getLinkage() != ExternalLinkage) {
+      SourceLocation &old = UndefinedInternals[Var->getCanonicalDecl()];
+      if (old.isInvalid()) old = Loc;
+    }
 
     D->setUsed(true);
     return;
diff --git a/test/Analysis/cxx-crashes.cpp b/test/Analysis/cxx-crashes.cpp
index cebc55bd42..ae2f3cb5eb 100644
--- a/test/Analysis/cxx-crashes.cpp
+++ b/test/Analysis/cxx-crashes.cpp
@@ -18,7 +18,7 @@ namespace {
 
 struct A { };
 struct B {
-  operator A();
+  operator A() { return A(); }
 };
 
 A f(char *dst) {
diff --git a/test/Analysis/misc-ps-64.m b/test/Analysis/misc-ps-64.m
index 0dbd6cb948..f65673eea2 100644
--- a/test/Analysis/misc-ps-64.m
+++ b/test/Analysis/misc-ps-64.m
@@ -14,7 +14,7 @@ typedef unsigned char Boolean;
 typedef const struct __CFDictionary * CFDictionaryRef;
 
 extern Boolean CFDictionaryGetValueIfPresent(CFDictionaryRef theDict, const void *key, const void **value);
-static void shazam(NSUInteger i, unsigned char **out);
+void shazam(NSUInteger i, unsigned char **out);
 
 void rdar_6440393_1(NSDictionary *dict) {
   NSInteger x = 0;
diff --git a/test/CodeGen/regparm.c b/test/CodeGen/regparm.c
index dd0be96818..d628b685f9 100644
--- a/test/CodeGen/regparm.c
+++ b/test/CodeGen/regparm.c
@@ -11,8 +11,7 @@ typedef struct {
 typedef void (*FType)(int, int)      __attribute ((regparm (3), stdcall));
 FType bar;
 
-static void FASTCALL
-reduced(char b, double c, foo* d, double e, int f);
+extern void FASTCALL reduced(char b, double c, foo* d, double e, int f);
 
 // PR7025
 void FASTCALL f1(int i, int j, int k);
diff --git a/test/CodeGen/sizeof-vla.c b/test/CodeGen/sizeof-vla.c
index b0c514fd01..c5fc912519 100644
--- a/test/CodeGen/sizeof-vla.c
+++ b/test/CodeGen/sizeof-vla.c
@@ -2,7 +2,7 @@
 
 // PR3442
 
-static void *g(unsigned long len);
+void *g(unsigned long len);
 
 void
 f(int n)
diff --git a/test/CodeGenCXX/constructor-convert.cpp b/test/CodeGenCXX/constructor-convert.cpp
index 338febbe97..9122dae128 100644
--- a/test/CodeGenCXX/constructor-convert.cpp
+++ b/test/CodeGenCXX/constructor-convert.cpp
@@ -5,7 +5,7 @@ class Twine {
   Twine(const char *Str) { }
 };
 
-static void error(const Twine &Message);
+static void error(const Twine &Message) {}
 
 template<typename>
 struct opt_storage {
diff --git a/test/CodeGenCXX/internal-linkage.cpp b/test/CodeGenCXX/internal-linkage.cpp
index 9fdb7274e1..39bce8545f 100644
--- a/test/CodeGenCXX/internal-linkage.cpp
+++ b/test/CodeGenCXX/internal-linkage.cpp
@@ -1,11 +1,11 @@
 // RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
 
 struct Global { Global(); };
-template<typename T> struct X { X(); };
+template<typename T> struct X { X() {} };
 
 
 namespace {
-  struct Anon { Anon(); };
+  struct Anon { Anon() {} };
 
   // CHECK: @_ZN12_GLOBAL__N_15anon0E = internal global
   Global anon0;
diff --git a/test/CodeGenCXX/key-function-vtable.cpp b/test/CodeGenCXX/key-function-vtable.cpp
index 8378bbd234..8e474bdf95 100644
--- a/test/CodeGenCXX/key-function-vtable.cpp
+++ b/test/CodeGenCXX/key-function-vtable.cpp
@@ -30,6 +30,7 @@ void testf::a() {}
 namespace {
   struct testg { virtual void a(); };
 }
+void testg::a() {}
 testg *testgvar = new testg;
 
 struct X0 { virtual ~X0(); };
diff --git a/test/CodeGenCXX/thunks.cpp b/test/CodeGenCXX/thunks.cpp
index 238032cc6d..a74cc053db 100644
--- a/test/CodeGenCXX/thunks.cpp
+++ b/test/CodeGenCXX/thunks.cpp
@@ -88,31 +88,29 @@ void C::f() { }
 }
 
 // Check that the thunk gets internal linkage.
-namespace {
-
-struct A {
-  virtual void f();
-};
-
-struct B {
-  virtual void f();
-};
-
-struct C : A, B {
-  virtual void c();
-
-  virtual void f();
-};
+namespace Test4B {
+  struct A {
+    virtual void f();
+  };
 
-void C::f() { }
+  struct B {
+    virtual void f();
+  };
 
-}
+  namespace {
+    struct C : A, B {
+      virtual void c();
+      virtual void f();
+    };
+  }
+  void C::c() {}
+  void C::f() {}
 
-// Force C::f to be used.
-void f() { 
-  C c; 
-  
-  c.f();
+  // Force C::f to be used.
+  void f() { 
+    C c; 
+    c.f();
+  }
 }
 
 namespace Test5 {
@@ -283,4 +281,4 @@ namespace Test11 {
 
 // This is from Test5:
 // CHECK: define linkonce_odr void @_ZTv0_n24_N5Test51B1fEv
-// CHECK: define internal void @_ZThn8_N12_GLOBAL__N_11C1fEv(
+// CHECK: define internal void @_ZThn8_N6Test4B12_GLOBAL__N_11C1fEv(
diff --git a/test/Rewriter/properties.m b/test/Rewriter/properties.m
index 89177d7e64..ca4a199cc6 100644
--- a/test/Rewriter/properties.m
+++ b/test/Rewriter/properties.m
@@ -38,7 +38,7 @@ void *sel_registerName(const char *);
 
 @implementation Bar
 
-static int func(int i);
+static int func(int i) { return 0; }
 
 - (void)baz {
     Foo *obj1, *obj2;
diff --git a/test/SemaCXX/undefined-internal.cpp b/test/SemaCXX/undefined-internal.cpp
new file mode 100644
index 0000000000..bb87ce0f12
--- /dev/null
+++ b/test/SemaCXX/undefined-internal.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Make sure we don't produce invalid IR.
+// RUN: %clang_cc1 -emit-llvm-only %s
+
+namespace test1 {
+  static void foo(); // expected-warning {{function 'test1::foo' has internal linkage but is not defined}}
+  template <class T> static void bar(); // expected-warning {{function 'test1::bar<int>' has internal linkage but is not defined}}
+
+  void test() {
+    foo(); // expected-note {{used here}}
+    bar<int>(); // expected-note {{used here}}
+  }
+}
+
+namespace test2 {
+  namespace {
+    void foo(); // expected-warning {{function 'test2::<anonymous namespace>::foo' has internal linkage but is not defined}}
+    extern int var; // expected-warning {{variable 'test2::<anonymous namespace>::var' has internal linkage but is not defined}}
+    template <class T> void bar(); // expected-warning {{function 'test2::<anonymous namespace>::bar<int>' has internal linkage but is not defined}}
+  }
+  void test() {
+    foo(); // expected-note {{used here}}
+    var = 0; // expected-note {{used here}}
+    bar<int>(); // expected-note {{used here}}
+  }
+}
+
+namespace test3 {
+  namespace {
+    void foo();
+    extern int var;
+    template <class T> void bar();
+  }
+
+  void test() {
+    foo();
+    var = 0;
+    bar<int>();
+  }
+
+  namespace {
+    void foo() {}
+    int var = 0;
+    template <class T> void bar() {}
+  }
+}
+
+namespace test4 {
+  namespace {
+    struct A {
+      A(); // expected-warning {{function 'test4::<anonymous namespace>::A::A' has internal linkage but is not defined}}
+      ~A();// expected-warning {{function 'test4::<anonymous namespace>::A::~A' has internal linkage but is not defined}}
+      virtual void foo(); // expected-warning {{function 'test4::<anonymous namespace>::A::foo' has internal linkage but is not defined}}
+      virtual void bar() = 0;
+      virtual void baz(); // expected-warning {{function 'test4::<anonymous namespace>::A::baz' has internal linkage but is not defined}}
+    };
+  }
+
+  void test(A &a) {
+    a.foo(); // expected-note {{used here}}
+    a.bar();
+    a.baz(); // expected-note {{used here}}
+  }
+
+  struct Test : A {
+    Test() {} // expected-note 2 {{used here}}
+  };
+}
+
+// rdar://problem/9014651
+namespace test5 {
+  namespace {
+    struct A {};
+  }
+
+  template <class N> struct B {
+    static int var; // expected-warning {{variable 'test5::B<test5::<anonymous>::A>::var' has internal linkage but is not defined}}
+    static void foo(); // expected-warning {{function 'test5::B<test5::<anonymous>::A>::foo' has internal linkage but is not defined}}
+  };
+
+  void test() {
+    B<A>::var = 0; // expected-note {{used here}}
+    B<A>::foo(); // expected-note {{used here}}
+  }
+}