- User Since
- Mar 7 2019, 1:49 AM (80 w, 6 d)
Wed, Sep 16
Mon, Sep 14
Sun, Sep 13
I'm curious if we could make use of this in the Clang Static Analyzer.
We always hit the limitations of the current Constraint manager implementation.
What do you think about this @NoQ @xazax.hun @Szelethus?
Thu, Sep 10
I suspected this one due to my previous investigation.
Wed, Sep 9
I would like to discuss why don't we have a distinct checker managing the bookkeeping stuff of the CString lengths.
I just want a clean understanding and wide consensus about this.
Tue, Sep 8
Mon, Sep 7
Seems fine to me.
I completely agree with you.
I plan to further refactor the CStringChecker, but the related patches are pretty much stuck :D
Perfectly clear, thank you. However, I would still rely on the others to accept this :|
I have checked only your test, but the readability of the reports should be improved.
You frequently refer to previous events, such as This lock has already been unlocked, This lock has already been acquired, etc.
It isn't clear to the reader where do you refer to. IMO you should put a NoteTag at the interesting locations to achieve more readable diagnostics.
Ping @OikawaKirie .
How should we proceed?
I would happily participate in creating a minimal repro for this, but I need at least one crash.
- Fixed typo in the link Clang Static Analyser -> Clang Static Analyzer
Thank you very much. I tied to make it as short as possible while keeping all the necessary details to make my reasoning perfectly clean. Now I think I was on a bad track, but @martong helped me to pinpoint some issues.
In my example, I'm speaking about the case when it's nonloc::SymbolVal wrapping the expression x+1. So not every NonLoc is ConcreteInt.
- Mentioned the previous namespaces as well (GR, entoGR)
- Slightly rephrased previous sentences
- Updated the summary, to give a deeper dive into the history of the Clang Static Analyzer - for git blame
OK, after a few hours of debugging, the test code simplifies to this:
// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true %s -verify
Fri, Sep 4
Yes, obviously - but it's not a problem. See my next comment.
Thu, Sep 3
Wed, Sep 2
Thank you all for the comments!
- Added no-warning.
- Added test-case for __builtin_unique_stable_name as well.
We only analyze instantiated functions, which are not dependently typed.
Safe to assume that every encountered PredefinedExpression has a defined (non-null) function name.
- Added tests for Microsoft extensions.
- Added an assert requiring the PredefinedExpression to have a function name.
Tue, Sep 1
I tried to run the benchmark on the small set of projects but it would take an enormous time to analyze all such projects 20 times each...
Dedicating a box for this is unfeasible for me.
So I stuck with analyzing only tmux with 20 iterations.
The results are not convincing enough IMO.
Mon, Aug 31
I don't have much to say, but to keep up the good work xD
I will follow this and the rest of your patches @Szelethus.
Thu, Aug 27
Previously, I liked this. Now I love it!
The benchmarks look promising as well.
Wed, Aug 26
How about using the mistical SymbolDerived?
The clang-analyzer-guide says:
Another atomic symbol, closely related to SymbolRegionValue, is SymbolDerived. It represents a value of a region after another symbol was written into a direct or indirect super-region. SymbolDerived contains a reference to both the parent symbol and the parent region. This symbol is mostly a technical hack. Usually SymbolDerived appears after invalidation: the whole structure of a certain type gets smashed with a single SymbolConjured, and then values of its fields become represented with the help of SymbolDerived of that conjured symbol and the region of the field. In any case, SymbolDerived is similar to SymbolRegionValue, just refers to a value after a certain event during analysis rather than at start of analysis.
Could we use that here @NoQ?
Tue, Aug 25
I believe @baloghadamsoftware is on vacation.
I'm sure he won't be mad if you commit on his behalf.
I'm not sure about the status of this patch.
If you say that further improvements will be done later and this functionality is enough, I'm fine with that.
This is a huge change, I've got pretty tired at the end of the review.
I haven't checked the test-cases too deeply.
This preprocessor expansion stuff is definitely not my expertise, nvm here is my review.
Aug 24 2020
- Rephrase paragraph
- Use desktop site wikipedia link
Add link to the static analyzer llvm page.
I tried, but it lacks further fixes.
Oh thanks for the detailed guide, I will make the experiment.
Could you point me to some docs to help getting started? I haven't used this docker image.
Aug 22 2020
I tried to measure this with time, without much luck:
It might introduce a slight overhead since Z3 will parse longer the symbol names.
Use virtual getKindStr method for acquiring the kind name.
Aug 21 2020
Eh, I don't really know how to bench this.
Here is how I did it in nutshell:
Added STATISTIC counters to MemRegion ctor, dtor, getAsOffset begining and the path of just returning the cached value.
It seems that this patch is stuck.
How can I reproduce the crash? @OikawaKirie
Aug 20 2020
It would be nice to have this fix in clang11.
Do you think it's qualified for it?
Sure, I will leave a note.
Aug 19 2020
Aug 18 2020
- Refined documentation comments as noted.
- Added tests.
- Removed the complicated ByteOffsetOfElement lambda.
- Rename revision.
Yea, naming things is pretty hard. I'm open to have a less verbose one - especially since these API functions will live under some namespace.
Fixed Artem's inline comments:
- cstring::getCStringLength now takes StateRef by value
- cstring::dumpCStringLengths now takes by StateRef by non const value
I no longer think that we should support Symbol to Symbol comparisons.
It would introduce certain anomalies, like Why could the CM reason about this and that comparisons wile could not in others.
Add an extra RUN line without refutation.
The expected result is the same as with refutation.
Aug 10 2020
If index1 and index2 has the same value, we should not be confident that the x == y holds.
Aug 9 2020
I mean, this shouldn't be an issue. Since we already omitted the 'unnecessary' cast expressions... That decision is the root cause of this, we should have expected that.
Aug 8 2020
My primary objective is to fix all the crashes related to Range CM + Z3 refutation.
- Using dumps instead of reaching in tests.
- Not requiring complete enums anymore (unlike we did before the patch).
Aug 7 2020
- Moved the FIXME closer to the subject.
- Added tests for covering incomplete enums as well.
- Added REQUIRES: z3. This will mark the test case unsupported on every buildbots. See my notes about this behavior at D83677.
- Refined test file.