Fri Feb 18 18:53:41 PST 2011

Author: rjmccall
Date: Fri Feb 18 20:53:41 2011
New Revision: 126016

URL: http://llvm.org/viewvc/llvm-project?rev=126016&view=rev
Log:
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.


Added:
    cfe/trunk/test/SemaCXX/undefined-internal.cpp
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/AST/Decl.cpp
    cfe/trunk/lib/CodeGen/CodeGenModule.cpp
    cfe/trunk/lib/Sema/Sema.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/Analysis/cxx-crashes.cpp
    cfe/trunk/test/Analysis/misc-ps-64.m
    cfe/trunk/test/CodeGen/regparm.c
    cfe/trunk/test/CodeGen/sizeof-vla.c
    cfe/trunk/test/CodeGenCXX/constructor-convert.cpp
    cfe/trunk/test/CodeGenCXX/internal-linkage.cpp
    cfe/trunk/test/CodeGenCXX/key-function-vtable.cpp
    cfe/trunk/test/CodeGenCXX/thunks.cpp
    cfe/trunk/test/Rewriter/properties.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================

--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Feb 18 20:53:41 2011
@@ -1997,6 +1997,11 @@
 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">,

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Feb 18 20:53:41 2011
@@ -600,6 +600,10 @@
   // 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;
 

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Fri Feb 18 20:53:41 2011
@@ -637,14 +637,26 @@
   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 {

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Fri Feb 18 20:53:41 2011
@@ -983,7 +983,7 @@
     // 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);

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Fri Feb 18 20:53:41 2011
@@ -271,6 +271,65 @@
   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 @@
               << DiagD->getDeclName();
       }
     }
+
+    checkUndefinedInternals(*this);
   }
 
   TUScope = 0;

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Feb 18 20:53:41 2011
@@ -9284,6 +9284,9 @@
       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 @@
         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 @@
       }
     }
 
-    // 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;

Modified: cfe/trunk/test/Analysis/cxx-crashes.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-crashes.cpp?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/cxx-crashes.cpp (original)
+++ cfe/trunk/test/Analysis/cxx-crashes.cpp Fri Feb 18 20:53:41 2011
@@ -18,7 +18,7 @@
 
 struct A { };
 struct B {
-  operator A();
+  operator A() { return A(); }
 };
 
 A f(char *dst) {

Modified: cfe/trunk/test/Analysis/misc-ps-64.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps-64.m?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps-64.m (original)
+++ cfe/trunk/test/Analysis/misc-ps-64.m Fri Feb 18 20:53:41 2011
@@ -14,7 +14,7 @@
 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;

Modified: cfe/trunk/test/CodeGen/regparm.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/regparm.c?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/regparm.c (original)
+++ cfe/trunk/test/CodeGen/regparm.c Fri Feb 18 20:53:41 2011
@@ -11,8 +11,7 @@
 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);

Modified: cfe/trunk/test/CodeGen/sizeof-vla.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/sizeof-vla.c?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/sizeof-vla.c (original)
+++ cfe/trunk/test/CodeGen/sizeof-vla.c Fri Feb 18 20:53:41 2011
@@ -2,7 +2,7 @@
 
 // PR3442
 
-static void *g(unsigned long len);
+void *g(unsigned long len);
 
 void
 f(int n)

Modified: cfe/trunk/test/CodeGenCXX/constructor-convert.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/constructor-convert.cpp?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/constructor-convert.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/constructor-convert.cpp Fri Feb 18 20:53:41 2011
@@ -5,7 +5,7 @@
   Twine(const char *Str) { }
 };
 
-static void error(const Twine &Message);
+static void error(const Twine &Message) {}
 
 template<typename>
 struct opt_storage {

Modified: cfe/trunk/test/CodeGenCXX/internal-linkage.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/internal-linkage.cpp?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/internal-linkage.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/internal-linkage.cpp Fri Feb 18 20:53:41 2011
@@ -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;

Modified: cfe/trunk/test/CodeGenCXX/key-function-vtable.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/key-function-vtable.cpp?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/key-function-vtable.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/key-function-vtable.cpp Fri Feb 18 20:53:41 2011
@@ -30,6 +30,7 @@
 namespace {
   struct testg { virtual void a(); };
 }
+void testg::a() {}
 testg *testgvar = new testg;
 
 struct X0 { virtual ~X0(); };

Modified: cfe/trunk/test/CodeGenCXX/thunks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/thunks.cpp?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/thunks.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/thunks.cpp Fri Feb 18 20:53:41 2011
@@ -88,31 +88,29 @@
 }
 
 // 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 @@
 
 // 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(

Modified: cfe/trunk/test/Rewriter/properties.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Rewriter/properties.m?rev=126016&r1=126015&r2=126016&view=diff
==============================================================================
--- cfe/trunk/test/Rewriter/properties.m (original)
+++ cfe/trunk/test/Rewriter/properties.m Fri Feb 18 20:53:41 2011
@@ -38,7 +38,7 @@
 
 @implementation Bar
 
-static int func(int i);
+static int func(int i) { return 0; }
 
 - (void)baz {
     Foo *obj1, *obj2;

Added: cfe/trunk/test/SemaCXX/undefined-internal.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/undefined-internal.cpp?rev=126016&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/undefined-internal.cpp (added)
+++ cfe/trunk/test/SemaCXX/undefined-internal.cpp Fri Feb 18 20:53:41 2011
@@ -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}}
+  }
+}



