Ned Batchelder avatar Ned Batchelder committed faac189

Detect when our trace function is yanked out from under us, and warn the user. Finishes, but does not fix, issue #93.

Comments (0)

Files changed (5)

 - A little bit of Jython support: `coverage run` can now measure Jython
   execution by adapting when $py.class files are traced. Thanks, Adi Roiban.
 
+- Pathological code execution could disable the trace function behind our
+  backs, leading to incorrect code measurement.  Now if this happens,
+  coverage.py will issue a warning, at least alerting you to the problem.
+  Closes `issue 93`_.  Thanks to Marius Gedminas for the idea.
+
+.. _issue 93: http://bitbucket.org/ned/coveragepy/issue/93/copying-a-mock-object-breaks-coverage
+
 
 Version 3.4 --- 19 September 2010
 ---------------------------------

coverage/collector.py

         self.data = None
         self.should_trace = None
         self.should_trace_cache = None
+        self.warn = None
         self.cur_file_data = None
         self.last_line = 0
         self.data_stack = []
 
     def stop(self):
         """Stop this Tracer."""
+        if hasattr(sys, "gettrace") and self.warn:
+            if sys.gettrace() != self._trace:
+                msg = "Trace function changed, measurement is likely wrong: %r"
+                self.warn(msg % sys.gettrace())
         sys.settrace(None)
 
     def get_stats(self):
     # the top, and resumed when they become the top again.
     _collectors = []
 
-    def __init__(self, should_trace, timid, branch):
+    def __init__(self, should_trace, timid, branch, warn):
         """Create a collector.
 
         `should_trace` is a function, taking a filename, and returning a
         collecting data on which statements followed each other (arcs).  Use
         `get_arc_data` to get the arc data.
 
+        `warn` is a warning function, taking a single string message argument,
+        to be used if a warning needs to be issued.
+
         """
         self.should_trace = should_trace
+        self.warn = warn
         self.branch = branch
         self.reset()
 
         tracer.arcs = self.branch
         tracer.should_trace = self.should_trace
         tracer.should_trace_cache = self.should_trace_cache
+        tracer.warn = self.warn
         fn = tracer.start()
         self.tracers.append(tracer)
         return fn

coverage/control.py

         """
         from coverage import __version__
 
+        # A record of all the warnings that have been issued.
+        self._warnings = []
+
         # Build our configuration from a number of sources:
         # 1: defaults:
         self.config = CoverageConfig()
 
         self.collector = Collector(
             self._should_trace, timid=self.config.timid,
-            branch=self.config.branch
+            branch=self.config.branch, warn=self._warn
             )
 
         # Suffixes are a bit tricky.  We want to use the data suffix only when
 
     def _warn(self, msg):
         """Use `msg` as a warning."""
-        sys.stderr.write("Coverage.py warning: " + msg + "\n")
+        self._warnings.append(msg)
+        sys.stderr.write("Coverage.py warning: %s\n" % msg)
 
     def _abs_files(self, files):
         """Return a list of absolute file names for the names in `files`."""

coverage/tracer.c

 
     /* Python objects manipulated directly by the Collector class. */
     PyObject * should_trace;
+    PyObject * warn;
     PyObject * data;
     PyObject * should_trace_cache;
     PyObject * arcs;
 #endif /* COLLECT_STATS */
 
     self->should_trace = NULL;
+    self->warn = NULL;
     self->data = NULL;
     self->should_trace_cache = NULL;
     self->arcs = NULL;
     }
 
     Py_XDECREF(self->should_trace);
+    Py_XDECREF(self->warn);
     Py_XDECREF(self->data);
     Py_XDECREF(self->should_trace_cache);
 
     { "should_trace",       T_OBJECT, offsetof(Tracer, should_trace), 0,
             PyDoc_STR("Function indicating whether to trace a file.") },
 
+    { "warn",               T_OBJECT, offsetof(Tracer, warn), 0,
+            PyDoc_STR("Function for issuing warnings.") },
+
     { "data",               T_OBJECT, offsetof(Tracer, data), 0,
             PyDoc_STR("The raw dictionary of trace data.") },
 

test/test_oddball.py

                     return recur(n-1)+1
 
             recur(495)  # We can get at least this many stack frames.
+            i = 8       # and this line will be traced
             """,
-            [1,2,3,5,7], "")
+            [1,2,3,5,7,8], "")
 
     def test_long_recursion(self):
         # We can't finish a very deep recursion, but we don't crash.
             """,
             [1,2,3,5,7], "")
 
+    def test_long_recursion_recovery(self):
+        # Test the core of bug 93: http://bitbucket.org/ned/coveragepy/issue/93
+        # When recovering from a stack overflow, the Python trace function is
+        # disabled, but the C trace function is not.  So if we're using a
+        # Python trace function, we won't trace anything after the stack
+        # overflow, and there should be a warning about it.  If we're using
+        # the C trace function, only line 3 will be missing, and all else
+        # will be traced.
+
+        self.make_file("recur.py", """\
+            def recur(n):
+                if n == 0:
+                    return 0    # never hit
+                else:
+                    return recur(n-1)+1
+
+            try:
+                recur(100000)  # This is definitely too many frames.
+            except RuntimeError:
+                i = 10
+            i = 11
+            """)
+
+        cov = coverage.coverage()
+        cov.start()
+        self.import_local_file("recur")
+        cov.stop()
+
+        pytrace = (cov.collector.tracer_name() == "PyTracer")
+        expected_missing = [3]
+        if pytrace:
+            expected_missing += [9,10,11]
+
+        _, statements, missing, _ = cov.analysis("recur.py")
+        self.assertEqual(statements, [1,2,3,5,7,8,9,10,11])
+        self.assertEqual(missing, expected_missing)
+
+        # We can get a warning about the stackoverflow effect on the tracing
+        # function only if we have sys.gettrace
+        if pytrace and hasattr(sys, "gettrace"):
+            self.assertEqual(cov._warnings,
+                ["Trace function changed, measurement is likely wrong: None"]
+                )
+        else:
+            self.assertEqual(cov._warnings, [])
+
 
 class MemoryLeakTest(CoverageTest):
     """Attempt the impossible: test that memory doesn't leak."""
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.