From 097ce7616527b8948b2a69d1300a44f552959a43 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Fri, 25 Nov 2022 10:24:56 +0100 Subject: [PATCH] [analyzer] Deprecate FAM analyzer-config, recommend -fstrict-flex-arrays instead By default, clang assumes that all trailing array objects could be a FAM. So, an array of undefined size, size 0, size 1, or even size 42 is considered as FAMs for optimizations at least. One needs to override the default behavior by supplying the `-fstrict-flex-arrays=` flag, with `N > 0` value to reduce the set of FAM candidates. Value `3` is the most restrictive and `0` is the most permissive on this scale. 0: all trailing arrays are FAMs 1: only incomplete, zero and one-element arrays are FAMs 2: only incomplete, zero-element arrays are FAMs 3: only incomplete arrays are FAMs If the user is happy with consdering single-element arrays as FAMs, they just need to remove the `consider-single-element-arrays-as-flexible-array-members` from the command line. Otherwise, if they don't want to recognize such cases as FAMs, they should specify `-fstrict-flex-arrays` anyway, which will be picked up by CSA. Any use of the deprecated analyzer-config value will trigger a warning explaining what to use instead. The `-analyzer-config-help` is updated accordingly. Depends on D138657 Reviewed By: xazax.hun Differential Revision: https://reviews.llvm.org/D138659 --- clang/docs/ReleaseNotes.rst | 6 +++++ clang/include/clang/Basic/DiagnosticDriverKinds.td | 4 +++ .../clang/StaticAnalyzer/Core/AnalyzerOptions.def | 3 ++- clang/lib/Frontend/CompilerInvocation.cpp | 9 +++++++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp | 30 ++++++++++++++-------- .../test/Analysis/deprecated-flags-and-options.cpp | 9 +++++++ clang/test/Analysis/flexible-array-members.c | 25 ++++++++++-------- 7 files changed, 63 insertions(+), 23 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ac7a183..e9280ca 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -827,6 +827,12 @@ Static Analyzer ``scanbuild`` was also updated accordingly. Passing these flags will result in a hard error. +- Deprecate the ``consider-single-element-arrays-as-flexible-array-members`` + analyzer-config option. + This option will be still accepted, but a warning will be displayed. + This option will be rejected, thus turned into a hard error starting with + ``clang-17``. Use ``-fstrict-flex-array=`` instead if necessary. + - Trailing array objects of structs with single elements will be considered as flexible-array-members. Use ``-fstrict-flex-array=`` to define what should be considered as flexible-array-member if needed. diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index 119ad57..6479518 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -458,6 +458,10 @@ def warn_analyzer_deprecated_option : Warning< "analyzer option '%0' is deprecated. This flag will be removed in %1, and " "passing this option will be an error.">, InGroup; +def warn_analyzer_deprecated_option_with_alternative : Warning< + "analyzer option '%0' is deprecated. This flag will be removed in %1, and " + "passing this option will be an error. Use '%2' instead.">, + InGroup; def warn_drv_needs_hvx : Warning< "%0 requires HVX, use -mhvx/-mhvx= to enable it">, diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def index 1f22801..acfbcf6 100644 --- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def +++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def @@ -331,7 +331,8 @@ ANALYZER_OPTION( "consider-single-element-arrays-as-flexible-array-members", "Consider single element arrays as flexible array member candidates. " "This will prevent the analyzer from assuming that a single element array " - "holds a single element.", + "holds a single element. [DEPRECATED, removing in clang-17; " + "use '-fstrict-flex-arrays=' instead]", true) ANALYZER_OPTION( diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index a13da5a..9536319 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -1018,6 +1018,15 @@ static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args, A->claim(); Opts.Config[key] = std::string(val); + + // FIXME: Remove this hunk after clang-17 released. + constexpr auto SingleFAM = + "consider-single-element-arrays-as-flexible-array-members"; + if (key == SingleFAM) { + Diags.Report(diag::warn_analyzer_deprecated_option_with_alternative) + << SingleFAM << "clang-17" + << "-fstrict-flex-arrays="; + } } } diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp index 0b504c3..289b0a7 100644 --- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp +++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -790,22 +790,30 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const MemRegion *MR, return true; if (const auto *CAT = dyn_cast(AT)) { - const llvm::APInt &Size = CAT->getSize(); - if (Size.isZero()) - return true; - using FAMKind = LangOptions::StrictFlexArraysLevelKind; const FAMKind StrictFlexArraysLevel = Ctx.getLangOpts().getStrictFlexArraysLevel(); - if (StrictFlexArraysLevel == FAMKind::ZeroOrIncomplete || - StrictFlexArraysLevel == FAMKind::IncompleteOnly) - return false; - const AnalyzerOptions &Opts = SVB.getAnalyzerOptions(); - // FIXME: this option is probably redundant with -fstrict-flex-arrays=1. - if (Opts.ShouldConsiderSingleElementArraysAsFlexibleArrayMembers && - Size.isOne()) + const llvm::APInt &Size = CAT->getSize(); + + if (StrictFlexArraysLevel <= FAMKind::ZeroOrIncomplete && Size.isZero()) return true; + + // The "-fstrict-flex-arrays" should have precedence over + // consider-single-element-arrays-as-flexible-array-members + // analyzer-config when checking single element arrays. + if (StrictFlexArraysLevel == FAMKind::Default) { + // FIXME: After clang-17 released, we should remove this branch. + if (Opts.ShouldConsiderSingleElementArraysAsFlexibleArrayMembers && + Size.isOne()) + return true; + } else { + // -fstrict-flex-arrays was specified, since it's not the default, so + // ignore analyzer-config. + if (StrictFlexArraysLevel <= FAMKind::OneZeroOrIncomplete && + Size.isOne()) + return true; + } } return false; }; diff --git a/clang/test/Analysis/deprecated-flags-and-options.cpp b/clang/test/Analysis/deprecated-flags-and-options.cpp index 23272c1..6fbb311 100644 --- a/clang/test/Analysis/deprecated-flags-and-options.cpp +++ b/clang/test/Analysis/deprecated-flags-and-options.cpp @@ -9,6 +9,15 @@ // RUN: | FileCheck %s --check-prefixes=DEPRECATED-NESTED-BLOCKS // DEPRECATED-NESTED-BLOCKS: error: unknown argument: '-analyzer-opt-analyze-nested-blocks' +// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config consider-single-element-arrays-as-flexible-array-members=true %s 2>&1 \ +// RUN: | FileCheck %s --check-prefixes=CHECK,DEPRECATED-SINGLE-ELEM-FAM +// DEPRECATED-SINGLE-ELEM-FAM: warning: analyzer option 'consider-single-element-arrays-as-flexible-array-members' is deprecated. This flag will be removed in clang-17, and passing this option will be an error. Use '-fstrict-flex-arrays=' instead. + +// RUN: %clang_analyze_cc1 -analyzer-config-help 2>&1 \ +// RUN: | FileCheck %s --check-prefixes=CHECK-HELP +// CHECK-HELP: [DEPRECATED, removing in clang-17; use '-fstrict-flex-arrays=' +// CHECK-HELP-NEXT: instead] (default: true) + int empty(int x) { // CHECK: warning: Division by zero return x ? 0 : 0 / x; diff --git a/clang/test/Analysis/flexible-array-members.c b/clang/test/Analysis/flexible-array-members.c index a139883..dc131c6 100644 --- a/clang/test/Analysis/flexible-array-members.c +++ b/clang/test/Analysis/flexible-array-members.c @@ -1,27 +1,30 @@ +// -fstrict-flex-arrays=2 means that only undefined or zero element arrays are considered as FAMs. + // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c90 \ -// RUN: -analyzer-config consider-single-element-arrays-as-flexible-array-members=false +// RUN: -fstrict-flex-arrays=2 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c99 \ -// RUN: -analyzer-config consider-single-element-arrays-as-flexible-array-members=false +// RUN: -fstrict-flex-arrays=2 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c11 \ -// RUN: -analyzer-config consider-single-element-arrays-as-flexible-array-members=false +// RUN: -fstrict-flex-arrays=2 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c17 \ -// RUN: -analyzer-config consider-single-element-arrays-as-flexible-array-members=false +// RUN: -fstrict-flex-arrays=2 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++98 -x c++ \ -// RUN: -analyzer-config consider-single-element-arrays-as-flexible-array-members=false +// RUN: -fstrict-flex-arrays=2 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++03 -x c++ \ -// RUN: -analyzer-config consider-single-element-arrays-as-flexible-array-members=false +// RUN: -fstrict-flex-arrays=2 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++11 -x c++ \ -// RUN: -analyzer-config consider-single-element-arrays-as-flexible-array-members=false +// RUN: -fstrict-flex-arrays=2 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++14 -x c++ \ -// RUN: -analyzer-config consider-single-element-arrays-as-flexible-array-members=false +// RUN: -fstrict-flex-arrays=2 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++17 -x c++ \ -// RUN: -analyzer-config consider-single-element-arrays-as-flexible-array-members=false +// RUN: -fstrict-flex-arrays=2 +// By default, -fstrict-flex-arrays=0, which means that even single element arrays are considered as FAMs. // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c17 \ -// RUN: -analyzer-config consider-single-element-arrays-as-flexible-array-members=true -DSINGLE_ELEMENT_FAMS +// RUN: -DSINGLE_ELEMENT_FAMS // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++17 -x c++ \ -// RUN: -analyzer-config consider-single-element-arrays-as-flexible-array-members=true -DSINGLE_ELEMENT_FAMS +// RUN: -DSINGLE_ELEMENT_FAMS typedef __typeof(sizeof(int)) size_t; size_t clang_analyzer_getExtent(void *); -- 2.7.4