On 2013-09-11 07:33:59, Steve Beattie wrote: > On Wed, Sep 11, 2013 at 01:57:13AM -0700, Tyler Hicks wrote: > > On 2013-09-09 11:29:28, Steve Beattie wrote: > > > It's also finding what may be some false positives around > > > invalid reads around profile serialization (e.g. on > > > simple_tests/conditional/else_if_5.sd) and conditional > > > jump or move depends on uninitialized value(s) (e.g. > > > simple_tests/network/network_ok_1.sd) that need more investigation, > > > to determine whether it's a legitimate issue or a valgrind check that > > > hopefully can be disabled. > > > > The memory leak fixes get my > > > > Acked-by: Tyler Hicks <[email protected]> > > > > I rebased the C++ patches on that patch, so please go ahead and commit > > it when you get a chance. > > Committed. > > > I need some more time to look at the valgrind test patch. It doesn't > > like the C++ conversion and ever test was failing before I killed it. > > I'm not immediately sure what is happening. Running the parser through > > valgrind on the same testcases shows no issue. I'll have to look into > > more. > > Try the following version. It seems gcc emits code for some string loops > that reads past the end of the allocated area, even though the code > check guarantees that things won't go out of bounds, but valgrind can't > tell the difference. I added some suppressions to try to avoid that, but > it's awfully brittle. I may just convert it to suppressing any of the 4 > byte read errors.
Interesting. The suppressions allow many of the tests to pass with the C++ patches applied. There are still some new failures caused by the C++ patches. This new valgrind target will be helpful in fixing those issues. Acked-by: Tyler Hicks <[email protected]> Tyler > > Subject: parser - add simple valgrind wrapper tests > > This patch adds a test wrapper that runs valgrind on the parser over the > simple_tests tree (or other directory tree if passed on the command > line). An alternate parser location can also be passed on the command > line. > > Like the libapparmor python bindings test, this test uses a bit of magic > to generate tests that doesn't work with auto-detecting test utilities > like nose. > > Running valgrind on the parser over all 69000+ testcases takes several > hours, so while this patch includes a make target 'make valgrind', it > does not add it to the set of tests run when 'make check' is called. > Perhaps a 'make extra-tests' target is in order. > > Patch history: > v1: initial version > v2: add valgrind suppressions for overaggressive 4 byte reads past the > end of allocated storage. > > Signed-off-by: Steve Beattie <[email protected]> > --- > parser/tst/Makefile | 3 > parser/tst/valgrind_simple.py | 197 > ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 200 insertions(+) > > Index: b/parser/tst/valgrind_simple.py > =================================================================== > --- /dev/null > +++ b/parser/tst/valgrind_simple.py > @@ -0,0 +1,197 @@ > +#!/usr/bin/env python > +# ------------------------------------------------------------------ > +# > +# Copyright (C) 2013 Canonical Ltd. > +# Author: Steve Beattie <[email protected]> > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of version 2 of the GNU General Public > +# License published by the Free Software Foundation. > +# > +# ------------------------------------------------------------------ > + > +from optparse import OptionParser # deprecated, should move to argparse > eventually > +import os > +import signal > +import subprocess > +import tempfile > +import unittest > + > +DEFAULT_TESTDIR = "./simple_tests/vars" > +DEFAULT_PARSER = '../apparmor_parser' > +VALGRIND_ERROR_CODE = 151 > +TIMEOUT_ERROR_CODE = 152 > +VALGRIND_ARGS = ['--leak-check=full', '--error-exitcode=%d' % > (VALGRIND_ERROR_CODE)] > + > +VALGRIND_SUPPRESSIONS = ''' > +{ > + valgrind-add_search_dir-obsessive-overreads > + Memcheck:Addr4 > + fun:_Z*add_search_dir* > + fun:_Z*process_arg* > + fun:main > +} > + > +{ > + valgrind-yylex-obsessive-overreads > + Memcheck:Addr4 > + fun:_Z?yylex? > + fun:_Z*yyparse* > + fun:_Z*process_profile* > + fun:main > +} > + > +{ > + valgrind-serialize_profile-obsessive-overreads > + Memcheck:Addr4 > + fun:_Z*sd_serialize_profile* > + fun:_Z*sd_serialize_codomain* > + fun:_Z*load_codomain* > + fun:_Z*__load_flattened_hat* > + ... > + fun:twalk > + fun:_Z*load_flattened_hats* > + fun:_Z*sd_serialize_codomain* > + fun:_Z*load_codomain* > + fun:_Z*__load_policy* > +}''' > + > +# > http://www.chiark.greenend.org.uk/ucgi/~cjwatson/blosxom/2009-07-02-python-sigpipe.html > +# This is needed so that the subprocesses that produce endless output > +# actually quit when the reader goes away. > +def subprocess_setup(): > + # Python installs a SIGPIPE handler by default. This is usually not > + # what non-Python subprocesses expect. > + signal.signal(signal.SIGPIPE, signal.SIG_DFL) > + > + > +def run_cmd(command, input=None, stderr=subprocess.STDOUT, > stdout=subprocess.PIPE, stdin=None, timeout=120): > + '''Try to execute given command (array) and return its stdout, or > + return a textual error if it failed.''' > + > + try: > + sp = subprocess.Popen(command, stdin=stdin, stdout=stdout, > stderr=stderr, close_fds=True, preexec_fn=subprocess_setup) > + except OSError as e: > + return [127, str(e)] > + > + timeout_communicate = TimeoutFunction(sp.communicate, timeout) > + out, outerr = (None, None) > + try: > + out, outerr = timeout_communicate(input) > + rc = sp.returncode > + except TimeoutFunctionException as e: > + sp.terminate() > + outerr = b'test timed out, killed' > + rc = TIMEOUT_ERROR_CODE > + > + # Handle redirection of stdout > + if out is None: > + out = b'' > + # Handle redirection of stderr > + if outerr is None: > + outerr = b'' > + return [rc, out.decode('utf-8') + outerr.decode('utf-8')] > + > + > +# Timeout handler using alarm() from John P. Speno's Pythonic Avocado > +class TimeoutFunctionException(Exception): > + """Exception to raise on a timeout""" > + pass > + > + > +class TimeoutFunction: > + def __init__(self, function, timeout): > + self.timeout = timeout > + self.function = function > + > + def handle_timeout(self, signum, frame): > + raise TimeoutFunctionException() > + > + def __call__(self, *args, **kwargs): > + old = signal.signal(signal.SIGALRM, self.handle_timeout) > + signal.alarm(self.timeout) > + try: > + result = self.function(*args, **kwargs) > + finally: > + signal.signal(signal.SIGALRM, old) > + signal.alarm(0) > + return result > + > + > +class AAParserValgrindTests(unittest.TestCase): > + def setUp(self): > + # REPORT ALL THE OUTPUT > + self.maxDiff = None > + > + def _runtest(self, testname, config): > + parser_args = ['-Q', '-I', config.testdir] > + failure_rc = [VALGRIND_ERROR_CODE, TIMEOUT_ERROR_CODE] > + command = ['valgrind'] > + command.extend(VALGRIND_ARGS) > + command.append(config.parser) > + command.extend(parser_args) > + command.append(testname) > + rc, output = run_cmd(command, timeout=120) > + self.assertNotIn(rc, failure_rc, > + "valgrind returned error code %d, gave the following > output\n%s" % (rc, output)) > + > + > +def find_testcases(testdir): > + '''dig testcases out of passed directory''' > + > + for (fdir, direntries, files) in os.walk(testdir): > + for f in files: > + if f.endswith(".sd"): > + yield os.path.join(fdir, f) > + > + > +def create_suppressions(): > + '''generate valgrind suppressions file''' > + > + handle, name = tempfile.mkstemp(suffix='.suppressions', > prefix='aa-parser-valgrind') > + os.close(handle) > + handle = file(name,"w+") > + handle.write(VALGRIND_SUPPRESSIONS) > + handle.close() > + return name > + > +def main(): > + usage = "usage: %prog [options] [test_directory]" > + p = OptionParser(usage=usage) > + p.add_option('-p', '--parser', default=DEFAULT_PARSER, action="store", > type="string", dest='parser') > + p.add_option('-v', '--verbose', action="store_true", dest="verbose") > + p.add_option('-s', '--skip-suppressions', action="store_true", > dest="skip_suppressions") > + config, args = p.parse_args() > + > + verbosity = 1 > + if config.verbose: > + verbosity = 2 > + > + if len(args) == 1: > + config.testdir = args[0] > + else: > + config.testdir = DEFAULT_TESTDIR > + > + if not config.skip_suppressions: > + suppression_file = create_suppressions() > + VALGRIND_ARGS.append('--suppressions=%s' % (suppression_file)) > + > + for f in find_testcases(config.testdir): > + def stub_test(self, testname=f): > + self._runtest(testname, config) > + stub_test.__doc__ = "test %s" % (f) > + setattr(AAParserValgrindTests, 'test_%s' % (f), stub_test) > + test_suite = unittest.TestSuite() > + > test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AAParserValgrindTests)) > + rc = 0 > + try: > + rc = unittest.TextTestRunner(verbosity=verbosity).run(test_suite) > + except: > + rc = 1 > + finally: > + os.remove(suppression_file) > + > + return rc > + > +if __name__ == "__main__": > + main() > Index: b/parser/tst/Makefile > =================================================================== > --- a/parser/tst/Makefile > +++ b/parser/tst/Makefile > @@ -50,6 +50,9 @@ minimize: $(PARSER) > equality: $(PARSER) > LANG=C APPARMOR_PARSER="$(PARSER)" ./equality.sh > > +valgrind: $(PARSER) > + LANG=C ./valgrind_simple.py -p "$(PARSER)" -v simple_tests > + > $(PARSER): > make -C $(PARSER_DIR) $(PARSER_BIN) > > -- > Steve Beattie > <[email protected]> > http://NxNW.org/~steve/
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
