[clang] Rework dontcall attributes

To avoid using the AST when emitting diagnostics, split the "dontcall"
attribute into "dontcall-warn" and "dontcall-error", and also add the
frontend attribute value as the LLVM attribute value. This gives us all
the information to report diagnostics we need from within the IR (aside
from access to the original source).

One downside is we directly use LLVM's demangler rather than using the
existing Clang diagnostic pretty printing of symbols.

Reviewed By: nickdesaulniers

Differential Revision: https://reviews.llvm.org/D110364

diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 789c06a..6e1d7f3 100644 (file)
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -27,6 +27,7 @@
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/DiagnosticPrinter.h"
@@ -760,30 +761,18 @@ void BackendConsumer::OptimizationFailureHandler(
 }
 
 void BackendConsumer::DontCallDiagHandler(const DiagnosticInfoDontCall &D) {
-  if (const Decl *DE = Gen->GetDeclForMangledName(D.getFunctionName()))
-    if (const auto *FD = dyn_cast<FunctionDecl>(DE)) {
-      assert(FD->hasAttr<ErrorAttr>() &&
-             "expected error or warning function attribute");
-
-      if (const auto *EA = FD->getAttr<ErrorAttr>()) {
-        assert((EA->isError() || EA->isWarning()) &&
-               "ErrorAttr neither error or warning");
-
-        SourceLocation LocCookie =
-            SourceLocation::getFromRawEncoding(D.getLocCookie());
-
-        // FIXME: we can't yet diagnose indirect calls. When/if we can, we
-        // should instead assert that LocCookie.isValid().
-        if (!LocCookie.isValid())
-          return;
-
-        Diags.Report(LocCookie, EA->isError()
-                                    ? diag::err_fe_backend_error_attr
-                                    : diag::warn_fe_backend_warning_attr)
-            << FD << EA->getUserDiagnostic();
-      }
-    }
-  // TODO: assert if DE or FD were nullptr?
+  SourceLocation LocCookie =
+      SourceLocation::getFromRawEncoding(D.getLocCookie());
+
+  // FIXME: we can't yet diagnose indirect calls. When/if we can, we
+  // should instead assert that LocCookie.isValid().
+  if (!LocCookie.isValid())
+    return;
+
+  Diags.Report(LocCookie, D.getSeverity() == DiagnosticSeverity::DS_Error
+                              ? diag::err_fe_backend_error_attr
+                              : diag::warn_fe_backend_warning_attr)
+      << llvm::demangle(D.getFunctionName().str()) << D.getNote();
 }
 
 /// This function is invoked when the backend needs

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 8341801..d26b134 100644 (file)
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2138,8 +2138,12 @@ void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, llvm::Function *F,
   else if (const auto *SA = FD->getAttr<SectionAttr>())
      F->setSection(SA->getName());
 
