NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware,
eraman.
I don't see why BasicValueFactory::getTruthValue(bool, QualType) always returns
an unsigned `A
NoQ added a comment.
Welcome to the club!
https://reviews.llvm.org/D50211
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ updated this revision to Diff 159435.
NoQ added a comment.
Whoops, how did this `T, ` thing get in there.
https://reviews.llvm.org/D50363
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
test/Analysis/casts.c
test/Analysis/std-c-library-functions-inlined.c
NoQ accepted this revision.
NoQ added a comment.
Yay somebody reads those.
Repository:
rC Clang
https://reviews.llvm.org/D50382
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This looks roughly correct, but at the same time none of the tests actually
exercise the dynamic type propagation. In these tests all the necessary
information is obtained from the structure of the MemRegion (directly or via
the initial `StripCas
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Ok, let's commit this and see how to fix it later. I still think it's more
important to come up with clear rules of who is responsible for initializing
fields than making sure our warnings are prope
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Yup, fair enough.
https://reviews.llvm.org/D50408
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.
`CXXTemporaryObjectExpr` is a sub-class of `CXXConstructExpr` that represents a
construct-expression wr
NoQ added a comment.
Hmm, this might make a good `optin.*` checker. Also i see that this is a pure
AST-based checker; did you consider putting this into `clang-tidy`? Like, you
shouldn't if you're not using `clang-tidy` yourself (it's a bit messy) but this
is an option. Also we're actively usin
NoQ added a comment.
> I'm only a beginner, but here are some things that caught my eye. I really
> like the idea! :)
Thanks, i appreciate help with reviews greatly.
Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:93
+ const RecordDecl *RD =
+IterTy->ge
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
I think this is way easier to understand :)
https://reviews.llvm.org/D49570
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.o
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Yup, i hope it'll be comfy now.
https://reviews.llvm.org/D50594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/m
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: Szelethus.
Looks great, let's land this!
https://reviews.llvm.org/D32747
___
cfe-commits mailing list
cfe-commits@lists.llvm
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: Szelethus.
Looks good. I guess we may have to tone down the heuristic about "all template
functions" if we see it fail.
@a.sidorin and @whisperity have some valid minor co
NoQ added a comment.
I guess one of the things the analyzer could find with path-sensitive analysis
is direct comparison of non-aliasing pointers. Not only this is
non-deterministic, but there's a related problem that comparison for equality
would always yield false and is therefore useless. Ho
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: Szelethus.
I think this patch is in good shape.
In https://reviews.llvm.org/D32859#1187551, @baloghadamsoftware wrote:
> I do not see which lines exactly you commented
T
NoQ requested changes to this revision.
NoQ added a comment.
This revision now requires changes to proceed.
Herald added subscribers: Szelethus, mikhail.ramalho.
Let's see if this is still necessary after https://reviews.llvm.org/D49443.
Iterators will be constructed directly into the argument re
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: Szelethus.
This looks great, thanks, this is exactly how i imagined it!
Repository:
rC Clang
https://reviews.llvm.org/D48027
NoQ added inline comments.
Comment at: cfe/trunk/test/Analysis/cstring-syntax.c:45
+ strlcpy(dest, "012345678", sizeof(dest));
+ strlcat(dest, "910", sizeof(dest)); // expected-warning {{The third argument
allows to potentially copy more bytes than it should. Replace with the
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho,
baloghadamsoftware.
The analyzer doesn't need them and they seem to have pretty weird AST from time
to time, so
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho,
baloghadamsoftware.
Despite the effort to eliminate the need for
`skipRValueSubobjectAdjustments()`, we still e
NoQ updated this revision to Diff 161073.
NoQ added a comment.
Add comments for optional arguments.
The reason why i chose an out-parameter rather than returning a std::pair is
because most callers //presumably// don't need this feature.
https://reviews.llvm.org/D50855
Files:
include/clang/
NoQ added a comment.
> which is of course a bad thing to do because we should not bind `Loc`s to
> `prvalue` expressions
... of non-pointer type. On the other hand, binding `NonLoc`s to glvalue
expressions is always a bad thing to do; but, for the reference, adding this as
an assertion current
NoQ added a comment.
...and adding the aforementioned assertion for prvalues increases the number
of crashes on tests to 196. It seems that the analyzer assigns values of
improper loc-less very often, but then immediately overwrites them :/ I wonder
how much performance could be gained by fixi
NoQ created this revision.
NoQ added reviewers: dcoughlin, george.karpenkov.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho, a.sidorin,
szepet, baloghadamsoftware, xazax.hun.
CFRetainReleaseChecker is a tiny checker that verifies that arguments of
CoreFoundation retain/release
NoQ added inline comments.
Herald added subscribers: Szelethus, mikhail.ramalho.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1477-1530
+bool isPushBackCall(const FunctionDecl *Func) {
+ const auto *IdInfo = Func->getIdentifier();
+ if (!IdInfo)
+return false
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Yay.
Repository:
rC Clang
https://reviews.llvm.org/D50747
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mail
NoQ added inline comments.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:187-191
// If FR is a pointer pointing to a non-primitive type.
if (Optional RecordV =
DerefdV.getAs()) {
const TypedValueRegion *R = RecordV->
NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.
Comment at: www/analyzer/alpha_checks.html:334-336
+The checker regards inherited fields as direct fields, so one
+will recieve warnings for uninitialized inherited data member
NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:114-115
+ virtual void printNode(llvm::raw_ostream &Out) const override {
+Out
NoQ added inline comments.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:223
+ // int*).
+ while (auto Tmp = V.getAs()) {
+// We can't reason about symbolic regions, assume its initialized.
Hmm, i still have concerns
NoQ accepted this revision.
NoQ added a comment.
Which shouldn't prevent us from moving code around.
https://reviews.llvm.org/D50509
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ added inline comments.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:223
+ // int*).
+ while (auto Tmp = V.getAs()) {
+// We can't reason about symbolic regions, assume its initialized.
Szelethus wrote:
> NoQ wrot
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp:537
+ mutable APIMisuse BT{this, "null passed to CF memory management function"};
+ CallDescription CFRetain{"CFRetain", 1},
+ CFRelease{"CFRelease", 1},
--
NoQ added a comment.
Herald added a subscriber: Szelethus.
In https://reviews.llvm.org/D49811#1175927, @NoQ wrote:
> Devin has recently pointed out that we might have as well reordered CFG
> elements to have return statement kick in after automatic destructors, so
> that callbacks were called i
NoQ added a comment.
Yup, this looks great and that's exactly how i imagined it.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h:261-265
+inline bool isLocType(const QualType &T) {
+ return T->isAnyPointerType() || T->isReferenceType() ||
+
NoQ added inline comments.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:187-191
// If FR is a pointer pointing to a non-primitive type.
if (Optional RecordV =
DerefdV.getAs()) {
const TypedValueRegion *R = RecordV->
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:419
+ case CK_LValueBitCast:
+ case CK_FixedPointCast: {
state =
a.sidorin wrote:
> Should we consider this construction as unsupported rather than supported as
> a
NoQ added a comment.
In https://reviews.llvm.org/D48027#1207721, @rnkovacs wrote:
> I guess it is highly unlikely for someone to write namespaces like that, and
> if they do, I think they deserve to have them treated as a `std::string`.
Yup. On the other hand, if we specify our method as `{"ba
NoQ added a comment.
In https://reviews.llvm.org/D46187#1089087, @lebedev.ri wrote:
> > I believe we could also benefit from a method of extracting the analyzer's
> > `clang -cc1` run-line from clang-tidy. This would allow arbitrary debugging
> > over a single analyzer invocation.
>
> I'm not r
NoQ added a comment.
Thank you! Looks good overall.
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1013-1014
+
+ // Get the offset and the base region to figure out whether the offset of
+ // buffer is 0.
+ RegionOffset Offset = MR->getAsOffset();
---
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Thanks! I'll commit again.
https://reviews.llvm.org/D45177
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
NoQ added a comment.
Yay thanks!
I think some cornercases would need to be dealt with.
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1650
+
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
N
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
Plist diagnostics support multi-file reports since forever. HTML diagnostics
support multi-file reports since
https://r
NoQ added inline comments.
Comment at: test/Analysis/diagnostics/plist-multi-file.c:202
+// CHECK-NEXT:
+// CHECK-NEXT: report-{{([0-9a-f]{6})}}.html
+// CHECK-NEXT:
Yay we have regular expressions.
`()` are needed so that not to confuse `}``}}` with `}}``}
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Thanks! This looks great now.
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1055
+std::tie(StateNullChar, StateNonNullChar) =
+assumeZero(C, State, CharVal
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks good, thanks! Yeah, seems that these were accidentally omitted.
You may want to add the `const CXXRecordDecl *` variant as well.
Repository:
rC Clang
https://reviews.llvm.org/D46891
___
NoQ added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:732
+ const SubRegion *Super) const {
+ const auto Base = BaseSpec.getType()->getAsCXXRecordDecl();
+ return loc::MemRegionVal(
--
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1560-1566
// If the size is known to be zero, we're done.
if (StateZeroSize && !StateNonZeroSize) {
StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
NoQ added a comment.
Mmm, i think loop widening simply shouldn't invalidate references (though it
should invalidate objects bound to them). Simply because you can't really
reassign a reference.
Could we mark them as "preserve contents", like in
https://reviews.llvm.org/D45491?
Repository:
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
I've been thinking if we could de-duplicate this whole set of branches that
computes the return value so that we didn't have to fix every bug twice. Maybe
move it to an auxiliary function.
==
NoQ added a comment.
Hopefully.
Repository:
rC Clang
https://reviews.llvm.org/D47044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ added a comment.
Hmm, well, i guess it's not going to be a one-liner. You might have to iterate
through all live variables in the scope, see if they are references, and add
the trait. I think currently there's no way around scanning the current
function body (i.e. `LCtx->getDecl()`, where `
NoQ updated this revision to Diff 147635.
NoQ added a comment.
Switch to `skipRValueSubobjectAdjustments`. Yup, that's much better.
Add FIXME tests for lifetime extension through non-references that are still
not working - that correspond to a nearby FIXME in the code. I'm not planning
to fix t
NoQ added a comment.
@devnexen I think you should request commit access. You're already an active
contributor :)
Repository:
rC Clang
https://reviews.llvm.org/D47007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
NoQ updated this revision to Diff 147637.
NoQ added a comment.
Herald added a subscriber: baloghadamsoftware.
Rebase.
https://reviews.llvm.org/D44239
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprE
NoQ added a comment.
This looks great, i think we should make a single super simple mock test and
commit this.
@MTC, i really appreciate your help!
Comment at: lib/StaticAnalyzer/Checkers/DanglingStringPointerChecker.cpp:59
+ QualType RegType = TypedR->getValueType();
+ if
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
Reported by Mikael Holmén on
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180416/225349.html -
a combinati
NoQ added a reviewer: mikhail.ramalho.
NoQ added a subscriber: mikhail.ramalho.
NoQ added a comment.
@mikhail.ramalho Does it solve your problems with ffmpeg as well? :)
Repository:
rC Clang
https://reviews.llvm.org/D47155
___
cfe-commits mailing
NoQ added a comment.
We'll have to track `string_view` ourselves, not relying on `MallocChecker`. So
we only need an `AF_` for the pointer case.
`DanglingInternalBufferChecker` and `AF_InternalBuffer` sound great to me.
Repository:
rC Clang
https://reviews.llvm.org/D47135
___
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
As noticed in http://lists.llvm.org/pipermail/cfe-dev/2018-May/058055.html we
need a path-sensitive program state trait
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
`getLocationForConstructedObject()` looks at the construction context and
figures out what region should represent the o
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
As explained in http://lists.llvm.org/pipermail/cfe-dev/2018-May/058055.html,
`findDirectConstructorForCurrentCFGElement
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
Same as https://reviews.llvm.org/D47305 but for `CXXCtorInitializer`-based
constructors, based on the discussion in
htt
NoQ updated this revision to Diff 148502.
NoQ added a comment.
Add a forgotten FIXME to the test.
https://reviews.llvm.org/D47350
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Ana
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
The recent refactoring allows us to easily support C++17 "copy-elided"
constructor syntax for variables and constructor-
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks good!
Do you have commit access? I think you should get commit access.
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1650
+
+// If there is a list, but no
NoQ added a comment.
Ok! Putting any remaining work into follow-up patches would be great indeed.
Let's land this into `alpha.cplusplus` for now because i still haven't made up
my mind for how to organize the proposed `bugprone` category (sorry!).
Comment at: lib/StaticAnalyz
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:265-276
+ const llvm::APSInt &from = i->From(), &to = i->To();
+ const llvm::APSInt &newFrom = (to.isMinSignedValue() ?
+ BV.getMaxValue(to) :
+
NoQ updated this revision to Diff 148681.
NoQ added a comment.
Optimize `simplifySVal()` instead of reducing the threshold.
I'll address the memoization separately.
https://reviews.llvm.org/D47155
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
lib/StaticAnalyzer/Core/
NoQ added a comment.
I only essentially did one optimization - introduce a short path that returns
the original value if visiting its sub-values changed nothing, which is a
relatively common case. The reason it works though is that `evalBinOp()` will
be called later to combine the sub-values, w
NoQ updated this revision to Diff 148684.
NoQ added a comment.
Add an explicit brute-force protection against re-entering `simplifySVal()`.
Remove the threshold completely.
https://reviews.llvm.org/D47155
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
lib/StaticAnalyz
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:377
+ if (!State->contains(Key)) {
+return State->set(Key, V);
}
george.karpenkov wrote:
> nitpick: most C++ containers eliminate the need for two lookups by allowing
> the `ge
NoQ updated this revision to Diff 148692.
NoQ marked 3 inline comments as done.
NoQ added a comment.
Fix review comments.
https://reviews.llvm.org/D47303
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/Expr
NoQ added a comment.
Ouch, this one really got out of hand. Sorry.
Repository:
rC Clang
https://reviews.llvm.org/D41881
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
Memoize `SValBuilder::simplifySVal()` so that it didn't try to re-simplify the
same symbolic expression in the same prog
NoQ added a comment.
The remaining slowness is in `removeDead()`. It's going to be fun to optimize.
Some parts of it are already memoized (eg. the live set in `SymbolReaper`).
Memory usage on the artificial test seems stable.
Repository:
rC Clang
https://reviews.llvm.org/D47402
_
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
The refactoring conducted in https://reviews.llvm.org/D47304 made it easy for
the analyzer to find the target region for
NoQ added inline comments.
Comment at: test/Analysis/cxx17-mandatory-elision.cpp:185-191
+// Check if the last destructor is an automatic destructor.
+// A temporary destructor would have fired by now.
+#if __cplusplus >= 201703L
+clang_analyzer_eval(v.len == 1); // e
NoQ accepted this revision.
NoQ added a comment.
Looks great, thanks! I think you should add a check for UnknownVal and commit.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65
+ if (Call.isCalled(CStrFn)) {
+SymbolRef RawPtr = Call.getReturnVal
NoQ added a comment.
Nice catch!
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:399
+
+ C.addTransition(State);
}
MTC wrote:
> I have two questions may need @NoQ or @xazax.hun who is more familiar with
> the analyzer engine help to answer.
>
>
NoQ added a comment.
Yep, great stuff!
I guess, please add a comment on incomplete destructor support in the CFG
and/or analyzer core that may cause the region to be never cleaned up. Or
perform an extra cleanup pass - not sure if it's worth it, but it should be
easy and safe.
=
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Thx!
Repository:
rC Clang
https://reviews.llvm.org/D47451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mail
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:191
+}
+if (from.isMinSignedValue()) {
+ F.add(newRanges, Range(BV.getMinValue(from), BV.getMinValue(from)));
We'll also need to merge the two adjacent segments
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:192
+if (from.isMinSignedValue()) {
+ F.add(newRanges, Range(BV.getMinValue(from), BV.getMinValue(from)));
+}
NoQ wrote:
> Return value of `add` seems to be acc
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs, baloghadamsoftware.
Herald added a subscriber: cfe-commits.
A follow-up to https://reviews.llvm.org/D47496.
This would warn the developer on the very common bug that consists in w
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1292
+ Constraints =
+ Z3Expr::fromBinOp(Constraints, BO_LOr, SymRange, IsSignedTy);
+}
george.karpenkov wrote:
> I'm very confused as to why are we doing dis
NoQ updated this revision to Diff 116542.
NoQ added a comment.
@dcoughlin: You're right, my reasoning and understanding was not correct, and
your explanation is much more clear. My code still makes sense to me though, so
i updated the comments to match. And moved the unusual logic for the
lvalu
NoQ updated this revision to Diff 116673.
NoQ added a comment.
Add no-crash test cases from https://bugs.llvm.org/show_bug.cgi?id=34373 and
https://bugs.llvm.org/show_bug.cgi?id=34731 .
https://reviews.llvm.org/D37023
Files:
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
test/Analysis/nul
NoQ created this revision.
In `ProgramState::getSVal(Loc, QualType)`, the `QualType` parameter, which
represents the type of the expected value, is optional. If it is not supplied,
it is auto-detected by looking at the memory region in `Loc`. For typed-value
regions such autodetection is trivia
NoQ updated this revision to Diff 116986.
NoQ added a comment.
Add a forgotten comment.
https://reviews.llvm.org/D38358
Files:
lib/StaticAnalyzer/Core/RegionStore.cpp
test/Analysis/ctor.mm
test/Analysis/gtest.cpp
Index: test/Analysis/gtest.cpp
===
NoQ updated this revision to Diff 116992.
NoQ added a subscriber: alexfh.
NoQ added a comment.
Add @alexfh's small reproducer test case. It was so small i never noticed it
until now!
https://reviews.llvm.org/D38358
Files:
lib/StaticAnalyzer/Core/RegionStore.cpp
test/Analysis/ctor.mm
test
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1127
+// only consist of ObjC objects, and escapes of ObjC objects
+// aren't so important (eg., retain count checker ignores them).
+if (isa(Ex) ||
dcoughlin wr
NoQ updated this revision to Diff 117360.
NoQ added a comment.
Yeah, nice catch. So we need to either always tell the checkers to specify
their `CharTy` when they are dealing with void pointers, or to do our
substitution consistently, not only for `SymbolicRegion` but also for
`AllocaRegion` (d
NoQ added a comment.
Whoops forgot to submit inline comments.
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1404
+ // When trying to dereference a void pointer, read the first byte.
+ T = Ctx.CharTy;
+}
dcoughlin wrote:
> Nit: It
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1404
+ // When trying to dereference a void pointer, read the first byte.
+ T = Ctx.CharTy;
+}
dcoughlin wrote:
> NoQ wrote:
> > dcoughlin wrote:
> > > Nit: I
NoQ marked 3 inline comments as done.
NoQ added a comment.
In https://reviews.llvm.org/D35216#886212, @rsmith wrote:
> This is precisely how the rest of the compiler handles
> `CXXStdInitializerListExpr`
Wow. Cool. I'd see what I can do. Yeah, it seems that this is a great case for
us to patt
NoQ updated this revision to Diff 117699.
NoQ added a comment.
Herald added a subscriber: szepet.
Escape into array and dictionary literals, add relevant tests. Fix the null
statement check.
https://reviews.llvm.org/D35216
Files:
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/initial
NoQ added inline comments.
Comment at: test/Analysis/objc-for.m:328
for (id key in array)
clang_analyzer_eval(0); // expected-warning{{FALSE}}
}
I guess these old tests should be replaced with `warnIfReached()`.
https://reviews.llvm.org/D35216
_
NoQ added a comment.
In https://reviews.llvm.org/D35216#886212, @rsmith wrote:
> This is precisely how the rest of the compiler handles
> `CXXStdInitializerListExpr`
Wow. Cool. I'd see what I can do. Yeah, it seems that this is a great case for
us to pattern-match the implementations as well
1 - 100 of 3498 matches
Mail list logo
