krememek (Ted Kremenek)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 1 2013, 1:22 PM (215 w, 3 d)

Recent Activity

Nov 1 2016

krememek added a comment to D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue.

LGTM.

Nov 1 2016, 9:55 AM

Oct 25 2015

krememek accepted D10021: Refactor: Simplify boolean conditional return statements in lib/StaticAnalyzer/Checkers.

These all look good to me.

Oct 25 2015, 12:27 PM

Sep 12 2015

krememek closed D12827: [analyzer] fix an error finding clang path.
Sep 12 2015, 9:03 AM
krememek accepted D12827: [analyzer] fix an error finding clang path.

Applied in r247510.

Sep 12 2015, 9:03 AM

Sep 11 2015

krememek added a comment to D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large.

LGTM as well. Thanks Sean.

Sep 11 2015, 1:34 PM

Sep 7 2015

krememek closed D12673: [analyzer] Remove whitespace in source code.
Sep 7 2015, 8:53 PM
krememek accepted D12673: [analyzer] Remove whitespace in source code.

Committed in r246978.

Sep 7 2015, 8:52 PM
krememek added a comment to D12673: [analyzer] Remove whitespace in source code.

I am OK with taking these changes. For those using git, git blame -w should suffice to show the real blame history.

Sep 7 2015, 8:49 PM
krememek added a comment to D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large.

This looks fine, but I'm not a fan of the actual name chosen here. It's not very clear what it means by just looking at the name, and as we grow the number of configuration options that will become more important.

Sep 7 2015, 1:20 PM

Sep 2 2015

krememek added a comment to D12358: [Analyzer] Widening loops which do not exit.

To get the conservative invalidation that Anna suggests (actually, a little less conservative), I think you can just pass in a two MemRegions as the input to that method and get a new ProgramState with the appropriate regions invalidated.

Thank you for the suggestion. Unfortunately nothing seems to get invalidated when I call invalidateRegions, in the following code:

const StackFrameContext *STC = LCtx->getCurrentStackFrame();
MemRegionManager &MRMgr = svalBuilder.getRegionManager();
const MemRegion *Regions[] = {
  MRMgr.getStackLocalsRegion(STC),
  MRMgr.getStackArgumentsRegion(STC),
  MRMgr.getGlobalsRegion()
};
ProgramStateRef State;
State = PrevState->invalidateRegions(Regions, Cond, BlockCount, LCtx, false);

Do you have any suggestions on what I have done wrong?

Sep 2 2015, 11:20 PM

Aug 31 2015

krememek added a comment to D12358: [Analyzer] Widening loops which do not exit.

If I refactor this patch in its current state into a separate file and put it behind a flag, would it then be acceptable? I would then like to take some time to look at the invalidation improvements as discussed in this thread.

Aug 31 2015, 11:11 PM

Aug 28 2015

krememek added a comment to D12358: [Analyzer] Widening loops which do not exit.
Aug 28 2015, 12:11 AM
krememek added a comment to D12358: [Analyzer] Widening loops which do not exit.

Hi Sean,

Aug 28 2015, 12:01 AM

Aug 27 2015

krememek added a comment to D12358: [Analyzer] Widening loops which do not exit.

I think the functionality started here by doing something clever with loops is complex enough to warrant putting this into a separate .cpp file. We can keep this here for now, but this seems like complexity that is going to naturally creep up and make this file even more monolithic than it already is.

Aug 27 2015, 11:59 PM
krememek added a comment to D12358: [Analyzer] Widening loops which do not exit.

I accept that my current patch is not a comprehensive solution to the problem and that it may introduce > false positives, however I do think it is an improvement, where it is preferable to have false positives
over doing no analysis after the loop.

We try to avoid false positives as much as possible. They are very painful for users to deal with.

In my experience, constant bound loops are normally used to make simple modifications to fixed
length collections of data, I think the behaviour of the majority of these loops will be represented by
the first and last iteration.

The main issue with the patch is that it produces a false path on which value of only one of the variables is reset to the last iteration of the loop and the rest of them are set as if it is the 3d iteration. A way to solve this is to compute what can be invalidated by the loop and set those to unknown values (a widening operation).

You should develop this feature behind a flag. This would allow for incremental development and simplify evaluation.

Aug 27 2015, 11:47 PM

Aug 26 2015

krememek added a comment to D12358: [Analyzer] Widening loops which do not exit.

One thing about the tests passing: that's great, but that's obviously insufficient. We probably have fewer tests showing that the analyzer can't handle something than tests that show it does handle something.

Aug 26 2015, 9:53 PM
krememek added a comment to D12358: [Analyzer] Widening loops which do not exit.

FWIW, I do think this is a great problem to work on. It is easy to come up with solutions that work for specific examples but fall over on general code. I completely agree that failing to analyzing code after the loop is a major hole and lost opportunity to find bugs, but fixing that should not be at a tradeoff of a huge influx in false positives. Some basic invalidation of values touched by the loop, which includes possibly invalidating checker state, will likely be necessary. I think this is what Anna was getting to in her comment.

Aug 26 2015, 9:51 PM
krememek added a comment to D12358: [Analyzer] Widening loops which do not exit.