-  if (FD->hasAttr<ErrorAttr>())
-    F->addFnAttr("dontcall");
+  if (const auto *EA = FD->getAttr<ErrorAttr>()) {
+    if (EA->isError())
+      F->addFnAttr("dontcall-error", EA->getUserDiagnostic());
+    else if (EA->isWarning())
+      F->addFnAttr("dontcall-warn", EA->getUserDiagnostic());
+  }
 
   // If we plan on emitting this inline builtin, we can't treat it as a builtin.
   if (FD->isInlineBuiltinDeclaration()) {

diff --git a/clang/test/CodeGen/attr-error.c b/clang/test/CodeGen/attr-error.c
index da56793..a1b63ab 100644 (file)
--- a/clang/test/CodeGen/attr-error.c
+++ b/clang/test/CodeGen/attr-error.c
@@ -7,5 +7,5 @@ void bar(void) {
 
 // CHECK: call void @foo(), !srcloc [[SRCLOC:![0-9]+]]
 // CHECK: declare{{.*}} void @foo() [[ATTR:#[0-9]+]]
-// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall"
+// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall-error"="oh no"
 // CHECK: [[SRCLOC]] = !{i32 {{[0-9]+}}}

diff --git a/clang/test/CodeGen/attr-warning.c b/clang/test/CodeGen/attr-warning.c
index daa53b6..5c89066 100644 (file)
--- a/clang/test/CodeGen/attr-warning.c
+++ b/clang/test/CodeGen/attr-warning.c
@@ -7,5 +7,5 @@ void bar(void) {
 
 // CHECK: call void @foo(), !srcloc [[SRCLOC:![0-9]+]]
 // CHECK: declare{{.*}} void @foo() [[ATTR:#[0-9]+]]
-// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall"
+// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall-warn"="oh no"
 // CHECK: [[SRCLOC]] = !{i32 {{[0-9]+}}}

diff --git a/clang/test/Frontend/backend-attribute-error-warning-optimize.c b/clang/test/Frontend/backend-attribute-error-warning-optimize.c
index d3951e3..668a0a4 100644 (file)
--- a/clang/test/Frontend/backend-attribute-error-warning-optimize.c
+++ b/clang/test/Frontend/backend-attribute-error-warning-optimize.c
@@ -8,7 +8,7 @@ int x(void) {
   return 8 % 2 == 1;
 }
 void baz(void) {
-  foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh no foo}}
+  foo(); // expected-error {{call to foo declared with 'error' attribute: oh no foo}}
   if (x())
     bar();
 }

diff --git a/clang/test/Frontend/backend-attribute-error-warning.c b/clang/test/Frontend/backend-attribute-error-warning.c
index 4e96f77..a1450b4 100644 (file)
--- a/clang/test/Frontend/backend-attribute-error-warning.c
+++ b/clang/test/Frontend/backend-attribute-error-warning.c
@@ -1,7 +1,5 @@
 // RUN: %clang_cc1 -verify=expected,enabled -emit-codegen-only %s
-// RUN: %clang_cc1 -verify=expected,enabled -emit-codegen-only %s -x c++
 // RUN: %clang_cc1 -verify -emit-codegen-only -Wno-attribute-warning %s
-// RUN: %clang_cc1 -verify -emit-codegen-only -Wno-attribute-warning %s -x c++
 
 __attribute__((error("oh no foo"))) void foo(void);
 
@@ -24,38 +22,12 @@ void                                            // expected-note@-1 {{previous a
 duplicate_warnings(void);
 
 void baz(void) {
-  foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh no foo}}
+  foo(); // expected-error {{call to foo declared with 'error' attribute: oh no foo}}
   if (x())
-    bar(); // expected-error {{call to 'bar' declared with 'error' attribute: oh no bar}}
+    bar(); // expected-error {{call to bar declared with 'error' attribute: oh no bar}}
 
-  quux();                     // enabled-warning {{call to 'quux' declared with 'warning' attribute: oh no quux}}
-  __compiletime_assert_455(); // expected-error {{call to '__compiletime_assert_455' declared with 'error' attribute: demangle me}}
-  duplicate_errors();         // expected-error {{call to 'duplicate_errors' declared with 'error' attribute: two}}
-  duplicate_warnings();       // enabled-warning {{call to 'duplicate_warnings' declared with 'warning' attribute: two}}
+  quux();                     // enabled-warning {{call to quux declared with 'warning' attribute: oh no quux}}
+  __compiletime_assert_455(); // expected-error {{call to __compiletime_assert_455 declared with 'error' attribute: demangle me}}
+  duplicate_errors();         // expected-error {{call to duplicate_errors declared with 'error' attribute: two}}
+  duplicate_warnings();       // enabled-warning {{call to duplicate_warnings declared with 'warning' attribute: two}}
 }
-
-#ifdef __cplusplus
-template <typename T>
-__attribute__((error("demangle me, too")))
-T
-nocall(T t);
-
-struct Widget {
-  __attribute__((warning("don't call me!")))
-  operator int() { return 42; }
-};
-
-void baz_cpp(void) {
-  foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh no foo}}
-  if (x())
-    bar(); // expected-error {{call to 'bar' declared with 'error' attribute: oh no bar}}
-  quux();  // enabled-warning {{call to 'quux' declared with 'warning' attribute: oh no quux}}
-
-  // Test that we demangle correctly in the diagnostic for C++.
-  __compiletime_assert_455(); // expected-error {{call to '__compiletime_assert_455' declared with 'error' attribute: demangle me}}
-  nocall<int>(42);            // expected-error {{call to 'nocall<int>' declared with 'error' attribute: demangle me, too}}
-
-  Widget W;
-  int w = W; // enabled-warning {{'operator int' declared with 'warning' attribute: don't call me!}}
-}
-#endif

