-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Fix: Issue #165239 — resolve -Wuninitialized warnings #166991
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,8 +61,8 @@ static bool isTrackedVar(const VarDecl *vd, const DeclContext *dc) { | |
| vd->getDeclContext() == dc) { | ||
| QualType ty = vd->getType(); | ||
| if (const auto *RD = ty->getAsRecordDecl()) | ||
| return recordIsNotEmpty(RD); | ||
| return ty->isScalarType() || ty->isVectorType() || ty->isRVVSizelessBuiltinType(); | ||
| return recordIsNotEmpty(RD); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated changes? |
||
| return ty->isScalarType() || ty->isVectorType() || ty->isRVVSizelessBuiltinType() || ty->isArrayType(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add tests with arrays whose element type is |
||
| } | ||
| return false; | ||
| } | ||
|
|
@@ -291,6 +291,7 @@ class ClassifyRefs : public StmtVisitor<ClassifyRefs> { | |
| public: | ||
| ClassifyRefs(AnalysisDeclContext &AC) : DC(cast<DeclContext>(AC.getDecl())) {} | ||
|
|
||
| void VisitArraySubscriptExpr(ArraySubscriptExpr *ASE); | ||
| void VisitDeclStmt(DeclStmt *DS); | ||
| void VisitUnaryOperator(UnaryOperator *UO); | ||
| void VisitBinaryOperator(BinaryOperator *BO); | ||
|
|
@@ -310,6 +311,10 @@ class ClassifyRefs : public StmtVisitor<ClassifyRefs> { | |
| if (!VD || !isTrackedVar(VD)) | ||
| return Ignore; | ||
|
|
||
| // Default to Use instead of Init for arrays - CRITICAL FIX | ||
| if (VD->getType()->isArrayType()) | ||
| return Use; | ||
|
|
||
| return Init; | ||
| } | ||
| }; | ||
|
|
@@ -331,6 +336,26 @@ static const DeclRefExpr *getSelfInitExpr(VarDecl *VD) { | |
| void ClassifyRefs::classify(const Expr *E, Class C) { | ||
| // The result of a ?: could also be an lvalue. | ||
| E = E->IgnoreParens(); | ||
|
|
||
| // Handle array subscripts - THE KEY FIX | ||
| if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E)) { | ||
| // For ANY array subscript expression, the base array is being USED | ||
| // This is the critical fix - don't check C, just mark as Use | ||
| FindVarResult Var = findVar(ASE->getBase(), DC); | ||
| if (const VarDecl *VD = Var.getDecl()) { | ||
| if (VD->getType()->isArrayType()) { | ||
| // Directly mark the array DeclRefExpr as Use | ||
| if (const DeclRefExpr *DRE = Var.getDeclRefExpr()) | ||
| Classification[DRE] = Use; | ||
| } | ||
| } | ||
|
|
||
| // Process base and index normally | ||
| classify(ASE->getBase(), C); | ||
| Visit(const_cast<Expr*>(ASE->getIdx())); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should not call |
||
| return; | ||
| } | ||
|
|
||
| if (const auto *CO = dyn_cast<ConditionalOperator>(E)) { | ||
| classify(CO->getTrueExpr(), C); | ||
| classify(CO->getFalseExpr(), C); | ||
|
|
@@ -376,6 +401,13 @@ void ClassifyRefs::classify(const Expr *E, Class C) { | |
| } | ||
| } | ||
|
|
||
| void ClassifyRefs::VisitArraySubscriptExpr(ArraySubscriptExpr *ASE) { | ||
| // For array subscript expressions, classify the base as a use | ||
| classify(ASE->getBase(), Use); | ||
| // Also visit the index expression | ||
| Visit(ASE->getIdx()); | ||
| } | ||
|
|
||
| void ClassifyRefs::VisitDeclStmt(DeclStmt *DS) { | ||
| for (auto *DI : DS->decls()) { | ||
| auto *VD = dyn_cast<VarDecl>(DI); | ||
|
|
@@ -393,10 +425,27 @@ void ClassifyRefs::VisitBinaryOperator(BinaryOperator *BO) { | |
| // use. | ||
| if (BO->isCompoundAssignmentOp()) | ||
| classify(BO->getLHS(), Use); | ||
| else if (BO->getOpcode() == BO_Assign || BO->getOpcode() == BO_Comma) | ||
| classify(BO->getLHS(), Ignore); | ||
| else if (BO->getOpcode() == BO_Assign || BO->getOpcode() == BO_Comma) { | ||
| // For array subscript expressions on LHS of assignment, don't classify as use | ||
| if (isa<ArraySubscriptExpr>(BO->getLHS())) { | ||
| // Don't classify array base as use when it's being assigned to | ||
| // But we still need to visit the index expression | ||
| if (auto *ASE = dyn_cast<ArraySubscriptExpr>(BO->getLHS())) { | ||
|
Comment on lines
+430
to
+433
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicate checks? |
||
| Visit(ASE->getIdx()); | ||
| } | ||
| } else { | ||
| classify(BO->getLHS(), Ignore); | ||
| } | ||
| // ALWAYS visit the right-hand side - this is crucial for array subscripts | ||
| Visit(BO->getRHS()); | ||
| } else { | ||
| // For all other binary operators, visit both operands normally | ||
| Visit(BO->getLHS()); | ||
| Visit(BO->getRHS()); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| void ClassifyRefs::VisitUnaryOperator(UnaryOperator *UO) { | ||
| // Increment and decrement are uses despite there being no lvalue-to-rvalue | ||
| // conversion. | ||
|
|
@@ -491,6 +540,7 @@ class TransferFunctions : public StmtVisitor<TransferFunctions> { | |
| void reportConstRefUse(const Expr *ex, const VarDecl *vd); | ||
| void reportConstPtrUse(const Expr *ex, const VarDecl *vd); | ||
|
|
||
| void VisitArraySubscriptExpr(ArraySubscriptExpr *ASE); | ||
| void VisitBinaryOperator(BinaryOperator *bo); | ||
| void VisitBlockExpr(BlockExpr *be); | ||
| void VisitCallExpr(CallExpr *ce); | ||
|
|
@@ -661,8 +711,23 @@ class TransferFunctions : public StmtVisitor<TransferFunctions> { | |
|
|
||
| } // namespace | ||
|
|
||
| void TransferFunctions::reportUse(const Expr *ex, const VarDecl *vd) { | ||
| void TransferFunctions::VisitArraySubscriptExpr(ArraySubscriptExpr *ASE) { | ||
| // Handle array subscript expressions like arr[i] | ||
| // Check if the base array variable is uninitialized | ||
| FindVarResult Var = findVar(ASE->getBase()); | ||
|
|
||
| if (const VarDecl *VD = Var.getDecl()) { | ||
| if (VD->getType()->isArrayType()) | ||
| reportUse(ASE, VD); | ||
| } | ||
|
|
||
| // Also visit index expression | ||
| Visit(ASE->getIdx()); | ||
| } | ||
|
|
||
| void TransferFunctions::reportUse(const Expr *ex, const VarDecl *vd) { | ||
| Value v = vals[vd]; | ||
|
|
||
|
Comment on lines
+728
to
+730
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated changes? |
||
| if (isUninitialized(v)) | ||
| handler.handleUseOfUninitVariable(vd, getUninitUse(ex, vd, v)); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| // RUN: %clang_cc1 -Wall -Wextra -Wuninitialized -fsyntax-only %s 2>&1 | FileCheck %s | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| void test1() { | ||
| int arr[5]; | ||
| int x = arr[0]; // expected-warning{{variable 'arr' is uninitialized when used here}} | ||
| } | ||
|
|
||
| void test2() { | ||
| int a[3][3]; | ||
| int y = a[1][1]; // expected-warning{{variable 'a' is uninitialized when used here}} | ||
| } | ||
|
|
||
| void test3() { | ||
| int n; | ||
| int vla[n]; // expected-note{{declared here}} | ||
| int z = vla[2]; // expected-warning{{variable 'vla' is uninitialized when used here}} | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference the issue in text, not in code blocks.