https://github.com/wizardengineer updated https://github.com/llvm/llvm-project/pull/169081
>From d015af2cb7ca61a0445a78781b31178e3f6fa9f2 Mon Sep 17 00:00:00 2001 From: Jasmine Tang <[email protected]> Date: Thu, 20 Nov 2025 05:23:58 -0800 Subject: [PATCH 1/7] Skeleton --- .../include/clang/CIR/Sema/CIRAnalysisKind.h | 117 ++++++++++++ .../clang/CIR/Sema/FallThroughWarning.h | 66 +++++++ .../include/clang/Frontend/FrontendOptions.h | 3 + clang/include/clang/Options/Options.td | 5 + clang/lib/CIR/CMakeLists.txt | 1 + clang/lib/CIR/FrontendAction/CIRGenAction.cpp | 36 ++++ clang/lib/CIR/FrontendAction/CMakeLists.txt | 1 + clang/lib/CIR/Sema/CIRAnalysisKind.cpp | 67 +++++++ clang/lib/CIR/Sema/CMakeLists.txt | 19 ++ clang/lib/CIR/Sema/FallThroughWarning.cpp | 170 ++++++++++++++++++ 10 files changed, 485 insertions(+) create mode 100644 clang/include/clang/CIR/Sema/CIRAnalysisKind.h create mode 100644 clang/include/clang/CIR/Sema/FallThroughWarning.h create mode 100644 clang/lib/CIR/Sema/CIRAnalysisKind.cpp create mode 100644 clang/lib/CIR/Sema/CMakeLists.txt create mode 100644 clang/lib/CIR/Sema/FallThroughWarning.cpp diff --git a/clang/include/clang/CIR/Sema/CIRAnalysisKind.h b/clang/include/clang/CIR/Sema/CIRAnalysisKind.h new file mode 100644 index 0000000000000..304acfcf433db --- /dev/null +++ b/clang/include/clang/CIR/Sema/CIRAnalysisKind.h @@ -0,0 +1,117 @@ +//===--- CIRAnalysisKind.h - CIR Analysis Pass Kinds -----------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +/// \file +/// Defines the CIR analysis pass kinds enum and related utilities. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_CIR_SEMA_CIRANALYSISKIND_H +#define LLVM_CLANG_CIR_SEMA_CIRANALYSISKIND_H + +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/raw_ostream.h" +#include <optional> +#include <string> +#include <vector> + +namespace cir { + +/// Enumeration of available CIR semantic analysis passes +enum class CIRAnalysisKind : unsigned { + Unrecognized = 0, + FallThrough = 1 << 0, // Fallthrough warning analysis + UnreachableCode = 1 << 1, // Unreachable code detection + NullCheck = 1 << 2, // Null pointer checks + UninitializedVar = 1 << 3, // Uninitialized variable detection + // Add more analysis passes here as needed +}; + +/// A set of CIR analysis passes (bitmask) +class CIRAnalysisSet { + unsigned mask = 0; + +public: + CIRAnalysisSet() = default; + explicit CIRAnalysisSet(CIRAnalysisKind kind) + : mask(static_cast<unsigned>(kind)) {} + explicit CIRAnalysisSet(unsigned mask) : mask(mask) {} + + /// Check if a specific analysis is enabled + bool has(CIRAnalysisKind kind) const { + return (mask & static_cast<unsigned>(kind)) != 0; + } + + /// Enable a specific analysis + void enable(CIRAnalysisKind kind) { + mask |= static_cast<unsigned>(kind); + } + + /// Disable a specific analysis + void disable(CIRAnalysisKind kind) { + mask &= ~static_cast<unsigned>(kind); + } + + /// Check if any analysis is enabled + bool hasAny() const { return mask != 0; } + + /// Check if no analysis is enabled + bool empty() const { return mask == 0; } + + /// Get the raw mask value + unsigned getMask() const { return mask; } + + /// Union with another set + CIRAnalysisSet &operator|=(const CIRAnalysisSet &other) { + mask |= other.mask; + return *this; + } + + /// Union operator + CIRAnalysisSet operator|(const CIRAnalysisSet &other) const { + return CIRAnalysisSet(mask | other.mask); + } + + /// Intersection with another set + CIRAnalysisSet &operator&=(const CIRAnalysisSet &other) { + mask &= other.mask; + return *this; + } + + /// Intersection operator + CIRAnalysisSet operator&(const CIRAnalysisSet &other) const { + return CIRAnalysisSet(mask & other.mask); + } + + bool operator==(const CIRAnalysisSet &other) const { + return mask == other.mask; + } + + bool operator!=(const CIRAnalysisSet &other) const { + return mask != other.mask; + } + + /// Print the analysis set to an output stream + void print(llvm::raw_ostream &OS) const; +}; + +/// Parse a single analysis name into a CIRAnalysisKind +/// Returns std::nullopt if the name is not recognized +CIRAnalysisKind parseCIRAnalysisKind(llvm::StringRef name); + +/// Parse a list of analysis names (from command line) into a CIRAnalysisSet +/// Handles comma and semicolon separators +/// Invalid names are ignored and optionally reported via InvalidNames +CIRAnalysisSet parseCIRAnalysisList( + const std::vector<std::string> &analysisList, + llvm::SmallVectorImpl<std::string> *invalidNames = nullptr); + +} // namespace cir + +#endif // LLVM_CLANG_CIR_SEMA_CIRANALYSISKIND_H diff --git a/clang/include/clang/CIR/Sema/FallThroughWarning.h b/clang/include/clang/CIR/Sema/FallThroughWarning.h new file mode 100644 index 0000000000000..808cf43f92ce0 --- /dev/null +++ b/clang/include/clang/CIR/Sema/FallThroughWarning.h @@ -0,0 +1,66 @@ +//===--- FallThroughWarning.h - CIR Fall-Through Analysis ------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +/// \file +/// Defines the FallThroughWarningPass for CIR fall-through analysis. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_CIR_SEMA_FALLTHROUGHWARNING_H +#define LLVM_CLANG_CIR_SEMA_FALLTHROUGHWARNING_H + +#include "clang/AST/Type.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Sema/AnalysisBasedWarnings.h" + +#include "mlir/IR/Block.h" +#include "mlir/IR/Operation.h" +#include "mlir/Support/LLVM.h" +namespace cir { +class FuncOp; +} // namespace cir + +namespace clang { +class Sema; + +enum ControlFlowKind { + UnknownFallThrough, + NeverFallThrough, + MaybeFallThrough, + AlwaysFallThrough, + NeverFallThroughOrReturn +}; + +/// Configuration for fall-through diagnostics +struct CheckFallThroughDiagnostics { + unsigned diagFallThrough = 0; + unsigned diagReturn = 0; + unsigned diagFallThroughAttr = 0; + unsigned funKind = 0; + SourceLocation funcLoc; + + bool checkDiagnostics(DiagnosticsEngine &d, bool returnsVoid, + bool hasNoReturn) const; +}; + +/// Pass for analyzing fall-through behavior in CIR functions +class FallThroughWarningPass { +public: + FallThroughWarningPass() = default; + + /// Check fall-through behavior for a CIR function body + void checkFallThroughForFuncBody(Sema &s, cir::FuncOp cfg, QualType blockType, + const CheckFallThroughDiagnostics &cd); + ControlFlowKind checkFallThrough(cir::FuncOp cfg); + mlir::DenseSet<mlir::Block *> getLiveSet(cir::FuncOp cfg); +}; + +} // namespace clang + +#endif // LLVM_CLANG_CIR_SEMA_FALLTHROUGHWARNING_H diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h index c919a53ae089e..a152309212bf8 100644 --- a/clang/include/clang/Frontend/FrontendOptions.h +++ b/clang/include/clang/Frontend/FrontendOptions.h @@ -420,6 +420,9 @@ class FrontendOptions { LLVM_PREFERRED_TYPE(bool) unsigned ClangIRDisableCIRVerifier : 1; + /// List of ClangIR semantic analysis passes to enable + std::vector<std::string> ClangIRAnalysisList; + CodeCompleteOptions CodeCompleteOpts; /// Specifies the output format of the AST. diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index 2f7434d8afe11..16821a7b6f230 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -3150,6 +3150,11 @@ def clangir_disable_verifier : Flag<["-"], "clangir-disable-verifier">, HelpText<"ClangIR: Disable MLIR module verifier">, MarshallingInfoFlag<FrontendOpts<"ClangIRDisableCIRVerifier">>; +def fclangir_analysis_EQ : CommaJoined<["-"], "fclangir-analysis=">, + Visibility<[ClangOption, CC1Option]>, + HelpText<"Enable ClangIR semantic analysis passes. Pass comma or semicolon separated list of analysis names">, + MarshallingInfoStringVector<FrontendOpts<"ClangIRAnalysisList">>; + defm clangir : BoolFOption<"clangir", FrontendOpts<"UseClangIRPipeline">, DefaultFalse, PosFlag<SetTrue, [], [ClangOption, CC1Option], "Use the ClangIR pipeline to compile">, diff --git a/clang/lib/CIR/CMakeLists.txt b/clang/lib/CIR/CMakeLists.txt index 7bdf3fcc59035..f86ad0933bce6 100644 --- a/clang/lib/CIR/CMakeLists.txt +++ b/clang/lib/CIR/CMakeLists.txt @@ -17,3 +17,4 @@ add_subdirectory(CodeGen) add_subdirectory(FrontendAction) add_subdirectory(Interfaces) add_subdirectory(Lowering) +add_subdirectory(Sema) diff --git a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp index 67bb5657d4001..fb86b5f848628 100644 --- a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp +++ b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp @@ -10,12 +10,20 @@ #include "mlir/IR/MLIRContext.h" #include "mlir/IR/OwningOpRef.h" #include "clang/Basic/DiagnosticFrontend.h" +#include "clang/Basic/DiagnosticSema.h" #include "clang/CIR/CIRGenerator.h" #include "clang/CIR/CIRToCIRPasses.h" +#include "clang/CIR/Dialect/IR/CIRDialect.h" #include "clang/CIR/LowerToLLVM.h" +#include "clang/CIR/Sema/CIRAnalysisKind.h" +#include "clang/CIR/Sema/FallThroughWarning.h" #include "clang/CodeGen/BackendUtil.h" #include "clang/Frontend/CompilerInstance.h" +#include "clang/Sema/AnalysisBasedWarnings.h" +#include "clang/Sema/Sema.h" #include "llvm/IR/Module.h" +#include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/raw_ostream.h" using namespace cir; using namespace clang; @@ -108,6 +116,34 @@ class CIRGenConsumer : public clang::ASTConsumer { mlir::ModuleOp MlirModule = Gen->getModule(); mlir::MLIRContext &MlirCtx = Gen->getMLIRContext(); + // Run CIR analysis passes if requested + if (!FEOptions.ClangIRAnalysisList.empty()) { + CIRAnalysisSet AnalysisSet = parseCIRAnalysisList(FEOptions.ClangIRAnalysisList); + AnalysisSet.print(llvm::errs()); + + if (AnalysisSet.has(CIRAnalysisKind::FallThrough)) { + // Get Sema for diagnostics + if (CI.hasSema()) { + Sema &S = CI.getSema(); + FallThroughWarningPass FallThroughPass; + + // Iterate over all functions in the CIR module + MlirModule.walk([&](cir::FuncOp FuncOp) { + // TODO: Get the proper QualType for the function + // For now, use an invalid QualType as placeholder + + QualType FuncType; + + // Set up diagnostics configuration + CheckFallThroughDiagnostics Diags; + + // Run fall-through analysis on this function + FallThroughPass.checkFallThroughForFuncBody(S, FuncOp, FuncType, Diags); + }); + } + } + } + if (!FEOptions.ClangIRDisablePasses) { // Setup and run CIR pipeline. if (runCIRToCIRPasses(MlirModule, MlirCtx, C, diff --git a/clang/lib/CIR/FrontendAction/CMakeLists.txt b/clang/lib/CIR/FrontendAction/CMakeLists.txt index 50d6ea7108ce1..f43ef2d8cbbf5 100644 --- a/clang/lib/CIR/FrontendAction/CMakeLists.txt +++ b/clang/lib/CIR/FrontendAction/CMakeLists.txt @@ -18,6 +18,7 @@ add_clang_library(clangCIRFrontendAction clangBasic clangFrontend clangCIR + clangCIRSema clangCIRLoweringCommon clangCIRLoweringDirectToLLVM clangCodeGen diff --git a/clang/lib/CIR/Sema/CIRAnalysisKind.cpp b/clang/lib/CIR/Sema/CIRAnalysisKind.cpp new file mode 100644 index 0000000000000..3ae24979cfe68 --- /dev/null +++ b/clang/lib/CIR/Sema/CIRAnalysisKind.cpp @@ -0,0 +1,67 @@ +//===--- CIRAnalysisKind.cpp - CIR Analysis Pass Kinds -------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/CIR/Sema/CIRAnalysisKind.h" +#include "llvm/ADT/StringSwitch.h" +#include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/raw_ostream.h" +#include <optional> + +namespace cir { + +CIRAnalysisKind parseCIRAnalysisKind(llvm::StringRef name) { + auto parseResult = llvm::StringSwitch<CIRAnalysisKind>(name) + .Case("fallthrough", CIRAnalysisKind::FallThrough) + .Case("fall-through", CIRAnalysisKind::FallThrough) + .Default(CIRAnalysisKind::Unrecognized); + + return parseResult; +} + + +CIRAnalysisSet parseCIRAnalysisList( + const std::vector<std::string> &analysisList, + llvm::SmallVectorImpl<std::string> *invalidNames) { + CIRAnalysisSet result; + + for (const std::string &item : analysisList) { + llvm::StringRef remaining = item; + CIRAnalysisKind parseKind = parseCIRAnalysisKind(remaining); + if (parseKind == CIRAnalysisKind::Unrecognized) { + llvm::errs() << "Unrecognized CIR analysis option: " << remaining << "\n"; + continue; + } + result.enable(parseKind); + } + + return result; +} + +void CIRAnalysisSet::print(llvm::raw_ostream &OS) const { + if (empty()) { + OS << "none"; + return; + } + + bool first = true; + auto printIfEnabled = [&](CIRAnalysisKind kind, llvm::StringRef name) { + if (has(kind)) { + if (!first) + OS << ", "; + OS << name; + first = false; + } + }; + + printIfEnabled(CIRAnalysisKind::FallThrough, "fallthrough"); + printIfEnabled(CIRAnalysisKind::UnreachableCode, "unreachable-code"); + printIfEnabled(CIRAnalysisKind::NullCheck, "null-check"); + printIfEnabled(CIRAnalysisKind::UninitializedVar, "uninitialized-var"); +} + +} // namespace cir diff --git a/clang/lib/CIR/Sema/CMakeLists.txt b/clang/lib/CIR/Sema/CMakeLists.txt new file mode 100644 index 0000000000000..acf49742d8e4c --- /dev/null +++ b/clang/lib/CIR/Sema/CMakeLists.txt @@ -0,0 +1,19 @@ +add_clang_library(clangCIRSema + FallThroughWarning.cpp + CIRAnalysisKind.cpp + + DEPENDS + MLIRCIRPassIncGen + + LINK_LIBS PUBLIC + clangAST + clangBasic + + MLIRAnalysis + MLIRIR + MLIRPass + MLIRTransformUtils + + MLIRCIR + MLIRCIRInterfaces +) diff --git a/clang/lib/CIR/Sema/FallThroughWarning.cpp b/clang/lib/CIR/Sema/FallThroughWarning.cpp new file mode 100644 index 0000000000000..5fdc9ffc083cb --- /dev/null +++ b/clang/lib/CIR/Sema/FallThroughWarning.cpp @@ -0,0 +1,170 @@ +#include "clang/CIR/Sema/FallThroughWarning.h" +#include "clang/AST/ASTContext.h" +#include "clang/Basic/DiagnosticSema.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/TargetInfo.h" +#include "clang/CIR/Dialect/IR/CIRDialect.h" +#include "clang/CIR/Dialect/IR/CIRTypes.h" +#include "clang/Sema/Sema.h" +#include "llvm/Support/raw_ostream.h" + +using namespace mlir; +using namespace cir; +using namespace clang; + +namespace clang { + +//===----------------------------------------------------------------------===// +// Check for missing return value. +//===----------------------------------------------------------------------===// + +bool CheckFallThroughDiagnostics::checkDiagnostics(DiagnosticsEngine &d, + bool returnsVoid, + bool hasNoReturn) const { + if (funKind == diag::FalloffFunctionKind::Function) { + return (returnsVoid || d.isIgnored(diag::warn_falloff_nonvoid, funcLoc)) && + (!hasNoReturn || + d.isIgnored(diag::warn_noreturn_has_return_expr, funcLoc)) && + (!returnsVoid || + d.isIgnored(diag::warn_suggest_noreturn_block, funcLoc)); + } + if (funKind == diag::FalloffFunctionKind::Coroutine) { + return (returnsVoid || d.isIgnored(diag::warn_falloff_nonvoid, funcLoc)) && + (!hasNoReturn); + } + // For blocks / lambdas. + return returnsVoid && !hasNoReturn; +} + +// TODO: Add a class for fall through config later + +void FallThroughWarningPass::checkFallThroughForFuncBody( + Sema &s, cir::FuncOp cfg, QualType blockType, + const CheckFallThroughDiagnostics &cd) { + + llvm::errs() << "Hello world, you're in CIR sema analysis\n"; + bool returnsVoid = false; + bool hasNoReturn = false; + + // Supposedly all function in cir is FuncOp + // 1. If normal function (FunctionDecl), check if it's coroutine. + // 1a. if coroutine -> check the fallthrough handler (idk what this means, + // TODO for now) + if (cfg.getCoroutine()) { + // TODO: Let's not worry about coroutine for now + } else + returnsVoid = isa<cir::VoidType>(cfg.getFunctionType().getReturnType()); + + // TODO: Do we need to check for InferredNoReturnAttr just like in OG? + hasNoReturn = cfg.getFunctionType().getReturnTypes().empty(); + + DiagnosticsEngine &diags = s.getDiagnostics(); + if (cd.checkDiagnostics(diags, returnsVoid, hasNoReturn)) { + return; + } + + // cpu_dispatch functions permit empty function bodies for ICC compatibility. + // TODO: Do we have isCPUDispatchMultiVersion? + checkFallThrough(cfg); +} + +mlir::DenseSet<mlir::Block *> +FallThroughWarningPass::getLiveSet(cir::FuncOp cfg) { + mlir::DenseSet<mlir::Block *> liveSet; + if (cfg.getBody().empty()) + return liveSet; + + auto &first = cfg.getBody().getBlocks().front(); + + for (auto &block : cfg.getBody()) { + if (block.isReachable(&first)) + liveSet.insert(&block); + } + return liveSet; +} + +ControlFlowKind FallThroughWarningPass::checkFallThrough(cir::FuncOp cfg) { + + assert(cfg && "there can't be a null func op"); + + // TODO: Is no CFG akin to a declaration? + if (cfg.isDeclaration()) { + return UnknownFallThrough; + } + + mlir::DenseSet<mlir::Block *> liveSet = this->getLiveSet(cfg); + + unsigned count = liveSet.size(); + + bool hasLiveReturn = false; + bool hasFakeEdge = false; + bool hasPlainEdge = false; + bool hasAbnormalEdge = false; + + auto &exitBlock = cfg.getBody().back(); + // INFO: in OG clang CFG, they have an empty exit block, so when they query + // pred of exit OG, they get all exit blocks + // + // I guess in CIR, we can pretend exit blocks are all blocks that have no + // successor? + for (mlir::Block &pred : cfg.getBody().getBlocks()) { + if (!liveSet.contains(&pred)) + continue; + + // We consider no predecessors as 'exit blocks' + if (!pred.hasNoSuccessors()) + continue; + + if (!pred.mightHaveTerminator()) + continue; + + mlir::Operation *term = pred.getTerminator(); + if (isa<cir::ReturnOp>(term)) { + hasAbnormalEdge = true; + continue; + } + + // INFO: In OG, we'll be looking for destructor since it can appear past + // return but i guess not in CIR? In this case we'll only be examining the + // terminator + + if (isa<cir::TryOp>(term)) { + hasAbnormalEdge = true; + continue; + } + + // INFO: OG clang has this equals true whenever ri == re, which means this + // is true only when a block only has the terminator, or its size is 1. + hasPlainEdge = std::distance(pred.begin(), pred.end()) == 1; + + if (isa<cir::ReturnOp>(term)) { + hasLiveReturn = true; + continue; + } + if (isa<cir::TryOp>(term)) { + hasLiveReturn = true; + continue; + } + + // TODO: Maybe one day throw will be terminator? + // + // TODO: We need to add a microsoft inline assembly enum + + // TODO: We don't concer with try op either since it's not terminator + + hasPlainEdge = true; + } + + if (!hasPlainEdge) { + if (hasLiveReturn) + return NeverFallThrough; + return NeverFallThroughOrReturn; + } + if (hasAbnormalEdge || hasFakeEdge || hasLiveReturn) + return MaybeFallThrough; + // This says AlwaysFallThrough for calls to functions that are not marked + // noreturn, that don't return. If people would like this warning to be more + // accurate, such functions should be marked as noreturn. + return AlwaysFallThrough; +} +} // namespace clang >From 0eafeed1b29169fa5b8100084da13086855ca263 Mon Sep 17 00:00:00 2001 From: Jasmine Tang <[email protected]> Date: Thu, 20 Nov 2025 19:05:04 -0800 Subject: [PATCH 2/7] Fix reachable behaviors --- clang/lib/CIR/Sema/FallThroughWarning.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/CIR/Sema/FallThroughWarning.cpp b/clang/lib/CIR/Sema/FallThroughWarning.cpp index 5fdc9ffc083cb..dd22317fb48ba 100644 --- a/clang/lib/CIR/Sema/FallThroughWarning.cpp +++ b/clang/lib/CIR/Sema/FallThroughWarning.cpp @@ -77,7 +77,7 @@ FallThroughWarningPass::getLiveSet(cir::FuncOp cfg) { auto &first = cfg.getBody().getBlocks().front(); for (auto &block : cfg.getBody()) { - if (block.isReachable(&first)) + if (first.isReachable(&block)) liveSet.insert(&block); } return liveSet; >From 0e416f49e38bfef8b7a4520424e1495e17b78f86 Mon Sep 17 00:00:00 2001 From: Jasmine Tang <[email protected]> Date: Sun, 23 Nov 2025 01:33:00 -0800 Subject: [PATCH 3/7] Rename sema/ to analysis/ and add new test --- .../CIR/{Sema => Analysis}/CIRAnalysisKind.h | 0 .../{Sema => Analysis}/FallThroughWarning.h | 6 +- .../{Sema => Analysis}/CIRAnalysisKind.cpp | 2 +- .../lib/CIR/{Sema => Analysis}/CMakeLists.txt | 0 .../{Sema => Analysis}/FallThroughWarning.cpp | 58 ++++++++++++++++++- clang/lib/CIR/CMakeLists.txt | 2 +- clang/lib/CIR/FrontendAction/CIRGenAction.cpp | 4 +- clang/test/CIR/Analysis/fallthrough_1.c | 33 +++++++++++ clang/test/CIR/Analysis/fallthrough_2.c | 37 ++++++++++++ 9 files changed, 132 insertions(+), 10 deletions(-) rename clang/include/clang/CIR/{Sema => Analysis}/CIRAnalysisKind.h (100%) rename clang/include/clang/CIR/{Sema => Analysis}/FallThroughWarning.h (93%) rename clang/lib/CIR/{Sema => Analysis}/CIRAnalysisKind.cpp (97%) rename clang/lib/CIR/{Sema => Analysis}/CMakeLists.txt (100%) rename clang/lib/CIR/{Sema => Analysis}/FallThroughWarning.cpp (74%) create mode 100644 clang/test/CIR/Analysis/fallthrough_1.c create mode 100644 clang/test/CIR/Analysis/fallthrough_2.c diff --git a/clang/include/clang/CIR/Sema/CIRAnalysisKind.h b/clang/include/clang/CIR/Analysis/CIRAnalysisKind.h similarity index 100% rename from clang/include/clang/CIR/Sema/CIRAnalysisKind.h rename to clang/include/clang/CIR/Analysis/CIRAnalysisKind.h diff --git a/clang/include/clang/CIR/Sema/FallThroughWarning.h b/clang/include/clang/CIR/Analysis/FallThroughWarning.h similarity index 93% rename from clang/include/clang/CIR/Sema/FallThroughWarning.h rename to clang/include/clang/CIR/Analysis/FallThroughWarning.h index 808cf43f92ce0..3f87b3f867d65 100644 --- a/clang/include/clang/CIR/Sema/FallThroughWarning.h +++ b/clang/include/clang/CIR/Analysis/FallThroughWarning.h @@ -39,9 +39,9 @@ enum ControlFlowKind { /// Configuration for fall-through diagnostics struct CheckFallThroughDiagnostics { - unsigned diagFallThrough = 0; - unsigned diagReturn = 0; - unsigned diagFallThroughAttr = 0; + unsigned diagFallThroughHasNoReturn = 0; + unsigned diagFallThroughReturnsNonVoid = 0; + unsigned diagNeverFallThroughOrReturn = 0; unsigned funKind = 0; SourceLocation funcLoc; diff --git a/clang/lib/CIR/Sema/CIRAnalysisKind.cpp b/clang/lib/CIR/Analysis/CIRAnalysisKind.cpp similarity index 97% rename from clang/lib/CIR/Sema/CIRAnalysisKind.cpp rename to clang/lib/CIR/Analysis/CIRAnalysisKind.cpp index 3ae24979cfe68..f27f8dbeb731d 100644 --- a/clang/lib/CIR/Sema/CIRAnalysisKind.cpp +++ b/clang/lib/CIR/Analysis/CIRAnalysisKind.cpp @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// -#include "clang/CIR/Sema/CIRAnalysisKind.h" +#include "clang/CIR/Analysis/CIRAnalysisKind.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/raw_ostream.h" diff --git a/clang/lib/CIR/Sema/CMakeLists.txt b/clang/lib/CIR/Analysis/CMakeLists.txt similarity index 100% rename from clang/lib/CIR/Sema/CMakeLists.txt rename to clang/lib/CIR/Analysis/CMakeLists.txt diff --git a/clang/lib/CIR/Sema/FallThroughWarning.cpp b/clang/lib/CIR/Analysis/FallThroughWarning.cpp similarity index 74% rename from clang/lib/CIR/Sema/FallThroughWarning.cpp rename to clang/lib/CIR/Analysis/FallThroughWarning.cpp index dd22317fb48ba..d437eb6c377c4 100644 --- a/clang/lib/CIR/Sema/FallThroughWarning.cpp +++ b/clang/lib/CIR/Analysis/FallThroughWarning.cpp @@ -1,4 +1,4 @@ -#include "clang/CIR/Sema/FallThroughWarning.h" +#include "clang/CIR/Analysis/FallThroughWarning.h" #include "clang/AST/ASTContext.h" #include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/SourceLocation.h" @@ -6,6 +6,7 @@ #include "clang/CIR/Dialect/IR/CIRDialect.h" #include "clang/CIR/Dialect/IR/CIRTypes.h" #include "clang/Sema/Sema.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/raw_ostream.h" using namespace mlir; @@ -14,6 +15,36 @@ using namespace clang; namespace clang { +//===----------------------------------------------------------------------===// +// Helper function to lookup a Decl by name from ASTContext +//===----------------------------------------------------------------------===// + +/// Lookup a declaration by name in the translation unit. +/// \param Context The ASTContext to search in +/// \param Name The name of the declaration to find +/// \return The found Decl, or nullptr if not found + +/// WARN: I have to say, we only use this because a lot of the time, attribute +/// that we might need are not port to CIR currently so this function is +/// basically a crutch for that +static Decl *getDeclByName(ASTContext &context, StringRef name) { + // Get the identifier for the name + IdentifierInfo *ii = &context.Idents.get(name); + + // Create a DeclarationName from the identifier + DeclarationName dName(ii); + + // Lookup in the translation unit + TranslationUnitDecl *tu = context.getTranslationUnitDecl(); + DeclContext::lookup_result result = tu->lookup(dName); + + // Return the first match, or nullptr if not found + if (result.empty()) + return nullptr; + + return result.front(); +} + //===----------------------------------------------------------------------===// // Check for missing return value. //===----------------------------------------------------------------------===// @@ -41,8 +72,11 @@ bool CheckFallThroughDiagnostics::checkDiagnostics(DiagnosticsEngine &d, void FallThroughWarningPass::checkFallThroughForFuncBody( Sema &s, cir::FuncOp cfg, QualType blockType, const CheckFallThroughDiagnostics &cd) { - llvm::errs() << "Hello world, you're in CIR sema analysis\n"; + + auto *d = getDeclByName(s.getASTContext(), cfg.getName()); + assert(d && "we need non null decl"); + bool returnsVoid = false; bool hasNoReturn = false; @@ -65,7 +99,25 @@ void FallThroughWarningPass::checkFallThroughForFuncBody( // cpu_dispatch functions permit empty function bodies for ICC compatibility. // TODO: Do we have isCPUDispatchMultiVersion? - checkFallThrough(cfg); + + switch (ControlFlowKind fallThroughType = checkFallThrough(cfg)) { + case UnknownFallThrough: + case MaybeFallThrough: + case AlwaysFallThrough: + if (hasNoReturn && cd.diagFallThroughHasNoReturn) { + + } else if (!returnsVoid && cd.diagFallThroughReturnsNonVoid) { + + } + break; + case NeverFallThroughOrReturn: + if (returnsVoid && !hasNoReturn && cd.diagNeverFallThroughOrReturn) { + } + break; + + case NeverFallThrough: { + } break; + } } mlir::DenseSet<mlir::Block *> diff --git a/clang/lib/CIR/CMakeLists.txt b/clang/lib/CIR/CMakeLists.txt index f86ad0933bce6..c698267b04ef5 100644 --- a/clang/lib/CIR/CMakeLists.txt +++ b/clang/lib/CIR/CMakeLists.txt @@ -17,4 +17,4 @@ add_subdirectory(CodeGen) add_subdirectory(FrontendAction) add_subdirectory(Interfaces) add_subdirectory(Lowering) -add_subdirectory(Sema) +add_subdirectory(Analysis) diff --git a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp index fb86b5f848628..bcb8e8fbb644d 100644 --- a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp +++ b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp @@ -15,8 +15,8 @@ #include "clang/CIR/CIRToCIRPasses.h" #include "clang/CIR/Dialect/IR/CIRDialect.h" #include "clang/CIR/LowerToLLVM.h" -#include "clang/CIR/Sema/CIRAnalysisKind.h" -#include "clang/CIR/Sema/FallThroughWarning.h" +#include "clang/CIR/Analysis/CIRAnalysisKind.h" +#include "clang/CIR/Analysis/FallThroughWarning.h" #include "clang/CodeGen/BackendUtil.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Sema/AnalysisBasedWarnings.h" diff --git a/clang/test/CIR/Analysis/fallthrough_1.c b/clang/test/CIR/Analysis/fallthrough_1.c new file mode 100644 index 0000000000000..267745afeba5d --- /dev/null +++ b/clang/test/CIR/Analysis/fallthrough_1.c @@ -0,0 +1,33 @@ +// REQUIRES: false + +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir-analysis="fallthrough" -w +// INFO: These test cases are derived from clang/test/Sema/return.c +int unknown(void); + +int test7(void) { + unknown(); +} // expected-warning {{non-void function does not return a value}} + +int test8(void) { + (void)(1 + unknown()); +} // expected-warning {{non-void function does not return a value}} + + + +int test14(void) { + (void)(1 || unknown()); +} // expected-warning {{non-void function does not return a value}} + +int test15(void) { + (void)(0 || unknown()); +} // expected-warning {{non-void function does not return a value}} + +int test16(void) { + (void)(0 && unknown()); +} // expected-warning {{non-void function does not return a value}} + +int test17(void) { + (void)(1 && unknown()); +} // expected-warning {{non-void function does not return a value}} + + diff --git a/clang/test/CIR/Analysis/fallthrough_2.c b/clang/test/CIR/Analysis/fallthrough_2.c new file mode 100644 index 0000000000000..22c399730a4ff --- /dev/null +++ b/clang/test/CIR/Analysis/fallthrough_2.c @@ -0,0 +1,37 @@ +// REQUIRES: false + +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir-analysis="fallthrough" -w +// INFO: These test cases are derived from clang/test/Sema/return.c + +int test1(void) { +} // expected-warning {{non-void function does not return a value}} + +int test3(void) { + goto a; + a: ; +} // expected-warning {{non-void function does not return a value}} + +int test20(void) { + int i; + if (i) + return 0; + else if (0) + return 2; +} // expected-warning {{non-void function does not return a value in all control paths}} + +int test22(void) { + int i; + switch (i) default: ; +} // expected-warning {{non-void function does not return a value}} + +int test23(void) { + int i; + switch (i) { + case 0: + return 0; + case 2: + return 2; + } +} // expected-warning {{non-void function does not return a value in all control paths}} + + >From f7aa0ff92dcf744d463e058b921ed2f41fb09fbd Mon Sep 17 00:00:00 2001 From: Jasmine Tang <[email protected]> Date: Sun, 23 Nov 2025 03:50:45 -0800 Subject: [PATCH 4/7] test1 working --- .../clang/CIR/Analysis/FallThroughWarning.h | 10 +- clang/lib/CIR/Analysis/FallThroughWarning.cpp | 228 +++++++++++++++--- clang/lib/CIR/FrontendAction/CIRGenAction.cpp | 15 +- clang/lib/Sema/AnalysisBasedWarnings.cpp | 1 + 4 files changed, 215 insertions(+), 39 deletions(-) diff --git a/clang/include/clang/CIR/Analysis/FallThroughWarning.h b/clang/include/clang/CIR/Analysis/FallThroughWarning.h index 3f87b3f867d65..d0213bd677f85 100644 --- a/clang/include/clang/CIR/Analysis/FallThroughWarning.h +++ b/clang/include/clang/CIR/Analysis/FallThroughWarning.h @@ -21,6 +21,7 @@ #include "mlir/IR/Block.h" #include "mlir/IR/Operation.h" +#include "clang/CIR/Dialect/IR/CIRDialect.h" #include "mlir/Support/LLVM.h" namespace cir { class FuncOp; @@ -28,7 +29,7 @@ class FuncOp; namespace clang { class Sema; - +Decl *getDeclByName(ASTContext &context, StringRef name); enum ControlFlowKind { UnknownFallThrough, NeverFallThrough, @@ -45,9 +46,16 @@ struct CheckFallThroughDiagnostics { unsigned funKind = 0; SourceLocation funcLoc; + static CheckFallThroughDiagnostics makeForFunction(Sema &s, + const Decl *func); + static CheckFallThroughDiagnostics makeForCoroutine(const Decl *func); + static CheckFallThroughDiagnostics makeForBlock(); + static CheckFallThroughDiagnostics makeForLambda(); bool checkDiagnostics(DiagnosticsEngine &d, bool returnsVoid, bool hasNoReturn) const; }; +/// Check if a return operation returns a phony value (uninitialized __retval) +bool isPhonyReturn(cir::ReturnOp returnOp); /// Pass for analyzing fall-through behavior in CIR functions class FallThroughWarningPass { diff --git a/clang/lib/CIR/Analysis/FallThroughWarning.cpp b/clang/lib/CIR/Analysis/FallThroughWarning.cpp index d437eb6c377c4..d6bee5f826b4c 100644 --- a/clang/lib/CIR/Analysis/FallThroughWarning.cpp +++ b/clang/lib/CIR/Analysis/FallThroughWarning.cpp @@ -27,7 +27,7 @@ namespace clang { /// WARN: I have to say, we only use this because a lot of the time, attribute /// that we might need are not port to CIR currently so this function is /// basically a crutch for that -static Decl *getDeclByName(ASTContext &context, StringRef name) { +Decl *getDeclByName(ASTContext &context, StringRef name) { // Get the identifier for the name IdentifierInfo *ii = &context.Idents.get(name); @@ -45,6 +45,122 @@ static Decl *getDeclByName(ASTContext &context, StringRef name) { return result.front(); } +CheckFallThroughDiagnostics CheckFallThroughDiagnostics::makeForFunction(Sema &s, + const Decl *func) { + CheckFallThroughDiagnostics d; + d.funcLoc = func->getLocation(); + d.diagFallThroughHasNoReturn = diag::warn_noreturn_has_return_expr; + d.diagFallThroughReturnsNonVoid = diag::warn_falloff_nonvoid; + + // Don't suggest that virtual functions be marked "noreturn", since they + // might be overridden by non-noreturn functions. + bool isVirtualMethod = false; + if (const CXXMethodDecl *method = dyn_cast<CXXMethodDecl>(func)) + isVirtualMethod = method->isVirtual(); + + // Don't suggest that template instantiations be marked "noreturn" + bool isTemplateInstantiation = false; + if (const FunctionDecl *function = dyn_cast<FunctionDecl>(func)) { + isTemplateInstantiation = function->isTemplateInstantiation(); + if (!s.getLangOpts().CPlusPlus && !s.getLangOpts().C99 && + function->isMain()) { + d.diagFallThroughReturnsNonVoid = diag::ext_main_no_return; + } + } + + if (!isVirtualMethod && !isTemplateInstantiation) + d.diagNeverFallThroughOrReturn = diag::warn_suggest_noreturn_function; + + d.funKind = diag::FalloffFunctionKind::Function; + return d; +} + +CheckFallThroughDiagnostics CheckFallThroughDiagnostics::makeForCoroutine(const Decl *func) { + CheckFallThroughDiagnostics d; + d.funcLoc = func->getLocation(); + d.diagFallThroughReturnsNonVoid = diag::warn_falloff_nonvoid; + d.funKind = diag::FalloffFunctionKind::Coroutine; + return d; +} + +CheckFallThroughDiagnostics CheckFallThroughDiagnostics::makeForBlock() { + CheckFallThroughDiagnostics D; + D.diagFallThroughHasNoReturn = diag::err_noreturn_has_return_expr; + D.diagFallThroughReturnsNonVoid = diag::err_falloff_nonvoid; + D.funKind = diag::FalloffFunctionKind::Block; + return D; +} + +CheckFallThroughDiagnostics CheckFallThroughDiagnostics::makeForLambda() { + CheckFallThroughDiagnostics d; + d.diagFallThroughHasNoReturn = diag::err_noreturn_has_return_expr; + d.diagFallThroughReturnsNonVoid = diag::warn_falloff_nonvoid; + d.funKind = diag::FalloffFunctionKind::Lambda; + return d; +} + + +//===----------------------------------------------------------------------===// +// Check for phony return values (returning uninitialized __retval) +//===----------------------------------------------------------------------===// + +/// Check if a return operation returns a phony value. +/// A phony return is when a function returns a value loaded from an +/// uninitialized __retval alloca, which indicates the function doesn't +/// actually return a meaningful value. +/// +/// Example of phony return: +/// \code +/// cir.func @test1() -> !s32i { +/// %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] +/// %1 = cir.load %0 : !cir.ptr<!s32i>, !s32i +/// cir.return %1 : !s32i +/// } +/// \endcode +/// +/// \param returnOp The return operation to check +/// \return true if this is a phony return, false otherwise +bool isPhonyReturn(cir::ReturnOp returnOp) { + if (!returnOp) + return false; + + // Get the returned value - return operations use $input as the operand + if (!returnOp.hasOperand()) + return false; + + auto returnValue = returnOp.getInput()[0]; + + // Check if the return value comes from a load operation + auto loadOp = returnValue.getDefiningOp<cir::LoadOp>(); + if (!loadOp) + return false; + + // Check if the load is from an alloca + auto allocaOp = loadOp.getAddr().getDefiningOp<cir::AllocaOp>(); + if (!allocaOp) + return false; + + // Check if the alloca is named "__retval" + auto name = allocaOp.getName(); + if (name != "__retval") + return false; + + // Check if the alloca has any stores to it (if not, it's uninitialized) + // We need to search for store operations that write to this alloca + mlir::Value allocaResult = allocaOp.getResult(); + + for (auto *user : allocaResult.getUsers()) { + if (auto storeOp = dyn_cast<cir::StoreOp>(user)) { + // If there's a store to this alloca, it's not phony + // (assuming the store happens before the load in control flow) + return false; + } + } + + // No stores found to __retval alloca - this is a phony return + return true; +} + //===----------------------------------------------------------------------===// // Check for missing return value. //===----------------------------------------------------------------------===// @@ -52,19 +168,21 @@ static Decl *getDeclByName(ASTContext &context, StringRef name) { bool CheckFallThroughDiagnostics::checkDiagnostics(DiagnosticsEngine &d, bool returnsVoid, bool hasNoReturn) const { - if (funKind == diag::FalloffFunctionKind::Function) { - return (returnsVoid || d.isIgnored(diag::warn_falloff_nonvoid, funcLoc)) && - (!hasNoReturn || - d.isIgnored(diag::warn_noreturn_has_return_expr, funcLoc)) && - (!returnsVoid || - d.isIgnored(diag::warn_suggest_noreturn_block, funcLoc)); - } - if (funKind == diag::FalloffFunctionKind::Coroutine) { - return (returnsVoid || d.isIgnored(diag::warn_falloff_nonvoid, funcLoc)) && - (!hasNoReturn); - } - // For blocks / lambdas. - return returnsVoid && !hasNoReturn; + if (funKind == diag::FalloffFunctionKind::Function) { + return (returnsVoid || + d.isIgnored(diag::warn_falloff_nonvoid, funcLoc)) && + (d.isIgnored(diag::warn_noreturn_has_return_expr, funcLoc) || + !hasNoReturn) && + (!returnsVoid || + d.isIgnored(diag::warn_suggest_noreturn_block, funcLoc)); + } + if (funKind == diag::FalloffFunctionKind::Coroutine) { + return (returnsVoid || + d.isIgnored(diag::warn_falloff_nonvoid, funcLoc)) && + (!hasNoReturn); + } + // For blocks / lambdas. + return returnsVoid && !hasNoReturn; } // TODO: Add a class for fall through config later @@ -72,41 +190,83 @@ bool CheckFallThroughDiagnostics::checkDiagnostics(DiagnosticsEngine &d, void FallThroughWarningPass::checkFallThroughForFuncBody( Sema &s, cir::FuncOp cfg, QualType blockType, const CheckFallThroughDiagnostics &cd) { - llvm::errs() << "Hello world, you're in CIR sema analysis\n"; auto *d = getDeclByName(s.getASTContext(), cfg.getName()); + auto *body = d->getBody(); assert(d && "we need non null decl"); bool returnsVoid = false; bool hasNoReturn = false; + SourceLocation lBrace = body->getBeginLoc(), rBrace = body->getEndLoc(); // Supposedly all function in cir is FuncOp // 1. If normal function (FunctionDecl), check if it's coroutine. // 1a. if coroutine -> check the fallthrough handler (idk what this means, // TODO for now) - if (cfg.getCoroutine()) { - // TODO: Let's not worry about coroutine for now - } else - returnsVoid = isa<cir::VoidType>(cfg.getFunctionType().getReturnType()); - - // TODO: Do we need to check for InferredNoReturnAttr just like in OG? - hasNoReturn = cfg.getFunctionType().getReturnTypes().empty(); + if (const auto *fd = dyn_cast<FunctionDecl>(d)) { + if (const auto *cBody = dyn_cast<CoroutineBodyStmt>(d->getBody())) + returnsVoid = cBody->getFallthroughHandler() != nullptr; + else + returnsVoid = fd->getReturnType()->isVoidType(); + hasNoReturn = fd->isNoReturn() || fd->hasAttr<InferredNoReturnAttr>(); + } + else if (const auto *md = dyn_cast<ObjCMethodDecl>(d)) { + returnsVoid = md->getReturnType()->isVoidType(); + hasNoReturn = md->hasAttr<NoReturnAttr>(); + } + else if (isa<BlockDecl>(d)) { + if (const FunctionType *ft = + blockType->getPointeeType()->getAs<FunctionType>()) { + if (ft->getReturnType()->isVoidType()) + returnsVoid = true; + if (ft->getNoReturnAttr()) + hasNoReturn = true; + } + } DiagnosticsEngine &diags = s.getDiagnostics(); - if (cd.checkDiagnostics(diags, returnsVoid, hasNoReturn)) { - return; - } + + // Short circuit for compilation speed. + if (cd.checkDiagnostics(diags, returnsVoid, hasNoReturn)) + return; // cpu_dispatch functions permit empty function bodies for ICC compatibility. // TODO: Do we have isCPUDispatchMultiVersion? switch (ControlFlowKind fallThroughType = checkFallThrough(cfg)) { case UnknownFallThrough: + [[fallthrough]]; case MaybeFallThrough: + [[fallthrough]]; case AlwaysFallThrough: if (hasNoReturn && cd.diagFallThroughHasNoReturn) { } else if (!returnsVoid && cd.diagFallThroughReturnsNonVoid) { + // If the final statement is a call to an always-throwing function, + // don't warn about the fall-through. + if (d->getAsFunction()) { + if (const auto *cs = dyn_cast<CompoundStmt>(body); + cs && !cs->body_empty()) { + const Stmt *lastStmt = cs->body_back(); + // Unwrap ExprWithCleanups if necessary. + if (const auto *ewc = dyn_cast<ExprWithCleanups>(lastStmt)) { + lastStmt = ewc->getSubExpr(); + } + if (const auto *ce = dyn_cast<CallExpr>(lastStmt)) { + if (const FunctionDecl *callee = ce->getDirectCallee(); + callee && callee->hasAttr<InferredNoReturnAttr>()) { + return; // Don't warn about fall-through. + } + } + // Direct throw. + if (isa<CXXThrowExpr>(lastStmt)) { + return; // Don't warn about fall-through. + } + } + } + bool notInAllControlPaths = fallThroughType == MaybeFallThrough; + s.Diag(rBrace, cd.diagFallThroughReturnsNonVoid) + << cd.funKind << notInAllControlPaths; } break; @@ -129,7 +289,7 @@ FallThroughWarningPass::getLiveSet(cir::FuncOp cfg) { auto &first = cfg.getBody().getBlocks().front(); for (auto &block : cfg.getBody()) { - if (first.isReachable(&block)) + if (&first == &block || first.isReachable(&block)) liveSet.insert(&block); } return liveSet; @@ -171,10 +331,8 @@ ControlFlowKind FallThroughWarningPass::checkFallThrough(cir::FuncOp cfg) { continue; mlir::Operation *term = pred.getTerminator(); - if (isa<cir::ReturnOp>(term)) { - hasAbnormalEdge = true; - continue; - } + + // TODO: hasNoReturnElement() in OG here, not sure how to work it in here yet // INFO: In OG, we'll be looking for destructor since it can appear past // return but i guess not in CIR? In this case we'll only be examining the @@ -189,9 +347,11 @@ ControlFlowKind FallThroughWarningPass::checkFallThrough(cir::FuncOp cfg) { // is true only when a block only has the terminator, or its size is 1. hasPlainEdge = std::distance(pred.begin(), pred.end()) == 1; - if (isa<cir::ReturnOp>(term)) { - hasLiveReturn = true; - continue; + if (auto returnOp = dyn_cast<cir::ReturnOp>(term)) { + if (!isPhonyReturn(returnOp)) { + hasLiveReturn = true; + continue; + } } if (isa<cir::TryOp>(term)) { hasLiveReturn = true; @@ -217,6 +377,8 @@ ControlFlowKind FallThroughWarningPass::checkFallThrough(cir::FuncOp cfg) { // This says AlwaysFallThrough for calls to functions that are not marked // noreturn, that don't return. If people would like this warning to be more // accurate, such functions should be marked as noreturn. + // + // llvm_unreachable(""); return AlwaysFallThrough; } } // namespace clang diff --git a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp index bcb8e8fbb644d..1b06c311447af 100644 --- a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp +++ b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp @@ -119,10 +119,8 @@ class CIRGenConsumer : public clang::ASTConsumer { // Run CIR analysis passes if requested if (!FEOptions.ClangIRAnalysisList.empty()) { CIRAnalysisSet AnalysisSet = parseCIRAnalysisList(FEOptions.ClangIRAnalysisList); - AnalysisSet.print(llvm::errs()); if (AnalysisSet.has(CIRAnalysisKind::FallThrough)) { - // Get Sema for diagnostics if (CI.hasSema()) { Sema &S = CI.getSema(); FallThroughWarningPass FallThroughPass; @@ -135,10 +133,17 @@ class CIRGenConsumer : public clang::ASTConsumer { QualType FuncType; // Set up diagnostics configuration - CheckFallThroughDiagnostics Diags; - + // INFO: This is not full + Decl *D = getDeclByName(S.getASTContext(), FuncOp.getName()); + const CheckFallThroughDiagnostics &CD = + (isa<BlockDecl>(D) ? CheckFallThroughDiagnostics::makeForBlock() + : (isa<CXXMethodDecl>(D) && + cast<CXXMethodDecl>(D)->getOverloadedOperator() == OO_Call && + cast<CXXMethodDecl>(D)->getParent()->isLambda()) + ? CheckFallThroughDiagnostics::makeForLambda() + : CheckFallThroughDiagnostics::makeForFunction(S, D)); // Run fall-through analysis on this function - FallThroughPass.checkFallThroughForFuncBody(S, FuncOp, FuncType, Diags); + FallThroughPass.checkFallThroughForFuncBody(S, FuncOp, FuncType, CD); }); } } diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 41a98323450e4..a2f7f61b57d95 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -53,6 +53,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Debug.h" +#include "llvm/Support/ErrorHandling.h" #include <algorithm> #include <deque> #include <iterator> >From e4a96eff701bd35fe07a4316c3c62a33658ac99d Mon Sep 17 00:00:00 2001 From: Jasmine Tang <[email protected]> Date: Sun, 23 Nov 2025 04:53:49 -0800 Subject: [PATCH 5/7] Unknown should break --- .../clang/CIR/Analysis/FallThroughWarning.h | 5 +- clang/lib/CIR/Analysis/FallThroughWarning.cpp | 54 +++++++++---------- clang/lib/CIR/FrontendAction/CIRGenAction.cpp | 11 ++-- 3 files changed, 34 insertions(+), 36 deletions(-) diff --git a/clang/include/clang/CIR/Analysis/FallThroughWarning.h b/clang/include/clang/CIR/Analysis/FallThroughWarning.h index d0213bd677f85..f78a07d5212c3 100644 --- a/clang/include/clang/CIR/Analysis/FallThroughWarning.h +++ b/clang/include/clang/CIR/Analysis/FallThroughWarning.h @@ -21,8 +21,8 @@ #include "mlir/IR/Block.h" #include "mlir/IR/Operation.h" -#include "clang/CIR/Dialect/IR/CIRDialect.h" #include "mlir/Support/LLVM.h" +#include "clang/CIR/Dialect/IR/CIRDialect.h" namespace cir { class FuncOp; } // namespace cir @@ -46,8 +46,7 @@ struct CheckFallThroughDiagnostics { unsigned funKind = 0; SourceLocation funcLoc; - static CheckFallThroughDiagnostics makeForFunction(Sema &s, - const Decl *func); + static CheckFallThroughDiagnostics makeForFunction(Sema &s, const Decl *func); static CheckFallThroughDiagnostics makeForCoroutine(const Decl *func); static CheckFallThroughDiagnostics makeForBlock(); static CheckFallThroughDiagnostics makeForLambda(); diff --git a/clang/lib/CIR/Analysis/FallThroughWarning.cpp b/clang/lib/CIR/Analysis/FallThroughWarning.cpp index d6bee5f826b4c..0b4ddfa5cd113 100644 --- a/clang/lib/CIR/Analysis/FallThroughWarning.cpp +++ b/clang/lib/CIR/Analysis/FallThroughWarning.cpp @@ -45,8 +45,8 @@ Decl *getDeclByName(ASTContext &context, StringRef name) { return result.front(); } -CheckFallThroughDiagnostics CheckFallThroughDiagnostics::makeForFunction(Sema &s, - const Decl *func) { +CheckFallThroughDiagnostics +CheckFallThroughDiagnostics::makeForFunction(Sema &s, const Decl *func) { CheckFallThroughDiagnostics d; d.funcLoc = func->getLocation(); d.diagFallThroughHasNoReturn = diag::warn_noreturn_has_return_expr; @@ -75,7 +75,8 @@ CheckFallThroughDiagnostics CheckFallThroughDiagnostics::makeForFunction(Sema &s return d; } -CheckFallThroughDiagnostics CheckFallThroughDiagnostics::makeForCoroutine(const Decl *func) { +CheckFallThroughDiagnostics +CheckFallThroughDiagnostics::makeForCoroutine(const Decl *func) { CheckFallThroughDiagnostics d; d.funcLoc = func->getLocation(); d.diagFallThroughReturnsNonVoid = diag::warn_falloff_nonvoid; @@ -99,7 +100,6 @@ CheckFallThroughDiagnostics CheckFallThroughDiagnostics::makeForLambda() { return d; } - //===----------------------------------------------------------------------===// // Check for phony return values (returning uninitialized __retval) //===----------------------------------------------------------------------===// @@ -168,21 +168,19 @@ bool isPhonyReturn(cir::ReturnOp returnOp) { bool CheckFallThroughDiagnostics::checkDiagnostics(DiagnosticsEngine &d, bool returnsVoid, bool hasNoReturn) const { - if (funKind == diag::FalloffFunctionKind::Function) { - return (returnsVoid || - d.isIgnored(diag::warn_falloff_nonvoid, funcLoc)) && - (d.isIgnored(diag::warn_noreturn_has_return_expr, funcLoc) || - !hasNoReturn) && - (!returnsVoid || - d.isIgnored(diag::warn_suggest_noreturn_block, funcLoc)); - } - if (funKind == diag::FalloffFunctionKind::Coroutine) { - return (returnsVoid || - d.isIgnored(diag::warn_falloff_nonvoid, funcLoc)) && - (!hasNoReturn); - } - // For blocks / lambdas. - return returnsVoid && !hasNoReturn; + if (funKind == diag::FalloffFunctionKind::Function) { + return (returnsVoid || d.isIgnored(diag::warn_falloff_nonvoid, funcLoc)) && + (d.isIgnored(diag::warn_noreturn_has_return_expr, funcLoc) || + !hasNoReturn) && + (!returnsVoid || + d.isIgnored(diag::warn_suggest_noreturn_block, funcLoc)); + } + if (funKind == diag::FalloffFunctionKind::Coroutine) { + return (returnsVoid || d.isIgnored(diag::warn_falloff_nonvoid, funcLoc)) && + (!hasNoReturn); + } + // For blocks / lambdas. + return returnsVoid && !hasNoReturn; } // TODO: Add a class for fall through config later @@ -209,14 +207,12 @@ void FallThroughWarningPass::checkFallThroughForFuncBody( else returnsVoid = fd->getReturnType()->isVoidType(); hasNoReturn = fd->isNoReturn() || fd->hasAttr<InferredNoReturnAttr>(); - } - else if (const auto *md = dyn_cast<ObjCMethodDecl>(d)) { + } else if (const auto *md = dyn_cast<ObjCMethodDecl>(d)) { returnsVoid = md->getReturnType()->isVoidType(); hasNoReturn = md->hasAttr<NoReturnAttr>(); - } - else if (isa<BlockDecl>(d)) { + } else if (isa<BlockDecl>(d)) { if (const FunctionType *ft = - blockType->getPointeeType()->getAs<FunctionType>()) { + blockType->getPointeeType()->getAs<FunctionType>()) { if (ft->getReturnType()->isVoidType()) returnsVoid = true; if (ft->getNoReturnAttr()) @@ -228,14 +224,14 @@ void FallThroughWarningPass::checkFallThroughForFuncBody( // Short circuit for compilation speed. if (cd.checkDiagnostics(diags, returnsVoid, hasNoReturn)) - return; + return; // cpu_dispatch functions permit empty function bodies for ICC compatibility. // TODO: Do we have isCPUDispatchMultiVersion? switch (ControlFlowKind fallThroughType = checkFallThrough(cfg)) { case UnknownFallThrough: - [[fallthrough]]; + break; case MaybeFallThrough: [[fallthrough]]; case AlwaysFallThrough: @@ -267,7 +263,6 @@ void FallThroughWarningPass::checkFallThroughForFuncBody( bool notInAllControlPaths = fallThroughType == MaybeFallThrough; s.Diag(rBrace, cd.diagFallThroughReturnsNonVoid) << cd.funKind << notInAllControlPaths; - } break; case NeverFallThroughOrReturn: @@ -332,7 +327,8 @@ ControlFlowKind FallThroughWarningPass::checkFallThrough(cir::FuncOp cfg) { mlir::Operation *term = pred.getTerminator(); - // TODO: hasNoReturnElement() in OG here, not sure how to work it in here yet + // TODO: hasNoReturnElement() in OG here, not sure how to work it in here + // yet // INFO: In OG, we'll be looking for destructor since it can appear past // return but i guess not in CIR? In this case we'll only be examining the @@ -348,7 +344,7 @@ ControlFlowKind FallThroughWarningPass::checkFallThrough(cir::FuncOp cfg) { hasPlainEdge = std::distance(pred.begin(), pred.end()) == 1; if (auto returnOp = dyn_cast<cir::ReturnOp>(term)) { - if (!isPhonyReturn(returnOp)) { + if (!isPhonyReturn(returnOp)) { hasLiveReturn = true; continue; } diff --git a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp index 1b06c311447af..627265cec3d3b 100644 --- a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp +++ b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp @@ -118,7 +118,8 @@ class CIRGenConsumer : public clang::ASTConsumer { // Run CIR analysis passes if requested if (!FEOptions.ClangIRAnalysisList.empty()) { - CIRAnalysisSet AnalysisSet = parseCIRAnalysisList(FEOptions.ClangIRAnalysisList); + CIRAnalysisSet AnalysisSet = + parseCIRAnalysisList(FEOptions.ClangIRAnalysisList); if (AnalysisSet.has(CIRAnalysisKind::FallThrough)) { if (CI.hasSema()) { @@ -138,12 +139,14 @@ class CIRGenConsumer : public clang::ASTConsumer { const CheckFallThroughDiagnostics &CD = (isa<BlockDecl>(D) ? CheckFallThroughDiagnostics::makeForBlock() : (isa<CXXMethodDecl>(D) && - cast<CXXMethodDecl>(D)->getOverloadedOperator() == OO_Call && + cast<CXXMethodDecl>(D)->getOverloadedOperator() == + OO_Call && cast<CXXMethodDecl>(D)->getParent()->isLambda()) ? CheckFallThroughDiagnostics::makeForLambda() - : CheckFallThroughDiagnostics::makeForFunction(S, D)); + : CheckFallThroughDiagnostics::makeForFunction(S, D)); // Run fall-through analysis on this function - FallThroughPass.checkFallThroughForFuncBody(S, FuncOp, FuncType, CD); + FallThroughPass.checkFallThroughForFuncBody(S, FuncOp, FuncType, + CD); }); } } >From c87602f944baa43ef7774e8e834d6dfb7cb54b21 Mon Sep 17 00:00:00 2001 From: wizardengineer <[email protected]> Date: Fri, 5 Dec 2025 17:06:28 -0500 Subject: [PATCH 6/7] [CIR] Added switch support --- clang/lib/CIR/Analysis/FallThroughWarning.cpp | 106 ++++++++++++++++++ clang/test/CIR/Analysis/fallthrough_1.c | 4 +- clang/test/CIR/Analysis/fallthrough_2.c | 4 +- clang/test/CIR/Analysis/fallthrough_switch.c | 105 +++++++++++++++++ 4 files changed, 213 insertions(+), 6 deletions(-) create mode 100644 clang/test/CIR/Analysis/fallthrough_switch.c diff --git a/clang/lib/CIR/Analysis/FallThroughWarning.cpp b/clang/lib/CIR/Analysis/FallThroughWarning.cpp index 0b4ddfa5cd113..eede0f0a7517c 100644 --- a/clang/lib/CIR/Analysis/FallThroughWarning.cpp +++ b/clang/lib/CIR/Analysis/FallThroughWarning.cpp @@ -290,6 +290,101 @@ FallThroughWarningPass::getLiveSet(cir::FuncOp cfg) { return liveSet; } +//===----------------------------------------------------------------------===// +// Switch/Case Analysis Helpers +//===----------------------------------------------------------------------===// + +/// Check if a case region terminates with a return (not break/yield). +/// Returns true if the case ends with cir.return or never falls through. +/// +/// \param caseOp The case operation to check +/// \return true if the case returns (doesn't break or yield) +static bool caseTerminatesWithReturn(cir::CaseOp caseOp); + +/// Check if a switch operation returns on all code paths. +/// Returns true if: +/// - Switch is in simple form AND +/// - Has a default case that returns +/// +/// \param switchOp The switch operation to analyze +/// \return true if all paths through the switch return a value +static bool switchAllPathsReturn(cir::SwitchOp switchOp) { + llvm::SmallVector<cir::CaseOp> cases; + + // Check if switch is in simple form + if (!switchOp.isSimpleForm(cases)) { + // Non-simple form: conservatively assume may fall through + return false; + } + + if (cases.empty()) { + // Empty switch falls through + return false; + } + + // Find default case and check if it returns + for (auto caseOp : cases) { + if (caseOp.getKind() == cir::CaseOpKind::Default) { + // If default returns, all unhandled case values are covered + return caseTerminatesWithReturn(caseOp); + } + } + + // No default case: not all paths covered + return false; +} + +static bool caseTerminatesWithReturn(cir::CaseOp caseOp) { + // Get the case region + mlir::Region ®ion = caseOp.getCaseRegion(); + + if (region.empty()) { + return false; + } + + // Check all exit blocks in the case region + // (blocks with no successors within the region) + for (mlir::Block &block : region.getBlocks()) { + if (!block.hasNoSuccessors()) { + continue; + } + + if (!block.mightHaveTerminator()) { + return false; + } + + mlir::Operation *terminator = block.getTerminator(); + + // Return operations mean this path returns + if (isa<cir::ReturnOp>(terminator)) { + return true; + } + + // Break or yield means this case doesn't return + if (isa<cir::BreakOp>(terminator) || isa<cir::YieldOp>(terminator)) { + return false; + } + + // Recursively check nested switches + if (auto nestedSwitch = dyn_cast<cir::SwitchOp>(terminator)) { + if (!switchAllPathsReturn(nestedSwitch)) { + return false; + } + // If nested switch returns, continue checking other blocks + continue; + } + + // Try operations are abnormal flow + if (isa<cir::TryOp>(terminator)) { + // Conservative: assume try doesn't guarantee return + return false; + } + } + + // If we found no explicit return, the case doesn't return + return false; +} + ControlFlowKind FallThroughWarningPass::checkFallThrough(cir::FuncOp cfg) { assert(cfg && "there can't be a null func op"); @@ -354,6 +449,17 @@ ControlFlowKind FallThroughWarningPass::checkFallThrough(cir::FuncOp cfg) { continue; } + // Handle switch statements + if (auto switchOp = dyn_cast<cir::SwitchOp>(term)) { + if (switchAllPathsReturn(switchOp)) { + hasLiveReturn = true; + continue; + } + // Switch may fall through + hasPlainEdge = true; + continue; + } + // TODO: Maybe one day throw will be terminator? // // TODO: We need to add a microsoft inline assembly enum diff --git a/clang/test/CIR/Analysis/fallthrough_1.c b/clang/test/CIR/Analysis/fallthrough_1.c index 267745afeba5d..ef27ad51494b6 100644 --- a/clang/test/CIR/Analysis/fallthrough_1.c +++ b/clang/test/CIR/Analysis/fallthrough_1.c @@ -1,6 +1,4 @@ -// REQUIRES: false - -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir-analysis="fallthrough" -w +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir-analysis="fallthrough" -verify // INFO: These test cases are derived from clang/test/Sema/return.c int unknown(void); diff --git a/clang/test/CIR/Analysis/fallthrough_2.c b/clang/test/CIR/Analysis/fallthrough_2.c index 22c399730a4ff..74eb8b8284efb 100644 --- a/clang/test/CIR/Analysis/fallthrough_2.c +++ b/clang/test/CIR/Analysis/fallthrough_2.c @@ -1,6 +1,4 @@ -// REQUIRES: false - -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir-analysis="fallthrough" -w +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir-analysis="fallthrough" -verify // INFO: These test cases are derived from clang/test/Sema/return.c int test1(void) { diff --git a/clang/test/CIR/Analysis/fallthrough_switch.c b/clang/test/CIR/Analysis/fallthrough_switch.c new file mode 100644 index 0000000000000..b0e5bb00b1412 --- /dev/null +++ b/clang/test/CIR/Analysis/fallthrough_switch.c @@ -0,0 +1,105 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir-analysis="fallthrough" -verify +// INFO: Switch statement test cases for CIR fallthrough analysis +// Derived from clang/test/Sema/return.c + +// =========================================================================== +// Test cases where switch SHOULD NOT warn (all paths return) +// =========================================================================== + +// Switch with default - all cases return +int switch_all_return(int x) { + switch (x) { + case 0: + return 0; + case 1: + return 1; + default: + return -1; + } +} + +// Test from return.c: enum switch covering all cases +enum Cases { C1, C2, C3, C4 }; +int test_enum_cases(enum Cases C) { + switch (C) { + case C1: return 1; + case C2: return 2; + case C4: return 3; + case C3: return 4; + } +} // No warning - enum covers all cases (TODO: CIR may need enum coverage analysis) + +// =========================================================================== +// Test cases where switch SHOULD warn +// =========================================================================== + +// Switch without default +int switch_no_default(int x) { + switch (x) { + case 0: + return 0; + case 2: + return 2; + } +} // expected-warning {{non-void function does not return a value in all control paths}} + +// Switch with only default that doesn't return +int switch_default_no_return(int x) { + switch (x) default: ; +} // expected-warning {{non-void function does not return a value}} + +// Switch with break in one case +int switch_with_break(int x) { + switch (x) { + case 0: + return 0; + case 1: + break; + default: + return -1; + } +} // expected-warning {{non-void function does not return a value in all control paths}} + +// Fallthrough case (using fallthrough, not return) +int switch_fallthrough(int x) { + switch (x) { + case 0: + case 1: // fallthrough from case 0 + return 1; + default: + break; // falls through to end + } +} // expected-warning {{non-void function does not return a value in all control paths}} + +// =========================================================================== +// Nested switch statements +// =========================================================================== + +int nested_switch(int x, int y) { + switch (x) { + case 0: + switch (y) { + case 0: + return 0; + default: + return 1; + } + default: + return -1; + } +} + +int nested_switch_missing(int x, int y) { + switch (x) { + case 0: + switch (y) { + case 0: + return 0; + // No default in inner switch + } + break; // Falls through after inner switch + default: + return -1; + } +} // expected-warning {{non-void function does not return a value in all control paths}} + >From 090394d61c27029485189f388daefb1593881387 Mon Sep 17 00:00:00 2001 From: wizardengineer <[email protected]> Date: Fri, 5 Dec 2025 23:16:07 -0500 Subject: [PATCH 7/7] [CIR][Analyzer] Revamping codebase and switch implementation --- clang/include/clang/Basic/LangOptions.def | 2 + clang/lib/CIR/Analysis/FallThroughWarning.cpp | 234 +++++++++++++----- clang/lib/Frontend/CompilerInvocation.cpp | 6 + clang/lib/Sema/SemaDecl.cpp | 3 + clang/test/CIR/Analysis/fallthrough_1.c | 2 +- clang/test/CIR/Analysis/fallthrough_2.c | 2 +- clang/test/CIR/Analysis/fallthrough_2.cir | 141 +++++++++++ clang/test/CIR/Analysis/fallthrough_switch.c | 5 +- 8 files changed, 324 insertions(+), 71 deletions(-) create mode 100644 clang/test/CIR/Analysis/fallthrough_2.cir diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 40fc66ea12e34..6d44b4a382151 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -501,6 +501,8 @@ LANGOPT(EnableLifetimeSafety, 1, 0, NotCompatible, "Experimental lifetime safety LANGOPT(PreserveVec3Type, 1, 0, NotCompatible, "Preserve 3-component vector type") +LANGOPT(CIRFallThroughAnalysis, 1, 0, NotCompatible, "Use CIR fallthrough analysis") + #undef LANGOPT #undef ENUM_LANGOPT #undef VALUE_LANGOPT diff --git a/clang/lib/CIR/Analysis/FallThroughWarning.cpp b/clang/lib/CIR/Analysis/FallThroughWarning.cpp index eede0f0a7517c..82b32ecc8a4e0 100644 --- a/clang/lib/CIR/Analysis/FallThroughWarning.cpp +++ b/clang/lib/CIR/Analysis/FallThroughWarning.cpp @@ -1,4 +1,5 @@ #include "clang/CIR/Analysis/FallThroughWarning.h" +#include "mlir/IR/OpDefinition.h" #include "clang/AST/ASTContext.h" #include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/SourceLocation.h" @@ -130,7 +131,6 @@ bool isPhonyReturn(cir::ReturnOp returnOp) { auto returnValue = returnOp.getInput()[0]; - // Check if the return value comes from a load operation auto loadOp = returnValue.getDefiningOp<cir::LoadOp>(); if (!loadOp) return false; @@ -145,19 +145,23 @@ bool isPhonyReturn(cir::ReturnOp returnOp) { if (name != "__retval") return false; - // Check if the alloca has any stores to it (if not, it's uninitialized) - // We need to search for store operations that write to this alloca + // Check if there are ANY stores to __retval in the entire function. + // This is intentionally path-INsensitive - if there are stores on some + // paths, then this return is considered non-phony. + // The control flow analysis (hasLiveReturn + hasPlainEdge) will determine + // if all paths return properly. mlir::Value allocaResult = allocaOp.getResult(); for (auto *user : allocaResult.getUsers()) { if (auto storeOp = dyn_cast<cir::StoreOp>(user)) { - // If there's a store to this alloca, it's not phony - // (assuming the store happens before the load in control flow) - return false; + if (storeOp.getAddr() == allocaResult) { + // There's a store to __retval somewhere - not a phony return + return false; + } } } - // No stores found to __retval alloca - this is a phony return + // No stores to __retval anywhere - this is a phony return (uninitialized) return true; } @@ -190,8 +194,12 @@ void FallThroughWarningPass::checkFallThroughForFuncBody( const CheckFallThroughDiagnostics &cd) { auto *d = getDeclByName(s.getASTContext(), cfg.getName()); - auto *body = d->getBody(); assert(d && "we need non null decl"); + auto *body = d->getBody(); + + // Functions without bodies (declarations only) don't need fall-through analysis + if (!body) + return; bool returnsVoid = false; bool hasNoReturn = false; @@ -283,10 +291,25 @@ FallThroughWarningPass::getLiveSet(cir::FuncOp cfg) { auto &first = cfg.getBody().getBlocks().front(); + // First pass: normal reachability for (auto &block : cfg.getBody()) { if (&first == &block || first.isReachable(&block)) liveSet.insert(&block); } + + // Second pass: follow goto operations to find labeled blocks + // Goto creates blocks that aren't connected via normal CFG edges + cfg.walk([&](cir::GotoOp gotoOp) { + auto labelName = gotoOp.getLabel(); + // Find the block containing the matching label + cfg.walk([&](cir::LabelOp labelOp) { + if (labelOp.getLabel() == labelName) { + mlir::Block *labelBlock = labelOp->getBlock(); + liveSet.insert(labelBlock); + } + }); + }); + return liveSet; } @@ -322,16 +345,41 @@ static bool switchAllPathsReturn(cir::SwitchOp switchOp) { return false; } - // Find default case and check if it returns + // Check if there's a default case + bool hasDefault = false; + bool defaultReturns = false; + for (auto caseOp : cases) { if (caseOp.getKind() == cir::CaseOpKind::Default) { - // If default returns, all unhandled case values are covered - return caseTerminatesWithReturn(caseOp); + hasDefault = true; + defaultReturns = caseTerminatesWithReturn(caseOp); + break; } } - // No default case: not all paths covered - return false; + // If no default, not all paths covered + if (!hasDefault) { + return false; + } + + // If default doesn't return, switch falls through + if (!defaultReturns) { + return false; + } + + // Now check ALL cases (including non-default) + // If any case has a break or doesn't return, switch may fall through + for (auto caseOp : cases) { + if (caseOp.getKind() != cir::CaseOpKind::Default) { + if (!caseTerminatesWithReturn(caseOp)) { + // This case doesn't return (might have break) + return false; + } + } + } + + // All cases return and default returns + return true; } static bool caseTerminatesWithReturn(cir::CaseOp caseOp) { @@ -342,6 +390,53 @@ static bool caseTerminatesWithReturn(cir::CaseOp caseOp) { return false; } + // First, check if there are nested control flow operations (switch, if) + // that return on all paths. If so, any subsequent operations (like yield) + // are unreachable, and we should consider this case as terminating with return. + bool foundAllPathsReturn = false; + + region.walk([&](mlir::Operation *op) { + // Check for nested switches that return on all paths + if (auto nestedSwitch = dyn_cast<cir::SwitchOp>(op)) { + if (switchAllPathsReturn(nestedSwitch)) { + foundAllPathsReturn = true; + return mlir::WalkResult::interrupt(); + } + } + + // Check for nested if operations where both branches return + if (auto nestedIf = dyn_cast<cir::IfOp>(op)) { + bool thenReturns = false; + bool elseReturns = false; + + // Check then region + nestedIf.getThenRegion().walk([&](cir::ReturnOp ret) { + thenReturns = true; + return mlir::WalkResult::interrupt(); + }); + + // Check else region (if it exists) + if (!nestedIf.getElseRegion().empty()) { + nestedIf.getElseRegion().walk([&](cir::ReturnOp ret) { + elseReturns = true; + return mlir::WalkResult::interrupt(); + }); + } + + // If both branches return, subsequent code is unreachable + if (thenReturns && elseReturns) { + foundAllPathsReturn = true; + return mlir::WalkResult::interrupt(); + } + } + + return mlir::WalkResult::advance(); + }); + + if (foundAllPathsReturn) { + return true; + } + // Check all exit blocks in the case region // (blocks with no successors within the region) for (mlir::Block &block : region.getBlocks()) { @@ -406,68 +501,73 @@ ControlFlowKind FallThroughWarningPass::checkFallThrough(cir::FuncOp cfg) { auto &exitBlock = cfg.getBody().back(); // INFO: in OG clang CFG, they have an empty exit block, so when they query // pred of exit OG, they get all exit blocks - // - // I guess in CIR, we can pretend exit blocks are all blocks that have no - // successor? - for (mlir::Block &pred : cfg.getBody().getBlocks()) { - if (!liveSet.contains(&pred)) - continue; - - // We consider no predecessors as 'exit blocks' - if (!pred.hasNoSuccessors()) - continue; - - if (!pred.mightHaveTerminator()) - continue; - - mlir::Operation *term = pred.getTerminator(); - - // TODO: hasNoReturnElement() in OG here, not sure how to work it in here - // yet - - // INFO: In OG, we'll be looking for destructor since it can appear past - // return but i guess not in CIR? In this case we'll only be examining the - // terminator - - if (isa<cir::TryOp>(term)) { - hasAbnormalEdge = true; - continue; - } - - // INFO: OG clang has this equals true whenever ri == re, which means this - // is true only when a block only has the terminator, or its size is 1. - hasPlainEdge = std::distance(pred.begin(), pred.end()) == 1; - - if (auto returnOp = dyn_cast<cir::ReturnOp>(term)) { - if (!isPhonyReturn(returnOp)) { - hasLiveReturn = true; - continue; - } - } - if (isa<cir::TryOp>(term)) { + + // Walk all ReturnOp operations to find returns in nested regions + cfg.walk([&](cir::ReturnOp returnOp) { + mlir::Block *block = returnOp->getBlock(); + + // Check if this block is reachable: + // - For top-level blocks (parent is FuncOp): check liveSet + // - For nested blocks (parent is if/scope/switch/etc): assume reachable + mlir::Operation *parentOp = block->getParentOp(); + bool isTopLevel = isa<cir::FuncOp>(parentOp); + + if (isTopLevel && !liveSet.contains(block)) + return; + + // Check if this is a real return or a phony one + if (!isPhonyReturn(returnOp)) { hasLiveReturn = true; - continue; + } else { + // Phony return means the function falls through without an explicit + // return statement. + hasPlainEdge = true; } + }); - // Handle switch statements - if (auto switchOp = dyn_cast<cir::SwitchOp>(term)) { - if (switchAllPathsReturn(switchOp)) { - hasLiveReturn = true; - continue; - } + // Also walk switch operations to handle their fall-through behavior + cfg.walk([&](cir::SwitchOp switchOp) { + if (switchAllPathsReturn(switchOp)) { + hasLiveReturn = true; + } else { // Switch may fall through hasPlainEdge = true; - continue; + } + }); + + // Walk if operations - if they don't cover all paths, they may fall through + cfg.walk([&](cir::IfOp ifOp) { + // An if statement can fall through if: + // 1. It has no else region, or + // 2. The else region doesn't return + bool thenReturns = false; + bool elseReturns = false; + + // Check if then region returns + ifOp.getThenRegion().walk([&](cir::ReturnOp ret) { + thenReturns = true; + }); + + // Check if there's an else region and if it returns + if (!ifOp.getElseRegion().empty()) { + ifOp.getElseRegion().walk([&](cir::ReturnOp ret) { + elseReturns = true; + }); } - // TODO: Maybe one day throw will be terminator? - // - // TODO: We need to add a microsoft inline assembly enum - - // TODO: We don't concer with try op either since it's not terminator + // If both branches return, mark as liveReturn + // Otherwise, it may fall through + if (thenReturns && elseReturns) { + hasLiveReturn = true; + } else { + hasPlainEdge = true; + } + }); - hasPlainEdge = true; - } + // Walk TryOp for abnormal edges + cfg.walk([&](cir::TryOp tryOp) { + hasAbnormalEdge = true; + }); if (!hasPlainEdge) { if (hasLiveReturn) diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index f584a2a5824b2..7c0de4686d1bc 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -5066,6 +5066,12 @@ bool CompilerInvocation::CreateFromArgsImpl( isCodeGenAction(Res.getFrontendOpts().ProgramAction)) Diags.Report(diag::warn_drv_openacc_without_cir); + // Enable CIR Fallthrough analysis LangOpt if requested + for (const auto &Analysis : Res.getFrontendOpts().ClangIRAnalysisList) { + if (Analysis == "fallthrough") + LangOpts.CIRFallThroughAnalysis = true; + } + // Set the triple of the host for OpenMP device compile. if (LangOpts.OpenMPIsTargetDevice) Res.getTargetOpts().HostTriple = Res.getFrontendOpts().AuxTriple; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 25b89d65847ad..39008e73a17ad 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -16525,6 +16525,9 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body, bool IsInstantiation, FD->hasAttr<NakedAttr>()) WP.disableCheckFallThrough(); + if (getLangOpts().CIRFallThroughAnalysis) + WP.disableCheckFallThrough(); + // MSVC permits the use of pure specifier (=0) on function definition, // defined at class scope, warn about this non-standard construct. if (getLangOpts().MicrosoftExt && FD->isPureVirtual() && diff --git a/clang/test/CIR/Analysis/fallthrough_1.c b/clang/test/CIR/Analysis/fallthrough_1.c index ef27ad51494b6..6d5a215d6e8ff 100644 --- a/clang/test/CIR/Analysis/fallthrough_1.c +++ b/clang/test/CIR/Analysis/fallthrough_1.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir-analysis="fallthrough" -verify +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir -fclangir-analysis="fallthrough" -emit-cir -o /dev/null -verify // INFO: These test cases are derived from clang/test/Sema/return.c int unknown(void); diff --git a/clang/test/CIR/Analysis/fallthrough_2.c b/clang/test/CIR/Analysis/fallthrough_2.c index 74eb8b8284efb..57931de219491 100644 --- a/clang/test/CIR/Analysis/fallthrough_2.c +++ b/clang/test/CIR/Analysis/fallthrough_2.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir-analysis="fallthrough" -verify +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir -fclangir-analysis="fallthrough" -emit-cir -o /dev/null -verify // INFO: These test cases are derived from clang/test/Sema/return.c int test1(void) { diff --git a/clang/test/CIR/Analysis/fallthrough_2.cir b/clang/test/CIR/Analysis/fallthrough_2.cir new file mode 100644 index 0000000000000..d7a9d18daae93 --- /dev/null +++ b/clang/test/CIR/Analysis/fallthrough_2.cir @@ -0,0 +1,141 @@ +!s32i = !cir.int<s, 32> +module @"/Users/juliusalexandre/Projects/MainRepo/llvm-project/clang/test/CIR/Analysis/fallthrough_2.c" attributes {cir.lang = #cir.lang<c>, cir.module_asm = [], cir.triple = "x86_64-unknown-linux-gnu", dlti.dl_spec = #dlti.dl_spec<!llvm.ptr<270> = dense<32> : vector<4xi64>, !llvm.ptr<271> = dense<32> : vector<4xi64>, !llvm.ptr<272> = dense<64> : vector<4xi64>, i64 = dense<64> : vector<2xi64>, i128 = dense<128> : vector<2xi64>, f80 = dense<128> : vector<2xi64>, !llvm.ptr = dense<64> : vector<4xi64>, i1 = dense<8> : vector<2xi64>, i8 = dense<8> : vector<2xi64>, i16 = dense<16> : vector<2xi64>, i32 = dense<32> : vector<2xi64>, f16 = dense<16> : vector<2xi64>, f64 = dense<64> : vector<2xi64>, f128 = dense<128> : vector<2xi64>, "dlti.endianness" = "little", "dlti.mangling_mode" = "e", "dlti.legal_int_widths" = array<i32: 8, 16, 32, 64>, "dlti.stack_alignment" = 128 : i64>} { + cir.func dso_local @test1() -> !s32i inline(never) { + %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64} loc(#loc2) + %1 = cir.load %0 : !cir.ptr<!s32i>, !s32i loc(#loc2) + cir.return %1 : !s32i loc(#loc2) + } loc(#loc41) + cir.func dso_local @test3() -> !s32i inline(never) { + %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64} loc(#loc4) + cir.goto "a" loc(#loc43) + ^bb1: // no predecessors + cir.label "a" loc(#loc7) + %1 = cir.load %0 : !cir.ptr<!s32i>, !s32i loc(#loc4) + cir.return %1 : !s32i loc(#loc4) + } loc(#loc42) + cir.func dso_local @test20() -> !s32i inline(never) { + %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64} loc(#loc9) + %1 = cir.alloca !s32i, !cir.ptr<!s32i>, ["i"] {alignment = 4 : i64} loc(#loc45) + cir.scope { + %3 = cir.load align(4) %1 : !cir.ptr<!s32i>, !s32i loc(#loc14) + %4 = cir.cast int_to_bool %3 : !s32i -> !cir.bool loc(#loc14) + cir.if %4 { + %5 = cir.const #cir.int<0> : !s32i loc(#loc16) + cir.store %5, %0 : !s32i, !cir.ptr<!s32i> loc(#loc48) + %6 = cir.load %0 : !cir.ptr<!s32i>, !s32i loc(#loc48) + cir.return %6 : !s32i loc(#loc48) + } else { + cir.scope { + %5 = cir.const #cir.int<0> : !s32i loc(#loc18) + %6 = cir.cast int_to_bool %5 : !s32i -> !cir.bool loc(#loc18) + cir.if %6 { + %7 = cir.const #cir.int<2> : !s32i loc(#loc13) + cir.store %7, %0 : !s32i, !cir.ptr<!s32i> loc(#loc50) + %8 = cir.load %0 : !cir.ptr<!s32i>, !s32i loc(#loc50) + cir.return %8 : !s32i loc(#loc50) + } loc(#loc50) + } loc(#loc49) + } loc(#loc47) + } loc(#loc46) + %2 = cir.load %0 : !cir.ptr<!s32i>, !s32i loc(#loc9) + cir.return %2 : !s32i loc(#loc9) + } loc(#loc44) + cir.func dso_local @test22() -> !s32i inline(never) { + %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64} loc(#loc21) + %1 = cir.alloca !s32i, !cir.ptr<!s32i>, ["i"] {alignment = 4 : i64} loc(#loc52) + cir.scope { + %3 = cir.load align(4) %1 : !cir.ptr<!s32i>, !s32i loc(#loc26) + cir.switch (%3 : !s32i) { + cir.case(default, []) { + cir.yield loc(#loc27) + } loc(#loc27) + cir.yield loc(#loc24) + } loc(#loc24) + } loc(#loc53) + %2 = cir.load %0 : !cir.ptr<!s32i>, !s32i loc(#loc21) + cir.return %2 : !s32i loc(#loc21) + } loc(#loc51) + cir.func dso_local @test23() -> !s32i inline(never) { + %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64} loc(#loc29) + %1 = cir.alloca !s32i, !cir.ptr<!s32i>, ["i"] {alignment = 4 : i64} loc(#loc55) + cir.scope { + %3 = cir.load align(4) %1 : !cir.ptr<!s32i>, !s32i loc(#loc34) + cir.switch (%3 : !s32i) { + cir.case(equal, [#cir.int<0> : !s32i]) { + %4 = cir.const #cir.int<0> : !s32i loc(#loc36) + cir.store %4, %0 : !s32i, !cir.ptr<!s32i> loc(#loc57) + %5 = cir.load %0 : !cir.ptr<!s32i>, !s32i loc(#loc57) + cir.return %5 : !s32i loc(#loc57) + } loc(#loc35) + cir.case(equal, [#cir.int<2> : !s32i]) { + %4 = cir.const #cir.int<2> : !s32i loc(#loc39) + cir.store %4, %0 : !s32i, !cir.ptr<!s32i> loc(#loc58) + %5 = cir.load %0 : !cir.ptr<!s32i>, !s32i loc(#loc58) + cir.return %5 : !s32i loc(#loc58) + } loc(#loc38) + cir.yield loc(#loc32) + } loc(#loc32) + } loc(#loc56) + %2 = cir.load %0 : !cir.ptr<!s32i>, !s32i loc(#loc29) + cir.return %2 : !s32i loc(#loc29) + } loc(#loc54) +} loc(#loc) +#loc = loc("/Users/juliusalexandre/Projects/MainRepo/llvm-project/clang/test/CIR/Analysis/fallthrough_2.c":0:0) +#loc1 = loc("clang/test/CIR/Analysis/fallthrough_2.c":4:1) +#loc2 = loc("clang/test/CIR/Analysis/fallthrough_2.c":5:1) +#loc3 = loc("clang/test/CIR/Analysis/fallthrough_2.c":7:1) +#loc4 = loc("clang/test/CIR/Analysis/fallthrough_2.c":10:1) +#loc5 = loc("clang/test/CIR/Analysis/fallthrough_2.c":8:3) +#loc6 = loc("clang/test/CIR/Analysis/fallthrough_2.c":8:8) +#loc7 = loc("clang/test/CIR/Analysis/fallthrough_2.c":9:3) +#loc8 = loc("clang/test/CIR/Analysis/fallthrough_2.c":12:1) +#loc9 = loc("clang/test/CIR/Analysis/fallthrough_2.c":18:1) +#loc10 = loc("clang/test/CIR/Analysis/fallthrough_2.c":13:3) +#loc11 = loc("clang/test/CIR/Analysis/fallthrough_2.c":13:7) +#loc12 = loc("clang/test/CIR/Analysis/fallthrough_2.c":14:3) +#loc13 = loc("clang/test/CIR/Analysis/fallthrough_2.c":17:12) +#loc14 = loc("clang/test/CIR/Analysis/fallthrough_2.c":14:7) +#loc15 = loc("clang/test/CIR/Analysis/fallthrough_2.c":15:5) +#loc16 = loc("clang/test/CIR/Analysis/fallthrough_2.c":15:12) +#loc17 = loc("clang/test/CIR/Analysis/fallthrough_2.c":16:8) +#loc18 = loc("clang/test/CIR/Analysis/fallthrough_2.c":16:12) +#loc19 = loc("clang/test/CIR/Analysis/fallthrough_2.c":17:5) +#loc20 = loc("clang/test/CIR/Analysis/fallthrough_2.c":20:1) +#loc21 = loc("clang/test/CIR/Analysis/fallthrough_2.c":23:1) +#loc22 = loc("clang/test/CIR/Analysis/fallthrough_2.c":21:3) +#loc23 = loc("clang/test/CIR/Analysis/fallthrough_2.c":21:7) +#loc24 = loc("clang/test/CIR/Analysis/fallthrough_2.c":22:3) +#loc25 = loc("clang/test/CIR/Analysis/fallthrough_2.c":22:23) +#loc26 = loc("clang/test/CIR/Analysis/fallthrough_2.c":22:11) +#loc27 = loc("clang/test/CIR/Analysis/fallthrough_2.c":22:14) +#loc28 = loc("clang/test/CIR/Analysis/fallthrough_2.c":25:1) +#loc29 = loc("clang/test/CIR/Analysis/fallthrough_2.c":33:1) +#loc30 = loc("clang/test/CIR/Analysis/fallthrough_2.c":26:3) +#loc31 = loc("clang/test/CIR/Analysis/fallthrough_2.c":26:7) +#loc32 = loc("clang/test/CIR/Analysis/fallthrough_2.c":27:3) +#loc33 = loc("clang/test/CIR/Analysis/fallthrough_2.c":32:3) +#loc34 = loc("clang/test/CIR/Analysis/fallthrough_2.c":27:11) +#loc35 = loc("clang/test/CIR/Analysis/fallthrough_2.c":28:3) +#loc36 = loc("clang/test/CIR/Analysis/fallthrough_2.c":29:12) +#loc37 = loc("clang/test/CIR/Analysis/fallthrough_2.c":29:5) +#loc38 = loc("clang/test/CIR/Analysis/fallthrough_2.c":30:3) +#loc39 = loc("clang/test/CIR/Analysis/fallthrough_2.c":31:12) +#loc40 = loc("clang/test/CIR/Analysis/fallthrough_2.c":31:5) +#loc41 = loc(fused[#loc1, #loc2]) +#loc42 = loc(fused[#loc3, #loc4]) +#loc43 = loc(fused[#loc5, #loc6]) +#loc44 = loc(fused[#loc8, #loc9]) +#loc45 = loc(fused[#loc10, #loc11]) +#loc46 = loc(fused[#loc12, #loc13]) +#loc47 = loc(fused[#loc15, #loc16, #loc17, #loc13]) +#loc48 = loc(fused[#loc15, #loc16]) +#loc49 = loc(fused[#loc17, #loc13]) +#loc50 = loc(fused[#loc19, #loc13]) +#loc51 = loc(fused[#loc20, #loc21]) +#loc52 = loc(fused[#loc22, #loc23]) +#loc53 = loc(fused[#loc24, #loc25]) +#loc54 = loc(fused[#loc28, #loc29]) +#loc55 = loc(fused[#loc30, #loc31]) +#loc56 = loc(fused[#loc32, #loc33]) +#loc57 = loc(fused[#loc37, #loc36]) +#loc58 = loc(fused[#loc40, #loc39]) diff --git a/clang/test/CIR/Analysis/fallthrough_switch.c b/clang/test/CIR/Analysis/fallthrough_switch.c index b0e5bb00b1412..6a234dda5e559 100644 --- a/clang/test/CIR/Analysis/fallthrough_switch.c +++ b/clang/test/CIR/Analysis/fallthrough_switch.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir-analysis="fallthrough" -verify +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fclangir -fclangir-analysis="fallthrough" -emit-cir -o /dev/null -verify // INFO: Switch statement test cases for CIR fallthrough analysis // Derived from clang/test/Sema/return.c @@ -27,7 +27,8 @@ int test_enum_cases(enum Cases C) { case C4: return 3; case C3: return 4; } -} // No warning - enum covers all cases (TODO: CIR may need enum coverage analysis) +} // expected-warning {{non-void function does not return a value in all control paths}} +// TODO: Should not warn - enum covers all cases (needs enum coverage analysis) // =========================================================================== // Test cases where switch SHOULD warn _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
