Bug 1259850 - Test for sixgill exception handling + analysis constructor/destructor matching, r=terrence
☠☠ backed out by 69518db96a4d ☠ ☠
authorSteve Fink <sfink@mozilla.com>
Thu, 28 Apr 2016 15:05:09 -0700
changeset 338762 788ac18818c9b91f3057c4c382bf17be264eb03e
parent 338761 19c13aa9b5ada4d1284a09d8284b3231e0790ebe
child 338763 95107c3ad9cf5a173a17aee4fcd8635defbf0eb4
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1259850
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1259850 - Test for sixgill exception handling + analysis constructor/destructor matching, r=terrence MozReview-Commit-ID: H5NZlyS2rx5
js/src/devtools/rootAnalysis/analyzeRoots.js
js/src/devtools/rootAnalysis/computeCallgraph.js
js/src/devtools/rootAnalysis/run-test.py
js/src/devtools/rootAnalysis/t/exceptions/source.cpp
js/src/devtools/rootAnalysis/t/exceptions/test.py
js/src/devtools/rootAnalysis/t/testlib.py
--- a/js/src/devtools/rootAnalysis/analyzeRoots.js
+++ b/js/src/devtools/rootAnalysis/analyzeRoots.js
@@ -24,17 +24,17 @@ var gcFunctionsFile = scriptArgs[0] || "
 var gcEdgesFile = scriptArgs[1] || "gcEdges.txt";
 var suppressedFunctionsFile = scriptArgs[2] || "suppressedFunctions.lst";
 var gcTypesFile = scriptArgs[3] || "gcTypes.txt";
 var typeInfoFile = scriptArgs[4] || "typeInfo.txt";
 var batch = (scriptArgs[5]|0) || 1;
 var numBatches = (scriptArgs[6]|0) || 1;
 var tmpfile = scriptArgs[7] || "tmp.txt";
 
-GCSuppressionTypes = loadTypeInfo(typeInfoFile)["Suppress GC"];
+GCSuppressionTypes = loadTypeInfo(typeInfoFile)["Suppress GC"] || [];
 
 var gcFunctions = {};
 var text = snarf("gcFunctions.lst").split("\n");
 assert(text.pop().length == 0);
 for (var line of text)
     gcFunctions[mangled(line)] = true;
 
 var suppressedFunctions = {};
--- a/js/src/devtools/rootAnalysis/computeCallgraph.js
+++ b/js/src/devtools/rootAnalysis/computeCallgraph.js
@@ -303,17 +303,17 @@ function processBody(functionName, body)
             } else {
                 printErr("invalid " + callee.kind + " callee");
                 debugger;
             }
         }
     }
 }
 
-GCSuppressionTypes = loadTypeInfo(typeInfo_filename)["Suppress GC"];
+GCSuppressionTypes = loadTypeInfo(typeInfo_filename)["Suppress GC"] || [];
 
 var xdb = xdbLibrary();
 xdb.open("src_comp.xdb");
 
 var minStream = xdb.min_data_stream();
 var maxStream = xdb.max_data_stream();
 
 for (var csuIndex = minStream; csuIndex <= maxStream; csuIndex++) {
--- a/js/src/devtools/rootAnalysis/run-test.py
+++ b/js/src/devtools/rootAnalysis/run-test.py
@@ -35,17 +35,17 @@ parser.add_argument(
     help='Path to gcc')
 parser.add_argument(
     '--cxx', default=os.environ.get('CXX'),
     help='Path to g++')
 parser.add_argument(
     '--verbose', '-v', action='store_true',
     help='Display verbose output, including commands executed')
 parser.add_argument(
-    'tests', nargs='*', default=['sixgill-tree', 'suppression', 'hazards'],
+    'tests', nargs='*', default=['sixgill-tree', 'suppression', 'hazards', 'exceptions'],
     help='tests to run')
 
 cfg = parser.parse_args()
 
 if not cfg.js:
     exit('Must specify JS binary through environment variable or --js option')
 if not cfg.cc:
     if cfg.gccdir:
copy from js/src/devtools/rootAnalysis/t/suppression/source.cpp
copy to js/src/devtools/rootAnalysis/t/exceptions/source.cpp
--- a/js/src/devtools/rootAnalysis/t/suppression/source.cpp
+++ b/js/src/devtools/rootAnalysis/t/exceptions/source.cpp
@@ -1,64 +1,42 @@
 #define ANNOTATE(property) __attribute__((tag(property)))
 
 struct Cell { int f; } ANNOTATE("GC Thing");
 
-class AutoSuppressGC_Base {
-  public:
-    AutoSuppressGC_Base() {}
-    ~AutoSuppressGC_Base() {}
-} ANNOTATE("Suppress GC");
-
-class AutoSuppressGC_Child : public AutoSuppressGC_Base {
-  public:
-    AutoSuppressGC_Child() : AutoSuppressGC_Base() {}
-};
-
-class AutoSuppressGC {
-    AutoSuppressGC_Child helpImBeingSuppressed;
-
-  public:
-    AutoSuppressGC() {}
-};
-
 extern void GC() ANNOTATE("GC Call");
 
 void GC()
 {
     // If the implementation is too trivial, the function body won't be emitted at all.
     asm("");
 }
 
-extern void foo(Cell*);
-
-void suppressedFunction() {
-    GC(); // Calls GC, but is always called within AutoSuppressGC
-}
+class RAII_GC {
+  public:
+    RAII_GC() {}
+    ~RAII_GC() { GC(); }
+};
 
-void halfSuppressedFunction() {
-    GC(); // Calls GC, but is sometimes called within AutoSuppressGC
-}
+// ~AutoSomething calls GC because of the RAII_GC field. The constructor,
+// though, should *not* GC -- unless it throws an exception. Which is not
+// possible when compiled with -fno-exceptions.
+class AutoSomething {
+    RAII_GC gc;
+  public:
+    AutoSomething() : gc() {
+        asm(""); // Ooh, scary, this might throw an exception
+    }
+    ~AutoSomething() {
+        asm("");
+    }
+};
 
-void unsuppressedFunction() {
-    GC(); // Calls GC, never within AutoSuppressGC
-}
+extern void usevar(Cell* cell);
 
 void f() {
-    Cell* cell1 = nullptr;
-    Cell* cell2 = nullptr;
-    Cell* cell3 = nullptr;
+    Cell* thing = nullptr; // Live range starts here
+
     {
-        AutoSuppressGC nogc;
-        suppressedFunction();
-        halfSuppressedFunction();
-    }
-    foo(cell1);
-    halfSuppressedFunction();
-    foo(cell2);
-    unsuppressedFunction();
-    {
-        // Old bug: it would look from the first AutoSuppressGC constructor it
-        // found to the last destructor. This statement *should* have no effect.
-        AutoSuppressGC nogc;
-    }
-    foo(cell3);
+        AutoSomething smth; // Constructor can GC only if exceptions are enabled
+        usevar(thing); // Live range ends here
+    } // In particular, 'thing' is dead at the destructor, so no hazard
 }
