[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344879: [analyzer][UninitializedObjectChecker] No longer
using nonloc::LazyCompoundVal (authored by Szelethus, committed by ).
Herald added subscribers: llvm-commits, dkrupp, donat.nagy.
Changed prior to commit:
https://reviews.llvm.org/D51300?vs=168775=170361#toc
Repository:
rL LLVM
https://reviews.llvm.org/D51300
Files:
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
Index:
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
---
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -96,12 +96,11 @@
// Utility function declarations.
-/// Returns the object that was constructed by CtorDecl, or None if that isn't
-/// possible.
-// TODO: Refactor this function so that it returns the constructed object's
-// region.
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext );
+/// Returns the region that was constructed by CtorDecl, or nullptr if that
+/// isn't possible.
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext );
/// Checks whether the object constructed by \p Ctor will be analyzed later
/// (e.g. if the object is a field of another object, in which case we'd check
@@ -135,11 +134,11 @@
if (willObjectBeAnalyzedLater(CtorDecl, Context))
return;
- Optional Object = getObjectVal(CtorDecl, Context);
- if (!Object)
+ const TypedValueRegion *R = getConstructedRegion(CtorDecl, Context);
+ if (!R)
return;
- FindUninitializedFields F(Context.getState(), Object->getRegion(), Opts);
+ FindUninitializedFields F(Context.getState(), R, Opts);
const UninitFieldMap = F.getUninitFields();
@@ -400,25 +399,27 @@
// Utility functions.
//===--===//
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext ) {
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext ) {
- Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(),
+ Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl,
Context.getStackFrame());
- // Getting the value for 'this'.
- SVal This = Context.getState()->getSVal(ThisLoc);
- // Getting the value for '*this'.
- SVal Object = Context.getState()->getSVal(This.castAs());
+ SVal ObjectV = Context.getState()->getSVal(ThisLoc);
- return Object.getAs();
+ auto *R = ObjectV.getAsRegion()->getAs();
+ if (R && !R->getValueType()->getAsCXXRecordDecl())
+return nullptr;
+
+ return R;
}
static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
CheckerContext ) {
- Optional CurrentObject = getObjectVal(Ctor,
Context);
- if (!CurrentObject)
+ const TypedValueRegion *CurrRegion = getConstructedRegion(Ctor, Context);
+ if (!CurrRegion)
return false;
const LocationContext *LC = Context.getLocationContext();
@@ -429,14 +430,14 @@
if (!OtherCtor)
continue;
-Optional OtherObject =
-getObjectVal(OtherCtor, Context);
-if (!OtherObject)
+const TypedValueRegion *OtherRegion =
+getConstructedRegion(OtherCtor, Context);
+if (!OtherRegion)
continue;
-// If the CurrentObject is a subregion of OtherObject, it will be analyzed
-// during the analysis of OtherObject.
-if (CurrentObject->getRegion()->isSubRegionOf(OtherObject->getRegion()))
+// If the CurrRegion is a subregion of OtherRegion, it will be analyzed
+// during the analysis of OtherRegion.
+if (CurrRegion->isSubRegionOf(OtherRegion))
return true;
}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -96,12 +96,11 @@
// Utility function declarations.
-/// Returns the object that was constructed by CtorDecl, or None if that isn't
-/// possible.
-// TODO: Refactor this function so that it returns the constructed object's
-// region.
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext );
+/// Returns the region that was constructed by CtorDecl, or nullptr if that
+/// isn't possible.
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
Szelethus added a comment. Since a good amount of time passed since this patch was made, I'll re-analyze the LLVM project with this patch rebased on trunk just to be sure that nothing breaks. https://reviews.llvm.org/D51300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
Szelethus updated this revision to Diff 168775.
Szelethus added a comment.
This revision is now accepted and ready to land.
Finally managed to run creduce. I added the test that caused a crash on our
server, but as I said, I couldn't replicate it elsewhere.
https://reviews.llvm.org/D51300
Files:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object.cpp
Index: test/Analysis/cxx-uninitialized-object.cpp
===
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -1130,3 +1130,29 @@
// TODO: we'd expect the warning: {{2 uninitializeds field}}
CXX11MemberInitTest2(); // no-warning
}
+
+//===--===//
+// Test for cases when the constructed region after the constructor call isn't
+// CXXRecordDecl, when it should be.
+//===--===//
+
+struct UndefinedStruct;
+
+struct CXXRecordDeclRegionBugTest {
+ CXXRecordDeclRegionBugTest(const UndefinedStruct );
+
+ struct UserDefinedDefaultCtorStruct {
+UserDefinedDefaultCtorStruct() {}
+ };
+};
+
+using size_t = long unsigned;
+void *operator new[](size_t, const UndefinedStruct &) throw() {
+ return nullptr;
+}
+
+CXXRecordDeclRegionBugTest::CXXRecordDeclRegionBugTest(
+const UndefinedStruct ) {
+ size_t N;
+ new (U) UserDefinedDefaultCtorStruct[N];
+}
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -124,12 +124,11 @@
// Utility function declarations.
-/// Returns the object that was constructed by CtorDecl, or None if that isn't
-/// possible.
-// TODO: Refactor this function so that it returns the constructed object's
-// region.
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext );
+/// Returns the region that was constructed by CtorDecl, or nullptr if that
+/// isn't possible.
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext );
/// Checks whether the object constructed by \p Ctor will be analyzed later
/// (e.g. if the object is a field of another object, in which case we'd check
@@ -159,12 +158,11 @@
if (willObjectBeAnalyzedLater(CtorDecl, Context))
return;
- Optional Object = getObjectVal(CtorDecl, Context);
- if (!Object)
+ const TypedValueRegion *R = getConstructedRegion(CtorDecl, Context);
+ if (!R)
return;
- FindUninitializedFields F(Context.getState(), Object->getRegion(),
-CheckPointeeInitialization);
+ FindUninitializedFields F(Context.getState(), R, CheckPointeeInitialization);
const UninitFieldMap = F.getUninitFields();
@@ -446,25 +444,27 @@
// Utility functions.
//===--===//
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext ) {
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext ) {
- Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(),
-Context.getStackFrame());
- // Getting the value for 'this'.
- SVal This = Context.getState()->getSVal(ThisLoc);
+ Loc ThisLoc =
+ Context.getSValBuilder().getCXXThis(CtorDecl, Context.getStackFrame());
- // Getting the value for '*this'.
- SVal Object = Context.getState()->getSVal(This.castAs());
+ SVal ObjectV = Context.getState()->getSVal(ThisLoc);
- return Object.getAs();
+ auto *R = ObjectV.getAsRegion()->getAs();
+ if (R && !R->getValueType()->getAsCXXRecordDecl())
+return nullptr;
+
+ return R;
}
static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
CheckerContext ) {
- Optional CurrentObject = getObjectVal(Ctor, Context);
- if (!CurrentObject)
+ const TypedValueRegion *CurrRegion = getConstructedRegion(Ctor, Context);
+ if (!CurrRegion)
return false;
const LocationContext *LC = Context.getLocationContext();
@@ -475,14 +475,14 @@
if (!OtherCtor)
continue;
-Optional OtherObject =
-getObjectVal(OtherCtor, Context);
-if (!OtherObject)
+const TypedValueRegion *OtherRegion =
+getConstructedRegion(OtherCtor, Context);
+if (!OtherRegion)
continue;
-// If the CurrentObject is a subregion of OtherObject, it will be analyzed
-// during the analysis of OtherObject.
-if
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
Szelethus added a comment. There are some nasty environment issues I'm dealing with, so the issue is the lack of a well functioning creduce, but I'll get it done eventually. https://reviews.llvm.org/D51300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
NoQ added a comment. Very easy: 1. Put preprocessed file that crashes into `repro.cpp` (`.c`, `.m`, whatever). 2. Write `repro.sh` as follows: #!/bin/bash blah/blah/path/to/clang -cc1 -analyze -blah-blah-arguments-you-need-to-cause-a-crash repro.cpp 2>&1 | grep "Any assertion message or whatever" 3. Do: creduce repro.sh repro.cpp 4. Wait until it finishes. 5. `repro.cpp` now contains the reduced test case. Tip for speed: - The repro.sh script should be as fast as possible. Use a release+asserts build of clang. - Also `-analyze-function top_function_that_we_analyze_when_we_crash` is a great flag to add to the run-line if possible, because it'll make the test much faster; see `-analyzer-display-progress` to figure out how to specify the function; if it's a lambda or a block you're out of luck because you'll need to specify line number which would change during reduce; you may also fail because top-level analyses are interacting a bit, so it may fail to crash if you restrict to a function. https://reviews.llvm.org/D51300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
Szelethus planned changes to this revision. Szelethus added a comment. > Sounds like a test case would be great to have. Aye. I'd be good to land this with that testcase. There are a variety of other projects I'm working on (related to the checker), and none of them depend on this patch, so I'll commit this once I figure out how to use `creduce` >< https://reviews.llvm.org/D51300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
NoQ added a comment. In https://reviews.llvm.org/D51300#1220537, @Szelethus wrote: > Would you be comfortable me commiting this without that assert Yup sure. In https://reviews.llvm.org/D51300#1223042, @Szelethus wrote: > I'm not even sure how this is possible -- and unfortunately I've been unable > to create a minimal (not) working example for this, and I wasn't even able to > recreate the error locally. Sounds like a test case would be great to have. Consider extracting a preprocessed file and running it under //creduce//, that's a great generic method for obtaining small reproducers for crashes and regressions (but not for false positives). As far as i understand, it's a crash on llvm codebase, which should be easy to re-analyze locally, even just one file, because llvm is built with cmake and dumps compilation databases, so just use clang-check on a single file, or simply append --analyze to the compilation database run-line. This specific problem sounds elusive because it's a problem with pointer casts, and pointer casts are currently a mess. I cannot state for sure that typed this-region type must always be a record, but it's definitely a bad smell when it is't. So i recommend a quick investigation of whether the region in question is (1) well-formed and (2) correctly reflects the semantics of the program. https://reviews.llvm.org/D51300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
Szelethus updated this revision to Diff 163813.
Szelethus added a comment.
Fixed a crash where the returned region's type wasn't `CXXRecordDecl`. I'm not
even sure how this is possible -- and unfortunately I've been unable to create
a minimal (not) working example for this, and I wasn't even able to recreate
the error locally. However, on the server where I could repeatedly cause a
crash with the analysis of `lib/AST/Expr.cpp`, this fix resolved the issue.
Note that this assert failure didn't happen inside `willBeAnalyzedLater`, where
`getConstructedRegion` is used with a different `CXXConstructorDecl` then the
one on top of the stack frame.
https://reviews.llvm.org/D51300
Files:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
Index:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
---
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -124,12 +124,11 @@
// Utility function declarations.
-/// Returns the object that was constructed by CtorDecl, or None if that isn't
-/// possible.
-// TODO: Refactor this function so that it returns the constructed object's
-// region.
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext );
+/// Returns the region that was constructed by CtorDecl, or nullptr if that
+/// isn't possible.
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext );
/// Checks whether the object constructed by \p Ctor will be analyzed later
/// (e.g. if the object is a field of another object, in which case we'd check
@@ -159,12 +158,11 @@
if (willObjectBeAnalyzedLater(CtorDecl, Context))
return;
- Optional Object = getObjectVal(CtorDecl, Context);
- if (!Object)
+ const TypedValueRegion *R = getConstructedRegion(CtorDecl, Context);
+ if (!R)
return;
- FindUninitializedFields F(Context.getState(), Object->getRegion(),
-CheckPointeeInitialization);
+ FindUninitializedFields F(Context.getState(), R, CheckPointeeInitialization);
const UninitFieldMap = F.getUninitFields();
@@ -443,25 +441,27 @@
// Utility functions.
//===--===//
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext ) {
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext ) {
- Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(),
+ Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl,
Context.getStackFrame());
- // Getting the value for 'this'.
- SVal This = Context.getState()->getSVal(ThisLoc);
- // Getting the value for '*this'.
- SVal Object = Context.getState()->getSVal(This.castAs());
+ SVal ObjectV = Context.getState()->getSVal(ThisLoc);
- return Object.getAs();
+ auto *R = ObjectV.getAsRegion()->getAs();
+ if (R && !R->getValueType()->getAsCXXRecordDecl())
+return nullptr;
+
+ return R;
}
static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
CheckerContext ) {
- Optional CurrentObject = getObjectVal(Ctor,
Context);
- if (!CurrentObject)
+ const TypedValueRegion *CurrRegion = getConstructedRegion(Ctor, Context);
+ if (!CurrRegion)
return false;
const LocationContext *LC = Context.getLocationContext();
@@ -472,14 +472,14 @@
if (!OtherCtor)
continue;
-Optional OtherObject =
-getObjectVal(OtherCtor, Context);
-if (!OtherObject)
+const TypedValueRegion *OtherRegion =
+getConstructedRegion(OtherCtor, Context);
+if (!OtherRegion)
continue;
-// If the CurrentObject is a subregion of OtherObject, it will be analyzed
-// during the analysis of OtherObject.
-if (CurrentObject->getRegion()->isSubRegionOf(OtherObject->getRegion()))
+// If the CurrRegion is a subregion of OtherRegion, it will be analyzed
+// during the analysis of OtherRegion.
+if (CurrRegion->isSubRegionOf(OtherRegion))
return true;
}
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -124,12 +124,11 @@
// Utility function declarations.
-/// Returns the object that was constructed by CtorDecl, or None if that isn't
-/// possible.
-// TODO: Refactor this function so that it returns the constructed object's
-// region.
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
Szelethus added a comment. Would you be comfortable me commiting this without that assert, or should I come up with a more risk-free solution? Repository: rC Clang https://reviews.llvm.org/D51300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
NoQ added inline comments.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:448-449
Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(),
Context.getStackFrame());
Szelethus wrote:
> NoQ wrote:
> > This totally needs `assert(CtorDecl ==
> > Context.getStackFrame()->getDecl())`. Otherwise we're in big trouble
> > because we'll be looking into a this-region that doesn't exist on this
> > stack frame.
> >
> > On second thought, though, i guess we should put this assertion into the
> > constructor of `CXXThisRegion`. I'll do this.
> >
> > Also there's an overload of `getCXXThis` that accepts the method itself, no
> > need to get parent.
> U that wouldn't be very nice, because...
Yeah, i guess i'll have to think a bit deeper about this. I really want to
prevent invalid `CXXThisRegion`s from appearing, but it might be not that
simple.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:456-483
static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
CheckerContext ) {
- Optional CurrentObject = getObjectVal(Ctor,
Context);
- if (!CurrentObject)
+ const TypedValueRegion *CurrRegion = getConstructedRegion(Ctor, Context);
+ if (!CurrRegion)
return false;
Szelethus wrote:
> ...`willBeAnalyzerLater()` relies on this, and it uses all sorts of
> constructor decls to check whether `Context.getLocationContext()->getDecl()`
> would be a subregion of another object. Are you sure that this is incorrect?
I mean not the this-region of the object, but the `CXXThisRegion` itself, in
which this-region is stored. It is definitely not aliased across stack frames.
Repository:
rC Clang
https://reviews.llvm.org/D51300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
Szelethus added inline comments.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:448-449
Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(),
Context.getStackFrame());
NoQ wrote:
> This totally needs `assert(CtorDecl == Context.getStackFrame()->getDecl())`.
> Otherwise we're in big trouble because we'll be looking into a this-region
> that doesn't exist on this stack frame.
>
> On second thought, though, i guess we should put this assertion into the
> constructor of `CXXThisRegion`. I'll do this.
>
> Also there's an overload of `getCXXThis` that accepts the method itself, no
> need to get parent.
U that wouldn't be very nice, because...
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:456-483
static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
CheckerContext ) {
- Optional CurrentObject = getObjectVal(Ctor,
Context);
- if (!CurrentObject)
+ const TypedValueRegion *CurrRegion = getConstructedRegion(Ctor, Context);
+ if (!CurrRegion)
return false;
...`willBeAnalyzerLater()` relies on this, and it uses all sorts of constructor
decls to check whether `Context.getLocationContext()->getDecl()` would be a
subregion of another object. Are you sure that this is incorrect?
Repository:
rC Clang
https://reviews.llvm.org/D51300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Yup, looks correct to me! Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:448-449 Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(), Context.getStackFrame()); This totally needs `assert(CtorDecl == Context.getStackFrame()->getDecl())`. Otherwise we're in big trouble because we'll be looking into a this-region that doesn't exist on this stack frame. On second thought, though, i guess we should put this assertion into the constructor of `CXXThisRegion`. I'll do this. Also there's an overload of `getCXXThis` that accepts the method itself, no need to get parent. Repository: rC Clang https://reviews.llvm.org/D51300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet,
whisperity.
As rightly pointed out by @NoQ, `nonloc::LazyCompoundVal`s were only used to
acquire a constructed object's region, which isn't what `LazyCompoundVal` was
made for.
Repository:
rC Clang
https://reviews.llvm.org/D51300
Files:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
Index:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
---
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -124,12 +124,11 @@
// Utility function declarations.
-/// Returns the object that was constructed by CtorDecl, or None if that isn't
-/// possible.
-// TODO: Refactor this function so that it returns the constructed object's
-// region.
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext );
+/// Returns the region that was constructed by CtorDecl, or nullptr if that
+/// isn't possible.
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext );
/// Checks whether the object constructed by \p Ctor will be analyzed later
/// (e.g. if the object is a field of another object, in which case we'd check
@@ -159,12 +158,11 @@
if (willObjectBeAnalyzedLater(CtorDecl, Context))
return;
- Optional Object = getObjectVal(CtorDecl, Context);
- if (!Object)
+ const TypedValueRegion *R = getConstructedRegion(CtorDecl, Context);
+ if (!R)
return;
- FindUninitializedFields F(Context.getState(), Object->getRegion(),
-CheckPointeeInitialization);
+ FindUninitializedFields F(Context.getState(), R, CheckPointeeInitialization);
const UninitFieldMap = F.getUninitFields();
@@ -443,25 +441,23 @@
// Utility functions.
//===--===//
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext ) {
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext ) {
Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(),
Context.getStackFrame());
- // Getting the value for 'this'.
- SVal This = Context.getState()->getSVal(ThisLoc);
- // Getting the value for '*this'.
- SVal Object = Context.getState()->getSVal(This.castAs());
+ SVal ObjectV = Context.getState()->getSVal(ThisLoc);
- return Object.getAs();
+ return ObjectV.getAsRegion()->getAs();
}
static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
CheckerContext ) {
- Optional CurrentObject = getObjectVal(Ctor,
Context);
- if (!CurrentObject)
+ const TypedValueRegion *CurrRegion = getConstructedRegion(Ctor, Context);
+ if (!CurrRegion)
return false;
const LocationContext *LC = Context.getLocationContext();
@@ -472,14 +468,14 @@
if (!OtherCtor)
continue;
-Optional OtherObject =
-getObjectVal(OtherCtor, Context);
-if (!OtherObject)
+const TypedValueRegion *OtherRegion =
+getConstructedRegion(OtherCtor, Context);
+if (!OtherRegion)
continue;
-// If the CurrentObject is a subregion of OtherObject, it will be analyzed
-// during the analysis of OtherObject.
-if (CurrentObject->getRegion()->isSubRegionOf(OtherObject->getRegion()))
+// If the CurrRegion is a subregion of OtherRegion, it will be analyzed
+// during the analysis of OtherRegion.
+if (CurrRegion->isSubRegionOf(OtherRegion))
return true;
}
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -124,12 +124,11 @@
// Utility function declarations.
-/// Returns the object that was constructed by CtorDecl, or None if that isn't
-/// possible.
-// TODO: Refactor this function so that it returns the constructed object's
-// region.
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext );
+/// Returns the region that was constructed by CtorDecl, or nullptr if that
+/// isn't possible.
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext );
/// Checks whether the object constructed by \p Ctor will be analyzed later
/// (e.g. if the object is a field
