arphaman updated this revision to Diff 79556.
arphaman added a comment.
The updated diff has the following changes:
- The default value for '-fstrict-return' is now the same across all platforms,
the Darwin specific -fno-strict-return was removed.
- The 'fno-strict-return' optimisation avoidance can only be done by functions
that return trivially copyable types.
- A new test verifies that Objective-C++ methods and blocks never get the
return optimisation regardless of the '-fstrict-return' flag.
- The updated test adds explanation comments and uses Mehdi's advice for LABEL
checks and common prefix.
Repository:
rL LLVM
https://reviews.llvm.org/D27163
Files:
include/clang/AST/Decl.h
include/clang/Basic/LangOptions.def
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
include/clang/Sema/AnalysisBasedWarnings.h
include/clang/Sema/Sema.h
lib/CodeGen/CodeGenFunction.cpp
lib/Driver/Tools.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/Sema.cpp
test/CodeGenCXX/return.cpp
test/CodeGenObjCXX/return.mm
test/Driver/clang_f_opts.c
Index: test/Driver/clang_f_opts.c
===================================================================
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -469,3 +469,8 @@
// CHECK-WCHAR2: -fshort-wchar
// CHECK-WCHAR2-NOT: -fno-short-wchar
// DELIMITERS: {{^ *"}}
+
+// RUN: %clang -### -S -fstrict-return %s 2>&1 | FileCheck -check-prefix=CHECK-STRICT-RETURN %s
+// RUN: %clang -### -S -fno-strict-return %s 2>&1 | FileCheck -check-prefix=CHECK-NO-STRICT-RETURN %s
+// CHECK-STRICT-RETURN-NOT: "-fno-strict-return"
+// CHECK-NO-STRICT-RETURN: "-fno-strict-return"
Index: test/CodeGenObjCXX/return.mm
===================================================================
--- /dev/null
+++ test/CodeGenObjCXX/return.mm
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -emit-llvm -fblocks -triple x86_64-apple-darwin -fstrict-return -o - %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -fblocks -triple x86_64-apple-darwin -fstrict-return -O -o - %s | FileCheck %s
+
+@interface I
+@end
+
+@implementation I
+
+- (int)method {
+}
+
+@end
+
+enum Enum {
+ a
+};
+
+int (^block)(Enum) = ^int(Enum e) {
+ switch (e) {
+ case a:
+ return 1;
+ }
+};
+
+// Ensure that both methods and blocks don't use the -fstrict-return undefined
+// behaviour optimization.
+
+// CHECK-NOT: call void @llvm.trap
+// CHECK-NOT: unreachable
Index: test/CodeGenCXX/return.cpp
===================================================================
--- test/CodeGenCXX/return.cpp
+++ test/CodeGenCXX/return.cpp
@@ -1,12 +1,105 @@
-// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
-// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -O -o - %s | FileCheck %s --check-prefix=CHECK-OPT
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -std=c++11 -o - %s | FileCheck --check-prefixes=CHECK,CHECK-COMMON %s
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -std=c++11 -O -o - %s | FileCheck %s --check-prefixes=CHECK-OPT,CHECK-COMMON
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -std=c++11 -fno-strict-return -o - %s | FileCheck %s --check-prefixes=CHECK-NOSTRICT,CHECK-COMMON
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -std=c++11 -fno-strict-return -Wno-return-type -o - %s | FileCheck %s --check-prefixes=CHECK-NOSTRICT,CHECK-COMMON
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -std=c++11 -fno-strict-return -O -o - %s | FileCheck %s --check-prefixes=CHECK-NOSTRICT-OPT,CHECK-COMMON
-// CHECK: @_Z9no_return
-// CHECK-OPT: @_Z9no_return
+// CHECK-COMMON-LABEL: @_Z9no_return
int no_return() {
// CHECK: call void @llvm.trap
// CHECK-NEXT: unreachable
// CHECK-OPT-NOT: call void @llvm.trap
// CHECK-OPT: unreachable
+
+ // -fno-strict-return should not emit trap + unreachable but it should return
+ // an undefined value instead.
+
+ // CHECK-NOSTRICT: entry:
+ // CHECK-NOSTRICT-NEXT: alloca
+ // CHECK-NOSTRICT-NEXT: load
+ // CHECK-NOSTRICT-NEXT: ret i32
+ // CHECK-NOSTRICT-NEXT: }
+
+ // CHECK-NOSTRICT-OPT: entry:
+ // CHECK-NOSTRICT-OPT: ret i32 undef
+}
+
+enum Enum {
+ A, B
+};
+
+// CHECK-COMMON-LABEL: @_Z21alwaysOptimizedReturn4Enum
+int alwaysOptimizedReturn(Enum e) {
+ switch (e) {
+ case A: return 1;
+ case B: return 2;
+ }
+ // This function covers all the cases of 'Enum', so the undefined behaviour
+ // optimization is allowed even if -fno-strict-return is used.
+
+ // CHECK-NOSTRICT: call void @llvm.trap()
+ // CHECK-NOSTRICT-NEXT: unreachable
+}
+
+// CHECK-NOSTRICT-LABEL: @_Z23strictlyOptimizedReturn4Enum
+int strictlyOptimizedReturn(Enum e) {
+ switch (e) {
+ case A: return 22;
+ }
+ // Undefined behaviour optimization shouldn't be used when -fno-strict-return
+ // is turned on, as not all enum cases are covered in this function.
+
+ // CHECK-NOSTRICT-NOT: call void @llvm.trap
+ // CHECK-NOSTRICT-NOT: unreachable
+}
+
+struct Trivial {
+ int x;
+};
+
+// CHECK-NOSTRICT-LABEL: @_Z7trivialv
+Trivial trivial() {
+ // This function returns a trivial record so -fno-strict-return should avoid
+ // the undefined behaviour optimization.
+
+ // CHECK-NOSTRICT-NOT: call void @llvm.trap
+ // CHECK-NOSTRICT-NOT: unreachable
+}
+
+// Functions that return non-trivial records should always use the
+// -fstrict-return optimization.
+
+struct NonTrivialCopy {
+ NonTrivialCopy(const NonTrivialCopy &);
+};
+
+// CHECK-NOSTRICT-LABEL: @_Z14nonTrivialCopyv
+NonTrivialCopy nonTrivialCopy() {
+ // CHECK-NOSTRICT: call void @llvm.trap
+ // CHECK-NOSTRICT-NEXT: unreachable
+}
+
+struct NonTrivialDestructor {
+ ~NonTrivialDestructor();
+};
+
+// CHECK-NOSTRICT-LABEL: @_Z20nonTrivialDestructorv
+NonTrivialDestructor nonTrivialDestructor() {
+ // CHECK-NOSTRICT: call void @llvm.trap
+ // CHECK-NOSTRICT-NEXT: unreachable
+}
+
+// The behavior for lambdas should be identical to functions.
+// CHECK-COMMON-LABEL: @_Z10lambdaTestv
+void lambdaTest() {
+ auto lambda1 = []() -> int {
+ };
+ lambda1();
+
+ // CHECK: call void @llvm.trap
+ // CHECK-NEXT: unreachable
+
+ // CHECK-NOSTRICT-NOT: call void @llvm.trap
+ // CHECK-NOSTRICT-NOT: unreachable
}
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -1153,7 +1153,7 @@
}
void Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP,
- const Decl *D, const BlockExpr *blkExpr) {
+ Decl *D, const BlockExpr *blkExpr) {
FunctionScopeInfo *Scope = FunctionScopes.pop_back_val();
assert(!FunctionScopes.empty() && "mismatched push/pop!");
Index: lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -521,13 +521,22 @@
} // anonymous namespace
+/// The given declaration \p D is marked as associated with a non-void return
+/// warning.
+static void markHasNonVoidReturnWarning(Decl *D) {
+ if (auto *FD = dyn_cast<FunctionDecl>(D))
+ FD->setHasNonVoidReturnWarning();
+ // Don't need to mark Objective-C methods or blocks since the IRGen doesn't
+ // emit undefined behaviour optimization IR for them.
+}
+
/// CheckFallThroughForFunctionDef - Check that we don't fall off the end of a
/// function that should return a value. Check that we don't fall off the end
/// of a noreturn function. We assume that functions and blocks not marked
/// noreturn will return.
-static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
+static void CheckFallThroughForBody(Sema &S, Decl *D, const Stmt *Body,
const BlockExpr *blkExpr,
- const CheckFallThroughDiagnostics& CD,
+ const CheckFallThroughDiagnostics &CD,
AnalysisDeclContext &AC) {
bool ReturnsVoid = false;
@@ -558,8 +567,9 @@
DiagnosticsEngine &Diags = S.getDiagnostics();
// Short circuit for compilation speed.
- if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn))
- return;
+ if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn) &&
+ !S.getLangOpts().CheckReturnControlFlow)
+ return;
SourceLocation LBrace = Body->getLocStart(), RBrace = Body->getLocEnd();
// Either in a function body compound statement, or a function-try-block.
@@ -570,14 +580,18 @@
case MaybeFallThrough:
if (HasNoReturn)
S.Diag(RBrace, CD.diag_MaybeFallThrough_HasNoReturn);
- else if (!ReturnsVoid)
+ else if (!ReturnsVoid) {
+ markHasNonVoidReturnWarning(D);
S.Diag(RBrace, CD.diag_MaybeFallThrough_ReturnsNonVoid);
+ }
break;
case AlwaysFallThrough:
if (HasNoReturn)
S.Diag(RBrace, CD.diag_AlwaysFallThrough_HasNoReturn);
- else if (!ReturnsVoid)
+ else if (!ReturnsVoid) {
+ markHasNonVoidReturnWarning(D);
S.Diag(RBrace, CD.diag_AlwaysFallThrough_ReturnsNonVoid);
+ }
break;
case NeverFallThroughOrReturn:
if (ReturnsVoid && !HasNoReturn && CD.diag_NeverFallThroughOrReturn) {
@@ -1891,10 +1905,9 @@
S.Diag(D.Loc, D.PD);
}
-void clang::sema::
-AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
- sema::FunctionScopeInfo *fscope,
- const Decl *D, const BlockExpr *blkExpr) {
+void clang::sema::AnalysisBasedWarnings::IssueWarnings(
+ sema::AnalysisBasedWarnings::Policy P, sema::FunctionScopeInfo *fscope,
+ Decl *D, const BlockExpr *blkExpr) {
// We avoid doing analysis-based warnings when there are errors for
// two reasons:
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -597,6 +597,7 @@
Opts.NoDwarfDirectoryAsm = Args.hasArg(OPT_fno_dwarf_directory_asm);
Opts.SoftFloat = Args.hasArg(OPT_msoft_float);
Opts.StrictEnums = Args.hasArg(OPT_fstrict_enums);
+ Opts.StrictReturn = !Args.hasArg(OPT_fno_strict_return);
Opts.StrictVTablePointers = Args.hasArg(OPT_fstrict_vtable_pointers);
Opts.UnsafeFPMath = Args.hasArg(OPT_menable_unsafe_fp_math) ||
Args.hasArg(OPT_cl_unsafe_math_optimizations) ||
@@ -2230,6 +2231,7 @@
Opts.SanitizeAddressFieldPadding =
getLastArgIntValue(Args, OPT_fsanitize_address_field_padding, 0, Diags);
Opts.SanitizerBlacklistFiles = Args.getAllArgValues(OPT_fsanitize_blacklist);
+ Opts.CheckReturnControlFlow = Args.hasArg(OPT_fno_strict_return);
}
static void ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
Index: lib/Driver/Tools.cpp
===================================================================
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4387,6 +4387,9 @@
if (Args.hasFlag(options::OPT_fstrict_enums, options::OPT_fno_strict_enums,
false))
CmdArgs.push_back("-fstrict-enums");
+ if (!Args.hasFlag(options::OPT_fstrict_return, options::OPT_fno_strict_return,
+ true))
+ CmdArgs.push_back("-fno-strict-return");
if (Args.hasFlag(options::OPT_fstrict_vtable_pointers,
options::OPT_fno_strict_vtable_pointers,
false))
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -1067,6 +1067,22 @@
return ResTy;
}
+/// Return true if the given function \p FD should use the undefined behavior
+/// return optimization.
+static bool
+shouldUseUndefinedBehaviorReturnOptimization(const FunctionDecl *FD,
+ const ASTContext &Context) {
+ if (!FD->hasNonVoidReturnWarning())
+ return true;
+ QualType T = FD->getReturnType();
+ if (T.isTriviallyCopyableType(Context)) {
+ // Avoid the optimization for functions that return trivially copyable
+ // types with a trivial destructor.
+ return false;
+ }
+ return true;
+}
+
void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
const CGFunctionInfo &FnInfo) {
const FunctionDecl *FD = cast<FunctionDecl>(GD.getDecl());
@@ -1151,11 +1167,15 @@
EmitCheck(std::make_pair(IsFalse, SanitizerKind::Return),
"missing_return", EmitCheckSourceLocation(FD->getLocation()),
None);
- } else if (CGM.getCodeGenOpts().OptimizationLevel == 0) {
- EmitTrapCall(llvm::Intrinsic::trap);
+ Builder.CreateUnreachable();
+ Builder.ClearInsertionPoint();
+ } else if (CGM.getCodeGenOpts().StrictReturn ||
+ shouldUseUndefinedBehaviorReturnOptimization(FD, getContext())) {
+ if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+ EmitTrapCall(llvm::Intrinsic::trap);
+ Builder.CreateUnreachable();
+ Builder.ClearInsertionPoint();
}
- Builder.CreateUnreachable();
- Builder.ClearInsertionPoint();
}
// Emit the standard function epilogue.
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1190,8 +1190,7 @@
CapturedRegionKind K);
void
PopFunctionScopeInfo(const sema::AnalysisBasedWarnings::Policy *WP = nullptr,
- const Decl *D = nullptr,
- const BlockExpr *blkExpr = nullptr);
+ Decl *D = nullptr, const BlockExpr *blkExpr = nullptr);
sema::FunctionScopeInfo *getCurFunction() const {
return FunctionScopes.back();
Index: include/clang/Sema/AnalysisBasedWarnings.h
===================================================================
--- include/clang/Sema/AnalysisBasedWarnings.h
+++ include/clang/Sema/AnalysisBasedWarnings.h
@@ -90,8 +90,8 @@
public:
AnalysisBasedWarnings(Sema &s);
- void IssueWarnings(Policy P, FunctionScopeInfo *fscope,
- const Decl *D, const BlockExpr *blkExpr);
+ void IssueWarnings(Policy P, FunctionScopeInfo *fscope, Decl *D,
+ const BlockExpr *blkExpr);
Policy getDefaultPolicy() { return DefaultPolicy; }
Index: include/clang/Frontend/CodeGenOptions.def
===================================================================
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -256,6 +256,10 @@
/// Whether copy relocations support is available when building as PIE.
CODEGENOPT(PIECopyRelocations, 1, 0)
+/// Whether we should use the undefined behaviour optimization for control flow
+/// paths that reach the end of a function without executing a required return.
+CODEGENOPT(StrictReturn, 1, 1)
+
#undef CODEGENOPT
#undef ENUM_CODEGENOPT
#undef VALUE_CODEGENOPT
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1317,6 +1317,12 @@
def fno_unique_section_names : Flag <["-"], "fno-unique-section-names">,
Group<f_Group>, Flags<[CC1Option]>;
+def fstrict_return : Flag<["-"], "fstrict-return">, Group<f_Group>,
+ Flags<[CC1Option]>,
+ HelpText<"Use C++ undefined behaviour optimization for control flow paths"
+ "that reach the end of the function without executing a required return">;
+def fno_strict_return : Flag<["-"], "fno-strict-return">, Group<f_Group>,
+ Flags<[CC1Option]>;
def fdebug_types_section: Flag <["-"], "fdebug-types-section">, Group<f_Group>,
Flags<[CC1Option]>, HelpText<"Place debug types in their own section (ELF Only)">;
Index: include/clang/Basic/LangOptions.def
===================================================================
--- include/clang/Basic/LangOptions.def
+++ include/clang/Basic/LangOptions.def
@@ -259,6 +259,10 @@
"field padding (0: none, 1:least "
"aggressive, 2: more aggressive)")
+LANGOPT(CheckReturnControlFlow, 1, 0, "Should the control flow for missing"
+ "returns be checked even if the"
+ "corresponding diagnostic is disabled")
+
#undef LANGOPT
#undef COMPATIBLE_LANGOPT
#undef BENIGN_LANGOPT
Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -1637,6 +1637,10 @@
/// hopes this is simpler.)
unsigned WillHaveBody : 1;
+ /// Indicates if analysis has found that control may reach the end of this
+ /// function without executing an appropriate return.
+ unsigned HasNonVoidReturnWarning : 1;
+
/// \brief End part of this FunctionDecl's source range.
///
/// We could compute the full range in getSourceRange(). However, when we're
@@ -1719,8 +1723,8 @@
IsExplicitlyDefaulted(false), HasImplicitReturnZero(false),
IsLateTemplateParsed(false), IsConstexpr(isConstexprSpecified),
UsesSEHTry(false), HasSkippedBody(false), WillHaveBody(false),
- EndRangeLoc(NameInfo.getEndLoc()), TemplateOrSpecialization(),
- DNLoc(NameInfo.getInfo()) {}
+ HasNonVoidReturnWarning(false), EndRangeLoc(NameInfo.getEndLoc()),
+ TemplateOrSpecialization(), DNLoc(NameInfo.getInfo()) {}
typedef Redeclarable<FunctionDecl> redeclarable_base;
FunctionDecl *getNextRedeclarationImpl() override {
@@ -2006,6 +2010,13 @@
bool willHaveBody() const { return WillHaveBody; }
void setWillHaveBody(bool V = true) { WillHaveBody = V; }
+ /// True if analysis has found that control may reach the end of this function
+ /// without executing an appropriate return.
+ bool hasNonVoidReturnWarning() const { return HasNonVoidReturnWarning; }
+ void setHasNonVoidReturnWarning(bool V = true) {
+ HasNonVoidReturnWarning = V;
+ }
+
void setPreviousDeclaration(FunctionDecl * PrevDecl);
FunctionDecl *getCanonicalDecl() override;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
