arphaman created this revision.
arphaman added reviewers: rjmccall, rsmith, mehdi_amini.
arphaman added a subscriber: cfe-commits.
arphaman set the repository for this revision to rL LLVM.

This patch adds a new clang flag called `-f[no-]strict-return`. The purpose of 
this flag is to control how the code generator applies the undefined behaviour 
return optimisation to value returning functions that flow-off without a 
required return:

- If -fstrict-return is on (default for non Darwin targets), then the code 
generator follows the current behaviour: it emits the IR for the undefined 
behaviour (trap with unreachable).

- Otherwise, the code generator emits the IR for the undefined behaviour only 
if the function avoided -Wreturn-type warnings. This avoidance is detected even 
if -Wreturn-type warnings are disabled (-Wno-return-type).




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/Driver/clang_f_opts.c
  test/Driver/darwin-no-strict-return.c

Index: test/Driver/darwin-no-strict-return.c
===================================================================
--- /dev/null
+++ test/Driver/darwin-no-strict-return.c
@@ -0,0 +1,7 @@
+// Darwin should have -fno-strict-return turned on by default
+// rdar://13102603
+
+// RUN: %clang -### -S -target x86_64-apple-macosx10.7.0 %s 2>&1 | FileCheck -check-prefix=CHECK-NO-STRICT-RETURN %s
+// RUN: %clang -### -S -target x86_64-apple-macosx10.7.0 -fstrict-return %s 2>&1 | FileCheck -check-prefix=CHECK-STRICT-RETURN %s
+// CHECK-NO-STRICT-RETURN: "-fno-strict-return"
+// CHECK-STRICT-RETURN-NOT: "-fno-strict-return"
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/CodeGenCXX/return.cpp
===================================================================
--- test/CodeGenCXX/return.cpp
+++ test/CodeGenCXX/return.cpp
@@ -1,12 +1,61 @@
 // 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 -fno-strict-return -o - %s | FileCheck %s --check-prefix=CHECK-NOSTRICT
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -fno-strict-return -Wno-return-type -o - %s | FileCheck %s --check-prefix=CHECK-NOSTRICT
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -fno-strict-return -O -o - %s | FileCheck %s --check-prefix=CHECK-NOSTRICT-OPT
 
 // CHECK:     @_Z9no_return
 // CHECK-OPT: @_Z9no_return
+// CHECK-NOSTRICT:     @_Z9no_return
+// CHECK-NOSTRICT-OPT: @_Z9no_return
 int no_return() {
   // CHECK:      call void @llvm.trap
   // CHECK-NEXT: unreachable
 
   // CHECK-OPT-NOT: call void @llvm.trap
   // CHECK-OPT:     unreachable
+
+  // 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:     @_Z21alwaysOptimizedReturn4Enum
+// CHECK-OPT: @_Z21alwaysOptimizedReturn4Enum
+// CHECK-NOSTRICT:     @_Z21alwaysOptimizedReturn4Enum
+// CHECK-NOSTRICT-OPT: @_Z21alwaysOptimizedReturn4Enum
+int alwaysOptimizedReturn(Enum e) {
+  switch (e) {
+  case A: return 0;
+  case B: return 1;
+  }
+  // CHECK-NOSTRICT:      call void @llvm.trap()
+  // CHECK-NOSTRICT-NEXT: unreachable
+
+  // CHECK-NOSTRICT-OPT: entry:
+  // CHECK-NOSTRICT-OPT-NEXT: icmp
+  // CHECK-NOSTRICT-OPT-NEXT: zext
+  // CHECK-NOSTRICT-OPT-NEXT: ret i32
+}
+
+// CHECK-NOSTRICT:     @_Z23strictlyOptimizedReturn4Enum
+// CHECK-NOSTRICT-OPT: @_Z23strictlyOptimizedReturn4Enum
+int strictlyOptimizedReturn(Enum e) {
+  switch (e) {
+  case A: return 22;
+  }
+  // CHECK-NOSTRICT-NOT: call void @llvm.trap
+  // CHECK-NOSTRICT-NOT: unreachable
+
+  // CHECK-NOSTRICT-OPT:     entry:
+  // CHECK-NOSTRICT-OPT-NEXT: ret i32 22
 }
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -1154,7 +1154,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 \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 undefined
+  // behaviour optimization isn't used 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,10 @@
   if (Args.hasFlag(options::OPT_fstrict_enums, options::OPT_fno_strict_enums,
                    false))
     CmdArgs.push_back("-fstrict-enums");
+  // -fno-strict-return is turned on by default on Darwin.
+  if (!Args.hasFlag(options::OPT_fstrict_return, options::OPT_fno_strict_return,
+                    !getToolChain().getTriple().isOSDarwin()))
+    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
@@ -1151,11 +1151,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 ||
+               !FD->hasNonVoidReturnWarning()) {
+      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
@@ -1193,8 +1193,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

Reply via email to