- User Since
- Jul 19 2017, 6:59 AM (104 w, 1 d)
Rebase on top master. Putting this on the bottom of the patch stack because this really deserves it's own analysis. (Side note, I completely messed up like ~40 hrs worth of analysis because I didn't check which branches do I have stacked on top of each other, so this might take a while...)
I think you forgot to remove /* */ and clang formatting before uploading the patch.
Hmm, okay, so we convert -1 from the config file to UINT_MAX in the code, I like it!
Tue, Jul 16
Starting to look real good!
Mon, Jul 15
Also, shouldn't we add this to the release notes? In general, it's be around time to sort it out (might do that myself before the new branch).
I don't see obvious red flags strictly regarding the analyzer!
Sun, Jul 14
Would you say it's good to go? :)
Fri, Jul 12
Hmmm, did this result in an assertion?
Thu, Jul 11
Wed, Jul 10
Np, please leave it in! :)
I guess any time we modify analyzer stuff, we may invite the main analyzer developers to the patch review as well.
Please know that I'm currently out of town, so it'll be a while before I can formally accept. Its on top of my list when I get home though! :^)
Just thinking aloud!
Tue, Jul 9
Sat, Jul 6
I happen to have very recent analyses on a couple projects, I'll throw this in: LLVM+Clang+Clang-tools-extra. No findings on Xerces or Bitcoin.
Rebase after D64271 being abandoned.
You're right. If condition tracking only adds necessary information anyways, this shouldn't hurt that much anyways.
Fri, Jul 5
Could you please elaborate? Which of the modified test cases (or any other) do you think falls under "being tracked too far" and why? Whenever the CFG where the value isn't linear, I think the information could be valuable, see the inline.
I.e., i suspect that your "mild tracking mode" would get rid of a lot of those automagically.
Thu, Jul 4
- Add two more test cases when a "Returning value" note is meaningful, and one where it's not
- Fix inlines!
Wed, Jul 3
This checker isn't in alpha -- did you evaluate it on LLVM? Other than that, looks great!
Add one more assert to GetExprText.
- Bail out if the actual terminator isn't a branch
- Bail out if the number of successors is less than 2
- LLVM-ify the code as suggested!
- Add some unit tests (I mean, you can kinda see how it was duct taped together, but it's maybe a hair better than nothing?)
Let's not try to tinker with something in a way that could have unforeseen consequences. I added a new method to simply get the condition the way I (and probably @xazax.hun) will need it.