copy from js/src/devtools/rootAnalysis/t/suppression/test.py
copy to js/src/devtools/rootAnalysis/t/exceptions/test.py
--- a/js/src/devtools/rootAnalysis/t/suppression/test.py
+++ b/js/src/devtools/rootAnalysis/t/exceptions/test.py
@@ -1,23 +1,19 @@
-test.compile("source.cpp")
-test.run_analysis_script('gcTypes', upto='gcFunctions')
+test.compile("source.cpp", '-fno-exceptions')
+test.run_analysis_script('gcTypes')
 
-# The suppressions file uses only mangled names since it's for internal use,
-# though I may change that soon given (1) the unfortunate non-uniqueness of
-# mangled constructor names, and (2) the usefulness of this file for
-# mrgiggles's reporting.
-suppressed = test.load_suppressed_functions()
+hazards = test.load_hazards()
+assert(len(hazards) == 0)
+
+# If we compile with exceptions, then there *should* be a hazard because
+# AutoSomething::AutoSomething might throw an exception, which would cause the
+# partially-constructed value to be torn down, which will call ~RAII_GC.
 
-# Only one of these is fully suppressed (ie, *always* called within the scope
-# of an AutoSuppressGC).
-assert(len(filter(lambda f: 'suppressedFunction' in f, suppressed)) == 1)
-assert(len(filter(lambda f: 'halfSuppressedFunction' in f, suppressed)) == 0)
-assert(len(filter(lambda f: 'unsuppressedFunction' in f, suppressed)) == 0)
+test.compile("source.cpp", '-fexceptions')
+test.run_analysis_script('gcTypes')
 
-# gcFunctions should be the inverse, but we get to rely on unmangled names here.
-gcFunctions = test.load_gcFunctions()
-print(gcFunctions)
-assert('void GC()' in gcFunctions)
-assert('void suppressedFunction()' not in gcFunctions)
-assert('void halfSuppressedFunction()' in gcFunctions)
-assert('void unsuppressedFunction()' in gcFunctions)
-assert('void f()' in gcFunctions)
+hazards = test.load_hazards()
+assert(len(hazards) == 1)
+hazard = hazards[0]
+assert(hazard.function == 'void f()')
+assert(hazard.variable == 'thing')
+assert("AutoSomething::AutoSomething" in hazard.GCFunction)
--- a/js/src/devtools/rootAnalysis/t/testlib.py
+++ b/js/src/devtools/rootAnalysis/t/testlib.py
@@ -26,20 +26,21 @@ class Test(object):
         self.cfg = cfg
 
     def infile(self, path):
         return os.path.join(self.indir, path)
 
     def binpath(self, prog):
         return os.path.join(self.cfg.sixgill_bin, prog)
 
-    def compile(self, source):
-        cmd = "{CXX} -c {source} -O3 -std=c++11 -fplugin={sixgill} -fplugin-arg-xgill-mangle=1".format(
+    def compile(self, source, options = ''):
+        cmd = "{CXX} -c {source} -O3 -std=c++11 -fplugin={sixgill} -fplugin-arg-xgill-mangle=1 {options}".format(
             source=self.infile(source),
-            CXX=self.cfg.cxx, sixgill=self.cfg.sixgill_plugin)
+            CXX=self.cfg.cxx, sixgill=self.cfg.sixgill_plugin,
+            options=options)
         if self.cfg.verbose:
             print("Running %s" % cmd)
         subprocess.check_call(["sh", "-c", cmd])
 
     def load_db_entry(self, dbname, pattern):
         '''Look up an entry from an XDB database file, 'pattern' may be an exact
         matching string, or an re pattern object matching a single entry.'''
 
@@ -70,17 +71,17 @@ sixgill_bin = '{bindir}'
             cmd.append("--verbose")
             print("Running " + " ".join(cmd))
         subprocess.check_call(cmd)
 
     def computeGCTypes(self):
         self.run_analysis_script("gcTypes", upto="gcTypes")
 
     def computeHazards(self):
-        self.run_analysis_script("callgraph")
+        self.run_analysis_script("gcTypes")
 
     def load_text_file(self, filename, extract=lambda l: l):
         fullpath = os.path.join(self.outdir, filename)
         values = (extract(line.strip()) for line in file(fullpath))
         return filter(lambda _: _ is not None, values)
 
     def load_suppressed_functions(self):
         return set(self.load_text_file("suppressedFunctions.lst"))