diff --git a/clang/test/Frontend/backend-attribute-error-warning.cpp b/clang/test/Frontend/backend-attribute-error-warning.cpp
new file mode 100644 (file)
index 0000000..0338fe4
--- /dev/null
+++ b/clang/test/Frontend/backend-attribute-error-warning.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -verify=expected,enabled -emit-codegen-only %s
+// RUN: %clang_cc1 -verify -emit-codegen-only -Wno-attribute-warning %s
+
+__attribute__((error("oh no foo"))) void foo(void);
+
+__attribute__((error("oh no bar"))) void bar(void);
+
+int x(void) {
+  return 8 % 2 == 1;
+}
+
+__attribute__((warning("oh no quux"))) void quux(void);
+
+__attribute__((error("demangle me"))) void __compiletime_assert_455(void);
+
+__attribute__((error("one"), error("two"))) // expected-warning {{attribute 'error' is already applied with different arguments}}
+void                                        // expected-note@-1 {{previous attribute is here}}
+duplicate_errors(void);
+
+__attribute__((warning("one"), warning("two"))) // expected-warning {{attribute 'warning' is already applied with different arguments}}
+void                                            // expected-note@-1 {{previous attribute is here}}
+duplicate_warnings(void);
+
+void baz(void) {
+  foo(); // expected-error {{call to foo() declared with 'error' attribute: oh no foo}}
+  if (x())
+    bar(); // expected-error {{call to bar() declared with 'error' attribute: oh no bar}}
+
+  quux();                     // enabled-warning {{call to quux() declared with 'warning' attribute: oh no quux}}
+  __compiletime_assert_455(); // expected-error {{call to __compiletime_assert_455() declared with 'error' attribute: demangle me}}
+  duplicate_errors();         // expected-error {{call to duplicate_errors() declared with 'error' attribute: two}}
+  duplicate_warnings();       // enabled-warning {{call to duplicate_warnings() declared with 'warning' attribute: two}}
+}
+
+#ifdef __cplusplus
+template <typename T>
+__attribute__((error("demangle me, too")))
+T
+nocall(T t);
+
+struct Widget {
+  __attribute__((warning("don't call me!")))
+  operator int() { return 42; }
+};
+
+void baz_cpp(void) {
+  foo(); // expected-error {{call to foo() declared with 'error' attribute: oh no foo}}
+  if (x())
+    bar(); // expected-error {{call to bar() declared with 'error' attribute: oh no bar}}
+  quux();  // enabled-warning {{call to quux() declared with 'warning' attribute: oh no quux}}
+
+  // Test that we demangle correctly in the diagnostic for C++.
+  __compiletime_assert_455(); // expected-error {{call to __compiletime_assert_455() declared with 'error' attribute: demangle me}}
+  nocall<int>(42);            // expected-error {{call to int nocall<int>(int) declared with 'error' attribute: demangle me, too}}
+
+  Widget W;
+  int w = W; // enabled-warning {{Widget::operator int() declared with 'warning' attribute: don't call me!}}
+}
+#endif

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 8240b8d..a40a161 100644 (file)
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -1594,12 +1594,18 @@ example:
     ``disable_sanitizer_instrumentation`` disables all kinds of instrumentation,
     taking precedence over the ``sanitize_<name>`` attributes and other compiler
     flags.
-``"dontcall"``
-    This attribute denotes that a diagnostic should be emitted when a call of a
-    function with this attribute is not eliminated via optimization. Front ends
-    can provide optional ``srcloc`` metadata nodes on call sites of such
-    callees to attach information about where in the source language such a
-    call came from.
+``"dontcall-error"``
+    This attribute denotes that an error diagnostic should be emitted when a
+    call of a function with this attribute is not eliminated via optimization.
+    Front ends can provide optional ``srcloc`` metadata nodes on call sites of
+    such callees to attach information about where in the source language such a
+    call came from. A string value can be provided as a note.
+``"dontcall-warn"``
+    This attribute denotes that a warning diagnostic should be emitted when a
+    call of a function with this attribute is not eliminated via optimization.
+    Front ends can provide optional ``srcloc`` metadata nodes on call sites of
+    such callees to attach information about where in the source language such a
+    call came from. A string value can be provided as a note.
 ``"frame-pointer"``
     This attribute tells the code generator whether the function
     should keep the frame pointer. The code generator may emit the frame pointer

