Add --force-analyze-debug-code option to scan-build to force debug build and hopefully enable more precise warnings.

[ygribov]

Summary

Static Analyzer is much more efficient when built in debug mode (-UNDEBUG) so we advice users to enable it manually. This may be inconvenient in case of large complex projects (think about Linux distros e.g. Android or Tizen). This patch adds a flag to scan-build which inserts -UNDEBUG automatically.

I've checked this on Android where it removed a bunch of false positives and also uncovered several errors which were present only in assertion code (e.g. use-after-free, etc.).

[zaks.anna]

It would be great if we could make this option more discoverable by the users..

scan-build is being re-implemented in python. Please, add this option to scan-build-py as well; otherwise, it will get lost when we transition.

tools/scan-build/bin/scan-build

1126 ↗

(On Diff #44905)

How about:
"to hopefully get more precise results" -> "to enable more precise results"

www/analyzer/scan-build.html

230 ↗

(On Diff #44905)

"which would enable"?

[a.sidorin]

tools/scan-build/bin/scan-build

1123 ↗

(On Diff #44905)

How about more informative name like '--force-analyze-debug-code'? (The same for $ForceDebug variable name).

[ygribov]

scan-build is being re-implemented in python. Please, add this option to scan-build-py as well; otherwise, it will get lost when we transition.

Anna, how am I supposed to test scan-build-py? It does not seem to be built/installed with default make all and also there are no instructions about manual installation in tools/scan-build-py folder.

[ygribov]

Updated the patch with draft scan-build-py support which I don't know how to test. Also renamed option according to Alexey's comment.

[dcoughlin]

In D16200#329173, @ygribov wrote:

Updated the patch with draft scan-build-py support which I don't know how to test.

Like the perl scan-build, you shouldn't need to install scan-build-py to run it. You can just run the scan-build script in the 'tools/scan-build-py/bin/' directory in the clang source directory.

As for real tests, scan-build-py has custom unit-ish tests in 'tools/scan-build-py/tests/' that you might extend. These are not run automatically yet.

We have plans to add lit-based tests for scan-build. This will help with qualifying scan-build-py before it replaces the perl scan-build and will also provide a more solid testing foundation moving forward.

[dcoughlin]

Adding Laszlo as a reviewer for the scan-build-py bits.

[rizsotto.mailinglist]

hey Yury,

nice idea and good work! i personally think that even more aggressive ways to explore bugs. (like call it with and without optimization/debug flags, collect the report from multiple runs and merge it into a single report.) anyway, here it is how to run the python tests. (run it from the scan-build-py directory.)

$ PATH=$(pwd)/bin:$PATH python -m unittest -v

tools/scan-build-py/libscanbuild/runner.py

182 ↗

(On Diff #45157)

can you move this logic into a separate method, and write tests around it? (it also will give you better names than new_args or args :)) unit tests for this module are in tests/unit/test_runner.py.

also, iterate through a list in python should do differently. (please take a look at the command module classify_parameters method.)

and i am not sure about the check you do with the line if 'force_analyze_debug_code' in args. you want to check opt instead of args, don't you? and since you were required to pass this parameter, you can write if opts['force_analyze_debug_code'].

[ygribov]

Addressed some of the comments, seems to work.

[ygribov]

Like the perl scan-build, you shouldn't need to install scan-build-py to run it.
You can just run the scan-build script in the 'tools/scan-build-py/bin/' directory in the clang source directory.

Not really, because files in scan-build-py/bin lack executable perms and also scan-build-py uses system clang instead of the one you want (and for some reasone updating PATH does not help). These are probably nits but makes testing kind of inconvenient.

[ygribov]

nice idea and good work! i personally think that even more aggressive ways to explore bugs. (like call it with and without optimization/debug flags,
collect the report from multiple runs and merge it into a single report.)

Makes good sense.

anyway, here it is how to run the python tests. (run it from the scan-build-py directory.)

$ PATH=$(pwd)/bin:$PATH python -m unittest -v

I don't think so:

$ python -m unittest -v

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK

whereas

$ for t in tests/unit/*.py; do t=$(echo $t | sed -e 's/\..*//'); python -m unittest tests.unit.$(basename $t); done

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
...............................................
----------------------------------------------------------------------
Ran 47 tests in 0.452s

OK
etc. etc.

leaving me puzzled about what exactly needs to be tested.

As for adding the unit test, is there currently a way to analyze C file and verify that output contains particular analyzer warning? I haven't found anything like this in tests/.

[rizsotto.mailinglist]

$ PATH=$(pwd)/bin:$PATH python -m unittest -v

I don't think so:

the command i gave to you works fine on my environment. would be curious to know what OS/python do you use to have the differences. anyway, to run python unittest see more on this link https://docs.python.org/2/library/unittest.html

leaving me puzzled about what exactly needs to be tested.

As for adding the unit test, is there currently a way to analyze C file and verify that output contains particular analyzer warning? I haven't found anything like this in tests/.

i would make this method do only the release flag filtering. (maybe call the method something like that too. eg.: debug_compile_flags) and call it only if the force_analyze_debug_code was set. this way you have a method you can test easy. pass some input flags, and check are they as expected.... assertEquals(['-Dkey=value'], debug_compile_flags(['-Dkey=value', '-DNDEBUG'])). or you can extend the create_commands method with special cases.

minor comments: you should extend the required decorator with force_analyze_debug_code on create_commands method... please call the new_args as result which is more descriptive... please add documentation to this new method...

thanks

[ygribov]

$ PATH=$(pwd)/bin:$PATH python -m unittest -v

I don't think so:

the command i gave to you works fine on my environment.
would be curious to know what OS/python do you use to have the differences.

I'm on Ubuntu 14.04 with it's Python 2.7.6 (default, Jun 22 2015, 17:58:13).

anyway, to run python unittest see more on this link https://docs.python.org/2/library/unittest.html

Ah, thanks, it works now but only if I ran it as

$ python -m unittest  tests.__init__

and add

unittest.TextTestRunner(verbosity=2).run(suite())

to tests/__init__.py (and this has 41 error).

i would make this method do only the release flag filtering.

Got it.

minor comments: you should extend the required decorator with force_analyze_debug_code on create_commands method...
please call the new_args as result which is more descriptive... please add documentation to this new method...

Right.

[ygribov]

Addressed Laszlo's comments.

[ygribov]

Laszlo, Anna, does this look better?

[rizsotto.mailinglist]

thanks Yury for the update.

tools/scan-build-py/tests/unit/test_runner.py

216 ↗

(On Diff #45683)

interesting to see that the method is changing the flags. (glue them together) my expectation would be that it only filters, and do not modify it. (i understand that semantically it is the same.)

and would like to see a few more tests... ['-Dkey=val'], ['-DNDEBUG'], ['-D', 'NDEBUG'], ['-DNDEBUG=1'], ['-DDEBUG'], ['-Dkey=val', '-DNDEBUG'], ['-Dkey=val', '-D', 'NDEBUG'], ['-DNDEBUG', '-Dkey=val'], ['-Dkey1=val1', '-DNDEBUG=1', '-Dkey2=val2']

[ygribov]

Addressed Laszlo's comments.

[rizsotto.mailinglist]

thanks Yury!

[zaks.anna]

Thanks!

[rizsotto.mailinglist]

sorry for it, but was thinking on the weekend and i have a lame question after the hard work: why not just append a -UNDEBUG to the end of the argument list? would not it make the same effect?

[ygribov]

why not just append a -UNDEBUG to the end of the argument list? would not it make the same effect?

I think for Perl version of scan-build I was just following the general practice of stripping/modifying interesting arguments. And scan-build-py then just followed the same pattern. -UNDEBUG is supported by all major compiler so it's probably ok to use it instead. Should I switch?

[rizsotto.mailinglist]

to me it appears a more reasonable solution. (it does not require a separate method and unit test around it.) maybe the same effort could be focus to write a functional test for it... that's only my thoughts. Devin or Anna, could you share your opinion?

[zaks.anna]

If the implementation is simpler and there are no other downsides, we should go for it.

[ygribov]

Append -UNDEBUG instead of filtering flags, per Laszlo's advice.

[ygribov]

Ping.

[zaks.anna]

See some nits below. Otherwise, LGTM.

Thank you!
Anna.

tools/scan-build-py/libscanbuild/analyze.py

463 ↗

(On Diff #47184)

"(to enable more precise results)" -> enabling more precise results

tools/scan-build-py/libscanbuild/runner.py

169 ↗

(On Diff #47184)

Please, update or remove the comment.

tools/scan-build-py/tests/unit/test_runner.py

222 ↗

(On Diff #47184)

Is '-' missing in front of NDEBUG?

tools/scan-build/bin/scan-build

1126 ↗

(On Diff #47184)

please remove '('.

[zaks.anna]

LGTM.
Thank you.

cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py

def exclude(filename):
         return any(re.match(r'^' + directory, filename)
                    for directory in args.excludes)
 
     consts = {
         'clang': args.clang,
         'output_dir': output_dir,
         'output_format': args.output_format,
         'output_failures': args.output_failures,
-        'direct_args': analyzer_params(args)
+        'direct_args': analyzer_params(args),
+        'force_analyze_debug_code' : args.force_analyze_debug_code
     }
 
     logging.debug('run analyzer against compilation database')
     with open(args.cdb, 'r') as handle:
         generator = (dict(cmd, **consts)
                      for cmd in json.load(handle) if not exclude(cmd['file']))
         # when verbose output requested execute sequentially
         pool = multiprocessing.Pool(1 if args.verbose > 2 else None)
environment.update({
         'CXX': os.path.join(bin_dir, COMPILER_WRAPPER_CXX),
         'ANALYZE_BUILD_CC': args.cc,
         'ANALYZE_BUILD_CXX': args.cxx,
         'ANALYZE_BUILD_CLANG': args.clang if need_analyzer(args.build) else '',
         'ANALYZE_BUILD_VERBOSE': 'DEBUG' if args.verbose > 2 else 'WARNING',
         'ANALYZE_BUILD_REPORT_DIR': destination,
         'ANALYZE_BUILD_REPORT_FORMAT': args.output_format,
         'ANALYZE_BUILD_REPORT_FAILURES': 'yes' if args.output_failures else '',
-        'ANALYZE_BUILD_PARAMETERS': ' '.join(analyzer_params(args))
+        'ANALYZE_BUILD_PARAMETERS': ' '.join(analyzer_params(args)),
+        'ANALYZE_BUILD_FORCE_ANALYZE_DEBUG_CODE'
+            : 'yes' if args.force_analyze_debug_code else ''
     })
     return environment
 
 
 def analyze_build_wrapper(cplusplus):
     """ Entry point for `analyze-cc` and `analyze-c++` compiler wrappers. """
 
     # initialize wrapper logging
try:
         # collect the needed parameters from environment, crash when missing
         consts = {
             'clang': os.getenv('ANALYZE_BUILD_CLANG'),
             'output_dir': os.getenv('ANALYZE_BUILD_REPORT_DIR'),
             'output_format': os.getenv('ANALYZE_BUILD_REPORT_FORMAT'),
             'output_failures': os.getenv('ANALYZE_BUILD_REPORT_FAILURES'),
             'direct_args': os.getenv('ANALYZE_BUILD_PARAMETERS',
                                      '').split(' '),
+            'force_analyze_debug_code':
+                os.getenv('ANALYZE_BUILD_FORCE_ANALYZE_DEBUG_CODE'),
             'directory': os.getcwd(),
         }
         # get relevant parameters from command line arguments
         args = classify_parameters(sys.argv)
         filenames = args.pop('files', [])
         for filename in (name for name in filenames if classify_source(name)):
             parameters = dict(args, file=filename, **consts)
             logging.debug('analyzer parameters %s', parameters)
advanced.add_argument(
         dest='excludes',
         action='append',
         default=[],
         help="""Do not run static analyzer against files found in this
                 directory. (You can specify this option multiple times.)
                 Could be usefull when project contains 3rd party libraries.
                 The directory path shall be absolute path as file names in
                 the compilation database.""")
+    advanced.add_argument(
+        '--force-analyze-debug-code',
+        dest='force_analyze_debug_code',
+        action='store_true',
+        help="""Tells analyzer to enable assertions in code even if they were
+                disabled during compilation, enabling more precise
+                results.""")
 
     plugins = parser.add_argument_group('checker options')
     plugins.add_argument(
         '--load-plugin', '-load-plugin',
         metavar='<plugin library>',
         dest='plugins',
         action='append',
         help="""Loading external checkers using the clang plugin interface.""")

cfe/trunk/tools/scan-build-py/libscanbuild/runner.py

def decorator(function):
 
         return wrapper
 
     return decorator
 
 
 @require(['command', 'directory', 'file',  # an entry from compilation database
           'clang', 'direct_args',  # compiler name, and arguments from command
+          'force_analyze_debug_code',  # preprocessing options
           'output_dir', 'output_format', 'output_failures'])
 def run(opts):
     """ Entry point to run (or not) static analyzer against a single entry
     of the compilation database.
 
     This complex task is decomposed into smaller methods which are calling
     each other in chain. If the analyzis is not possibe the given method
     just return and break the chain.
if opts.get('output_format') in {'plist', 'plist-html'}:
                                          delete=False,
                                          dir=opts['output_dir']) as output:
             opts.update({'output': ['-o', output.name]})
             return continuation(opts)
     else:
         opts.update({'output': ['-o', opts['output_dir']]})
         return continuation(opts)
 
-
-@require(['file', 'directory', 'clang', 'direct_args', 'language',
-          'output_dir', 'output_format', 'output_failures'])
+def force_analyze_debug_code(cmd):
+    """ Enable assert()'s by undefining NDEBUG. """
+    cmd.append('-UNDEBUG')
+
+@require(['file', 'directory', 'clang', 'direct_args',
+          'force_analyze_debug_code', 'language', 'output_dir',
+          'output_format', 'output_failures'])
 def create_commands(opts, continuation=set_analyzer_output):
     """ Create command to run analyzer or failure report generation.
 
     It generates commands (from compilation database entries) which contains
     enough information to run the analyzer (and the crash report generation
     if that was requested). """
 
     common = []
     if 'arch' in opts:
         common.extend(['-arch', opts.pop('arch')])
     common.extend(opts.pop('compile_options', []))
+    if opts['force_analyze_debug_code']:
+        force_analyze_debug_code(common)
     common.extend(['-x', opts['language']])
     common.append(os.path.relpath(opts['file'], opts['directory']))
 
     opts.update({
         'analyze': ['--analyze'] + opts['direct_args'] + common,
         'report': ['-fsyntax-only', '-E'] + common
     })
 

cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py

def test_method_with_expecteds(self):
         self.assertRaises(KeyError, method_with_expecteds, dict())
         self.assertRaises(KeyError, method_with_expecteds, {})
         self.assertRaises(KeyError, method_with_expecteds, {'this': 2})
         self.assertRaises(KeyError, method_with_expecteds, {'that': 3})
         self.assertEqual(method_with_expecteds({'this': 0, 'that': 3}), 0)
 
     def test_method_exception_not_caught(self):
         self.assertRaises(Exception, method_exception_from_inside, dict())
+
+class ForceAnalyzeDebugTest(unittest.TestCase):
+
+    def test_force_analyze_debug_code(self):
+        for a, b in [
+                ([], ['-UNDEBUG']),
+                (['-O2'], ['-O2', '-UNDEBUG']),
+                (['-Dkey=val'], ['-Dkey=val', '-UNDEBUG']),
+                (['-D', 'NDEBUG'], ['-D', 'NDEBUG', '-UNDEBUG']) ]:
+            sut.force_analyze_debug_code(a)
+            self.assertEqual(a, b)

cfe/trunk/tools/scan-build/bin/scan-build

my %Options = (
   InternalStats => undef,
   OutputFormat => "html",
   ConfigOptions => [],       # Options to pass through to the analyzer's -analyzer-config flag.
   ReportFailures => undef,
   AnalyzerStats => 0,
   MaxLoop => 0,
   PluginsToLoad => [],
   AnalyzerDiscoveryMethod => undef,
-  OverrideCompiler => 0      # The flag corresponding to the --override-compiler command line option.
+  OverrideCompiler => 0,      # The flag corresponding to the --override-compiler command line option.
+  ForceAnalyzeDebugCode => 0
 );
 lock_keys(%Options);
 
 ##----------------------------------------------------------------------------##
 # Diagnostics
 ##----------------------------------------------------------------------------##
 
 sub Diag {
sub SetEnv {
   }
   foreach my $var ('CCC_ANALYZER_STORE_MODEL',
                    'CCC_ANALYZER_CONSTRAINTS_MODEL',
                    'CCC_ANALYZER_INTERNAL_STATS',
                    'CCC_ANALYZER_OUTPUT_FORMAT',
                    'CCC_CC',
                    'CCC_CXX',
                    'CCC_REPORT_FAILURES',
-                   'CLANG_ANALYZER_TARGET') {
+                   'CLANG_ANALYZER_TARGET',
+                   'CCC_ANALYZER_FORCE_ANALYZE_DEBUG_CODE') {
     my $x = $EnvVars->{$var};
     if (defined $x) { $ENV{$var} = $x }
   }
   my $Verbose = $EnvVars->{'VERBOSE'};
   if ($Verbose >= 2) {
     $ENV{'CCC_ANALYZER_VERBOSE'} = 1;
   }
   if ($Verbose >= 3) {

 print <<ENDTEXT;
 OPTIONS:
 
  -analyze-headers
 
    Also analyze functions in #included files.  By default, such functions
    are skipped unless they are called by functions within the main source file.
 
+ --force-analyze-debug-code
+
+   Tells analyzer to enable assertions in code even if they were disabled
+   during compilation to enable more precise results.
+
  -o <output location>
 
    Specifies the output directory for analyzer reports. Subdirectories will be
    created as needed to represent separate "runs" of the analyzer. If this
    option is not specified, a directory is created in /tmp (TMPDIR on Mac OS X)
    to store the reports.
 
  -h
while (@$Args) {
     }
 
     if ($arg eq "--override-compiler") {
       shift @$Args;
       $Options{OverrideCompiler} = 1;
       next;
     }
 
+    if ($arg eq "--force-analyze-debug-code") {
+      shift @$Args;
+      $Options{ForceAnalyzeDebugCode} = 1;
+      next;
+    }
+
     DieDiag("unrecognized option '$arg'\n") if ($arg =~ /^-/);
 
     $NumArgs--;
     last;
   }
   return $NumArgs;
 }
 
my %EnvVars = (
   'OUTPUT_DIR' => $Options{OutputDir},
   'CCC_CC' => $Options{UseCC},
   'CCC_CXX' => $Options{UseCXX},
   'CCC_REPORT_FAILURES' => $Options{ReportFailures},
   'CCC_ANALYZER_STORE_MODEL' => $Options{StoreModel},
   'CCC_ANALYZER_CONSTRAINTS_MODEL' => $Options{ConstraintsModel},
   'CCC_ANALYZER_INTERNAL_STATS' => $Options{InternalStats},
   'CCC_ANALYZER_OUTPUT_FORMAT' => $Options{OutputFormat},
-  'CLANG_ANALYZER_TARGET' => $Options{AnalyzerTarget}
+  'CLANG_ANALYZER_TARGET' => $Options{AnalyzerTarget},
+  'CCC_ANALYZER_FORCE_ANALYZE_DEBUG_CODE' => $Options{ForceAnalyzeDebugCode}
 );
 
 # Run the build.
 my $ExitStatus = RunBuildCommand(\@ARGV, $Options{IgnoreErrors}, $Cmd, $CmdCXX,
                                 \%EnvVars);
 
 if (defined $Options{OutputFormat}) {
   if ($Options{OutputFormat} =~ /plist/) {

cfe/trunk/tools/scan-build/libexec/ccc-analyzer

 # Determine the level of verbosity.
 my $Verbose = 0;
 if (defined $ENV{'CCC_ANALYZER_VERBOSE'}) { $Verbose = 1; }
 if (defined $ENV{'CCC_ANALYZER_LOG'}) { $Verbose = 2; }
 
 # Get the HTML output directory.
 my $HtmlDir = $ENV{'CCC_ANALYZER_HTML'};
 
+# Get force-analyze-debug-code option.
+my $ForceAnalyzeDebugCode = $ENV{'CCC_ANALYZER_FORCE_ANALYZE_DEBUG_CODE'};
+
 my %DisabledArchs = ('ppc' => 1, 'ppc64' => 1);
 my %ArchsSeen;
 my $HadArch = 0;
 my $HasSDK = 0;
 
 # Process the arguments.
 foreach (my $i = 0; $i < scalar(@ARGV); ++$i) {
   my $Arg = $ARGV[$i];
foreach (my $i = 0; $i < scalar(@ARGV); ++$i) {
   }
 
   if (!($Arg =~ /^-/)) {
     push @Files, $Arg;
     next;
   }
 }
 
+# Forcedly enable debugging if requested by user.
+if ($ForceAnalyzeDebugCode) {
+  push @CompileOpts, '-UNDEBUG';
+}
+
 # If we are on OSX and have an installation where the
 # default SDK is inferred by xcrun use xcrun to infer
 # the SDK.
 if (not $HasSDK and $UseXCRUN) {
   my $sdk = `/usr/bin/xcrun --show-sdk-path -sdk macosx`;
   chomp $sdk;
   push @CompileOpts, "-isysroot", $sdk;
 }

cfe/trunk/www/analyzer/scan-build.html

 
 <h3 id="recommended_debug">ALWAYS analyze a project in its &quot;debug&quot; configuration</h3>
 
 <p>Most projects can be built in a &quot;debug&quot; mode that enables assertions.
 Assertions are picked up by the static analyzer to prune infeasible paths, which
 in some cases can greatly reduce the number of false positives (bogus error
 reports) emitted by the tool.</p>
 
+<p>Another option is to use <tt>--force-analyze-debug-code</tt> flag of
+<b>scan-build</b> tool which would enable assertions automatically.</p>
+
 <h3 id="recommend_verbose">Use verbose output when debugging scan-build</h3>
 
 <p><tt>scan-build</tt> takes a <b>-v</b> option to emit verbose output about
 what it's doing; two <b>-v</b> options emit more information. Redirecting the
 output of <tt>scan-build</tt> to a text file (make sure to redirect standard
 error) is useful for filing bug reports against <tt>scan-build</tt> or the
 analyzer, as we can see the exact options (and files) passed to the analyzer.
 For more comprehensible logs, don't perform a parallel build.</p>