A way this could be improved is by invalidating all the values that the loops effects, both the values on the stack and on the heap. (We could even start overly conservative and invalidate all the values in the state; setting the known values to unknown values.)

Aug 26 2015, 9:39 PM

Aug 24 2015

krememek added inline comments to D11700: Added remove taint support to ProgramState..
Aug 24 2015, 9:37 PM
krememek added inline comments to D11700: Added remove taint support to ProgramState..
Aug 24 2015, 9:36 PM

Aug 21 2015

krememek added inline comments to D12119: Analyzer: Fix a crasher in UbigraphViz.
Aug 21 2015, 10:11 PM
krememek added inline comments to D12119: Analyzer: Fix a crasher in UbigraphViz.
Aug 21 2015, 10:09 PM

Aug 19 2015

krememek added a comment to D12144: Fix 4 typos in lib/Analysis/.

Looks fine in UninitializedValues.

Aug 19 2015, 8:59 AM

Aug 18 2015

krememek added a comment to D12123: [analyzer] Skip Pre/Post handlers for ObjC calls when receiver is nil..

I think this is a great refinement overall, with a few minor nits. It also isn't clear what the test does.

Aug 18 2015, 3:50 PM
krememek added inline comments to D12119: Analyzer: Fix a crasher in UbigraphViz.
Aug 18 2015, 3:20 PM
krememek added inline comments to D12119: Analyzer: Fix a crasher in UbigraphViz.
Aug 18 2015, 3:18 PM

Aug 8 2015

krememek added a comment to D10356: scan-build: Add --analyzer-target option.

Committed r244400

Aug 8 2015, 10:59 AM

Aug 7 2015

krememek added a comment to D10356: scan-build: Add --analyzer-target option.
Aug 7 2015, 3:18 PM

Aug 3 2015

krememek added a comment to D10356: scan-build: Add --analyzer-target option.

Hi all,

I wrote this patch because I had some problem generating a clang static analysis report for ARM Linux kernel code. Since it uses
some target specific flags for ARM, I got the following error message during analysis.

error: unknown target CPU 'armv7-a'

I have checked scan-build script, but there was no way to pass target triple information to clang, so clang thinks that the source
code can be compiled for host machine, x86_64. This patch adds --triple option to fix this problem.

With this patch, I was able to generate clang static analysis report for ARM Linux kernel code with the following command:
$ scan-build --use-cc=arm-linux-gcc --triple=arm-linux-gnueabi make ARCH=arm CROSS_COMPILE=arm-linux-

Please let me know your opinion.
I appreciate all your comment.

Thanks,
Honggyu

Aug 3 2015, 2:19 PM
krememek added a comment to D10356: scan-build: Add --analyzer-target option.

I have updated the new patchset with more information in help message and changed the option name to —analyzer-target.
If you think the "extra flags to pass to the compiler” is still needed, I will write another patch for that.
I appreciate your comment and suggestion.

Aug 3 2015, 2:17 PM

Jul 31 2015

krememek added a comment to D11700: Added remove taint support to ProgramState..

In general I'm not a fan of the taint methods being directly on ProgramState, but this extends the current pattern. We could move the APIs elsewhere later.

Jul 31 2015, 10:49 PM
krememek added a comment to D11700: Added remove taint support to ProgramState..

I don't remember why the 'tainted' methods were added to ProgramState in the first place, but it doesn't seem quite right. Taint can easily be modeled as a set of APIs that modify and produce new ProgramStates, but I don't see why it should be part of ProgramState itself.

Jul 31 2015, 9:44 PM

Jul 30 2015

krememek added inline comments to D10356: scan-build: Add --analyzer-target option.
Jul 30 2015, 3:13 PM

Sep 9 2014

krememek added a comment to D5247: [analyzer] Move function declarations of PthreadLockChecker's tests to a separate system-like header.

This feels like a big hammer to wield just to prevent the conservative invalidation done by the static analyzer for calls to functions whose bodies are not available.

Sep 9 2014, 11:40 AM

Aug 19 2014

krememek added a comment to D3967: [Proposal] [Analyzer] Individual options for checkers.

Thanks Aleksei. That explanation helps quite a bit. I've responded in Phabricator with some comments.

Aug 19 2014, 2:53 PM
krememek added inline comments to D3967: [Proposal] [Analyzer] Individual options for checkers.
Aug 19 2014, 2:52 PM
krememek added a comment to D4974: [analyzer][Bugfix] RegionStore: use pointee type to create one-element regions.

Can you please provide a test case that shows what problem this fixed?

Aug 19 2014, 12:02 PM
krememek accepted D4973: [analyzer] [Refactoring+bugfix] Replace copy-paste function with a correctly implemented one.
Aug 19 2014, 10:48 AM
krememek added a comment to D4973: [analyzer] [Refactoring+bugfix] Replace copy-paste function with a correctly implemented one.

Looks good to me.

Aug 19 2014, 10:48 AM

Aug 16 2014

krememek added a comment to D3967: [Proposal] [Analyzer] Individual options for checkers.

Hi Aleksei,

Aug 16 2014, 11:03 AM