diff --git a/llvm/include/llvm/IR/DiagnosticInfo.h b/llvm/include/llvm/IR/DiagnosticInfo.h
index 44047d9..73b0be4 100644 (file)
--- a/llvm/include/llvm/IR/DiagnosticInfo.h
+++ b/llvm/include/llvm/IR/DiagnosticInfo.h
@@ -33,6 +33,7 @@ namespace llvm {
 
 // Forward declarations.
 class DiagnosticPrinter;
+class CallInst;
 class Function;
 class Instruction;
 class InstructionCost;
@@ -1070,15 +1071,20 @@ public:
   }
 };
 
+void diagnoseDontCall(const CallInst &CI);
+
 class DiagnosticInfoDontCall : public DiagnosticInfo {
   StringRef CalleeName;
+  StringRef Note;
   unsigned LocCookie;
 
 public:
-  DiagnosticInfoDontCall(StringRef CalleeName, unsigned LocCookie)
-      : DiagnosticInfo(DK_DontCall, DS_Error), CalleeName(CalleeName),
+  DiagnosticInfoDontCall(StringRef CalleeName, StringRef Note,
+                         DiagnosticSeverity DS, unsigned LocCookie)
+      : DiagnosticInfo(DK_DontCall, DS), CalleeName(CalleeName), Note(Note),
         LocCookie(LocCookie) {}
   StringRef getFunctionName() const { return CalleeName; }
+  StringRef getNote() const { return Note; }
   unsigned getLocCookie() const { return LocCookie; }
   void print(DiagnosticPrinter &DP) const override;
   static bool classof(const DiagnosticInfo *DI) {

diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index e27abdc..13a0da1 100644 (file)
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2344,14 +2344,7 @@ bool IRTranslator::translateCall(const User &U, MachineIRBuilder &MIRBuilder) {
   if (CI.isInlineAsm())
     return translateInlineAsm(CI, MIRBuilder);
 
-  if (F && F->hasFnAttribute("dontcall")) {
-    unsigned LocCookie = 0;
-    if (MDNode *MD = CI.getMetadata("srcloc"))
-      LocCookie =
-          mdconst::extract<ConstantInt>(MD->getOperand(0))->getZExtValue();
-    DiagnosticInfoDontCall D(F->getName(), LocCookie);
-    F->getContext().diagnose(D);
-  }
+  diagnoseDontCall(CI);
 
   Intrinsic::ID ID = Intrinsic::not_intrinsic;
   if (F && F->isIntrinsic()) {

diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
index 251e698..7af0db1 100644 (file)
--- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1152,15 +1152,7 @@ bool FastISel::lowerCall(const CallInst *CI) {
   CLI.setCallee(RetTy, FuncTy, CI->getCalledOperand(), std::move(Args), *CI)
       .setTailCall(IsTailCall);
 
-  if (const Function *F = CI->getCalledFunction())
-    if (F->hasFnAttribute("dontcall")) {
-      unsigned LocCookie = 0;
-      if (MDNode *MD = CI->getMetadata("srcloc"))
-        LocCookie =
-            mdconst::extract<ConstantInt>(MD->getOperand(0))->getZExtValue();
-      DiagnosticInfoDontCall D(F->getName(), LocCookie);
-      F->getContext().diagnose(D);
-    }
+  diagnoseDontCall(*CI);
 
   return lowerCallTo(CLI);
 }

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 6b09d39..7c16652 100644 (file)
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -8036,14 +8036,7 @@ void SelectionDAGBuilder::visitCall(const CallInst &I) {
   }
 
   if (Function *F = I.getCalledFunction()) {
-    if (F->hasFnAttribute("dontcall")) {
-      unsigned LocCookie = 0;
-      if (MDNode *MD = I.getMetadata("srcloc"))
-        LocCookie =
-            mdconst::extract<ConstantInt>(MD->getOperand(0))->getZExtValue();
-      DiagnosticInfoDontCall D(F->getName(), LocCookie);
-      DAG.getContext()->diagnose(D);
-    }
+    diagnoseDontCall(I);
 
     if (F->isDeclaration()) {
       // Is this an LLVM intrinsic or a target-specific intrinsic?

diff --git a/llvm/lib/IR/DiagnosticInfo.cpp b/llvm/lib/IR/DiagnosticInfo.cpp
index 4492e2e..3d0f13b 100644 (file)
--- a/llvm/lib/IR/DiagnosticInfo.cpp
+++ b/llvm/lib/IR/DiagnosticInfo.cpp
@@ -400,6 +400,34 @@ std::string DiagnosticInfoOptimizationBase::getMsg() const {
 void OptimizationRemarkAnalysisFPCommute::anchor() {}
 void OptimizationRemarkAnalysisAliasing::anchor() {}
 
+void llvm::diagnoseDontCall(const CallInst &CI) {
+  auto *F = CI.getCalledFunction();
+  if (!F)
+    return;
+
+  for (int i = 0; i != 2; ++i) {
+    auto AttrName = i == 0 ? "dontcall-error" : "dontcall-warn";
+    auto Sev = i == 0 ? DS_Error : DS_Warning;
+
+    if (F->hasFnAttribute(AttrName)) {
+      unsigned LocCookie = 0;
+      auto A = F->getFnAttribute(AttrName);
+      if (MDNode *MD = CI.getMetadata("srcloc"))
+        LocCookie =
+            mdconst::extract<ConstantInt>(MD->getOperand(0))->getZExtValue();
+      DiagnosticInfoDontCall D(F->getName(), A.getValueAsString(), Sev,
+                               LocCookie);
+      F->getContext().diagnose(D);
+    }
+  }
+}
+
 void DiagnosticInfoDontCall::print(DiagnosticPrinter &DP) const {
-  DP << "call to " << getFunctionName() << " marked \"dontcall\"";
+  DP << "call to " << getFunctionName() << " marked \"dontcall-";
+  if (getSeverity() == DiagnosticSeverity::DS_Error)
+    DP << "error\"";
+  else
+    DP << "warn\"";
+  if (!getNote().empty())
+    DP << ": " << getNote();
 }

diff --git a/llvm/test/CodeGen/X86/attr-dontcall.ll b/llvm/test/CodeGen/X86/attr-dontcall.ll
index 7cdac32..4c2595f 100644 (file)
--- a/llvm/test/CodeGen/X86/attr-dontcall.ll
+++ b/llvm/test/CodeGen/X86/attr-dontcall.ll
@@ -2,10 +2,24 @@
 ; RUN: not llc -mtriple=x86_64 -global-isel=0 -fast-isel=1 -stop-after=finalize-isel < %s 2>&1 | FileCheck %s
 ; RUN: not llc -mtriple=x86_64 -global-isel=1 -fast-isel=0 -stop-after=irtranslator -global-isel-abort=0 < %s 2>&1 | FileCheck %s
 
-declare void @foo() "dontcall"
+declare void @foo() "dontcall-error"="e"
 define void @bar() {
   call void @foo()
   ret void
 }
 
-; CHECK: error: call to foo marked "dontcall"
+declare void @foo2() "dontcall-warn"="w"
+define void @bar2() {
+  call void @foo2()
+  ret void
+}
+
+declare void @foo3() "dontcall-warn"
+define void @bar3() {
+  call void @foo3()
+  ret void
+}
+
+; CHECK: error: call to foo marked "dontcall-error": e
+; CHECK: warning: call to foo2 marked "dontcall-warn": w
+; CHECK: warning: call to foo3 marked "dontcall-warn"{{$}}

diff --git a/llvm/test/ThinLTO/X86/dontcall.ll b/llvm/test/ThinLTO/X86/dontcall.ll
index ef5eae9..baacf29 100644 (file)
--- a/llvm/test/ThinLTO/X86/dontcall.ll
+++ b/llvm/test/ThinLTO/X86/dontcall.ll
@@ -7,16 +7,16 @@
 ; RUN:   -r=%t/b.bc,caller,px
 
 ; TODO: As part of LTO, we check that types match, but *we don't yet check that
-; attributes match!!! What should happen if we remove "dontcall" from the
+; attributes match!!! What should happen if we remove "dontcall-error" from the
 ; definition or declaration of @callee?
 
-; CHECK: call to callee marked "dontcall"
+; CHECK: call to callee marked "dontcall-error"
 
 ;--- a.s
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-define i32 @callee() "dontcall" noinline {
+define i32 @callee() "dontcall-error" noinline {
   ret i32 42
 }
 
@@ -24,7 +24,7 @@ define i32 @callee() "dontcall" noinline {
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-declare i32 @callee() "dontcall"
+declare i32 @callee() "dontcall-error"
 
 define i32 @caller() {
 entry: