[clang] 170b686 - [Clang] Add a new warning to warn when passing uninitialized variables as const reference parameters to a function
Zequan Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 2 10:22:57 PDT 2020
Author: Zequan Wu
Date: 2020-06-02T10:21:02-07:00
New Revision: 170b6869b563dd3393d99f3e03d389b9058d5f24
URL: https://github.com/llvm/llvm-project/commit/170b6869b563dd3393d99f3e03d389b9058d5f24
DIFF: https://github.com/llvm/llvm-project/commit/170b6869b563dd3393d99f3e03d389b9058d5f24.diff
LOG: [Clang] Add a new warning to warn when passing uninitialized variables as const reference parameters to a function
Summary:
Add a new warning -Wuninitialized-const-reference as a subgroup of -Wuninitialized to address a bug filed here: https://bugs.llvm.org/show_bug.cgi?id=45624
This warning is controlled by -Wuninitialized and can be disabled by -Wno-uninitialized-const-reference.
The warning is diagnosed when passing uninitialized variables as const reference parameters to a function.
Differential Revision: https://reviews.llvm.org/D79895
Added:
clang/test/SemaCXX/warn-uninitialized-const-reference.cpp
Modified:
clang/include/clang/Analysis/Analyses/UninitializedValues.h
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/UninitializedValues.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/Misc/warning-wall.c
clang/test/SemaCXX/uninit-variables.cpp
clang/test/SemaCXX/uninitialized.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/Analyses/UninitializedValues.h b/clang/include/clang/Analysis/Analyses/UninitializedValues.h
index 479be1fec048..a2b37deddcec 100644
--- a/clang/include/clang/Analysis/Analyses/UninitializedValues.h
+++ b/clang/include/clang/Analysis/Analyses/UninitializedValues.h
@@ -110,6 +110,10 @@ class UninitVariablesHandler {
virtual void handleUseOfUninitVariable(const VarDecl *vd,
const UninitUse &use) {}
+ /// Called when the uninitialized variable is used as const refernce argument.
+ virtual void handleConstRefUseOfUninitVariable(const VarDecl *vd,
+ const UninitUse &use) {}
+
/// Called when the uninitialized variable analysis detects the
/// idiom 'int x = x'. All other uses of 'x' within the initializer
/// are handled by handleUseOfUninitVariable.
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 3f0521615a5e..451310a8c3a6 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -624,8 +624,10 @@ def Unicode : DiagGroup<"unicode">;
def UninitializedMaybe : DiagGroup<"conditional-uninitialized">;
def UninitializedSometimes : DiagGroup<"sometimes-uninitialized">;
def UninitializedStaticSelfInit : DiagGroup<"static-self-init">;
+def UninitializedConstReference : DiagGroup<"uninitialized-const-reference">;
def Uninitialized : DiagGroup<"uninitialized", [UninitializedSometimes,
- UninitializedStaticSelfInit]>;
+ UninitializedStaticSelfInit,
+ UninitializedConstReference]>;
def IgnoredPragmaIntrinsic : DiagGroup<"ignored-pragma-intrinsic">;
// #pragma optimize is often used to avoid to work around MSVC codegen bugs or
// to disable inlining. It's not completely clear what alternative to suggest
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index dd45b21724d2..5138b69eee74 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2111,6 +2111,10 @@ def err_list_init_in_parens : Error<
"cannot initialize %select{non-class|reference}0 type %1 with a "
"parenthesized initializer list">;
+def warn_uninit_const_reference : Warning<
+ "variable %0 is uninitialized when passed as a const reference argument "
+ "here">, InGroup<UninitializedConstReference>, DefaultIgnore;
+
def warn_unsequenced_mod_mod : Warning<
"multiple unsequenced modifications to %0">, InGroup<Unsequenced>;
def warn_unsequenced_mod_use : Warning<
diff --git a/clang/lib/Analysis/UninitializedValues.cpp b/clang/lib/Analysis/UninitializedValues.cpp
index ba64fbb81884..facda6ee616e 100644
--- a/clang/lib/Analysis/UninitializedValues.cpp
+++ b/clang/lib/Analysis/UninitializedValues.cpp
@@ -268,6 +268,7 @@ class ClassifyRefs : public StmtVisitor<ClassifyRefs> {
Init,
Use,
SelfInit,
+ ConstRefUse,
Ignore
};
@@ -413,14 +414,16 @@ void ClassifyRefs::VisitCallExpr(CallExpr *CE) {
return;
}
- // If a value is passed by const pointer or by const reference to a function,
+ // If a value is passed by const pointer to a function,
// we should not assume that it is initialized by the call, and we
// conservatively do not assume that it is used.
+ // If a value is passed by const reference to a function,
+ // it should already be initialized.
for (CallExpr::arg_iterator I = CE->arg_begin(), E = CE->arg_end();
I != E; ++I) {
if ((*I)->isGLValue()) {
if ((*I)->getType().isConstQualified())
- classify((*I), Ignore);
+ classify((*I), ConstRefUse);
} else if (isPointerToConst((*I)->getType())) {
const Expr *Ex = stripCasts(DC->getParentASTContext(), *I);
const auto *UO = dyn_cast<UnaryOperator>(Ex);
@@ -469,6 +472,7 @@ class TransferFunctions : public StmtVisitor<TransferFunctions> {
handler(handler) {}
void reportUse(const Expr *ex, const VarDecl *vd);
+ void reportConstRefUse(const Expr *ex, const VarDecl *vd);
void VisitBinaryOperator(BinaryOperator *bo);
void VisitBlockExpr(BlockExpr *be);
@@ -667,6 +671,12 @@ void TransferFunctions::reportUse(const Expr *ex, const VarDecl *vd) {
handler.handleUseOfUninitVariable(vd, getUninitUse(ex, vd, v));
}
+void TransferFunctions::reportConstRefUse(const Expr *ex, const VarDecl *vd) {
+ Value v = vals[vd];
+ if (isUninitialized(v))
+ handler.handleConstRefUseOfUninitVariable(vd, getUninitUse(ex, vd, v));
+}
+
void TransferFunctions::VisitObjCForCollectionStmt(ObjCForCollectionStmt *FS) {
// This represents an initialization of the 'element' value.
if (const auto *DS = dyn_cast<DeclStmt>(FS->getElement())) {
@@ -734,7 +744,10 @@ void TransferFunctions::VisitDeclRefExpr(DeclRefExpr *dr) {
vals[cast<VarDecl>(dr->getDecl())] = Initialized;
break;
case ClassifyRefs::SelfInit:
- handler.handleSelfInit(cast<VarDecl>(dr->getDecl()));
+ handler.handleSelfInit(cast<VarDecl>(dr->getDecl()));
+ break;
+ case ClassifyRefs::ConstRefUse:
+ reportConstRefUse(dr, cast<VarDecl>(dr->getDecl()));
break;
}
}
@@ -867,6 +880,12 @@ struct PruneBlocksHandler : public UninitVariablesHandler {
hadAnyUse = true;
}
+ void handleConstRefUseOfUninitVariable(const VarDecl *vd,
+ const UninitUse &use) override {
+ hadUse[currentBlock] = true;
+ hadAnyUse = true;
+ }
+
/// Called when the uninitialized variable analysis detects the
/// idiom 'int x = x'. All other uses of 'x' within the initializer
/// are handled by handleUseOfUninitVariable.
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 04611dadde66..995d776d6565 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -974,6 +974,14 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
<< Use.getUser()->getSourceRange();
}
+/// Diagnose uninitialized const reference usages.
+static bool DiagnoseUninitializedConstRefUse(Sema &S, const VarDecl *VD,
+ const UninitUse &Use) {
+ S.Diag(Use.getUser()->getBeginLoc(), diag::warn_uninit_const_reference)
+ << VD->getDeclName() << Use.getUser()->getSourceRange();
+ return true;
+}
+
/// DiagnoseUninitializedUse -- Helper function for diagnosing uses of an
/// uninitialized variable. This manages the
diff erent forms of diagnostic
/// emitted for particular types of uses. Returns true if the use was diagnosed
@@ -1506,13 +1514,14 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
// order of diagnostics when calling flushDiagnostics().
typedef llvm::MapVector<const VarDecl *, MappedType> UsesMap;
UsesMap uses;
+ UsesMap constRefUses;
public:
UninitValsDiagReporter(Sema &S) : S(S) {}
~UninitValsDiagReporter() override { flushDiagnostics(); }
- MappedType &getUses(const VarDecl *vd) {
- MappedType &V = uses[vd];
+ MappedType &getUses(UsesMap &um, const VarDecl *vd) {
+ MappedType &V = um[vd];
if (!V.getPointer())
V.setPointer(new UsesVec());
return V;
@@ -1520,11 +1529,17 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
void handleUseOfUninitVariable(const VarDecl *vd,
const UninitUse &use) override {
- getUses(vd).getPointer()->push_back(use);
+ getUses(uses, vd).getPointer()->push_back(use);
+ }
+
+ void handleConstRefUseOfUninitVariable(const VarDecl *vd,
+ const UninitUse &use) override {
+ getUses(constRefUses, vd).getPointer()->push_back(use);
}
void handleSelfInit(const VarDecl *vd) override {
- getUses(vd).setInt(true);
+ getUses(uses, vd).setInt(true);
+ getUses(constRefUses, vd).setInt(true);
}
void flushDiagnostics() {
@@ -1571,6 +1586,32 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
}
uses.clear();
+
+ // Flush all const reference uses diags.
+ for (const auto &P : constRefUses) {
+ const VarDecl *vd = P.first;
+ const MappedType &V = P.second;
+
+ UsesVec *vec = V.getPointer();
+ bool hasSelfInit = V.getInt();
+
+ if (!vec->empty() && hasSelfInit && hasAlwaysUninitializedUse(vec))
+ DiagnoseUninitializedUse(S, vd,
+ UninitUse(vd->getInit()->IgnoreParenCasts(),
+ /* isAlwaysUninit */ true),
+ /* alwaysReportSelfInit */ true);
+ else {
+ for (const auto &U : *vec) {
+ if (DiagnoseUninitializedConstRefUse(S, vd, U))
+ break;
+ }
+ }
+
+ // Release the uses vector.
+ delete vec;
+ }
+
+ constRefUses.clear();
}
private:
@@ -2184,7 +2225,8 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
if (!Diags.isIgnored(diag::warn_uninit_var, D->getBeginLoc()) ||
!Diags.isIgnored(diag::warn_sometimes_uninit_var, D->getBeginLoc()) ||
- !Diags.isIgnored(diag::warn_maybe_uninit_var, D->getBeginLoc())) {
+ !Diags.isIgnored(diag::warn_maybe_uninit_var, D->getBeginLoc()) ||
+ !Diags.isIgnored(diag::warn_uninit_const_reference, D->getBeginLoc())) {
if (CFG *cfg = AC.getCFG()) {
UninitValsDiagReporter reporter(S);
UninitVariablesAnalysisStats stats;
diff --git a/clang/test/Misc/warning-wall.c b/clang/test/Misc/warning-wall.c
index d0a0172dff5f..c63d4beecff0 100644
--- a/clang/test/Misc/warning-wall.c
+++ b/clang/test/Misc/warning-wall.c
@@ -55,6 +55,7 @@ CHECK-NEXT: -Wtrigraphs
CHECK-NEXT: -Wuninitialized
CHECK-NEXT: -Wsometimes-uninitialized
CHECK-NEXT: -Wstatic-self-init
+CHECK-NEXT: -Wuninitialized-const-reference
CHECK-NEXT: -Wunknown-pragmas
CHECK-NEXT: -Wunused
CHECK-NEXT: -Wunused-argument
diff --git a/clang/test/SemaCXX/uninit-variables.cpp b/clang/test/SemaCXX/uninit-variables.cpp
index 4dcd34858abb..90d1ddb31718 100644
--- a/clang/test/SemaCXX/uninit-variables.cpp
+++ b/clang/test/SemaCXX/uninit-variables.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -fsyntax-only -fcxx-exceptions %s -verify -std=c++1y
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wno-uninitialized-const-reference -fsyntax-only -fcxx-exceptions %s -verify -std=c++1y
// Stub out types for 'typeid' to work.
namespace std { class type_info {}; }
diff --git a/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp
index 63d5e7b92eac..cdfcb2a9cbed 100644
--- a/clang/test/SemaCXX/uninitialized.cpp
+++ b/clang/test/SemaCXX/uninitialized.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wall -Wuninitialized -Wno-unused-value -Wno-unused-lambda-capture -std=c++1z -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wall -Wuninitialized -Wno-unused-value -Wno-unused-lambda-capture -Wno-uninitialized-const-reference -std=c++1z -verify %s
// definitions for std::move
namespace std {
diff --git a/clang/test/SemaCXX/warn-uninitialized-const-reference.cpp b/clang/test/SemaCXX/warn-uninitialized-const-reference.cpp
new file mode 100644
index 000000000000..de0c124e1a09
--- /dev/null
+++ b/clang/test/SemaCXX/warn-uninitialized-const-reference.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized-const-reference -verify %s
+
+class A {
+public:
+ int i;
+ A(){};
+ A(const A &a){};
+ A(int i) {}
+ bool operator!=(const A &);
+};
+
+A const_ref_use_A(const A &a);
+int const_ref_use(const int &i);
+A const_use_A(const A a);
+int const_use(const int i);
+
+void f() {
+ int i;
+ const_ref_use(i); // expected-warning {{variable 'i' is uninitialized when passed as a const reference argument here}}
+ int j = j + const_ref_use(j); // expected-warning {{variable 'j' is uninitialized when used within its own initialization}} expected-warning {{variable 'j' is uninitialized when passed as a const reference argument here}}
+ A a1 = const_ref_use_A(a1); // expected-warning {{variable 'a1' is uninitialized when passed as a const reference argument here}}
+ int k = const_use(k); // expected-warning {{variable 'k' is uninitialized when used within its own initialization}}
+ A a2 = const_use_A(a2); // expected-warning {{variable 'a2' is uninitialized when used within its own initialization}}
+ A a3(const_ref_use_A(a3)); // expected-warning {{variable 'a3' is uninitialized when passed as a const reference argument here}}
+ A a4 = a3 != a4; // expected-warning {{variable 'a4' is uninitialized when used within its own initialization}} expected-warning {{variable 'a4' is uninitialized when passed as a const reference argument here}}
+ int n = n; // expected-warning {{variable 'n' is uninitialized when used within its own initialization}}
+ const_ref_use(n);
+
+ A a5;
+ const_ref_use_A(a5);
+}
More information about the cfe-commits
mailing list