nickdesaulniers created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
nickdesaulniers requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
Debugging functions that have fallen through to the next function is not
fun. So much so that the Linux kernel has a host utility called objtool
that can report such cases of fallthrough (for architectures objtool
supports). Example:
drivers/media/i2c/m5mols/m5mols.o: warning: objtool:
m5mols_set_fmt() falls through to next function __cfi_m5mols_open()
With this new diagnostic, -Wfunction-fallthrough, we can produce a
similar diagnostic:
drivers/media/i2c/m5mols/m5mols_core.c:573:12: warning: function
'm5mols_set_fmt' falls through to next function
[-Wfunction-fallthrough]
static int m5mols_set_fmt(struct v4l2_subdev *sd,
^
This can help diagnose the improper usage of __builtin_unreachable(). It
can also help us spot potential codegen bugs in LLVM itself.
Example issues faced in the past:
https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aissue+falls+label%3A%22%5BTOOL%5D+objtool%22+
Feature request:
https://lore.kernel.org/llvm/CAHk-=wgtsdkybmb1jym5vmhmcd9j9uzr0mn7boym_ludrp+...@mail.gmail.com/
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D146466
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticFrontendKinds.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/lib/CodeGen/CodeGenAction.cpp
clang/test/Frontend/backend-function-fallthrough.c
llvm/include/llvm/IR/DiagnosticInfo.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/IR/DiagnosticInfo.cpp
llvm/test/CodeGen/AArch64/function-fallthrough.ll
Index: llvm/test/CodeGen/AArch64/function-fallthrough.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/AArch64/function-fallthrough.ll
@@ -0,0 +1,8 @@
+; RUN: llc %s 2>&1 | FileCheck %s
+declare void @foo ()
+
+define void @bar () {
+; CHECK: warning: function 'bar' falls through to next function
+entry:
+ unreachable
+}
Index: llvm/lib/IR/DiagnosticInfo.cpp
===================================================================
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -450,3 +450,7 @@
if (!getNote().empty())
DP << ": " << getNote();
}
+
+void DiagnosticInfoFunctionFallthrough::print(DiagnosticPrinter &DP) const {
+ DP << "function '" << getFunction().getName() << "' falls through to next function";
+}
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===================================================================
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -66,6 +66,7 @@
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/IR/EHPersonalities.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GCStrategy.h"
@@ -2388,6 +2389,30 @@
return Res.first->second;
}
+static void diagnoseFunctionFallthrough(MachineFunction &MF) {
+ bool FT = false;
+ MachineBasicBlock &LastMBB = MF.back();
+ if (LastMBB.canFallThrough())
+ FT = true;
+ // TODO: MachineBasicBlock::canFallThrough is too conservative; it will not
+ // catch this!
+ else if (LastMBB.empty())
+ FT = true;
+ // TODO: TII::isTailCall?
+ else if (LastMBB.succ_empty() && !LastMBB.isReturnBlock())
+ // TODO: MachineBasicBlock::canFallThrough is too conservative; it will not
+ // catch this!
+ // Don't call MachineBasicBlock::back() without a
+ // !MachineBasicBlock::empty() guard.
+ if (const Function *Callee = LastMBB.back().getCalledFunction())
+ FT = !Callee->hasFnAttribute(Attribute::NoReturn);
+ if (FT) {
+ Function &F = MF.getFunction();
+ DiagnosticInfoFunctionFallthrough D(F);
+ F.getContext().diagnose(D);
+ }
+}
+
void AsmPrinter::SetupMachineFunction(MachineFunction &MF) {
this->MF = &MF;
const Function &F = MF.getFunction();
@@ -2436,6 +2461,8 @@
}
ORE = &getAnalysis<MachineOptimizationRemarkEmitterPass>().getORE();
+
+ diagnoseFunctionFallthrough(MF);
}
namespace {
Index: llvm/include/llvm/IR/DiagnosticInfo.h
===================================================================
--- llvm/include/llvm/IR/DiagnosticInfo.h
+++ llvm/include/llvm/IR/DiagnosticInfo.h
@@ -20,6 +20,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
#include "llvm/IR/DebugLoc.h"
+#include "llvm/IR/Function.h"
#include "llvm/Support/CBindingWrapping.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/SourceMgr.h"
@@ -38,7 +39,6 @@
class DIFile;
class DISubprogram;
class CallInst;
-class Function;
class Instruction;
class InstructionCost;
class Module;
@@ -86,6 +86,7 @@
DK_SrcMgr,
DK_DontCall,
DK_MisExpect,
+ DK_FunctionFallthrough,
DK_FirstPluginKind // Must be last value to work with
// getNextAvailablePluginDiagnosticKind
};
@@ -1051,6 +1052,17 @@
const Twine &Msg;
};
+struct DiagnosticInfoFunctionFallthrough : public DiagnosticInfoWithLocationBase {
+ DiagnosticInfoFunctionFallthrough(Function &F) :
+ DiagnosticInfoWithLocationBase(DK_FunctionFallthrough, DS_Warning, F,
+ F.getSubprogram())
+ {}
+ static bool classof(const DiagnosticInfo *DI) {
+ return DI->getKind() == DK_FunctionFallthrough;
+ }
+ void print(DiagnosticPrinter &DP) const override;
+};
+
static DiagnosticSeverity getDiagnosticSeverity(SourceMgr::DiagKind DK) {
switch (DK) {
case llvm::SourceMgr::DK_Error:
Index: clang/test/Frontend/backend-function-fallthrough.c
===================================================================
--- /dev/null
+++ clang/test/Frontend/backend-function-fallthrough.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -O0 -verify -emit-codegen-only %s
+// RUN: %clang_cc1 -O2 -verify=expected,optimization -emit-codegen-only %s
+void foo (void);
+int bar (int x) { // optimization-warning {{function 'bar' falls through to next function}}
+ if (x) {
+ foo();
+ __builtin_unreachable();
+ }
+ return 42;
+}
+void baz (void) { // expected-warning {{function 'baz' falls through to next function}}
+ __builtin_unreachable();
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -462,6 +462,7 @@
/// Specialized handler for misexpect warnings.
/// Note that misexpect remarks are emitted through ORE
void MisExpectDiagHandler(const llvm::DiagnosticInfoMisExpect &D);
+ void FunctionFallthroughHandler(const DiagnosticInfoFunctionFallthrough &DI);
};
void BackendConsumer::anchor() {}
@@ -875,6 +876,11 @@
<< Filename << Line << Column;
}
+void BackendConsumer::FunctionFallthroughHandler(const DiagnosticInfoFunctionFallthrough &DI) {
+ if (std::optional<FullSourceLoc> Loc = getFunctionSourceLocation(DI.getFunction()))
+ Diags.Report(*Loc, diag::warn_fe_function_fallthrough) << DI.getFunction().getName();
+}
+
/// This function is invoked when the backend needs
/// to report something to the user.
void BackendConsumer::DiagnosticHandlerImpl(const DiagnosticInfo &DI) {
@@ -957,6 +963,9 @@
case llvm::DK_MisExpect:
MisExpectDiagHandler(cast<DiagnosticInfoMisExpect>(DI));
return;
+ case llvm::DK_FunctionFallthrough:
+ FunctionFallthroughHandler(cast<DiagnosticInfoFunctionFallthrough>(DI));
+ return;
default:
// Plugin IDs are not bound to any value as they are set dynamically.
ComputeDiagRemarkID(Severity, backend_plugin, DiagID);
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1296,6 +1296,12 @@
def BackendOptimizationRemarkAnalysis : DiagGroup<"pass-analysis">;
def BackendOptimizationFailure : DiagGroup<"pass-failed">;
def BackendWarningAttributes : DiagGroup<"attribute-warning">;
+def BackendWarningFunctionFallthrough : DiagGroup<"function-fallthrough">{
+ code Documentation = [{
+The function falls through to the next function. This can occur due to either
+the incorrect use of __builtin_unreachable() or potentially a bug in LLVM.
+}];
+}
// Instrumentation based profiling warnings.
def ProfileInstrMissing : DiagGroup<"profile-instr-missing">;
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -90,6 +90,9 @@
def warn_fe_backend_warning_attr :
Warning<"call to '%0' declared with 'warning' attribute: %1">, BackendInfo,
InGroup<BackendWarningAttributes>;
+def warn_fe_function_fallthrough:
+ Warning<"function '%0' falls through to next function">,
+ InGroup<BackendWarningFunctionFallthrough>;
def err_fe_invalid_code_complete_file : Error<
"cannot locate code-completion file %0">, DefaultFatal;
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -179,6 +179,8 @@
- Clang now avoids duplicate warnings on unreachable ``[[fallthrough]];`` statements
previously issued from ``-Wunreachable-code`` and ``-Wunreachable-code-fallthrough``
by prioritizing ``-Wunreachable-code-fallthrough``.
+- New diagnostic ``-Wfunction-fallthrough`` warns when a function falls through
+ to the next function.
Bug Fixes in This Version
-------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
