From 8fca6cf5969b00357ecf6b33a008f2633c5f0b93 Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Wed, 4 Jan 2017 13:40:34 +0000 Subject: [PATCH] Add -f[no-]strict-return flag that can be used to avoid undefined behaviour in non-void functions that fall off at the end without returning a value when compiling C++. Clang uses the new compiler flag to determine when it should treat control flow paths that fall off the end of a non-void function as unreachable. If -fno-strict-return is on, the code generator emits the ureachable and trap IR only when the function returns either a record type with a non-trivial destructor or another non-trivially copyable type. The primary goal of this flag is to avoid treating falling off the end of a non-void function as undefined behaviour. The burden of undefined behaviour is placed on the caller instead: if the caller ignores the returned value then the undefined behaviour is avoided. This kind of behaviour is useful in several cases, e.g. when compiling C code in C++ mode. rdar://13102603 Differential Revision: https://reviews.llvm.org/D27163 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@290960 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Driver/Options.td | 6 ++ include/clang/Frontend/CodeGenOptions.def | 4 + lib/CodeGen/CodeGenFunction.cpp | 27 +++++- lib/Driver/Tools.cpp | 3 + lib/Frontend/CompilerInvocation.cpp | 1 + test/CodeGenCXX/return.cpp | 101 +++++++++++++++++++++- test/CodeGenObjCXX/return.mm | 29 +++++++ test/Driver/clang_f_opts.c | 5 ++ 8 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 test/CodeGenObjCXX/return.mm diff --git a/include/clang/Driver/Options.td b/include/clang/Driver/Options.td index 1f1222e106..f102559551 100644 --- a/include/clang/Driver/Options.td +++ b/include/clang/Driver/Options.td @@ -1331,6 +1331,12 @@ def funique_section_names : Flag <["-"], "funique-section-names">, def fno_unique_section_names : Flag <["-"], "fno-unique-section-names">, Group, Flags<[CC1Option]>; +def fstrict_return : Flag<["-"], "fstrict-return">, Group, + Flags<[CC1Option]>, + HelpText<"Always treat control flow paths that fall off the end of a non-void" + "function as unreachable">; +def fno_strict_return : Flag<["-"], "fno-strict-return">, Group, + Flags<[CC1Option]>; def fdebug_types_section: Flag <["-"], "fdebug-types-section">, Group, Flags<[CC1Option]>, HelpText<"Place debug types in their own section (ELF Only)">; diff --git a/include/clang/Frontend/CodeGenOptions.def b/include/clang/Frontend/CodeGenOptions.def index 1f0c83b5bf..54c9f81265 100644 --- a/include/clang/Frontend/CodeGenOptions.def +++ b/include/clang/Frontend/CodeGenOptions.def @@ -251,6 +251,10 @@ CODEGENOPT(DiagnosticsWithHotness, 1, 0) /// 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 diff --git a/lib/CodeGen/CodeGenFunction.cpp b/lib/CodeGen/CodeGenFunction.cpp index a954f487d1..7cab13de92 100644 --- a/lib/CodeGen/CodeGenFunction.cpp +++ b/lib/CodeGen/CodeGenFunction.cpp @@ -1049,6 +1049,19 @@ QualType CodeGenFunction::BuildFunctionArgList(GlobalDecl GD, return ResTy; } +static bool +shouldUseUndefinedBehaviorReturnOptimization(const FunctionDecl *FD, + const ASTContext &Context) { + QualType T = FD->getReturnType(); + // Avoid the optimization for functions that return a record type with a + // trivial destructor or another trivially copyable type. + if (const RecordType *RT = T.getCanonicalType()->getAs()) { + if (const auto *ClassDecl = dyn_cast(RT->getDecl())) + return !ClassDecl->hasTrivialDestructor(); + } + return !T.isTriviallyCopyableType(Context); +} + void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn, const CGFunctionInfo &FnInfo) { const FunctionDecl *FD = cast(GD.getDecl()); @@ -1127,17 +1140,23 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn, // function call is used by the caller, the behavior is undefined. if (getLangOpts().CPlusPlus && !FD->hasImplicitReturnZero() && !SawAsmBlock && !FD->getReturnType()->isVoidType() && Builder.GetInsertBlock()) { + bool ShouldEmitUnreachable = + CGM.getCodeGenOpts().StrictReturn || + shouldUseUndefinedBehaviorReturnOptimization(FD, getContext()); if (SanOpts.has(SanitizerKind::Return)) { SanitizerScope SanScope(this); llvm::Value *IsFalse = Builder.getFalse(); EmitCheck(std::make_pair(IsFalse, SanitizerKind::Return), SanitizerHandler::MissingReturn, EmitCheckSourceLocation(FD->getLocation()), None); - } else if (CGM.getCodeGenOpts().OptimizationLevel == 0) { - EmitTrapCall(llvm::Intrinsic::trap); + } else if (ShouldEmitUnreachable) { + if (CGM.getCodeGenOpts().OptimizationLevel == 0) + EmitTrapCall(llvm::Intrinsic::trap); + } + if (SanOpts.has(SanitizerKind::Return) || ShouldEmitUnreachable) { + Builder.CreateUnreachable(); + Builder.ClearInsertionPoint(); } - Builder.CreateUnreachable(); - Builder.ClearInsertionPoint(); } // Emit the standard function epilogue. diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp index 01a17dbf51..874799b110 100644 --- a/lib/Driver/Tools.cpp +++ b/lib/Driver/Tools.cpp @@ -4462,6 +4462,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, 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)) diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index a0682e26e7..ca4a7655a3 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -602,6 +602,7 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, 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) || diff --git a/test/CodeGenCXX/return.cpp b/test/CodeGenCXX/return.cpp index 5c1cfdaeed..6166cccd72 100644 --- a/test/CodeGenCXX/return.cpp +++ b/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: @_Z27returnNotViableDontOptimize4Enum +int returnNotViableDontOptimize(Enum e) { + switch (e) { + case A: return 1; + case B: return 2; + } + // Undefined behaviour optimization shouldn't be used when -fno-strict-return + // is turned on, even if all the 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 +} + +struct NonTrivialCopy { + NonTrivialCopy(const NonTrivialCopy &); +}; + +// CHECK-NOSTRICT-LABEL: @_Z14nonTrivialCopyv +NonTrivialCopy nonTrivialCopy() { + // CHECK-NOSTRICT-NOT: call void @llvm.trap + // CHECK-NOSTRICT-NOT: unreachable +} + +struct NonTrivialDefaultConstructor { + int x; + + NonTrivialDefaultConstructor() { } +}; + +// CHECK-NOSTRICT-LABEL: @_Z28nonTrivialDefaultConstructorv +NonTrivialDefaultConstructor nonTrivialDefaultConstructor() { + // CHECK-NOSTRICT-NOT: call void @llvm.trap + // CHECK-NOSTRICT-NOT: unreachable +} + +// Functions that return records with non-trivial destructors should always use +// the -fstrict-return optimization. + +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 } diff --git a/test/CodeGenObjCXX/return.mm b/test/CodeGenObjCXX/return.mm new file mode 100644 index 0000000000..53343e12b2 --- /dev/null +++ b/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 diff --git a/test/Driver/clang_f_opts.c b/test/Driver/clang_f_opts.c index 9056621721..210e16935d 100644 --- a/test/Driver/clang_f_opts.c +++ b/test/Driver/clang_f_opts.c @@ -477,3 +477,8 @@ // CHECK-NEW-PM-NOT: -fno-experimental-new-pass-manager // CHECK-NO-NEW-PM: -fno-experimental-new-pass-manager // CHECK-NO-NEW-PM-NOT: -fexperimental-new-pass-manager + +// 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" -- 2.40.0