From a95d709e9ec6e01b1844ed40dd627213160800b3 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Thu, 19 Jul 2018 11:57:54 +0200 Subject: [PATCH 01/37] Bug 1476574 - Support resetting/dumping code coverage counters before/after web-platform-tests. r=jgraham --HG-- extra : rebase_source : dc3375090f952bd58e61c07e927aa00b1c337f50 --- testing/marionette/jar.mn | 1 + .../wptrunner/executors/executormarionette.py | 66 ++++++++++++++++++- .../wptrunner/wptrunner/executors/protocol.py | 17 +++++ 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/testing/marionette/jar.mn b/testing/marionette/jar.mn index f23195e2bb67..214612c9e3e7 100644 --- a/testing/marionette/jar.mn +++ b/testing/marionette/jar.mn @@ -47,3 +47,4 @@ marionette.jar: content/test_nested_iframe.xul (chrome/test_nested_iframe.xul) content/test.xul (chrome/test.xul) #endif + content/PerTestCoverageUtils.jsm (../../tools/code-coverage/PerTestCoverageUtils.jsm) diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py index 54802faf83b1..6f1d35a749b7 100644 --- a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py +++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py @@ -29,7 +29,8 @@ from .protocol import (AssertsProtocolPart, SelectorProtocolPart, ClickProtocolPart, SendKeysProtocolPart, - TestDriverProtocolPart) + TestDriverProtocolPart, + CoverageProtocolPart) from ..testrunner import Stop from ..webdriver_server import GeckoDriverServer @@ -371,6 +372,53 @@ class MarionetteTestDriverProtocolPart(TestDriverProtocolPart): self.parent.base.execute_script("window.postMessage(%s, '*')" % json.dumps(obj)) +class MarionetteCoverageProtocolPart(CoverageProtocolPart): + def setup(self): + self.marionette = self.parent.marionette + script = """ + ChromeUtils.import("chrome://marionette/content/PerTestCoverageUtils.jsm"); + return PerTestCoverageUtils.enabled; + """ + with self.marionette.using_context(self.marionette.CONTEXT_CHROME): + self.is_enabled = self.marionette.execute_script(script) + + def reset(self): + script = """ + var callback = arguments[arguments.length - 1]; + + ChromeUtils.import("chrome://marionette/content/PerTestCoverageUtils.jsm"); + PerTestCoverageUtils.beforeTest().then(callback, callback); + """ + with self.marionette.using_context(self.marionette.CONTEXT_CHROME): + try: + error = self.marionette.execute_async_script(script) + if error is not None: + raise Exception('Failure while resetting counters: %s' % json.dumps(error)) + except (errors.MarionetteException, socket.error): + # This usually happens if the process crashed + pass + + def dump(self): + if len(self.marionette.window_handles): + handle = self.marionette.window_handles[0] + self.marionette.switch_to_window(handle) + + script = """ + var callback = arguments[arguments.length - 1]; + + ChromeUtils.import("chrome://marionette/content/PerTestCoverageUtils.jsm"); + PerTestCoverageUtils.afterTest().then(callback, callback); + """ + with self.marionette.using_context(self.marionette.CONTEXT_CHROME): + try: + error = self.marionette.execute_async_script(script) + if error is not None: + raise Exception('Failure while dumping counters: %s' % json.dumps(error)) + except (errors.MarionetteException, socket.error): + # This usually happens if the process crashed + pass + + class MarionetteProtocol(Protocol): implements = [MarionetteBaseProtocolPart, MarionetteTestharnessProtocolPart, @@ -380,7 +428,8 @@ class MarionetteProtocol(Protocol): MarionetteClickProtocolPart, MarionetteSendKeysProtocolPart, MarionetteTestDriverProtocolPart, - MarionetteAssertsProtocolPart] + MarionetteAssertsProtocolPart, + MarionetteCoverageProtocolPart] def __init__(self, executor, browser, capabilities=None, timeout_multiplier=1, e10s=True): do_delayed_imports() @@ -599,6 +648,9 @@ class MarionetteTestharnessExecutor(TestharnessExecutor): else: timeout_ms = "null" + if self.protocol.coverage.is_enabled: + self.protocol.coverage.reset() + format_map = {"abs_url": url, "url": strip_server(url), "window_id": self.window_id, @@ -621,6 +673,10 @@ class MarionetteTestharnessExecutor(TestharnessExecutor): done, rv = handler(result) if done: break + + if self.protocol.coverage.is_enabled: + self.protocol.coverage.dump() + return rv @@ -689,8 +745,14 @@ class MarionetteRefTestExecutor(RefTestExecutor): self.protocol.base.set_window(self.protocol.marionette.window_handles[-1]) self.has_window = True + if self.protocol.coverage.is_enabled: + self.protocol.coverage.reset() + result = self.implementation.run_test(test) + if self.protocol.coverage.is_enabled: + self.protocol.coverage.dump() + if self.debug: assertion_count = self.protocol.asserts.get() if "extra" not in result: diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py index 2e48162e0fd3..71fc3c9a8f65 100644 --- a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py +++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py @@ -303,3 +303,20 @@ class AssertsProtocolPart(ProtocolPart): def get(self): """Get a count of assertions since the last browser start""" pass + + +class CoverageProtocolPart(ProtocolPart): + """Protocol part for collecting per-test coverage data.""" + __metaclass__ = ABCMeta + + name = "coverage" + + @abstractmethod + def reset(self): + """Reset coverage counters""" + pass + + @abstractmethod + def dump(self): + """Dump coverage counters""" + pass From 57da0df50577196a15b5bb590659e196245e22b3 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Thu, 19 Jul 2018 11:56:43 +0200 Subject: [PATCH 02/37] Bug 1476574 - Enable reset/dump for wpt and cleanup harness code for supporting reset/dump now that it is supported by all test suites. r=jmaher --HG-- extra : rebase_source : 554f640652e8a44e53d37ca8b4a139845efd75ec extra : source : c1b3950cc12f1642ad60338d2a8701e2b60131ea --- .../mozilla/testing/codecoverage.py | 31 +++++++++++++------ .../mozharness/scripts/desktop_unittest.py | 21 +++---------- .../mozharness/scripts/web_platform_tests.py | 8 +++-- 3 files changed, 31 insertions(+), 29 deletions(-) diff --git a/testing/mozharness/mozharness/mozilla/testing/codecoverage.py b/testing/mozharness/mozharness/mozilla/testing/codecoverage.py index 129d87b60f41..7390dd7102fa 100644 --- a/testing/mozharness/mozharness/mozilla/testing/codecoverage.py +++ b/testing/mozharness/mozharness/mozilla/testing/codecoverage.py @@ -204,16 +204,20 @@ class CodeCoverageMixin(SingleTestMixin): def coverage_args(self): return [] - def set_coverage_env(self, env): + def set_coverage_env(self, env, is_baseline_test=False): # Set the GCOV directory. - gcov_dir = tempfile.mkdtemp() - env['GCOV_PREFIX'] = gcov_dir + self.gcov_dir = tempfile.mkdtemp() + env['GCOV_PREFIX'] = self.gcov_dir + + # Set the GCOV directory where counters will be dumped in per-test mode. + # Resetting/dumping is only available on Linux for the time being + # (https://bugzilla.mozilla.org/show_bug.cgi?id=1471576). + if self.per_test_coverage and not is_baseline_test and self._is_linux(): + env['GCOV_RESULTS_DIR'] = tempfile.mkdtemp() # Set JSVM directory. - jsvm_dir = tempfile.mkdtemp() - env['JS_CODE_COVERAGE_OUTPUT_DIR'] = jsvm_dir - - return (gcov_dir, jsvm_dir) + self.jsvm_dir = tempfile.mkdtemp() + env['JS_CODE_COVERAGE_OUTPUT_DIR'] = self.jsvm_dir @PreScriptAction('run-tests') def _set_gcov_prefix(self, action): @@ -223,7 +227,7 @@ class CodeCoverageMixin(SingleTestMixin): if self.per_test_coverage: return - self.gcov_dir, self.jsvm_dir = self.set_coverage_env(os.environ) + self.set_coverage_env(os.environ) def parse_coverage_artifacts(self, gcov_dir, @@ -286,9 +290,11 @@ class CodeCoverageMixin(SingleTestMixin): else: return grcov_output_file, jsvm_output_file - def add_per_test_coverage_report(self, gcov_dir, jsvm_dir, suite, test): + def add_per_test_coverage_report(self, env, suite, test): + gcov_dir = env['GCOV_RESULTS_DIR'] if 'GCOV_RESULTS_DIR' in env else self.gcov_dir + grcov_file = self.parse_coverage_artifacts( - gcov_dir, jsvm_dir, merge=True, output_format='coveralls', + gcov_dir, self.jsvm_dir, merge=True, output_format='coveralls', filter_covered=True, ) @@ -300,6 +306,11 @@ class CodeCoverageMixin(SingleTestMixin): assert test not in self.per_test_reports[suite] self.per_test_reports[suite][test] = report_file + if 'GCOV_RESULTS_DIR' in env: + # In this case, parse_coverage_artifacts has removed GCOV_RESULTS_DIR + # so we need to remove GCOV_PREFIX. + shutil.rmtree(self.gcov_dir) + def is_covered(self, sf): # For C/C++ source files, we can consider a file as being uncovered # when all its source lines are uncovered. diff --git a/testing/mozharness/scripts/desktop_unittest.py b/testing/mozharness/scripts/desktop_unittest.py index e18937a7a6c1..c699c1731655 100755 --- a/testing/mozharness/scripts/desktop_unittest.py +++ b/testing/mozharness/scripts/desktop_unittest.py @@ -15,7 +15,6 @@ import re import sys import copy import shutil -import tempfile import glob import imp @@ -889,28 +888,18 @@ class DesktopUnittest(TestingMixin, MercurialScript, MozbaseMixin, final_cmd = copy.copy(cmd) final_cmd.extend(per_test_args) + final_env = copy.copy(env) + if self.per_test_coverage: - gcov_dir, jsvm_dir = self.set_coverage_env(env) - # Per-test reset/dump is only supported on Linux for the time being. - if not is_baseline_test and \ - self._is_linux(): - env['GCOV_RESULTS_DIR'] = tempfile.mkdtemp() + self.set_coverage_env(final_env) return_code = self.run_command(final_cmd, cwd=dirs['abs_work_dir'], output_timeout=cmd_timeout, output_parser=parser, - env=env) + env=final_env) if self.per_test_coverage: - self.add_per_test_coverage_report( - env['GCOV_RESULTS_DIR'] if 'GCOV_RESULTS_DIR' in env else gcov_dir, - jsvm_dir, - suite, - per_test_args[-1] - ) - if 'GCOV_RESULTS_DIR' in env: - shutil.rmtree(gcov_dir) - del env['GCOV_RESULTS_DIR'] + self.add_per_test_coverage_report(final_env, suite, per_test_args[-1]) # mochitest, reftest, and xpcshell suites do not return # appropriate return codes. Therefore, we must parse the output diff --git a/testing/mozharness/scripts/web_platform_tests.py b/testing/mozharness/scripts/web_platform_tests.py index b0610a1e5c83..e5f5ccc3eee3 100755 --- a/testing/mozharness/scripts/web_platform_tests.py +++ b/testing/mozharness/scripts/web_platform_tests.py @@ -377,17 +377,19 @@ class WebPlatformTest(TestingMixin, MercurialScript, CodeCoverageMixin): cmd = self._query_cmd(test_types) cmd.extend(per_test_args) + final_env = copy.copy(env) + if self.per_test_coverage: - gcov_dir, jsvm_dir = self.set_coverage_env(env) + self.set_coverage_env(final_env, is_baseline_test) return_code = self.run_command(cmd, cwd=dirs['abs_work_dir'], output_timeout=1000, output_parser=parser, - env=env) + env=final_env) if self.per_test_coverage: - self.add_per_test_coverage_report(gcov_dir, jsvm_dir, suite, per_test_args[-1]) + self.add_per_test_coverage_report(final_env, suite, per_test_args[-1]) tbpl_status, log_level, summary = parser.evaluate_parser(return_code, previous_summary=summary) From 28cfdf45e088d2cb13a124fa7c0420f0c3ee4561 Mon Sep 17 00:00:00 2001 From: Sebastian Hengst Date: Thu, 19 Jul 2018 23:03:38 +0300 Subject: [PATCH 03/37] Bug 1354232 - Annotate html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html as passing on Linux asan. r=RyanVM on IRC --- .../resolving-urls/query-encoding/navigation.sub.html.ini | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/testing/web-platform/meta/html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html.ini b/testing/web-platform/meta/html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html.ini index ca6172fda954..326a54db62ac 100644 --- a/testing/web-platform/meta/html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html.ini +++ b/testing/web-platform/meta/html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html.ini @@ -1,10 +1,9 @@ [navigation.sub.html?encoding=x-cp1251] expected: if not debug and not webrender and e10s and (os == "linux") and (version == "Ubuntu 16.04") and (processor == "x86") and (bits == 32): TIMEOUT - if not debug and not webrender and e10s and (os == "linux") and (version == "Ubuntu 16.04") and (processor == "x86_64") and (bits == 64): TIMEOUT + if not debug and not asan and e10s and (os == "linux") and (version == "Ubuntu 16.04") and (processor == "x86_64") and (bits == 64): TIMEOUT if not debug and not webrender and e10s and (os == "win") and (version == "6.1.7601") and (processor == "x86") and (bits == 32): TIMEOUT if not debug and not webrender and e10s and (os == "win") and (version == "10.0.15063") and (processor == "x86_64") and (bits == 64): TIMEOUT - if not debug and webrender and e10s and (os == "linux") and (version == "Ubuntu 16.04") and (processor == "x86_64") and (bits == 64): TIMEOUT if not debug and not webrender and e10s and (os == "mac") and (version == "OS X 10.10.5") and (processor == "x86_64") and (bits == 64): TIMEOUT [follow hyperlink ] expected: FAIL @@ -25,10 +24,9 @@ [navigation.sub.html?encoding=utf8] expected: if not debug and not webrender and e10s and (os == "linux") and (version == "Ubuntu 16.04") and (processor == "x86") and (bits == 32): TIMEOUT - if not debug and not webrender and e10s and (os == "linux") and (version == "Ubuntu 16.04") and (processor == "x86_64") and (bits == 64): TIMEOUT + if not debug and not asan and e10s and (os == "linux") and (version == "Ubuntu 16.04") and (processor == "x86_64") and (bits == 64): TIMEOUT if not debug and not webrender and e10s and (os == "win") and (version == "6.1.7601") and (processor == "x86") and (bits == 32): TIMEOUT if not debug and not webrender and e10s and (os == "win") and (version == "10.0.15063") and (processor == "x86_64") and (bits == 64): TIMEOUT - if not debug and webrender and e10s and (os == "linux") and (version == "Ubuntu 16.04") and (processor == "x86_64") and (bits == 64): TIMEOUT if not debug and not webrender and e10s and (os == "mac") and (version == "OS X 10.10.5") and (processor == "x86_64") and (bits == 64): TIMEOUT [hyperlink auditing ] expected: TIMEOUT From e63e92aa342b4650d0ae61ca59fe931c0e5bf99d Mon Sep 17 00:00:00 2001 From: Eric Rahm Date: Tue, 20 Mar 2018 17:52:16 -0700 Subject: [PATCH 04/37] Bug 1442765 - Part 1: Add intptr_t hashkey type. r=froydnj This adds a hashkey that operates on a uintptr_t. --HG-- extra : rebase_source : e10b00f7735bf63cb578d18acd03c3f3e919e304 --- xpcom/ds/nsHashKeys.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/xpcom/ds/nsHashKeys.h b/xpcom/ds/nsHashKeys.h index ee4fea5af759..e8a996e2166b 100644 --- a/xpcom/ds/nsHashKeys.h +++ b/xpcom/ds/nsHashKeys.h @@ -20,6 +20,7 @@ #include "nsUnicharUtils.h" #include "nsPointerHashKeys.h" +#include #include #include @@ -54,6 +55,7 @@ HashString(const nsACString& aStr) * nsUint32HashKey * nsUint64HashKey * nsFloatHashKey + * IntPtrHashKey * nsPtrHashKey * nsClearingPtrHashKey * nsVoidPtrHashKey @@ -284,6 +286,35 @@ private: const float mValue; }; +/** + * hashkey wrapper using intptr_t KeyType + * + * @see nsTHashtable::EntryType for specification + */ +class IntPtrHashKey : public PLDHashEntryHdr +{ +public: + typedef const intptr_t& KeyType; + typedef const intptr_t* KeyTypePointer; + + explicit IntPtrHashKey(KeyTypePointer aKey) : mValue(*aKey) {} + IntPtrHashKey(const IntPtrHashKey& aToCopy) : mValue(aToCopy.mValue) {} + ~IntPtrHashKey() {} + + KeyType GetKey() const { return mValue; } + bool KeyEquals(KeyTypePointer aKey) const { return *aKey == mValue; } + + static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; } + static PLDHashNumber HashKey(KeyTypePointer aKey) + { + return mozilla::HashGeneric(*aKey); + } + enum { ALLOW_MEMMOVE = true }; + +private: + const intptr_t mValue; +}; + /** * hashkey wrapper using nsISupports* KeyType * From cf21a1de96804cb74e6c9bd904b6065b03bb970b Mon Sep 17 00:00:00 2001 From: Eric Rahm Date: Mon, 5 Mar 2018 16:50:00 -0800 Subject: [PATCH 05/37] Bug 1442765 - Part 2: Switch nsTraceRefcnt's hashtables to use xpcom hashtables. r=mccr8 --HG-- extra : rebase_source : 5fecdd86a3ef27d211cb43d4c602162db7554f2f --- xpcom/base/nsTraceRefcnt.cpp | 364 ++++++++++------------------------- 1 file changed, 99 insertions(+), 265 deletions(-) diff --git a/xpcom/base/nsTraceRefcnt.cpp b/xpcom/base/nsTraceRefcnt.cpp index 596c29e06a01..7feb306cb3d7 100644 --- a/xpcom/base/nsTraceRefcnt.cpp +++ b/xpcom/base/nsTraceRefcnt.cpp @@ -11,7 +11,9 @@ #include "mozilla/StaticPtr.h" #include "nsXPCOMPrivate.h" #include "nscore.h" +#include "nsClassHashtable.h" #include "nsISupports.h" +#include "nsHashKeys.h" #include "nsTArray.h" #include "nsTHashtable.h" #include "prenv.h" @@ -52,8 +54,6 @@ //////////////////////////////////////////////////////////////////////////////// -#include "plhash.h" - #include "prthread.h" // We use a spin lock instead of a regular mutex because this lock is usually @@ -80,10 +80,19 @@ struct MOZ_STACK_CLASS AutoTraceLogLock final ~AutoTraceLogLock() { if (doRelease) gTraceLogLocked = 0; } }; -static PLHashTable* gBloatView; -static PLHashTable* gTypesToLog; -static PLHashTable* gObjectsToLog; -static PLHashTable* gSerialNumbers; +class BloatEntry; +struct SerialNumberRecord; + +using BloatHash = nsClassHashtable; +using CharPtrSet = nsTHashtable; +using IntPtrSet = nsTHashtable; +using SerialHash = nsClassHashtable; + +static StaticAutoPtr gBloatView; +static StaticAutoPtr gTypesToLog; +static StaticAutoPtr gObjectsToLog; +static StaticAutoPtr gSerialNumbers; + static intptr_t gNextSerialNumber; static bool gDumpedStatistics = false; static bool gLogJSStacks = false; @@ -205,56 +214,6 @@ AssertActivityIsLegal() # define ASSERT_ACTIVITY_IS_LEGAL do { } while(0) #endif // DEBUG -// These functions are copied from nsprpub/lib/ds/plhash.c, with changes -// to the functions not called Default* to free the SerialNumberRecord or -// the BloatEntry. - -static void* -DefaultAllocTable(void* aPool, size_t aSize) -{ - return malloc(aSize); -} - -static void -DefaultFreeTable(void* aPool, void* aItem) -{ - free(aItem); -} - -static PLHashEntry* -DefaultAllocEntry(void* aPool, const void* aKey) -{ - return (PLHashEntry*) malloc(sizeof(PLHashEntry)); -} - -static void -SerialNumberFreeEntry(void* aPool, PLHashEntry* aHashEntry, unsigned aFlag) -{ - if (aFlag == HT_FREE_ENTRY) { - delete static_cast(aHashEntry->value); - free(aHashEntry); - } -} - -static void -TypesToLogFreeEntry(void* aPool, PLHashEntry* aHashEntry, unsigned aFlag) -{ - if (aFlag == HT_FREE_ENTRY) { - free(const_cast(static_cast(aHashEntry->key))); - free(aHashEntry); - } -} - -static const PLHashAllocOps serialNumberHashAllocOps = { - DefaultAllocTable, DefaultFreeTable, - DefaultAllocEntry, SerialNumberFreeEntry -}; - -static const PLHashAllocOps typesToLogHashAllocOps = { - DefaultAllocTable, DefaultFreeTable, - DefaultAllocEntry, TypesToLogFreeEntry -}; - //////////////////////////////////////////////////////////////////////////////// class CodeAddressServiceStringTable final @@ -338,24 +297,6 @@ public: mStats.mDestroys++; } - static int DumpEntry(PLHashEntry* aHashEntry, int aIndex, void* aArg) - { - BloatEntry* entry = (BloatEntry*)aHashEntry->value; - if (entry) { - static_cast*>(aArg)->AppendElement(entry); - } - return HT_ENUMERATE_NEXT; - } - - static int TotalEntries(PLHashEntry* aHashEntry, int aIndex, void* aArg) - { - BloatEntry* entry = (BloatEntry*)aHashEntry->value; - if (entry && nsCRT::strcmp(entry->mClassName, "TOTAL") != 0) { - entry->Total((BloatEntry*)aArg); - } - return HT_ENUMERATE_NEXT; - } - void Total(BloatEntry* aTotal) { aTotal->mStats.mCreates += mStats.mCreates; @@ -411,29 +352,10 @@ protected: nsTraceRefcntStats mStats; }; -static void -BloatViewFreeEntry(void* aPool, PLHashEntry* aHashEntry, unsigned aFlag) -{ - if (aFlag == HT_FREE_ENTRY) { - BloatEntry* entry = static_cast(aHashEntry->value); - delete entry; - free(aHashEntry); - } -} - -const static PLHashAllocOps bloatViewHashAllocOps = { - DefaultAllocTable, DefaultFreeTable, - DefaultAllocEntry, BloatViewFreeEntry -}; - static void RecreateBloatView() { - gBloatView = PL_NewHashTable(256, - PL_HashString, - PL_CompareStrings, - PL_CompareValues, - &bloatViewHashAllocOps, nullptr); + gBloatView = new BloatHash(256); } static BloatEntry* @@ -442,48 +364,39 @@ GetBloatEntry(const char* aTypeName, uint32_t aInstanceSize) if (!gBloatView) { RecreateBloatView(); } - BloatEntry* entry = nullptr; - if (gBloatView) { - entry = (BloatEntry*)PL_HashTableLookup(gBloatView, aTypeName); - if (!entry && aInstanceSize > 0) { - - entry = new BloatEntry(aTypeName, aInstanceSize); - PLHashEntry* e = PL_HashTableAdd(gBloatView, aTypeName, entry); - if (!e) { - delete entry; - entry = nullptr; - } - } else { - MOZ_ASSERT(aInstanceSize == 0 || entry->GetClassSize() == aInstanceSize, - "Mismatched sizes were recorded in the memory leak logging table. " - "The usual cause of this is having a templated class that uses " - "MOZ_COUNT_{C,D}TOR in the constructor or destructor, respectively. " - "As a workaround, the MOZ_COUNT_{C,D}TOR calls can be moved to a " - "non-templated base class. Another possible cause is a runnable with " - "an mName that matches another refcounted class."); - } + BloatEntry* entry = gBloatView->Get(aTypeName); + if (!entry && aInstanceSize > 0) { + entry = new BloatEntry(aTypeName, aInstanceSize); + gBloatView->Put(aTypeName, entry); + } else { + MOZ_ASSERT(aInstanceSize == 0 || entry->GetClassSize() == aInstanceSize, + "Mismatched sizes were recorded in the memory leak logging table. " + "The usual cause of this is having a templated class that uses " + "MOZ_COUNT_{C,D}TOR in the constructor or destructor, respectively. " + "As a workaround, the MOZ_COUNT_{C,D}TOR calls can be moved to a " + "non-templated base class. Another possible cause is a runnable with " + "an mName that matches another refcounted class."); } return entry; } -static int -DumpSerialNumbers(PLHashEntry* aHashEntry, int aIndex, void* aClosure) +static void +DumpSerialNumbers(const SerialHash::Iterator& aHashEntry, FILE* aFd) { - SerialNumberRecord* record = - static_cast(aHashEntry->value); - auto* outputFile = static_cast(aClosure); + SerialNumberRecord* record = aHashEntry.Data(); + auto* outputFile = aFd; #ifdef HAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR fprintf(outputFile, "%" PRIdPTR " @%p (%d references; %d from COMPtrs)\n", record->serialNumber, - aHashEntry->key, + aHashEntry.Key(), record->refCount, record->COMPtrCount); #else fprintf(outputFile, "%" PRIdPTR " @%p (%d references)\n", record->serialNumber, - aHashEntry->key, + aHashEntry.Key(), record->refCount); #endif if (!record->allocationStack.empty()) { @@ -506,8 +419,6 @@ DumpSerialNumbers(PLHashEntry* aHashEntry, int aIndex, void* aClosure) fprintf(outputFile, "There is no JS context on the stack.\n"); } } - - return HT_ENUMERATE_NEXT; } @@ -545,7 +456,13 @@ nsTraceRefcnt::DumpStatistics() gLogging = NoLogging; BloatEntry total("TOTAL", 0); - PL_HashTableEnumerateEntries(gBloatView, BloatEntry::TotalEntries, &total); + for (auto iter = gBloatView->Iter(); !iter.Done(); iter.Next()) { + BloatEntry* entry = iter.Data(); + if (nsCRT::strcmp(entry->GetClassName(), "TOTAL") != 0) { + entry->Total(&total); + } + } + const char* msg; if (gLogLeaksOnly) { msg = "ALL (cumulative) LEAK STATISTICS"; @@ -555,7 +472,10 @@ nsTraceRefcnt::DumpStatistics() const bool leaked = total.PrintDumpHeader(gBloatLog, msg); nsTArray entries; - PL_HashTableEnumerateEntries(gBloatView, BloatEntry::DumpEntry, &entries); + for (auto iter = gBloatView->Iter(); !iter.Done(); iter.Next()) { + entries.AppendElement(iter.Data()); + } + const uint32_t count = entries.Length(); if (!gLogLeaksOnly || leaked) { @@ -574,7 +494,9 @@ nsTraceRefcnt::DumpStatistics() if (gSerialNumbers) { fprintf(gBloatLog, "\nSerial Numbers of Leaked Objects:\n"); - PL_HashTableEnumerateEntries(gSerialNumbers, DumpSerialNumbers, gBloatLog); + for (auto iter = gSerialNumbers->Iter(); !iter.Done(); iter.Next()) { + DumpSerialNumbers(iter, gBloatLog); + } } return NS_OK; @@ -584,87 +506,40 @@ void nsTraceRefcnt::ResetStatistics() { AutoTraceLogLock lock; - if (gBloatView) { - PL_HashTableDestroy(gBloatView); - gBloatView = nullptr; - } -} - -static bool -LogThisType(const char* aTypeName) -{ - void* he = PL_HashTableLookup(gTypesToLog, aTypeName); - return he != nullptr; -} - -static PLHashNumber -HashNumber(const void* aKey) -{ - return PLHashNumber(NS_PTR_TO_INT32(aKey)); + gBloatView = nullptr; } static intptr_t GetSerialNumber(void* aPtr, bool aCreate) { - PLHashEntry** hep = PL_HashTableRawLookup(gSerialNumbers, - HashNumber(aPtr), - aPtr); - if (hep && *hep) { - MOZ_RELEASE_ASSERT(!aCreate, "If an object already has a serial number, we should be destroying it."); - return static_cast((*hep)->value)->serialNumber; - } - if (!aCreate) { - return 0; + auto record = gSerialNumbers->Get(aPtr); + return record ? record->serialNumber : 0; } - SerialNumberRecord* record = new SerialNumberRecord(); + auto entry = gSerialNumbers->LookupForAdd(aPtr); + if (entry) { + MOZ_CRASH("If an object already has a serial number, we should be destroying it."); + } + + auto record = entry.OrInsert([]() { return new SerialNumberRecord(); }); WalkTheStackSavingLocations(record->allocationStack); - PL_HashTableRawAdd(gSerialNumbers, hep, HashNumber(aPtr), - aPtr, static_cast(record)); if (gLogJSStacks) { record->SaveJSStack(); } return gNextSerialNumber; } -static int32_t* -GetRefCount(void* aPtr) -{ - PLHashEntry** hep = PL_HashTableRawLookup(gSerialNumbers, - HashNumber(aPtr), - aPtr); - if (hep && *hep) { - return &(static_cast((*hep)->value)->refCount); - } else { - return nullptr; - } -} - -#ifdef HAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR -static int32_t* -GetCOMPtrCount(void* aPtr) -{ - PLHashEntry** hep = PL_HashTableRawLookup(gSerialNumbers, - HashNumber(aPtr), - aPtr); - if (hep && *hep) { - return &(static_cast((*hep)->value)->COMPtrCount); - } - return nullptr; -} -#endif // HAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR - static void RecycleSerialNumberPtr(void* aPtr) { - PL_HashTableRemove(gSerialNumbers, aPtr); + gSerialNumbers->Remove(aPtr); } static bool LogThisObj(intptr_t aSerialNumber) { - return (bool)PL_HashTableLookup(gObjectsToLog, (const void*)aSerialNumber); + return gObjectsToLog->Contains(aSerialNumber); } using EnvCharType = mozilla::filesystem::Path::value_type; @@ -802,54 +677,33 @@ InitTraceLog() if (classes) { // if XPCOM_MEM_LOG_CLASSES was set to some value, the value is interpreted // as a list of class names to track - gTypesToLog = PL_NewHashTable(256, - PL_HashString, - PL_CompareStrings, - PL_CompareValues, - &typesToLogHashAllocOps, nullptr); - if (!gTypesToLog) { - NS_WARNING("out of memory"); - fprintf(stdout, "### XPCOM_MEM_LOG_CLASSES defined -- unable to log specific classes\n"); - } else { - fprintf(stdout, "### XPCOM_MEM_LOG_CLASSES defined -- only logging these classes: "); - const char* cp = classes; - for (;;) { - char* cm = (char*)strchr(cp, ','); - if (cm) { - *cm = '\0'; - } - PL_HashTableAdd(gTypesToLog, strdup(cp), (void*)1); - fprintf(stdout, "%s ", cp); - if (!cm) { - break; - } - *cm = ','; - cp = cm + 1; + gTypesToLog = new CharPtrSet(256); + + fprintf(stdout, "### XPCOM_MEM_LOG_CLASSES defined -- only logging these classes: "); + const char* cp = classes; + for (;;) { + char* cm = (char*)strchr(cp, ','); + if (cm) { + *cm = '\0'; } - fprintf(stdout, "\n"); + gTypesToLog->PutEntry(cp); + fprintf(stdout, "%s ", cp); + if (!cm) { + break; + } + *cm = ','; + cp = cm + 1; } + fprintf(stdout, "\n"); - gSerialNumbers = PL_NewHashTable(256, - HashNumber, - PL_CompareValues, - PL_CompareValues, - &serialNumberHashAllocOps, nullptr); - - + gSerialNumbers = new SerialHash(256); } const char* objects = getenv("XPCOM_MEM_LOG_OBJECTS"); if (objects) { - gObjectsToLog = PL_NewHashTable(256, - HashNumber, - PL_CompareValues, - PL_CompareValues, - nullptr, nullptr); + gObjectsToLog = new IntPtrSet(256); - if (!gObjectsToLog) { - NS_WARNING("out of memory"); - fprintf(stdout, "### XPCOM_MEM_LOG_OBJECTS defined -- unable to log specific objects\n"); - } else if (!(gRefcntsLog || gAllocLog || gCOMPtrLog)) { + if (!(gRefcntsLog || gAllocLog || gCOMPtrLog)) { fprintf(stdout, "### XPCOM_MEM_LOG_OBJECTS defined -- but none of XPCOM_MEM_(REFCNT|ALLOC|COMPTR)_LOG is defined\n"); } else { fprintf(stdout, "### XPCOM_MEM_LOG_OBJECTS defined -- only logging these objects: "); @@ -875,7 +729,7 @@ InitTraceLog() bottom = top; } for (intptr_t serialno = bottom; serialno <= top; serialno++) { - PL_HashTableAdd(gObjectsToLog, (const void*)serialno, (void*)1); + gObjectsToLog->PutEntry(serialno); fprintf(stdout, "%" PRIdPTR " ", serialno); } if (!cm) { @@ -1093,18 +947,17 @@ NS_LogAddRef(void* aPtr, nsrefcnt aRefcnt, // Here's the case where MOZ_COUNT_CTOR was not used, // yet we still want to see creation information: - bool loggingThisType = (!gTypesToLog || LogThisType(aClass)); + bool loggingThisType = (!gTypesToLog || gTypesToLog->Contains(aClass)); intptr_t serialno = 0; if (gSerialNumbers && loggingThisType) { serialno = GetSerialNumber(aPtr, aRefcnt == 1); MOZ_ASSERT(serialno != 0, "Serial number requested for unrecognized pointer! " "Are you memmoving a refcounted object?"); - int32_t* count = GetRefCount(aPtr); - if (count) { - (*count)++; + auto record = gSerialNumbers->Get(aPtr); + if (record) { + ++record->refCount; } - } bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno)); @@ -1143,18 +996,17 @@ NS_LogRelease(void* aPtr, nsrefcnt aRefcnt, const char* aClass) } } - bool loggingThisType = (!gTypesToLog || LogThisType(aClass)); + bool loggingThisType = (!gTypesToLog || gTypesToLog->Contains(aClass)); intptr_t serialno = 0; if (gSerialNumbers && loggingThisType) { serialno = GetSerialNumber(aPtr, false); MOZ_ASSERT(serialno != 0, "Serial number requested for unrecognized pointer! " "Are you memmoving a refcounted object?"); - int32_t* count = GetRefCount(aPtr); - if (count) { - (*count)--; + auto record = gSerialNumbers->Get(aPtr); + if (record) { + --record->refCount; } - } bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno)); @@ -1202,7 +1054,7 @@ NS_LogCtor(void* aPtr, const char* aType, uint32_t aInstanceSize) } } - bool loggingThisType = (!gTypesToLog || LogThisType(aType)); + bool loggingThisType = (!gTypesToLog || gTypesToLog->Contains(aType)); intptr_t serialno = 0; if (gSerialNumbers && loggingThisType) { serialno = GetSerialNumber(aPtr, true); @@ -1239,7 +1091,7 @@ NS_LogDtor(void* aPtr, const char* aType, uint32_t aInstanceSize) } } - bool loggingThisType = (!gTypesToLog || LogThisType(aType)); + bool loggingThisType = (!gTypesToLog || gTypesToLog->Contains(aType)); intptr_t serialno = 0; if (gSerialNumbers && loggingThisType) { serialno = GetSerialNumber(aPtr, false); @@ -1285,16 +1137,13 @@ NS_LogCOMPtrAddRef(void* aCOMPtr, nsISupports* aObject) return; } - int32_t* count = GetCOMPtrCount(object); - if (count) { - (*count)++; - } - + auto record = gSerialNumbers->Get(object); + int32_t count = record ? ++record->COMPtrCount : -1; bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno)); if (gCOMPtrLog && loggingThisObject) { fprintf(gCOMPtrLog, "\n %p %" PRIdPTR " nsCOMPtrAddRef %d %p\n", - object, serialno, count ? (*count) : -1, aCOMPtr); + object, serialno, count, aCOMPtr); WalkTheStackCached(gCOMPtrLog); } } @@ -1326,16 +1175,13 @@ NS_LogCOMPtrRelease(void* aCOMPtr, nsISupports* aObject) return; } - int32_t* count = GetCOMPtrCount(object); - if (count) { - (*count)--; - } - + auto record = gSerialNumbers->Get(object); + int32_t count = record ? --record->COMPtrCount : -1; bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno)); if (gCOMPtrLog && loggingThisObject) { fprintf(gCOMPtrLog, "\n %p %" PRIdPTR " nsCOMPtrRelease %d %p\n", - object, serialno, count ? (*count) : -1, aCOMPtr); + object, serialno, count, aCOMPtr); WalkTheStackCached(gCOMPtrLog); } } @@ -1346,22 +1192,10 @@ void nsTraceRefcnt::Shutdown() { gCodeAddressService = nullptr; - if (gBloatView) { - PL_HashTableDestroy(gBloatView); - gBloatView = nullptr; - } - if (gTypesToLog) { - PL_HashTableDestroy(gTypesToLog); - gTypesToLog = nullptr; - } - if (gObjectsToLog) { - PL_HashTableDestroy(gObjectsToLog); - gObjectsToLog = nullptr; - } - if (gSerialNumbers) { - PL_HashTableDestroy(gSerialNumbers); - gSerialNumbers = nullptr; - } + gBloatView = nullptr; + gTypesToLog = nullptr; + gObjectsToLog = nullptr; + gSerialNumbers = nullptr; maybeUnregisterAndCloseFile(gBloatLog); maybeUnregisterAndCloseFile(gRefcntsLog); maybeUnregisterAndCloseFile(gAllocLog); From 96c18b6c4afc8c244e4c643f6f93972422307058 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Thu, 19 Jul 2018 14:14:50 -0600 Subject: [PATCH 06/37] Bug 1467889 - Adjust some uses of XPCOM strings. r=mrbkap r=mstange MozReview-Commit-ID: 5AG4WAmbLZz --- dom/base/nsContentUtils.cpp | 8 ++++---- dom/ipc/ContentChild.cpp | 2 +- dom/ipc/TabParent.cpp | 2 +- tools/profiler/gecko/nsProfiler.cpp | 4 ++-- widget/nsClipboardProxy.cpp | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 57d134e198f8..40916ed72fa3 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -7769,8 +7769,8 @@ nsContentUtils::IPCTransferableToTransferable(const IPCDataTransfer& aDataTransf // The buffer contains the terminating null. Shmem itemData = item.data().get_Shmem(); - const nsDependentCString text(itemData.get(), - itemData.Size()); + const nsDependentCSubstring text(itemData.get(), + itemData.Size()); rv = dataWrapper->SetData(text); NS_ENSURE_SUCCESS(rv, rv); @@ -7951,7 +7951,7 @@ ConvertToShmem(mozilla::dom::nsIContentChild* aChild, : static_cast(aParent); Shmem result; - if (!allocator->AllocShmem(aInput.Length() + 1, + if (!allocator->AllocShmem(aInput.Length(), SharedMemory::TYPE_BASIC, &result)) { return result; @@ -7959,7 +7959,7 @@ ConvertToShmem(mozilla::dom::nsIContentChild* aChild, memcpy(result.get(), aInput.BeginReading(), - aInput.Length() + 1); + aInput.Length()); return result; } diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index baaaac9bd736..4ed3b0ac12db 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -3343,7 +3343,7 @@ ContentChild::RecvInvokeDragSession(nsTArray&& aTransfers, variant->SetAsAString(data); } else if (item.data().type() == IPCDataTransferData::TShmem) { Shmem data = item.data().get_Shmem(); - variant->SetAsACString(nsDependentCString(data.get(), data.Size())); + variant->SetAsACString(nsDependentCSubstring(data.get(), data.Size())); Unused << DeallocShmem(data); } else if (item.data().type() == IPCDataTransferData::TIPCBlob) { RefPtr blobImpl = diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index e12f9118b646..91ce38727e45 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -3402,7 +3402,7 @@ TabParent::AddInitialDnDDataTo(DataTransfer* aDataTransfer, variant->SetAsISupports(imageContainer); } else { Shmem data = item.data().get_Shmem(); - variant->SetAsACString(nsDependentCString(data.get(), data.Size())); + variant->SetAsACString(nsDependentCSubstring(data.get(), data.Size())); } mozilla::Unused << DeallocShmem(item.data().get_Shmem()); diff --git a/tools/profiler/gecko/nsProfiler.cpp b/tools/profiler/gecko/nsProfiler.cpp index 7499f16a1c74..ac09c1d30e60 100644 --- a/tools/profiler/gecko/nsProfiler.cpp +++ b/tools/profiler/gecko/nsProfiler.cpp @@ -613,8 +613,8 @@ nsProfiler::StartGathering(double aSinceTime) for (auto profile : profiles) { profile->Then(GetMainThreadSerialEventTarget(), __func__, [self](const mozilla::ipc::Shmem& aResult) { - const nsDependentCString profileString(aResult.get(), - aResult.Size()); + const nsDependentCSubstring profileString(aResult.get(), + aResult.Size() - 1); self->GatheredOOPProfile(profileString); }, [self](ipc::ResponseRejectReason aReason) { diff --git a/widget/nsClipboardProxy.cpp b/widget/nsClipboardProxy.cpp index 040a91636f76..867be48d8b98 100644 --- a/widget/nsClipboardProxy.cpp +++ b/widget/nsClipboardProxy.cpp @@ -99,7 +99,7 @@ nsClipboardProxy::GetData(nsITransferable *aTransferable, int32_t aWhichClipboar nsCOMPtr stream; NS_NewCStringInputStream(getter_AddRefs(stream), - nsDependentCString(data.get(), data.Size())); + nsDependentCSubstring(data.get(), data.Size())); rv = aTransferable->SetTransferData(flavor.get(), stream, sizeof(nsISupports*)); NS_ENSURE_SUCCESS(rv, rv); @@ -110,7 +110,7 @@ nsClipboardProxy::GetData(nsITransferable *aTransferable, int32_t aWhichClipboar do_CreateInstance(NS_SUPPORTS_CSTRING_CONTRACTID, &rv); NS_ENSURE_SUCCESS(rv, rv); - rv = dataWrapper->SetData(nsDependentCString(data.get(), data.Size())); + rv = dataWrapper->SetData(nsDependentCSubstring(data.get(), data.Size())); NS_ENSURE_SUCCESS(rv, rv); rv = aTransferable->SetTransferData(item.flavor().get(), dataWrapper, From 7c156959da372f8af79492dae0718f913b1d8bcf Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Thu, 19 Jul 2018 16:19:59 -0400 Subject: [PATCH 07/37] Bug 1476796 - Enable AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor() to notify consumers about completion of asynchronous results; r=baku Right now consumers can't know when the parent process has finished talking to the permission manager. It would be nice to enable consumers to depend on the status of the asynchronous task using a promise. --- dom/base/nsDocument.cpp | 5 ++- dom/base/nsGlobalWindowOuter.cpp | 4 +- dom/ipc/ContentParent.cpp | 6 ++- dom/ipc/ContentParent.h | 3 +- dom/ipc/PContent.ipdl | 3 +- .../antitracking/AntiTrackingCommon.cpp | 40 ++++++++++++++----- .../antitracking/AntiTrackingCommon.h | 12 +++++- 7 files changed, 53 insertions(+), 20 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 525dbee9e65a..34b62f7d2069 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -12517,8 +12517,9 @@ nsIDocument::MaybeAllowStorageForOpener() return; } - AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, - openerInner); + // We don't care when the asynchronous work finishes here. + Unused << AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, + openerInner); } bool diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp index f1c4e18af2b4..f1673146d91b 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/dom/base/nsGlobalWindowOuter.cpp @@ -7148,7 +7148,9 @@ nsGlobalWindowOuter::MaybeAllowStorageForOpenedWindow(nsIURI* aURI) return; } - AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, inner); + // We don't care when the asynchronous work finishes here. + Unused << AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, + inner); } //***************************************************************************** diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 0ddc2fffe673..ba7287412f00 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -5780,10 +5780,12 @@ ContentParent::RecvBHRThreadHang(const HangDetails& aDetails) mozilla::ipc::IPCResult ContentParent::RecvFirstPartyStorageAccessGrantedForOrigin(const Principal& aParentPrincipal, const nsCString& aTrackingOrigin, - const nsCString& aGrantedOrigin) + const nsCString& aGrantedOrigin, + FirstPartyStorageAccessGrantedForOriginResolver&& aResolver) { AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(aParentPrincipal, aTrackingOrigin, - aGrantedOrigin); + aGrantedOrigin, + std::move(aResolver)); return IPC_OK(); } diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index 28e833ce2618..5c8a1141fd71 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -1227,7 +1227,8 @@ public: virtual mozilla::ipc::IPCResult RecvFirstPartyStorageAccessGrantedForOrigin(const Principal& aParentPrincipal, const nsCString& aTrackingOrigin, - const nsCString& aGrantedOrigin) override; + const nsCString& aGrantedOrigin, + FirstPartyStorageAccessGrantedForOriginResolver&& aResolver) override; // Notify the ContentChild to enable the input event prioritization when // initializing. diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 8a1b11541f8a..9e61cadbdb15 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -1165,7 +1165,8 @@ parent: */ async FirstPartyStorageAccessGrantedForOrigin(Principal aParentPrincipal, nsCString aTrackingOrigin, - nsCString aGrantedOrigin); + nsCString aGrantedOrigin) + returns (bool unused); both: async AsyncMessage(nsString aMessage, CpowEntry[] aCpows, diff --git a/toolkit/components/antitracking/AntiTrackingCommon.cpp b/toolkit/components/antitracking/AntiTrackingCommon.cpp index b48c468eb978..8784b831217b 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.cpp +++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp @@ -7,6 +7,8 @@ #include "AntiTrackingCommon.h" #include "mozilla/dom/ContentChild.h" +#include "mozilla/ipc/MessageChannel.h" +#include "mozilla/AbstractThread.h" #include "mozilla/StaticPrefs.h" #include "mozIThirdPartyUtil.h" #include "nsContentUtils.h" @@ -71,14 +73,14 @@ CreatePermissionKey(const nsCString& aTrackingOrigin, } // anonymous -/* static */ void +/* static */ RefPtr AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigin, nsPIDOMWindowInner* aParentWindow) { MOZ_ASSERT(aParentWindow); if (!StaticPrefs::privacy_restrict3rdpartystorage_enabled()) { - return; + return StorageAccessGrantPromise::CreateAndResolve(true, __func__); } nsCOMPtr topLevelStoragePrincipal; @@ -88,7 +90,7 @@ AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigi nsGlobalWindowOuter* outerParentWindow = nsGlobalWindowOuter::Cast(parentWindow->GetOuterWindow()); if (NS_WARN_IF(!outerParentWindow)) { - return; + return StorageAccessGrantPromise::CreateAndReject(false, __func__); } // We are a first party resource. @@ -96,23 +98,27 @@ AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigi CopyUTF16toUTF8(aOrigin, trackingOrigin); topLevelStoragePrincipal = parentWindow->GetPrincipal(); if (NS_WARN_IF(!topLevelStoragePrincipal)) { - return; + return StorageAccessGrantPromise::CreateAndReject(false, __func__); } // We are a 3rd party source. } else if (!GetParentPrincipalAndTrackingOrigin(parentWindow, getter_AddRefs(topLevelStoragePrincipal), trackingOrigin)) { - return; + return StorageAccessGrantPromise::CreateAndReject(false, __func__); } NS_ConvertUTF16toUTF8 grantedOrigin(aOrigin); if (XRE_IsParentProcess()) { + RefPtr p = new StorageAccessGrantPromise::Private(__func__); SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(topLevelStoragePrincipal, trackingOrigin, - grantedOrigin); - return; + grantedOrigin, + [p] (bool success) { + p->Resolve(success, __func__); + }); + return p; } ContentChild* cc = ContentChild::GetSingleton(); @@ -120,25 +126,36 @@ AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigi // This is not really secure, because here we have the content process sending // the request of storing a permission. - Unused << cc->SendFirstPartyStorageAccessGrantedForOrigin(IPC::Principal(topLevelStoragePrincipal), - trackingOrigin, - grantedOrigin); + RefPtr p = new StorageAccessGrantPromise::Private(__func__); + cc->SendFirstPartyStorageAccessGrantedForOrigin(IPC::Principal(topLevelStoragePrincipal), + trackingOrigin, + grantedOrigin) + ->Then(GetCurrentThreadSerialEventTarget(), __func__, + [p] (bool success) { + p->Resolve(success, __func__); + }, [p] (ipc::ResponseRejectReason aReason) { + p->Reject(false, __func__); + }); + return p; } /* static */ void AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(nsIPrincipal* aParentPrincipal, const nsCString& aTrackingOrigin, - const nsCString& aGrantedOrigin) + const nsCString& aGrantedOrigin, + FirstPartyStorageAccessGrantedForOriginResolver&& aResolver) { MOZ_ASSERT(XRE_IsParentProcess()); if (NS_WARN_IF(!aParentPrincipal)) { // The child process is sending something wrong. Let's ignore it. + aResolver(false); return; } nsCOMPtr pm = services::GetPermissionManager(); if (NS_WARN_IF(!pm)) { + aResolver(false); return; } @@ -154,6 +171,7 @@ AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(n nsIPermissionManager::ALLOW_ACTION, nsIPermissionManager::EXPIRE_TIME, when); Unused << NS_WARN_IF(NS_FAILED(rv)); + aResolver(NS_SUCCEEDED(rv)); } bool diff --git a/toolkit/components/antitracking/AntiTrackingCommon.h b/toolkit/components/antitracking/AntiTrackingCommon.h index 21742995462c..0cca926146f3 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.h +++ b/toolkit/components/antitracking/AntiTrackingCommon.h @@ -8,6 +8,9 @@ #define mozilla_antitrackingservice_h #include "nsString.h" +#include "mozilla/dom/PContentParent.h" +#include "mozilla/MozPromise.h" +#include "mozilla/RefPtr.h" class nsIHttpChannel; class nsIPrincipal; @@ -19,6 +22,9 @@ namespace mozilla { class AntiTrackingCommon final { public: + typedef dom::PContentParent::FirstPartyStorageAccessGrantedForOriginResolver + FirstPartyStorageAccessGrantedForOriginResolver; + // This method returns true if the URI has first party storage access when // loaded inside the passed 3rd party context tracking resource window. // If the window is first party context, please use @@ -59,7 +65,8 @@ public: // Ex: example.net import tracker.com/script.js which does opens a popup and // the user interacts with it. tracker.com is allowed when loaded by // example.net. - static void + typedef MozPromise StorageAccessGrantPromise; + static MOZ_MUST_USE RefPtr AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigin, nsPIDOMWindowInner* aParentWindow); @@ -67,7 +74,8 @@ public: static void SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(nsIPrincipal* aPrincipal, const nsCString& aParentOrigin, - const nsCString& aGrantedOrigin); + const nsCString& aGrantedOrigin, + FirstPartyStorageAccessGrantedForOriginResolver&& aResolver); }; From 46afadc431a4963aaf7f68699f1f0344b5b0ca18 Mon Sep 17 00:00:00 2001 From: Coroiu Cristina Date: Thu, 19 Jul 2018 23:40:24 +0300 Subject: [PATCH 08/37] Backed out 2 changesets (bug 1476574) for browser-chrome failures at browser/base/content/test/static/browser_all_files_referenced.js Backed out changeset 4ea7997194e9 (bug 1476574) Backed out changeset ec37892ce390 (bug 1476574) --- testing/marionette/jar.mn | 1 - .../mozilla/testing/codecoverage.py | 31 +++------ .../mozharness/scripts/desktop_unittest.py | 21 ++++-- .../mozharness/scripts/web_platform_tests.py | 8 +-- .../wptrunner/executors/executormarionette.py | 66 +------------------ .../wptrunner/wptrunner/executors/protocol.py | 17 ----- 6 files changed, 31 insertions(+), 113 deletions(-) diff --git a/testing/marionette/jar.mn b/testing/marionette/jar.mn index 214612c9e3e7..f23195e2bb67 100644 --- a/testing/marionette/jar.mn +++ b/testing/marionette/jar.mn @@ -47,4 +47,3 @@ marionette.jar: content/test_nested_iframe.xul (chrome/test_nested_iframe.xul) content/test.xul (chrome/test.xul) #endif - content/PerTestCoverageUtils.jsm (../../tools/code-coverage/PerTestCoverageUtils.jsm) diff --git a/testing/mozharness/mozharness/mozilla/testing/codecoverage.py b/testing/mozharness/mozharness/mozilla/testing/codecoverage.py index 7390dd7102fa..129d87b60f41 100644 --- a/testing/mozharness/mozharness/mozilla/testing/codecoverage.py +++ b/testing/mozharness/mozharness/mozilla/testing/codecoverage.py @@ -204,20 +204,16 @@ class CodeCoverageMixin(SingleTestMixin): def coverage_args(self): return [] - def set_coverage_env(self, env, is_baseline_test=False): + def set_coverage_env(self, env): # Set the GCOV directory. - self.gcov_dir = tempfile.mkdtemp() - env['GCOV_PREFIX'] = self.gcov_dir - - # Set the GCOV directory where counters will be dumped in per-test mode. - # Resetting/dumping is only available on Linux for the time being - # (https://bugzilla.mozilla.org/show_bug.cgi?id=1471576). - if self.per_test_coverage and not is_baseline_test and self._is_linux(): - env['GCOV_RESULTS_DIR'] = tempfile.mkdtemp() + gcov_dir = tempfile.mkdtemp() + env['GCOV_PREFIX'] = gcov_dir # Set JSVM directory. - self.jsvm_dir = tempfile.mkdtemp() - env['JS_CODE_COVERAGE_OUTPUT_DIR'] = self.jsvm_dir + jsvm_dir = tempfile.mkdtemp() + env['JS_CODE_COVERAGE_OUTPUT_DIR'] = jsvm_dir + + return (gcov_dir, jsvm_dir) @PreScriptAction('run-tests') def _set_gcov_prefix(self, action): @@ -227,7 +223,7 @@ class CodeCoverageMixin(SingleTestMixin): if self.per_test_coverage: return - self.set_coverage_env(os.environ) + self.gcov_dir, self.jsvm_dir = self.set_coverage_env(os.environ) def parse_coverage_artifacts(self, gcov_dir, @@ -290,11 +286,9 @@ class CodeCoverageMixin(SingleTestMixin): else: return grcov_output_file, jsvm_output_file - def add_per_test_coverage_report(self, env, suite, test): - gcov_dir = env['GCOV_RESULTS_DIR'] if 'GCOV_RESULTS_DIR' in env else self.gcov_dir - + def add_per_test_coverage_report(self, gcov_dir, jsvm_dir, suite, test): grcov_file = self.parse_coverage_artifacts( - gcov_dir, self.jsvm_dir, merge=True, output_format='coveralls', + gcov_dir, jsvm_dir, merge=True, output_format='coveralls', filter_covered=True, ) @@ -306,11 +300,6 @@ class CodeCoverageMixin(SingleTestMixin): assert test not in self.per_test_reports[suite] self.per_test_reports[suite][test] = report_file - if 'GCOV_RESULTS_DIR' in env: - # In this case, parse_coverage_artifacts has removed GCOV_RESULTS_DIR - # so we need to remove GCOV_PREFIX. - shutil.rmtree(self.gcov_dir) - def is_covered(self, sf): # For C/C++ source files, we can consider a file as being uncovered # when all its source lines are uncovered. diff --git a/testing/mozharness/scripts/desktop_unittest.py b/testing/mozharness/scripts/desktop_unittest.py index c699c1731655..e18937a7a6c1 100755 --- a/testing/mozharness/scripts/desktop_unittest.py +++ b/testing/mozharness/scripts/desktop_unittest.py @@ -15,6 +15,7 @@ import re import sys import copy import shutil +import tempfile import glob import imp @@ -888,18 +889,28 @@ class DesktopUnittest(TestingMixin, MercurialScript, MozbaseMixin, final_cmd = copy.copy(cmd) final_cmd.extend(per_test_args) - final_env = copy.copy(env) - if self.per_test_coverage: - self.set_coverage_env(final_env) + gcov_dir, jsvm_dir = self.set_coverage_env(env) + # Per-test reset/dump is only supported on Linux for the time being. + if not is_baseline_test and \ + self._is_linux(): + env['GCOV_RESULTS_DIR'] = tempfile.mkdtemp() return_code = self.run_command(final_cmd, cwd=dirs['abs_work_dir'], output_timeout=cmd_timeout, output_parser=parser, - env=final_env) + env=env) if self.per_test_coverage: - self.add_per_test_coverage_report(final_env, suite, per_test_args[-1]) + self.add_per_test_coverage_report( + env['GCOV_RESULTS_DIR'] if 'GCOV_RESULTS_DIR' in env else gcov_dir, + jsvm_dir, + suite, + per_test_args[-1] + ) + if 'GCOV_RESULTS_DIR' in env: + shutil.rmtree(gcov_dir) + del env['GCOV_RESULTS_DIR'] # mochitest, reftest, and xpcshell suites do not return # appropriate return codes. Therefore, we must parse the output diff --git a/testing/mozharness/scripts/web_platform_tests.py b/testing/mozharness/scripts/web_platform_tests.py index e5f5ccc3eee3..b0610a1e5c83 100755 --- a/testing/mozharness/scripts/web_platform_tests.py +++ b/testing/mozharness/scripts/web_platform_tests.py @@ -377,19 +377,17 @@ class WebPlatformTest(TestingMixin, MercurialScript, CodeCoverageMixin): cmd = self._query_cmd(test_types) cmd.extend(per_test_args) - final_env = copy.copy(env) - if self.per_test_coverage: - self.set_coverage_env(final_env, is_baseline_test) + gcov_dir, jsvm_dir = self.set_coverage_env(env) return_code = self.run_command(cmd, cwd=dirs['abs_work_dir'], output_timeout=1000, output_parser=parser, - env=final_env) + env=env) if self.per_test_coverage: - self.add_per_test_coverage_report(final_env, suite, per_test_args[-1]) + self.add_per_test_coverage_report(gcov_dir, jsvm_dir, suite, per_test_args[-1]) tbpl_status, log_level, summary = parser.evaluate_parser(return_code, previous_summary=summary) diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py index 6f1d35a749b7..54802faf83b1 100644 --- a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py +++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py @@ -29,8 +29,7 @@ from .protocol import (AssertsProtocolPart, SelectorProtocolPart, ClickProtocolPart, SendKeysProtocolPart, - TestDriverProtocolPart, - CoverageProtocolPart) + TestDriverProtocolPart) from ..testrunner import Stop from ..webdriver_server import GeckoDriverServer @@ -372,53 +371,6 @@ class MarionetteTestDriverProtocolPart(TestDriverProtocolPart): self.parent.base.execute_script("window.postMessage(%s, '*')" % json.dumps(obj)) -class MarionetteCoverageProtocolPart(CoverageProtocolPart): - def setup(self): - self.marionette = self.parent.marionette - script = """ - ChromeUtils.import("chrome://marionette/content/PerTestCoverageUtils.jsm"); - return PerTestCoverageUtils.enabled; - """ - with self.marionette.using_context(self.marionette.CONTEXT_CHROME): - self.is_enabled = self.marionette.execute_script(script) - - def reset(self): - script = """ - var callback = arguments[arguments.length - 1]; - - ChromeUtils.import("chrome://marionette/content/PerTestCoverageUtils.jsm"); - PerTestCoverageUtils.beforeTest().then(callback, callback); - """ - with self.marionette.using_context(self.marionette.CONTEXT_CHROME): - try: - error = self.marionette.execute_async_script(script) - if error is not None: - raise Exception('Failure while resetting counters: %s' % json.dumps(error)) - except (errors.MarionetteException, socket.error): - # This usually happens if the process crashed - pass - - def dump(self): - if len(self.marionette.window_handles): - handle = self.marionette.window_handles[0] - self.marionette.switch_to_window(handle) - - script = """ - var callback = arguments[arguments.length - 1]; - - ChromeUtils.import("chrome://marionette/content/PerTestCoverageUtils.jsm"); - PerTestCoverageUtils.afterTest().then(callback, callback); - """ - with self.marionette.using_context(self.marionette.CONTEXT_CHROME): - try: - error = self.marionette.execute_async_script(script) - if error is not None: - raise Exception('Failure while dumping counters: %s' % json.dumps(error)) - except (errors.MarionetteException, socket.error): - # This usually happens if the process crashed - pass - - class MarionetteProtocol(Protocol): implements = [MarionetteBaseProtocolPart, MarionetteTestharnessProtocolPart, @@ -428,8 +380,7 @@ class MarionetteProtocol(Protocol): MarionetteClickProtocolPart, MarionetteSendKeysProtocolPart, MarionetteTestDriverProtocolPart, - MarionetteAssertsProtocolPart, - MarionetteCoverageProtocolPart] + MarionetteAssertsProtocolPart] def __init__(self, executor, browser, capabilities=None, timeout_multiplier=1, e10s=True): do_delayed_imports() @@ -648,9 +599,6 @@ class MarionetteTestharnessExecutor(TestharnessExecutor): else: timeout_ms = "null" - if self.protocol.coverage.is_enabled: - self.protocol.coverage.reset() - format_map = {"abs_url": url, "url": strip_server(url), "window_id": self.window_id, @@ -673,10 +621,6 @@ class MarionetteTestharnessExecutor(TestharnessExecutor): done, rv = handler(result) if done: break - - if self.protocol.coverage.is_enabled: - self.protocol.coverage.dump() - return rv @@ -745,14 +689,8 @@ class MarionetteRefTestExecutor(RefTestExecutor): self.protocol.base.set_window(self.protocol.marionette.window_handles[-1]) self.has_window = True - if self.protocol.coverage.is_enabled: - self.protocol.coverage.reset() - result = self.implementation.run_test(test) - if self.protocol.coverage.is_enabled: - self.protocol.coverage.dump() - if self.debug: assertion_count = self.protocol.asserts.get() if "extra" not in result: diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py index 71fc3c9a8f65..2e48162e0fd3 100644 --- a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py +++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py @@ -303,20 +303,3 @@ class AssertsProtocolPart(ProtocolPart): def get(self): """Get a count of assertions since the last browser start""" pass - - -class CoverageProtocolPart(ProtocolPart): - """Protocol part for collecting per-test coverage data.""" - __metaclass__ = ABCMeta - - name = "coverage" - - @abstractmethod - def reset(self): - """Reset coverage counters""" - pass - - @abstractmethod - def dump(self): - """Dump coverage counters""" - pass From edada2f46b0ab6fec009c3340f9c5897863d8258 Mon Sep 17 00:00:00 2001 From: Coroiu Cristina Date: Thu, 19 Jul 2018 23:51:35 +0300 Subject: [PATCH 09/37] Backed out changeset bc6d7dc3f10f (bug 1476796) for build bustage on a CLOSED TREE --- dom/base/nsDocument.cpp | 5 +-- dom/base/nsGlobalWindowOuter.cpp | 4 +- dom/ipc/ContentParent.cpp | 6 +-- dom/ipc/ContentParent.h | 3 +- dom/ipc/PContent.ipdl | 3 +- .../antitracking/AntiTrackingCommon.cpp | 40 +++++-------------- .../antitracking/AntiTrackingCommon.h | 12 +----- 7 files changed, 20 insertions(+), 53 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 34b62f7d2069..525dbee9e65a 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -12517,9 +12517,8 @@ nsIDocument::MaybeAllowStorageForOpener() return; } - // We don't care when the asynchronous work finishes here. - Unused << AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, - openerInner); + AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, + openerInner); } bool diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp index f1673146d91b..f1c4e18af2b4 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/dom/base/nsGlobalWindowOuter.cpp @@ -7148,9 +7148,7 @@ nsGlobalWindowOuter::MaybeAllowStorageForOpenedWindow(nsIURI* aURI) return; } - // We don't care when the asynchronous work finishes here. - Unused << AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, - inner); + AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, inner); } //***************************************************************************** diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index ba7287412f00..0ddc2fffe673 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -5780,12 +5780,10 @@ ContentParent::RecvBHRThreadHang(const HangDetails& aDetails) mozilla::ipc::IPCResult ContentParent::RecvFirstPartyStorageAccessGrantedForOrigin(const Principal& aParentPrincipal, const nsCString& aTrackingOrigin, - const nsCString& aGrantedOrigin, - FirstPartyStorageAccessGrantedForOriginResolver&& aResolver) + const nsCString& aGrantedOrigin) { AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(aParentPrincipal, aTrackingOrigin, - aGrantedOrigin, - std::move(aResolver)); + aGrantedOrigin); return IPC_OK(); } diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index 5c8a1141fd71..28e833ce2618 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -1227,8 +1227,7 @@ public: virtual mozilla::ipc::IPCResult RecvFirstPartyStorageAccessGrantedForOrigin(const Principal& aParentPrincipal, const nsCString& aTrackingOrigin, - const nsCString& aGrantedOrigin, - FirstPartyStorageAccessGrantedForOriginResolver&& aResolver) override; + const nsCString& aGrantedOrigin) override; // Notify the ContentChild to enable the input event prioritization when // initializing. diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 9e61cadbdb15..8a1b11541f8a 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -1165,8 +1165,7 @@ parent: */ async FirstPartyStorageAccessGrantedForOrigin(Principal aParentPrincipal, nsCString aTrackingOrigin, - nsCString aGrantedOrigin) - returns (bool unused); + nsCString aGrantedOrigin); both: async AsyncMessage(nsString aMessage, CpowEntry[] aCpows, diff --git a/toolkit/components/antitracking/AntiTrackingCommon.cpp b/toolkit/components/antitracking/AntiTrackingCommon.cpp index 8784b831217b..b48c468eb978 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.cpp +++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp @@ -7,8 +7,6 @@ #include "AntiTrackingCommon.h" #include "mozilla/dom/ContentChild.h" -#include "mozilla/ipc/MessageChannel.h" -#include "mozilla/AbstractThread.h" #include "mozilla/StaticPrefs.h" #include "mozIThirdPartyUtil.h" #include "nsContentUtils.h" @@ -73,14 +71,14 @@ CreatePermissionKey(const nsCString& aTrackingOrigin, } // anonymous -/* static */ RefPtr +/* static */ void AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigin, nsPIDOMWindowInner* aParentWindow) { MOZ_ASSERT(aParentWindow); if (!StaticPrefs::privacy_restrict3rdpartystorage_enabled()) { - return StorageAccessGrantPromise::CreateAndResolve(true, __func__); + return; } nsCOMPtr topLevelStoragePrincipal; @@ -90,7 +88,7 @@ AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigi nsGlobalWindowOuter* outerParentWindow = nsGlobalWindowOuter::Cast(parentWindow->GetOuterWindow()); if (NS_WARN_IF(!outerParentWindow)) { - return StorageAccessGrantPromise::CreateAndReject(false, __func__); + return; } // We are a first party resource. @@ -98,27 +96,23 @@ AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigi CopyUTF16toUTF8(aOrigin, trackingOrigin); topLevelStoragePrincipal = parentWindow->GetPrincipal(); if (NS_WARN_IF(!topLevelStoragePrincipal)) { - return StorageAccessGrantPromise::CreateAndReject(false, __func__); + return; } // We are a 3rd party source. } else if (!GetParentPrincipalAndTrackingOrigin(parentWindow, getter_AddRefs(topLevelStoragePrincipal), trackingOrigin)) { - return StorageAccessGrantPromise::CreateAndReject(false, __func__); + return; } NS_ConvertUTF16toUTF8 grantedOrigin(aOrigin); if (XRE_IsParentProcess()) { - RefPtr p = new StorageAccessGrantPromise::Private(__func__); SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(topLevelStoragePrincipal, trackingOrigin, - grantedOrigin, - [p] (bool success) { - p->Resolve(success, __func__); - }); - return p; + grantedOrigin); + return; } ContentChild* cc = ContentChild::GetSingleton(); @@ -126,36 +120,25 @@ AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigi // This is not really secure, because here we have the content process sending // the request of storing a permission. - RefPtr p = new StorageAccessGrantPromise::Private(__func__); - cc->SendFirstPartyStorageAccessGrantedForOrigin(IPC::Principal(topLevelStoragePrincipal), - trackingOrigin, - grantedOrigin) - ->Then(GetCurrentThreadSerialEventTarget(), __func__, - [p] (bool success) { - p->Resolve(success, __func__); - }, [p] (ipc::ResponseRejectReason aReason) { - p->Reject(false, __func__); - }); - return p; + Unused << cc->SendFirstPartyStorageAccessGrantedForOrigin(IPC::Principal(topLevelStoragePrincipal), + trackingOrigin, + grantedOrigin); } /* static */ void AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(nsIPrincipal* aParentPrincipal, const nsCString& aTrackingOrigin, - const nsCString& aGrantedOrigin, - FirstPartyStorageAccessGrantedForOriginResolver&& aResolver) + const nsCString& aGrantedOrigin) { MOZ_ASSERT(XRE_IsParentProcess()); if (NS_WARN_IF(!aParentPrincipal)) { // The child process is sending something wrong. Let's ignore it. - aResolver(false); return; } nsCOMPtr pm = services::GetPermissionManager(); if (NS_WARN_IF(!pm)) { - aResolver(false); return; } @@ -171,7 +154,6 @@ AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(n nsIPermissionManager::ALLOW_ACTION, nsIPermissionManager::EXPIRE_TIME, when); Unused << NS_WARN_IF(NS_FAILED(rv)); - aResolver(NS_SUCCEEDED(rv)); } bool diff --git a/toolkit/components/antitracking/AntiTrackingCommon.h b/toolkit/components/antitracking/AntiTrackingCommon.h index 0cca926146f3..21742995462c 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.h +++ b/toolkit/components/antitracking/AntiTrackingCommon.h @@ -8,9 +8,6 @@ #define mozilla_antitrackingservice_h #include "nsString.h" -#include "mozilla/dom/PContentParent.h" -#include "mozilla/MozPromise.h" -#include "mozilla/RefPtr.h" class nsIHttpChannel; class nsIPrincipal; @@ -22,9 +19,6 @@ namespace mozilla { class AntiTrackingCommon final { public: - typedef dom::PContentParent::FirstPartyStorageAccessGrantedForOriginResolver - FirstPartyStorageAccessGrantedForOriginResolver; - // This method returns true if the URI has first party storage access when // loaded inside the passed 3rd party context tracking resource window. // If the window is first party context, please use @@ -65,8 +59,7 @@ public: // Ex: example.net import tracker.com/script.js which does opens a popup and // the user interacts with it. tracker.com is allowed when loaded by // example.net. - typedef MozPromise StorageAccessGrantPromise; - static MOZ_MUST_USE RefPtr + static void AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigin, nsPIDOMWindowInner* aParentWindow); @@ -74,8 +67,7 @@ public: static void SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(nsIPrincipal* aPrincipal, const nsCString& aParentOrigin, - const nsCString& aGrantedOrigin, - FirstPartyStorageAccessGrantedForOriginResolver&& aResolver); + const nsCString& aGrantedOrigin); }; From 344152682077ed45665db143dd2ca5f40c5ac7d7 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Thu, 19 Jul 2018 16:19:59 -0400 Subject: [PATCH 10/37] Bug 1476796 - Enable AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor() to notify consumers about completion of asynchronous results; r=baku Right now consumers can't know when the parent process has finished talking to the permission manager. It would be nice to enable consumers to depend on the status of the asynchronous task using a promise. --- dom/base/nsDocument.cpp | 5 ++- dom/base/nsGlobalWindowOuter.cpp | 4 +- dom/ipc/ContentParent.cpp | 6 ++- dom/ipc/ContentParent.h | 3 +- dom/ipc/PContent.ipdl | 3 +- .../antitracking/AntiTrackingCommon.cpp | 40 ++++++++++++++----- .../antitracking/AntiTrackingCommon.h | 15 ++++++- 7 files changed, 56 insertions(+), 20 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 525dbee9e65a..34b62f7d2069 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -12517,8 +12517,9 @@ nsIDocument::MaybeAllowStorageForOpener() return; } - AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, - openerInner); + // We don't care when the asynchronous work finishes here. + Unused << AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, + openerInner); } bool diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp index f1c4e18af2b4..f1673146d91b 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/dom/base/nsGlobalWindowOuter.cpp @@ -7148,7 +7148,9 @@ nsGlobalWindowOuter::MaybeAllowStorageForOpenedWindow(nsIURI* aURI) return; } - AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, inner); + // We don't care when the asynchronous work finishes here. + Unused << AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(origin, + inner); } //***************************************************************************** diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 0ddc2fffe673..ba7287412f00 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -5780,10 +5780,12 @@ ContentParent::RecvBHRThreadHang(const HangDetails& aDetails) mozilla::ipc::IPCResult ContentParent::RecvFirstPartyStorageAccessGrantedForOrigin(const Principal& aParentPrincipal, const nsCString& aTrackingOrigin, - const nsCString& aGrantedOrigin) + const nsCString& aGrantedOrigin, + FirstPartyStorageAccessGrantedForOriginResolver&& aResolver) { AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(aParentPrincipal, aTrackingOrigin, - aGrantedOrigin); + aGrantedOrigin, + std::move(aResolver)); return IPC_OK(); } diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index 28e833ce2618..5c8a1141fd71 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -1227,7 +1227,8 @@ public: virtual mozilla::ipc::IPCResult RecvFirstPartyStorageAccessGrantedForOrigin(const Principal& aParentPrincipal, const nsCString& aTrackingOrigin, - const nsCString& aGrantedOrigin) override; + const nsCString& aGrantedOrigin, + FirstPartyStorageAccessGrantedForOriginResolver&& aResolver) override; // Notify the ContentChild to enable the input event prioritization when // initializing. diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 8a1b11541f8a..9e61cadbdb15 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -1165,7 +1165,8 @@ parent: */ async FirstPartyStorageAccessGrantedForOrigin(Principal aParentPrincipal, nsCString aTrackingOrigin, - nsCString aGrantedOrigin); + nsCString aGrantedOrigin) + returns (bool unused); both: async AsyncMessage(nsString aMessage, CpowEntry[] aCpows, diff --git a/toolkit/components/antitracking/AntiTrackingCommon.cpp b/toolkit/components/antitracking/AntiTrackingCommon.cpp index b48c468eb978..8784b831217b 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.cpp +++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp @@ -7,6 +7,8 @@ #include "AntiTrackingCommon.h" #include "mozilla/dom/ContentChild.h" +#include "mozilla/ipc/MessageChannel.h" +#include "mozilla/AbstractThread.h" #include "mozilla/StaticPrefs.h" #include "mozIThirdPartyUtil.h" #include "nsContentUtils.h" @@ -71,14 +73,14 @@ CreatePermissionKey(const nsCString& aTrackingOrigin, } // anonymous -/* static */ void +/* static */ RefPtr AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigin, nsPIDOMWindowInner* aParentWindow) { MOZ_ASSERT(aParentWindow); if (!StaticPrefs::privacy_restrict3rdpartystorage_enabled()) { - return; + return StorageAccessGrantPromise::CreateAndResolve(true, __func__); } nsCOMPtr topLevelStoragePrincipal; @@ -88,7 +90,7 @@ AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigi nsGlobalWindowOuter* outerParentWindow = nsGlobalWindowOuter::Cast(parentWindow->GetOuterWindow()); if (NS_WARN_IF(!outerParentWindow)) { - return; + return StorageAccessGrantPromise::CreateAndReject(false, __func__); } // We are a first party resource. @@ -96,23 +98,27 @@ AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigi CopyUTF16toUTF8(aOrigin, trackingOrigin); topLevelStoragePrincipal = parentWindow->GetPrincipal(); if (NS_WARN_IF(!topLevelStoragePrincipal)) { - return; + return StorageAccessGrantPromise::CreateAndReject(false, __func__); } // We are a 3rd party source. } else if (!GetParentPrincipalAndTrackingOrigin(parentWindow, getter_AddRefs(topLevelStoragePrincipal), trackingOrigin)) { - return; + return StorageAccessGrantPromise::CreateAndReject(false, __func__); } NS_ConvertUTF16toUTF8 grantedOrigin(aOrigin); if (XRE_IsParentProcess()) { + RefPtr p = new StorageAccessGrantPromise::Private(__func__); SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(topLevelStoragePrincipal, trackingOrigin, - grantedOrigin); - return; + grantedOrigin, + [p] (bool success) { + p->Resolve(success, __func__); + }); + return p; } ContentChild* cc = ContentChild::GetSingleton(); @@ -120,25 +126,36 @@ AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigi // This is not really secure, because here we have the content process sending // the request of storing a permission. - Unused << cc->SendFirstPartyStorageAccessGrantedForOrigin(IPC::Principal(topLevelStoragePrincipal), - trackingOrigin, - grantedOrigin); + RefPtr p = new StorageAccessGrantPromise::Private(__func__); + cc->SendFirstPartyStorageAccessGrantedForOrigin(IPC::Principal(topLevelStoragePrincipal), + trackingOrigin, + grantedOrigin) + ->Then(GetCurrentThreadSerialEventTarget(), __func__, + [p] (bool success) { + p->Resolve(success, __func__); + }, [p] (ipc::ResponseRejectReason aReason) { + p->Reject(false, __func__); + }); + return p; } /* static */ void AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(nsIPrincipal* aParentPrincipal, const nsCString& aTrackingOrigin, - const nsCString& aGrantedOrigin) + const nsCString& aGrantedOrigin, + FirstPartyStorageAccessGrantedForOriginResolver&& aResolver) { MOZ_ASSERT(XRE_IsParentProcess()); if (NS_WARN_IF(!aParentPrincipal)) { // The child process is sending something wrong. Let's ignore it. + aResolver(false); return; } nsCOMPtr pm = services::GetPermissionManager(); if (NS_WARN_IF(!pm)) { + aResolver(false); return; } @@ -154,6 +171,7 @@ AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(n nsIPermissionManager::ALLOW_ACTION, nsIPermissionManager::EXPIRE_TIME, when); Unused << NS_WARN_IF(NS_FAILED(rv)); + aResolver(NS_SUCCEEDED(rv)); } bool diff --git a/toolkit/components/antitracking/AntiTrackingCommon.h b/toolkit/components/antitracking/AntiTrackingCommon.h index 21742995462c..6298b0e0ac95 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.h +++ b/toolkit/components/antitracking/AntiTrackingCommon.h @@ -8,6 +8,8 @@ #define mozilla_antitrackingservice_h #include "nsString.h" +#include "mozilla/MozPromise.h" +#include "mozilla/RefPtr.h" class nsIHttpChannel; class nsIPrincipal; @@ -19,6 +21,13 @@ namespace mozilla { class AntiTrackingCommon final { public: + // Normally we would include PContentParent.h here and use the + // ipc::FirstPartyStorageAccessGrantedForOriginResolver type which maps to + // the same underlying type, but that results in Windows compilation errors, + // so we use the underlying type to avoid the #include here. + typedef std::function + FirstPartyStorageAccessGrantedForOriginResolver; + // This method returns true if the URI has first party storage access when // loaded inside the passed 3rd party context tracking resource window. // If the window is first party context, please use @@ -59,7 +68,8 @@ public: // Ex: example.net import tracker.com/script.js which does opens a popup and // the user interacts with it. tracker.com is allowed when loaded by // example.net. - static void + typedef MozPromise StorageAccessGrantPromise; + static MOZ_MUST_USE RefPtr AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigin, nsPIDOMWindowInner* aParentWindow); @@ -67,7 +77,8 @@ public: static void SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(nsIPrincipal* aPrincipal, const nsCString& aParentOrigin, - const nsCString& aGrantedOrigin); + const nsCString& aGrantedOrigin, + FirstPartyStorageAccessGrantedForOriginResolver&& aResolver); }; From 87fbf2563c9a30cfb562b6c7711f356b905f3561 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Thu, 19 Jul 2018 11:57:54 +0200 Subject: [PATCH 11/37] Bug 1476574 - Support resetting/dumping code coverage counters before/after web-platform-tests. r=jgraham --HG-- extra : source : ec37892ce390414a0349e16e7cdc5d07d2d158ff extra : histedit_source : 3f3e267d2c2385b35393c2e08621dabd47c07be4%2C03271be81bfe9f7b0d0dec52037606a20699bc3e --- .../static/browser_all_files_referenced.js | 1 + testing/marionette/jar.mn | 1 + .../wptrunner/executors/executormarionette.py | 66 ++++++++++++++++++- .../wptrunner/wptrunner/executors/protocol.py | 17 +++++ 4 files changed, 83 insertions(+), 2 deletions(-) diff --git a/browser/base/content/test/static/browser_all_files_referenced.js b/browser/base/content/test/static/browser_all_files_referenced.js index d6ea50fa7842..4df609d13aab 100644 --- a/browser/base/content/test/static/browser_all_files_referenced.js +++ b/browser/base/content/test/static/browser_all_files_referenced.js @@ -132,6 +132,7 @@ var whitelist = [ {file: "chrome://marionette/content/test_anonymous_content.xul"}, {file: "chrome://marionette/content/test_dialog.properties"}, {file: "chrome://marionette/content/test_dialog.xul"}, + {file: "chrome://marionette/content/PerTestCoverageUtils.jsm"}, // Bug 1348533 {file: "chrome://mozapps/skin/downloads/buttons.png", platforms: ["macosx"]}, {file: "chrome://mozapps/skin/downloads/downloadButtons.png", platforms: ["linux", "win"]}, diff --git a/testing/marionette/jar.mn b/testing/marionette/jar.mn index f23195e2bb67..8c5be664ba9b 100644 --- a/testing/marionette/jar.mn +++ b/testing/marionette/jar.mn @@ -46,4 +46,5 @@ marionette.jar: content/test_dialog.xul (chrome/test_dialog.xul) content/test_nested_iframe.xul (chrome/test_nested_iframe.xul) content/test.xul (chrome/test.xul) + content/PerTestCoverageUtils.jsm (../../tools/code-coverage/PerTestCoverageUtils.jsm) #endif diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py index 54802faf83b1..6f1d35a749b7 100644 --- a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py +++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py @@ -29,7 +29,8 @@ from .protocol import (AssertsProtocolPart, SelectorProtocolPart, ClickProtocolPart, SendKeysProtocolPart, - TestDriverProtocolPart) + TestDriverProtocolPart, + CoverageProtocolPart) from ..testrunner import Stop from ..webdriver_server import GeckoDriverServer @@ -371,6 +372,53 @@ class MarionetteTestDriverProtocolPart(TestDriverProtocolPart): self.parent.base.execute_script("window.postMessage(%s, '*')" % json.dumps(obj)) +class MarionetteCoverageProtocolPart(CoverageProtocolPart): + def setup(self): + self.marionette = self.parent.marionette + script = """ + ChromeUtils.import("chrome://marionette/content/PerTestCoverageUtils.jsm"); + return PerTestCoverageUtils.enabled; + """ + with self.marionette.using_context(self.marionette.CONTEXT_CHROME): + self.is_enabled = self.marionette.execute_script(script) + + def reset(self): + script = """ + var callback = arguments[arguments.length - 1]; + + ChromeUtils.import("chrome://marionette/content/PerTestCoverageUtils.jsm"); + PerTestCoverageUtils.beforeTest().then(callback, callback); + """ + with self.marionette.using_context(self.marionette.CONTEXT_CHROME): + try: + error = self.marionette.execute_async_script(script) + if error is not None: + raise Exception('Failure while resetting counters: %s' % json.dumps(error)) + except (errors.MarionetteException, socket.error): + # This usually happens if the process crashed + pass + + def dump(self): + if len(self.marionette.window_handles): + handle = self.marionette.window_handles[0] + self.marionette.switch_to_window(handle) + + script = """ + var callback = arguments[arguments.length - 1]; + + ChromeUtils.import("chrome://marionette/content/PerTestCoverageUtils.jsm"); + PerTestCoverageUtils.afterTest().then(callback, callback); + """ + with self.marionette.using_context(self.marionette.CONTEXT_CHROME): + try: + error = self.marionette.execute_async_script(script) + if error is not None: + raise Exception('Failure while dumping counters: %s' % json.dumps(error)) + except (errors.MarionetteException, socket.error): + # This usually happens if the process crashed + pass + + class MarionetteProtocol(Protocol): implements = [MarionetteBaseProtocolPart, MarionetteTestharnessProtocolPart, @@ -380,7 +428,8 @@ class MarionetteProtocol(Protocol): MarionetteClickProtocolPart, MarionetteSendKeysProtocolPart, MarionetteTestDriverProtocolPart, - MarionetteAssertsProtocolPart] + MarionetteAssertsProtocolPart, + MarionetteCoverageProtocolPart] def __init__(self, executor, browser, capabilities=None, timeout_multiplier=1, e10s=True): do_delayed_imports() @@ -599,6 +648,9 @@ class MarionetteTestharnessExecutor(TestharnessExecutor): else: timeout_ms = "null" + if self.protocol.coverage.is_enabled: + self.protocol.coverage.reset() + format_map = {"abs_url": url, "url": strip_server(url), "window_id": self.window_id, @@ -621,6 +673,10 @@ class MarionetteTestharnessExecutor(TestharnessExecutor): done, rv = handler(result) if done: break + + if self.protocol.coverage.is_enabled: + self.protocol.coverage.dump() + return rv @@ -689,8 +745,14 @@ class MarionetteRefTestExecutor(RefTestExecutor): self.protocol.base.set_window(self.protocol.marionette.window_handles[-1]) self.has_window = True + if self.protocol.coverage.is_enabled: + self.protocol.coverage.reset() + result = self.implementation.run_test(test) + if self.protocol.coverage.is_enabled: + self.protocol.coverage.dump() + if self.debug: assertion_count = self.protocol.asserts.get() if "extra" not in result: diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py index 2e48162e0fd3..71fc3c9a8f65 100644 --- a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py +++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py @@ -303,3 +303,20 @@ class AssertsProtocolPart(ProtocolPart): def get(self): """Get a count of assertions since the last browser start""" pass + + +class CoverageProtocolPart(ProtocolPart): + """Protocol part for collecting per-test coverage data.""" + __metaclass__ = ABCMeta + + name = "coverage" + + @abstractmethod + def reset(self): + """Reset coverage counters""" + pass + + @abstractmethod + def dump(self): + """Dump coverage counters""" + pass From bd3f040c33fccf759c3d5b3d29a698869d309150 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Thu, 19 Jul 2018 11:56:43 +0200 Subject: [PATCH 12/37] Bug 1476574 - Enable reset/dump for wpt and cleanup harness code for supporting reset/dump now that it is supported by all test suites. r=jmaher --HG-- extra : source : c1b3950cc12f1642ad60338d2a8701e2b60131ea extra : intermediate-source : 4ea7997194e9f995bb0b1e434524a0ea5596d758 extra : histedit_source : fffe672f456fd7c503792d2a3e99184c14bfd36c --- .../mozilla/testing/codecoverage.py | 31 +++++++++++++------ .../mozharness/scripts/desktop_unittest.py | 21 +++---------- .../mozharness/scripts/web_platform_tests.py | 8 +++-- 3 files changed, 31 insertions(+), 29 deletions(-) diff --git a/testing/mozharness/mozharness/mozilla/testing/codecoverage.py b/testing/mozharness/mozharness/mozilla/testing/codecoverage.py index 129d87b60f41..7390dd7102fa 100644 --- a/testing/mozharness/mozharness/mozilla/testing/codecoverage.py +++ b/testing/mozharness/mozharness/mozilla/testing/codecoverage.py @@ -204,16 +204,20 @@ class CodeCoverageMixin(SingleTestMixin): def coverage_args(self): return [] - def set_coverage_env(self, env): + def set_coverage_env(self, env, is_baseline_test=False): # Set the GCOV directory. - gcov_dir = tempfile.mkdtemp() - env['GCOV_PREFIX'] = gcov_dir + self.gcov_dir = tempfile.mkdtemp() + env['GCOV_PREFIX'] = self.gcov_dir + + # Set the GCOV directory where counters will be dumped in per-test mode. + # Resetting/dumping is only available on Linux for the time being + # (https://bugzilla.mozilla.org/show_bug.cgi?id=1471576). + if self.per_test_coverage and not is_baseline_test and self._is_linux(): + env['GCOV_RESULTS_DIR'] = tempfile.mkdtemp() # Set JSVM directory. - jsvm_dir = tempfile.mkdtemp() - env['JS_CODE_COVERAGE_OUTPUT_DIR'] = jsvm_dir - - return (gcov_dir, jsvm_dir) + self.jsvm_dir = tempfile.mkdtemp() + env['JS_CODE_COVERAGE_OUTPUT_DIR'] = self.jsvm_dir @PreScriptAction('run-tests') def _set_gcov_prefix(self, action): @@ -223,7 +227,7 @@ class CodeCoverageMixin(SingleTestMixin): if self.per_test_coverage: return - self.gcov_dir, self.jsvm_dir = self.set_coverage_env(os.environ) + self.set_coverage_env(os.environ) def parse_coverage_artifacts(self, gcov_dir, @@ -286,9 +290,11 @@ class CodeCoverageMixin(SingleTestMixin): else: return grcov_output_file, jsvm_output_file - def add_per_test_coverage_report(self, gcov_dir, jsvm_dir, suite, test): + def add_per_test_coverage_report(self, env, suite, test): + gcov_dir = env['GCOV_RESULTS_DIR'] if 'GCOV_RESULTS_DIR' in env else self.gcov_dir + grcov_file = self.parse_coverage_artifacts( - gcov_dir, jsvm_dir, merge=True, output_format='coveralls', + gcov_dir, self.jsvm_dir, merge=True, output_format='coveralls', filter_covered=True, ) @@ -300,6 +306,11 @@ class CodeCoverageMixin(SingleTestMixin): assert test not in self.per_test_reports[suite] self.per_test_reports[suite][test] = report_file + if 'GCOV_RESULTS_DIR' in env: + # In this case, parse_coverage_artifacts has removed GCOV_RESULTS_DIR + # so we need to remove GCOV_PREFIX. + shutil.rmtree(self.gcov_dir) + def is_covered(self, sf): # For C/C++ source files, we can consider a file as being uncovered # when all its source lines are uncovered. diff --git a/testing/mozharness/scripts/desktop_unittest.py b/testing/mozharness/scripts/desktop_unittest.py index e18937a7a6c1..c699c1731655 100755 --- a/testing/mozharness/scripts/desktop_unittest.py +++ b/testing/mozharness/scripts/desktop_unittest.py @@ -15,7 +15,6 @@ import re import sys import copy import shutil -import tempfile import glob import imp @@ -889,28 +888,18 @@ class DesktopUnittest(TestingMixin, MercurialScript, MozbaseMixin, final_cmd = copy.copy(cmd) final_cmd.extend(per_test_args) + final_env = copy.copy(env) + if self.per_test_coverage: - gcov_dir, jsvm_dir = self.set_coverage_env(env) - # Per-test reset/dump is only supported on Linux for the time being. - if not is_baseline_test and \ - self._is_linux(): - env['GCOV_RESULTS_DIR'] = tempfile.mkdtemp() + self.set_coverage_env(final_env) return_code = self.run_command(final_cmd, cwd=dirs['abs_work_dir'], output_timeout=cmd_timeout, output_parser=parser, - env=env) + env=final_env) if self.per_test_coverage: - self.add_per_test_coverage_report( - env['GCOV_RESULTS_DIR'] if 'GCOV_RESULTS_DIR' in env else gcov_dir, - jsvm_dir, - suite, - per_test_args[-1] - ) - if 'GCOV_RESULTS_DIR' in env: - shutil.rmtree(gcov_dir) - del env['GCOV_RESULTS_DIR'] + self.add_per_test_coverage_report(final_env, suite, per_test_args[-1]) # mochitest, reftest, and xpcshell suites do not return # appropriate return codes. Therefore, we must parse the output diff --git a/testing/mozharness/scripts/web_platform_tests.py b/testing/mozharness/scripts/web_platform_tests.py index b0610a1e5c83..e5f5ccc3eee3 100755 --- a/testing/mozharness/scripts/web_platform_tests.py +++ b/testing/mozharness/scripts/web_platform_tests.py @@ -377,17 +377,19 @@ class WebPlatformTest(TestingMixin, MercurialScript, CodeCoverageMixin): cmd = self._query_cmd(test_types) cmd.extend(per_test_args) + final_env = copy.copy(env) + if self.per_test_coverage: - gcov_dir, jsvm_dir = self.set_coverage_env(env) + self.set_coverage_env(final_env, is_baseline_test) return_code = self.run_command(cmd, cwd=dirs['abs_work_dir'], output_timeout=1000, output_parser=parser, - env=env) + env=final_env) if self.per_test_coverage: - self.add_per_test_coverage_report(gcov_dir, jsvm_dir, suite, per_test_args[-1]) + self.add_per_test_coverage_report(final_env, suite, per_test_args[-1]) tbpl_status, log_level, summary = parser.evaluate_parser(return_code, previous_summary=summary) From 4d3e4a0894ac66e41c12bbcdcc1169b160e1d79e Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Thu, 19 Jul 2018 14:27:42 -0700 Subject: [PATCH 13/37] Bug 1476828: Part 1 - Reduce the default thread manager thread stack size. r=erahm f=froydnj MozReview-Commit-ID: cRED4r0xAb --HG-- extra : rebase_source : 1d648a5c91e802554f044c395a39be8e7f30419b --- xpcom/threads/nsIThreadManager.idl | 46 +++++++++++++++--------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/xpcom/threads/nsIThreadManager.idl b/xpcom/threads/nsIThreadManager.idl index e347369a4f76..89d67081f822 100644 --- a/xpcom/threads/nsIThreadManager.idl +++ b/xpcom/threads/nsIThreadManager.idl @@ -29,32 +29,30 @@ interface nsIThreadManager : nsISupports { /** * Default number of bytes reserved for a thread's stack, if no stack size - * is specified in newThread(). 0 means use platform default. - */ - const unsigned long DEFAULT_STACK_SIZE = 0; - -%{C++ - /* DEFAULT_STACK_SIZE can be a little overzealous for many platforms. On - * Linux and OS X, for instance, the default thread stack size is whatever - * getrlimit(RLIMIT_STACK) returns, which is often set at 8MB. The - * default on Windows is 1MB, which is a little more reasonable. But - * for thread pools, each individual thread often doesn't need that much - * stack space. + * is specified in newThread(). * - * We therefore have a separate setting for a reasonable stack size for - * a thread pool worker thread. + * Defaults can be a little overzealous for many platforms. + * + * On Linux and OS X, for instance, the default thread stack size is whatever + * getrlimit(RLIMIT_STACK) returns, which is often set at 8MB. Or, on Linux, + * if the stack size is unlimited, we fall back to 2MB. This causes particular + * problems on Linux, which allocates 2MB huge VM pages, and will often + * immediately allocate them for any stacks which are 2MB or larger. + * + * The default on Windows is 1MB, which is a little more reasonable. But the + * vast majority of our threads don't need anywhere near that much space. + * + * ASan and TSan builds, however, often need a bit more, so give them a the + * platform default. */ +%{C++ #if defined(MOZ_ASAN) || defined(MOZ_TSAN) - // Use the system default in ASAN builds, because the default is assumed - // to be larger than the size we want to use and is hopefully sufficient - // for ASAN. - static const uint32_t kThreadPoolStackSize = DEFAULT_STACK_SIZE; -#elif defined(XP_WIN) || defined(XP_MACOSX) || defined(LINUX) - static const uint32_t kThreadPoolStackSize = (256 * 1024); + static constexpr uint32_t DEFAULT_STACK_SIZE = 0; #else - // All other platforms use their system default. - static const uint32_t kThreadPoolStackSize = DEFAULT_STACK_SIZE; + static constexpr uint32_t DEFAULT_STACK_SIZE = 256 * 1024; #endif + + static const uint32_t kThreadPoolStackSize = DEFAULT_STACK_SIZE; %} /** @@ -63,7 +61,8 @@ interface nsIThreadManager : nsISupports * @param creationFlags * Reserved for future use. Pass 0. * @param stackSize - * Number of bytes to reserve for the thread's stack. + * Number of bytes to reserve for the thread's stack. 0 means use platform + * default. * * @returns * The newly created nsIThread object. @@ -77,7 +76,8 @@ interface nsIThreadManager : nsISupports * The name of the thread. Passing an empty name is equivalent to * calling newThread(0, stackSize), i.e. the thread will not be named. * @param stackSize - * Number of bytes to reserve for the thread's stack. + * Number of bytes to reserve for the thread's stack. 0 means use platform + * default. * * @returns * The newly created nsIThread object. From 41c3c1071d6a5812e939f53076af535ec9d5f4ae Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Wed, 18 Jul 2018 17:52:39 -0700 Subject: [PATCH 14/37] Bug 1476828: Part 2 - Slightly decrease the DOM Worker thread stack size. r=erahm MozReview-Commit-ID: GEDanj1dAC6 --HG-- extra : rebase_source : 212d94df94f4243fcb58f940d4a023aa7014c552 --- dom/workers/WorkerThread.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dom/workers/WorkerThread.cpp b/dom/workers/WorkerThread.cpp index 5fbca6790a27..f060d459d4f0 100644 --- a/dom/workers/WorkerThread.cpp +++ b/dom/workers/WorkerThread.cpp @@ -30,7 +30,12 @@ namespace { // The C stack size. We use the same stack size on all platforms for // consistency. -const uint32_t kWorkerStackSize = 256 * sizeof(size_t) * 1024; +// +// Note: Our typical equation of 256 machine words works out to 2MB on 64-bit +// platforms. Since that works out to the size of a VM huge page, that can +// sometimes lead to an OS allocating an entire huge page for the stack at once. +// To avoid this, we subtract the size of 2 pages, to be safe. +const uint32_t kWorkerStackSize = 256 * sizeof(size_t) * 1024 - 8192; } // namespace From 8cfe473f10a506d1eddeef256313cf000aa1872d Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Thu, 5 Jul 2018 15:26:37 -0700 Subject: [PATCH 15/37] Bug 1473631: Part 15 - Use a single pref observer for all telemetry environment preferences. r=gfritzsche MozReview-Commit-ID: dGJDjUP4Vm --HG-- extra : rebase_source : d7e801b2d5b3b0e1b390a78c835e445eda1d9ac6 extra : amend_source : 342b24fec8ee3cea9677080f588593405b233734 --- .../components/telemetry/TelemetryEnvironment.jsm | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/toolkit/components/telemetry/TelemetryEnvironment.jsm b/toolkit/components/telemetry/TelemetryEnvironment.jsm index f2267ccb48cb..ed8ad9e76f12 100644 --- a/toolkit/components/telemetry/TelemetryEnvironment.jsm +++ b/toolkit/components/telemetry/TelemetryEnvironment.jsm @@ -1124,11 +1124,7 @@ EnvironmentCache.prototype = { _startWatchingPrefs() { this._log.trace("_startWatchingPrefs - " + this._watchedPrefs); - for (let [pref, options] of this._watchedPrefs) { - if (!("requiresRestart" in options) || !options.requiresRestart) { - Services.prefs.addObserver(pref, this, true); - } - } + Services.prefs.addObserver("", this, true); }, _onPrefChanged(aData) { @@ -1144,11 +1140,7 @@ EnvironmentCache.prototype = { _stopWatchingPrefs() { this._log.trace("_stopWatchingPrefs"); - for (let [pref, options] of this._watchedPrefs) { - if (!("requiresRestart" in options) || !options.requiresRestart) { - Services.prefs.removeObserver(pref, this); - } - } + Services.prefs.removeObserver("", this); }, _addObservers() { @@ -1220,7 +1212,8 @@ EnvironmentCache.prototype = { this._updateDefaultBrowser(); break; case PREF_CHANGED_TOPIC: - if (this._watchedPrefs.has(aData)) { + let options = this._watchedPrefs.get(aData); + if (options && !options.requiresRestart) { this._onPrefChanged(aData); } break; From d43db51b53c1ae54857d08166ff95741b0beb98a Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Sat, 7 Jul 2018 20:15:45 -0700 Subject: [PATCH 16/37] Bug 1474155: Part 3 - Move WebChannel message listeners to a separate JSM. r=mconley MozReview-Commit-ID: AHCTFDBnChn --HG-- rename : toolkit/content/browser-content.js => toolkit/modules/WebChannelContent.jsm extra : rebase_source : cd39745775f1b8ad94f9563f75faa9cb7d5249f1 extra : amend_source : 13950bf852f9bbe2534135c879bb639ea26ad5b7 --- toolkit/content/browser-content.js | 90 ++------------------- toolkit/modules/WebChannelContent.jsm | 110 ++++++++++++++++++++++++++ toolkit/modules/moz.build | 1 + 3 files changed, 117 insertions(+), 84 deletions(-) create mode 100644 toolkit/modules/WebChannelContent.jsm diff --git a/toolkit/content/browser-content.js b/toolkit/content/browser-content.js index 9e067da4fc91..005dcff3bd93 100644 --- a/toolkit/content/browser-content.js +++ b/toolkit/content/browser-content.js @@ -40,6 +40,9 @@ XPCOMUtils.defineLazyProxy(this, "PopupBlocking", () => { XPCOMUtils.defineLazyProxy(this, "SelectionSourceContent", "resource://gre/modules/SelectionSourceContent.jsm"); +XPCOMUtils.defineLazyProxy(this, "WebChannelContent", + "resource://gre/modules/WebChannelContent.jsm"); + XPCOMUtils.defineLazyProxy(this, "DateTimePickerContent", () => { let tmp = {}; ChromeUtils.import("resource://gre/modules/DateTimePickerContent.jsm", tmp); @@ -285,90 +288,9 @@ var FindBar = { }; FindBar.init(); -let WebChannelMessageToChromeListener = { - // Preference containing the list (space separated) of origins that are - // allowed to send non-string values through a WebChannel, mainly for - // backwards compatability. See bug 1238128 for more information. - URL_WHITELIST_PREF: "webchannel.allowObject.urlWhitelist", - - // Cached list of whitelisted principals, we avoid constructing this if the - // value in `_lastWhitelistValue` hasn't changed since we constructed it last. - _cachedWhitelist: [], - _lastWhitelistValue: "", - - init() { - addEventListener("WebChannelMessageToChrome", e => { - this._onMessageToChrome(e); - }, true, true); - }, - - _getWhitelistedPrincipals() { - let whitelist = Services.prefs.getCharPref(this.URL_WHITELIST_PREF); - if (whitelist != this._lastWhitelistValue) { - let urls = whitelist.split(/\s+/); - this._cachedWhitelist = urls.map(origin => - Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(origin)); - } - return this._cachedWhitelist; - }, - - _onMessageToChrome(e) { - // If target is window then we want the document principal, otherwise fallback to target itself. - let principal = e.target.nodePrincipal ? e.target.nodePrincipal : e.target.document.nodePrincipal; - - if (e.detail) { - if (typeof e.detail != "string") { - // Check if the principal is one of the ones that's allowed to send - // non-string values for e.detail. They're whitelisted by site origin, - // so we compare on originNoSuffix in order to avoid other origin attributes - // that are not relevant here, such as containers or private browsing. - let objectsAllowed = this._getWhitelistedPrincipals().some(whitelisted => - principal.originNoSuffix == whitelisted.originNoSuffix); - if (!objectsAllowed) { - Cu.reportError("WebChannelMessageToChrome sent with an object from a non-whitelisted principal"); - return; - } - } - sendAsyncMessage("WebChannelMessageToChrome", e.detail, { eventTarget: e.target }, principal); - } else { - Cu.reportError("WebChannel message failed. No message detail."); - } - } -}; - -WebChannelMessageToChromeListener.init(); - -// This should be kept in sync with /browser/base/content.js. -// Add message listener for "WebChannelMessageToContent" messages from chrome scripts. -addMessageListener("WebChannelMessageToContent", function(e) { - if (e.data) { - // e.objects.eventTarget will be defined if sending a response to - // a WebChannelMessageToChrome event. An unsolicited send - // may not have an eventTarget defined, in this case send to the - // main content window. - let eventTarget = e.objects.eventTarget || content; - - // Use nodePrincipal if available, otherwise fallback to document principal. - let targetPrincipal = eventTarget instanceof Ci.nsIDOMWindow ? eventTarget.document.nodePrincipal : eventTarget.nodePrincipal; - - if (e.principal.subsumes(targetPrincipal)) { - // If eventTarget is a window, use it as the targetWindow, otherwise - // find the window that owns the eventTarget. - let targetWindow = eventTarget instanceof Ci.nsIDOMWindow ? eventTarget : eventTarget.ownerGlobal; - - eventTarget.dispatchEvent(new targetWindow.CustomEvent("WebChannelMessageToContent", { - detail: Cu.cloneInto({ - id: e.data.id, - message: e.data.message, - }, targetWindow), - })); - } else { - Cu.reportError("WebChannel message failed. Principal mismatch."); - } - } else { - Cu.reportError("WebChannel message failed. No message data."); - } -}); +addEventListener("WebChannelMessageToChrome", WebChannelContent, + true, true); +addMessageListener("WebChannelMessageToContent", WebChannelContent); var AudioPlaybackListener = { QueryInterface: ChromeUtils.generateQI([Ci.nsIObserver]), diff --git a/toolkit/modules/WebChannelContent.jsm b/toolkit/modules/WebChannelContent.jsm new file mode 100644 index 000000000000..2defd44942a3 --- /dev/null +++ b/toolkit/modules/WebChannelContent.jsm @@ -0,0 +1,110 @@ +/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/* eslint no-unused-vars: ["error", {args: "none"}] */ + +var EXPORTED_SYMBOLS = ["WebChannelContent"]; + +ChromeUtils.import("resource://gre/modules/Services.jsm"); + +function getMessageManager(event) { + let window = Cu.getGlobalForObject(event.target); + + return window.document.docShell + .QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIContentFrameMessageManager); +} + +var WebChannelContent = { + // Preference containing the list (space separated) of origins that are + // allowed to send non-string values through a WebChannel, mainly for + // backwards compatability. See bug 1238128 for more information. + URL_WHITELIST_PREF: "webchannel.allowObject.urlWhitelist", + + // Cached list of whitelisted principals, we avoid constructing this if the + // value in `_lastWhitelistValue` hasn't changed since we constructed it last. + _cachedWhitelist: [], + _lastWhitelistValue: "", + + handleEvent(event) { + if (event.type === "WebChannelMessageToChrome") { + return this._onMessageToChrome(event); + } + return undefined; + }, + + receiveMessage(msg) { + if (msg.name === "WebChannelMessageToContent") { + return this._onMessageToContent(msg); + } + return undefined; + }, + + _getWhitelistedPrincipals() { + let whitelist = Services.prefs.getCharPref(this.URL_WHITELIST_PREF); + if (whitelist != this._lastWhitelistValue) { + let urls = whitelist.split(/\s+/); + this._cachedWhitelist = urls.map(origin => + Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(origin)); + } + return this._cachedWhitelist; + }, + + _onMessageToChrome(e) { + // If target is window then we want the document principal, otherwise fallback to target itself. + let principal = e.target.nodePrincipal ? e.target.nodePrincipal : e.target.document.nodePrincipal; + + if (e.detail) { + if (typeof e.detail != "string") { + // Check if the principal is one of the ones that's allowed to send + // non-string values for e.detail. They're whitelisted by site origin, + // so we compare on originNoSuffix in order to avoid other origin attributes + // that are not relevant here, such as containers or private browsing. + let objectsAllowed = this._getWhitelistedPrincipals().some(whitelisted => + principal.originNoSuffix == whitelisted.originNoSuffix); + if (!objectsAllowed) { + Cu.reportError("WebChannelMessageToChrome sent with an object from a non-whitelisted principal"); + return; + } + } + + let mm = getMessageManager(e); + + mm.sendAsyncMessage("WebChannelMessageToChrome", e.detail, { eventTarget: e.target }, principal); + } else { + Cu.reportError("WebChannel message failed. No message detail."); + } + }, + + _onMessageToContent(msg) { + if (msg.data) { + // msg.objects.eventTarget will be defined if sending a response to + // a WebChannelMessageToChrome event. An unsolicited send + // may not have an eventTarget defined, in this case send to the + // main content window. + let eventTarget = msg.objects.eventTarget || msg.target.content; + + // Use nodePrincipal if available, otherwise fallback to document principal. + let targetPrincipal = eventTarget instanceof Ci.nsIDOMWindow ? eventTarget.document.nodePrincipal : eventTarget.nodePrincipal; + + if (msg.principal.subsumes(targetPrincipal)) { + // If eventTarget is a window, use it as the targetWindow, otherwise + // find the window that owns the eventTarget. + let targetWindow = eventTarget instanceof Ci.nsIDOMWindow ? eventTarget : eventTarget.ownerGlobal; + + eventTarget.dispatchEvent(new targetWindow.CustomEvent("WebChannelMessageToContent", { + detail: Cu.cloneInto({ + id: msg.data.id, + message: msg.data.message, + }, targetWindow), + })); + } else { + Cu.reportError("WebChannel message failed. Principal mismatch."); + } + } else { + Cu.reportError("WebChannel message failed. No message data."); + } + }, +}; diff --git a/toolkit/modules/moz.build b/toolkit/modules/moz.build index 158b6a6de31a..cb931da6e6b4 100644 --- a/toolkit/modules/moz.build +++ b/toolkit/modules/moz.build @@ -253,6 +253,7 @@ EXTRA_JS_MODULES += [ 'Troubleshoot.jsm', 'UpdateUtils.jsm', 'WebChannel.jsm', + 'WebChannelContent.jsm', 'WindowDraggingUtils.jsm', 'ZipUtils.jsm', ] From 2162d7809755c6dc6e72f1eb8b2ec38d0762d35f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 19 Jul 2018 09:44:22 +1000 Subject: [PATCH 17/37] Bug 1476820 - Re-alphabetize the sections in StaticPrefList.h. r=glandium MozReview-Commit-ID: 3qPddiQY18N --HG-- extra : rebase_source : f1a9e7e76a2393c6096d7fe4f792a07fa52fe687 --- modules/libpref/init/StaticPrefList.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/modules/libpref/init/StaticPrefList.h b/modules/libpref/init/StaticPrefList.h index 66ef938a5ad6..3b9154b9d203 100644 --- a/modules/libpref/init/StaticPrefList.h +++ b/modules/libpref/init/StaticPrefList.h @@ -1141,17 +1141,7 @@ VARCACHE_PREF( PREF("preferences.allow.omt-write", bool, true) //--------------------------------------------------------------------------- -// View source prefs -//--------------------------------------------------------------------------- - -VARCACHE_PREF( - "view_source.editor.external", - view_source_editor_external, - bool, false -) - -//--------------------------------------------------------------------------- -// Anti-Tracking prefs +// Privacy prefs //--------------------------------------------------------------------------- VARCACHE_PREF( @@ -1173,6 +1163,16 @@ VARCACHE_PREF( uint32_t, 2592000 // 30 days (in seconds) ) +//--------------------------------------------------------------------------- +// View source prefs +//--------------------------------------------------------------------------- + +VARCACHE_PREF( + "view_source.editor.external", + view_source_editor_external, + bool, false +) + //--------------------------------------------------------------------------- // End of prefs //--------------------------------------------------------------------------- From fc1f4bb4ae3d61495ca499ee2ab5b8ef574ad5b5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 19 Jul 2018 10:43:29 +1000 Subject: [PATCH 18/37] Bug 1476820 - Convert some VarCache prefs in dom/security/ to use StaticPrefs. r=ckerschb Specifically: - "security.csp.enable" - "security.csp.experimentalEnabled" - "security.csp.enableStrictDynamic" - "security.csp.reporting.script-sample.max-length" - "security.csp.enable_violation_events" MozReview-Commit-ID: G1ie4ut9QaK --HG-- extra : rebase_source : d6b5a0e79eb7046a13a8b4fe957c82c11831c86c --- dom/base/nsDocument.cpp | 2 +- dom/html/HTMLMetaElement.cpp | 3 +- dom/security/nsCSPContext.cpp | 15 +--------- dom/security/nsCSPContext.h | 9 +++--- dom/security/nsCSPParser.cpp | 13 ++------- dom/security/nsCSPParser.h | 3 -- dom/security/nsCSPService.cpp | 13 ++++----- dom/security/nsCSPService.h | 1 - dom/workers/ScriptLoader.cpp | 2 +- modules/libpref/init/StaticPrefList.h | 40 +++++++++++++++++++++++++++ modules/libpref/init/all.js | 10 ------- parser/html/nsHtml5TreeOpExecutor.cpp | 2 +- 12 files changed, 59 insertions(+), 54 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 34b62f7d2069..d46e38500ad5 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -2843,7 +2843,7 @@ nsIDocument::InitCSP(nsIChannel* aChannel) { MOZ_ASSERT(!mScriptGlobalObject, "CSP must be initialized before mScriptGlobalObject is set!"); - if (!CSPService::sCSPEnabled) { + if (!StaticPrefs::security_csp_enable()) { MOZ_LOG(gCspPRLog, LogLevel::Debug, ("CSP is disabled, skipping CSP init for document %p", this)); return NS_OK; diff --git a/dom/html/HTMLMetaElement.cpp b/dom/html/HTMLMetaElement.cpp index f7871af045d8..5f52c6377f0d 100644 --- a/dom/html/HTMLMetaElement.cpp +++ b/dom/html/HTMLMetaElement.cpp @@ -94,7 +94,8 @@ HTMLMetaElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, nsContentUtils::ProcessViewportInfo(aDocument, content); } - if (CSPService::sCSPEnabled && aDocument && !aDocument->IsLoadedAsData() && + if (StaticPrefs::security_csp_enable() && aDocument && + !aDocument->IsLoadedAsData() && AttrValueIs(kNameSpaceID_None, nsGkAtoms::httpEquiv, nsGkAtoms::headerCSP, eIgnoreCase)) { // only accept if it appears diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index 54053ecf8849..9522c7d81864 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -320,25 +320,12 @@ NS_IMPL_ISUPPORTS_CI(nsCSPContext, nsIContentSecurityPolicy, nsISerializable) -int32_t nsCSPContext::sScriptSampleMaxLength; -bool nsCSPContext::sViolationEventsEnabled = false; - nsCSPContext::nsCSPContext() : mInnerWindowID(0) , mLoadingContext(nullptr) , mLoadingPrincipal(nullptr) , mQueueUpMessages(true) { - static bool sInitialized = false; - if (!sInitialized) { - Preferences::AddIntVarCache(&sScriptSampleMaxLength, - "security.csp.reporting.script-sample.max-length", - 40); - Preferences::AddBoolVarCache(&sViolationEventsEnabled, - "security.csp.enable_violation_events"); - sInitialized = true; - } - CSPCONTEXTLOG(("nsCSPContext::nsCSPContext")); } @@ -1201,7 +1188,7 @@ nsCSPContext::FireViolationEvent( Element* aTriggeringElement, const mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit) { - if (!sViolationEventsEnabled) { + if (!StaticPrefs::security_csp_enable_violation_events()) { return NS_OK; } diff --git a/dom/security/nsCSPContext.h b/dom/security/nsCSPContext.h index 35787d1ce266..99f7adca6865 100644 --- a/dom/security/nsCSPContext.h +++ b/dom/security/nsCSPContext.h @@ -9,6 +9,7 @@ #include "mozilla/dom/nsCSPUtils.h" #include "mozilla/dom/SecurityPolicyViolationEvent.h" +#include "mozilla/StaticPrefs.h" #include "nsDataHashtable.h" #include "nsIChannel.h" #include "nsIChannelEventSink.h" @@ -140,7 +141,9 @@ class nsCSPContext : public nsIContentSecurityPolicy static uint32_t ScriptSampleMaxLength() { - return std::max(sScriptSampleMaxLength, 0); + return std::max( + mozilla::StaticPrefs::security_csp_reporting_script_sample_max_length(), + 0); } private: @@ -165,10 +168,6 @@ class nsCSPContext : public nsIContentSecurityPolicy uint32_t aLineNumber, uint32_t aColumnNumber); - static int32_t sScriptSampleMaxLength; - - static bool sViolationEventsEnabled; - nsString mReferrer; uint64_t mInnerWindowID; // used for web console logging nsTArray mPolicies; diff --git a/dom/security/nsCSPParser.cpp b/dom/security/nsCSPParser.cpp index 34784dcede2e..4647fe6b39cf 100644 --- a/dom/security/nsCSPParser.cpp +++ b/dom/security/nsCSPParser.cpp @@ -6,6 +6,7 @@ #include "mozilla/ArrayUtils.h" #include "mozilla/Preferences.h" +#include "mozilla/StaticPrefs.h" #include "nsCOMPtr.h" #include "nsContentUtils.h" #include "nsCSPParser.h" @@ -61,8 +62,6 @@ static const char* const kStyle = "style"; static const char* const kScript = "script"; /* ===== nsCSPParser ==================== */ -bool nsCSPParser::sCSPExperimentalEnabled = false; -bool nsCSPParser::sStrictDynamicEnabled = false; nsCSPParser::nsCSPParser(policyTokens& aTokens, nsIURI* aSelfURI, @@ -84,12 +83,6 @@ nsCSPParser::nsCSPParser(policyTokens& aTokens, , mCSPContext(aCSPContext) , mDeliveredViaMetaTag(aDeliveredViaMetaTag) { - static bool initialized = false; - if (!initialized) { - initialized = true; - Preferences::AddBoolVarCache(&sCSPExperimentalEnabled, "security.csp.experimentalEnabled"); - Preferences::AddBoolVarCache(&sStrictDynamicEnabled, "security.csp.enableStrictDynamic"); - } CSPPARSERLOG(("nsCSPParser::nsCSPParser")); } @@ -488,7 +481,7 @@ nsCSPParser::keywordSource() if (CSP_IsKeyword(mCurToken, CSP_STRICT_DYNAMIC)) { // make sure strict dynamic is enabled - if (!sStrictDynamicEnabled) { + if (!StaticPrefs::security_csp_enableStrictDynamic()) { return nullptr; } if (!CSP_IsDirective(mCurDir[0], nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE)) { @@ -968,7 +961,7 @@ nsCSPParser::directiveName() // Check if it is a valid directive if (!CSP_IsValidDirective(mCurToken) || - (!sCSPExperimentalEnabled && + (!StaticPrefs::security_csp_experimentalEnabled() && CSP_IsDirective(mCurToken, nsIContentSecurityPolicy::REQUIRE_SRI_FOR))) { const char16_t* params[] = { mCurToken.get() }; logWarningErrorToConsole(nsIScriptError::warningFlag, "couldNotProcessUnknownDirective", diff --git a/dom/security/nsCSPParser.h b/dom/security/nsCSPParser.h index 16ac032acdf5..03d6c075657d 100644 --- a/dom/security/nsCSPParser.h +++ b/dom/security/nsCSPParser.h @@ -33,9 +33,6 @@ class nsCSPParser { nsCSPContext* aCSPContext, bool aDeliveredViaMetaTag); - static bool sCSPExperimentalEnabled; - static bool sStrictDynamicEnabled; - ~nsCSPParser(); diff --git a/dom/security/nsCSPService.cpp b/dom/security/nsCSPService.cpp index 24f0b08dacab..1722e19df6af 100644 --- a/dom/security/nsCSPService.cpp +++ b/dom/security/nsCSPService.cpp @@ -5,6 +5,8 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "mozilla/Logging.h" +#include "mozilla/Preferences.h" +#include "mozilla/StaticPrefs.h" #include "nsString.h" #include "nsCOMPtr.h" #include "nsIURI.h" @@ -16,21 +18,16 @@ #include "nsError.h" #include "nsIAsyncVerifyRedirectCallback.h" #include "nsAsyncRedirectVerifyHelper.h" -#include "mozilla/Preferences.h" #include "nsIScriptError.h" #include "nsContentUtils.h" #include "nsContentPolicyUtils.h" using namespace mozilla; -/* Keeps track of whether or not CSP is enabled */ -bool CSPService::sCSPEnabled = true; - static LazyLogModule gCspPRLog("CSP"); CSPService::CSPService() { - Preferences::AddBoolVarCache(&sCSPEnabled, "security.csp.enable"); } CSPService::~CSPService() @@ -152,7 +149,8 @@ CSPService::ShouldLoad(nsIURI *aContentLocation, // Please note, the correct way to opt-out of CSP using a custom // protocolHandler is to set one of the nsIProtocolHandler flags // that are whitelistet in subjectToCSP() - if (!sCSPEnabled || !subjectToCSP(aContentLocation, contentType)) { + if (!StaticPrefs::security_csp_enable() || + !subjectToCSP(aContentLocation, contentType)) { return NS_OK; } @@ -282,7 +280,8 @@ CSPService::AsyncOnChannelRedirect(nsIChannel *oldChannel, // protocolHandler is to set one of the nsIProtocolHandler flags // that are whitelistet in subjectToCSP() nsContentPolicyType policyType = loadInfo->InternalContentPolicyType(); - if (!sCSPEnabled || !subjectToCSP(newUri, policyType)) { + if (!StaticPrefs::security_csp_enable() || + !subjectToCSP(newUri, policyType)) { return NS_OK; } diff --git a/dom/security/nsCSPService.h b/dom/security/nsCSPService.h index 0eb991233c75..08f2b6eddf16 100644 --- a/dom/security/nsCSPService.h +++ b/dom/security/nsCSPService.h @@ -26,7 +26,6 @@ public: NS_DECL_NSICHANNELEVENTSINK CSPService(); - static bool sCSPEnabled; protected: virtual ~CSPService(); diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index fbc25d782810..0a8b485beae0 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -1259,7 +1259,7 @@ private: nsCOMPtr csp = mWorkerPrivate->GetCSP(); // We did inherit CSP in bug 1223647. If we do not already have a CSP, we // should get it from the HTTP headers on the worker script. - if (CSPService::sCSPEnabled) { + if (StaticPrefs::security_csp_enable()) { if (!csp) { rv = mWorkerPrivate->SetCSPFromHeaderValues(tCspHeaderValue, tCspROHeaderValue); diff --git a/modules/libpref/init/StaticPrefList.h b/modules/libpref/init/StaticPrefList.h index 3b9154b9d203..2a05da0efb70 100644 --- a/modules/libpref/init/StaticPrefList.h +++ b/modules/libpref/init/StaticPrefList.h @@ -1163,6 +1163,46 @@ VARCACHE_PREF( uint32_t, 2592000 // 30 days (in seconds) ) +//--------------------------------------------------------------------------- +// Security prefs +//--------------------------------------------------------------------------- + +VARCACHE_PREF( + "security.csp.enable", + security_csp_enable, + bool, true +) + +VARCACHE_PREF( + "security.csp.experimentalEnabled", + security_csp_experimentalEnabled, + bool, false +) + +VARCACHE_PREF( + "security.csp.enableStrictDynamic", + security_csp_enableStrictDynamic, + bool, true +) + +#ifdef NIGHTLY_BUILD +# define PREF_VALUE true +#else +# define PREF_VALUE false +#endif +VARCACHE_PREF( + "security.csp.enable_violation_events", + security_csp_enable_violation_events, + bool, PREF_VALUE +) +#undef PREF_VALUE + +VARCACHE_PREF( + "security.csp.reporting.script-sample.max-length", + security_csp_reporting_script_sample_max_length, + int32_t, 40 +) + //--------------------------------------------------------------------------- // View source prefs //--------------------------------------------------------------------------- diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 91b95acb7038..85825efdf666 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -2518,21 +2518,11 @@ pref("security.directory", ""); pref("security.dialog_enable_delay", 1000); pref("security.notification_enable_delay", 500); -pref("security.csp.enable", true); -pref("security.csp.experimentalEnabled", false); -pref("security.csp.enableStrictDynamic", true); - #if defined(DEBUG) && !defined(ANDROID) // about:welcome has been added until Bug 1448359 is fixed at which time home, newtab, and welcome will all be removed. pref("csp.content_privileged_about_uris_without_csp", "blank,home,newtab,printpreview,srcdoc,welcome"); #endif -#ifdef NIGHTLY_BUILD -pref("security.csp.enable_violation_events", true); -#else -pref("security.csp.enable_violation_events", false); -#endif - // Default Content Security Policy to apply to signed contents. pref("security.signed_content.CSP.default", "script-src 'self'; style-src 'self'"); diff --git a/parser/html/nsHtml5TreeOpExecutor.cpp b/parser/html/nsHtml5TreeOpExecutor.cpp index 5e02755ef9ca..09418be30617 100644 --- a/parser/html/nsHtml5TreeOpExecutor.cpp +++ b/parser/html/nsHtml5TreeOpExecutor.cpp @@ -1135,7 +1135,7 @@ nsHtml5TreeOpExecutor::SetSpeculationReferrerPolicy( void nsHtml5TreeOpExecutor::AddSpeculationCSP(const nsAString& aCSP) { - if (!CSPService::sCSPEnabled) { + if (!StaticPrefs::security_csp_enable()) { return; } From bfdb43b04dcb2f1650fb238317e4ce54acbc4ba5 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Wed, 18 Jul 2018 15:22:21 -0700 Subject: [PATCH 19/37] Bug 1476383 - cx->pod_callocCanGC can set an exception when succeeding, r=pbone --HG-- extra : rebase_source : e46376e318e4b7880a2b09c8e26bd1d406fe084b extra : source : c04c0005e4857d5cf9bbba9be845ae175e17963b --- .../non262/regress/regress-1476383-calloc-exc.js | 15 +++++++++++++++ js/src/vm/JSContext.h | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 js/src/tests/non262/regress/regress-1476383-calloc-exc.js diff --git a/js/src/tests/non262/regress/regress-1476383-calloc-exc.js b/js/src/tests/non262/regress/regress-1476383-calloc-exc.js new file mode 100644 index 000000000000..72b8c1df6ffc --- /dev/null +++ b/js/src/tests/non262/regress/regress-1476383-calloc-exc.js @@ -0,0 +1,15 @@ +if (!this.oomTest) { + this.reportCompare && reportCompare(true, true); + quit(0); +} + +let lfPreamble = ` +`; +oomTest(new Function(`var TOTAL_MEMORY = 50 * 1024 * 1024; +HEAP = IHEAP = new Int32Array(TOTAL_MEMORY); +function __Z9makeFastaI10RandomizedEvPKcS2_jRT_(\$id, \$desc, \$N, \$output) +{ +} +`)); + +this.reportCompare && reportCompare(true, true); diff --git a/js/src/vm/JSContext.h b/js/src/vm/JSContext.h index 59af137b973b..9eeb5bb634c9 100644 --- a/js/src/vm/JSContext.h +++ b/js/src/vm/JSContext.h @@ -177,7 +177,7 @@ struct JSContext : public JS::RootingContext, */ template T* pod_callocCanGC(size_t numElems, arena_id_t arena = js::MallocArena) { - T* p = pod_calloc(numElems, arena); + T* p = maybe_pod_calloc(numElems, arena); if (MOZ_LIKELY(!!p)) return p; size_t bytes; From 848dfa9d92f5fdbc6acc07fdfb57432ea935578e Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Fri, 20 Jul 2018 03:31:39 +0200 Subject: [PATCH 20/37] Bug 1471339 - Use Clang 7 for opt Linux ccov builds too. r=me --HG-- extra : rebase_source : 75edec2c4ccccee9864dfe22dccf5c798841a861 --- taskcluster/ci/build/linux.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taskcluster/ci/build/linux.yml b/taskcluster/ci/build/linux.yml index 943a316e554f..54fc52518c54 100644 --- a/taskcluster/ci/build/linux.yml +++ b/taskcluster/ci/build/linux.yml @@ -1000,7 +1000,7 @@ linux64-ccov/opt: tooltool-downloads: public need-xvfb: true toolchains: - - linux64-clang + - linux64-clang-7 - linux64-rust - linux64-gcc - linux64-sccache From ea1b68d0ac1d8b9bcff741de2aedb7281a990cfb Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Tue, 17 Jul 2018 21:12:53 -0700 Subject: [PATCH 21/37] Remove some excess qualification in various tokenizing-related template class instantiations. No bug, r=me as trivial ("if it compiles, it's good") --HG-- extra : rebase_source : 90f8645c28f36df69b9c27a38b96fcdfb1d622f8 --- js/src/frontend/TokenStream.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/js/src/frontend/TokenStream.cpp b/js/src/frontend/TokenStream.cpp index 474745d5f294..76433e182e03 100644 --- a/js/src/frontend/TokenStream.cpp +++ b/js/src/frontend/TokenStream.cpp @@ -2615,21 +2615,21 @@ TokenKindToString(TokenKind tt) } #endif -template class frontend::TokenStreamCharsBase; -template class frontend::TokenStreamCharsBase; +template class TokenStreamCharsBase; +template class TokenStreamCharsBase; -template class frontend::TokenStreamChars; -template class frontend::TokenStreamSpecific; +template class TokenStreamChars; +template class TokenStreamSpecific; template class -frontend::TokenStreamChars>>; +TokenStreamChars>>; template class -frontend::TokenStreamChars>>; +TokenStreamChars>>; template class -frontend::TokenStreamSpecific>>; +TokenStreamSpecific>>; template class -frontend::TokenStreamSpecific>>; +TokenStreamSpecific>>; } // namespace frontend From cfce8c373ba0d90fec51f76e06c5bffdf1fb7f1f Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Tue, 17 Jul 2018 21:00:55 -0700 Subject: [PATCH 22/37] Bug 1476866 - Fix the TokenStreamChars::getNonAsciiCodePoint doc comment to be clearer about |lead| being non-ASCII but still potentially (for UTF-16) a full code point. r=arai --HG-- extra : rebase_source : 1eecf289078b6a70091b95f348221851208eee39 --- js/src/frontend/TokenStream.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/js/src/frontend/TokenStream.h b/js/src/frontend/TokenStream.h index 538ccae09afc..c2af44dc90a2 100644 --- a/js/src/frontend/TokenStream.h +++ b/js/src/frontend/TokenStream.h @@ -1503,16 +1503,17 @@ class TokenStreamChars } /** - * Given a just-consumed non-ASCII code unit (and maybe point) |lead|, - * consume a full code point or LineTerminatorSequence (normalizing it to - * '\n') and store it in |*codePoint|. Return true on success, otherwise - * return false and leave |*codePoint| undefined on failure. + * Given a just-consumed non-ASCII code unit |lead| (which may also be a + * full code point, for UTF-16), consume a full code point or + * LineTerminatorSequence (normalizing it to '\n') and store it in + * |*codePoint|. Return true on success, otherwise return false and leave + * |*codePoint| undefined on failure. * * If a LineTerminatorSequence was consumed, also update line/column info. * * This may change the current |sourceUnits| offset. */ - MOZ_MUST_USE bool getNonAsciiCodePoint(int32_t lead, int32_t* cp); + MOZ_MUST_USE bool getNonAsciiCodePoint(int32_t lead, int32_t* codePoint); /** * Unget a full code point (ASCII or not) without altering line/column From 344a84f8398c7d01c011cb69f2a76653b5f71401 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Fri, 6 Jul 2018 17:12:53 -0700 Subject: [PATCH 23/37] Bug 1476866 - Add a SpecializedTokenStreamCharsBase class to the hierarchy as a clear collection point for functions entirely specific to UTF-8 or UTF-16 tokenizing only. r=arai --HG-- extra : rebase_source : c8fec271374098b7fb7de882cb478f7455f1d53e --- js/src/frontend/TokenStream.h | 64 +++++++++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/js/src/frontend/TokenStream.h b/js/src/frontend/TokenStream.h index c2af44dc90a2..463fe299c95c 100644 --- a/js/src/frontend/TokenStream.h +++ b/js/src/frontend/TokenStream.h @@ -60,23 +60,41 @@ * for it. (ParserBase isn't the only instance of this, but it's certainly the * biggest case of it.) Ergo, TokenStreamAnyChars. * - * == TokenStreamCharsBase → ∅ == + * == TokenStreamCharsShared → ∅ == * - * Certain data structures in tokenizing are character-type-specific: + * Some functionality has meaning independent of character type, yet has no use + * *unless* you know the character type in actual use. It *could* live in + * TokenStreamAnyChars, but it makes more sense to live in a separate class + * that character-aware token information can simply inherit. + * + * This class currently exists only to contain a char16_t buffer, transiently + * used to accumulate strings in tricky cases that can't just be read directly + * from source text. It's not used outside character-aware tokenizing, so it + * doesn't make sense in TokenStreamAnyChars. + * + * == TokenStreamCharsBase → TokenStreamCharsShared == + * + * Certain data structures in tokenizing are character-type-specific: namely, * the various pointers identifying the source text (including current offset - * and end) , and the temporary vector into which characters are read/written - * in certain cases (think writing out the actual codepoints identified by an - * identifier containing a Unicode escape, to create the atom for the - * identifier: |a\u0062c| versus |abc|, for example). + * and end). * * Additionally, some functions operating on this data are defined the same way - * no matter what character type you have -- the offset being |offset - start| - * no matter whether those two variables are single- or double-byte pointers. + * no matter what character type you have (e.g. current offset in code units + * into the source text) or share a common interface regardless of character + * type (e.g. consume the next code unit if it has a given value). * * All such functionality lives in TokenStreamCharsBase. * + * == SpecializedTokenStreamCharsBase → TokenStreamCharsBase == + * + * Certain tokenizing functionality is specific to a single character type. + * For example, JS's UTF-16 encoding recognizes no coding errors, because lone + * surrogates are not an error; but a UTF-8 encoding must recognize a variety + * of validation errors. Such functionality is defined only in the appropriate + * SpecializedTokenStreamCharsBase specialization. + * * == GeneralTokenStreamChars → - * TokenStreamCharsBase == + * SpecializedTokenStreamCharsBase == * * Some functionality operates differently on different character types, just * as for TokenStreamCharsBase, but additionally requires access to character- @@ -1279,6 +1297,29 @@ TokenStreamCharsBase::fillCharBufferWithTemplateStringContents(const c return true; } +template +class SpecializedTokenStreamCharsBase; + +template<> +class SpecializedTokenStreamCharsBase + : public TokenStreamCharsBase +{ + using CharsBase = TokenStreamCharsBase; + + protected: + using CharsBase::CharsBase; +}; + +template<> +class SpecializedTokenStreamCharsBase + : public TokenStreamCharsBase +{ + using CharsBase = TokenStreamCharsBase; + + protected: + using CharsBase::CharsBase; +}; + /** A small class encapsulating computation of the start-offset of a Token. */ class TokenStart { @@ -1302,9 +1343,10 @@ class TokenStart template class GeneralTokenStreamChars - : public TokenStreamCharsBase + : public SpecializedTokenStreamCharsBase { using CharsBase = TokenStreamCharsBase; + using SpecializedCharsBase = SpecializedTokenStreamCharsBase; Token* newTokenInternal(TokenKind kind, TokenStart start, TokenKind* out); @@ -1335,7 +1377,7 @@ class GeneralTokenStreamChars using typename CharsBase::SourceUnits; protected: - using CharsBase::CharsBase; + using SpecializedCharsBase::SpecializedCharsBase; TokenStreamAnyChars& anyCharsAccess() { return AnyCharsAccess::anyChars(this); From 7208d297c5a049d5a78168ce316f380351efc2df Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Fri, 29 Jun 2018 13:46:09 -0700 Subject: [PATCH 24/37] Bug 1476866 - Move consumeRestOfSingleLineComment into TokenStreamChars (and the infallible UTF-16 implementation into SpecializedTokenStreamCharsBase) so that it can be easily specialized for UTF-8. r=arai --HG-- extra : rebase_source : 4650b9958962167e8a754b25d6a7835383f73715 --- js/src/frontend/TokenStream.cpp | 26 ++++++---- js/src/frontend/TokenStream.h | 92 ++++++++++++++++++++++++--------- 2 files changed, 85 insertions(+), 33 deletions(-) diff --git a/js/src/frontend/TokenStream.cpp b/js/src/frontend/TokenStream.cpp index 76433e182e03..a326b7dc0636 100644 --- a/js/src/frontend/TokenStream.cpp +++ b/js/src/frontend/TokenStream.cpp @@ -1511,16 +1511,16 @@ static const uint8_t firstCharKinds[] = { static_assert(LastCharKind < (1 << (sizeof(firstCharKinds[0]) * 8)), "Elements of firstCharKinds[] are too small"); -template void -GeneralTokenStreamChars::consumeRestOfSingleLineComment() +SpecializedTokenStreamCharsBase::infallibleConsumeRestOfSingleLineComment() { - int32_t c; - do { - c = getCodeUnit(); - } while (c != EOF && !SourceUnits::isRawEOLChar(c)); + while (MOZ_LIKELY(!this->sourceUnits.atEnd())) { + char16_t unit = this->sourceUnits.peekCodeUnit(); + if (SourceUnits::isRawEOLChar(unit)) + return; - ungetCodeUnit(c); + this->sourceUnits.consumeKnownCodeUnit(unit); + } } template @@ -2097,7 +2097,9 @@ TokenStreamSpecific::getTokenInternal(TokenKind* const tt if (matchCodeUnit('!')) { if (matchCodeUnit('-')) { if (matchCodeUnit('-')) { - consumeRestOfSingleLineComment(); + if (!consumeRestOfSingleLineComment()) + return false; + continue; } ungetCodeUnit('-'); @@ -2142,7 +2144,9 @@ TokenStreamSpecific::getTokenInternal(TokenKind* const tt ungetCodeUnit(unit); } - consumeRestOfSingleLineComment(); + if (!consumeRestOfSingleLineComment()) + return false; + continue; } @@ -2199,7 +2203,9 @@ TokenStreamSpecific::getTokenInternal(TokenKind* const tt !anyCharsAccess().flags.isDirtyLine) { if (matchCodeUnit('>')) { - consumeRestOfSingleLineComment(); + if (!consumeRestOfSingleLineComment()) + return false; + continue; } } diff --git a/js/src/frontend/TokenStream.h b/js/src/frontend/TokenStream.h index 463fe299c95c..c04065e31a52 100644 --- a/js/src/frontend/TokenStream.h +++ b/js/src/frontend/TokenStream.h @@ -1076,6 +1076,12 @@ class SourceUnits return false; } + void consumeKnownCodeUnit(CharT c) { + MOZ_ASSERT(ptr, "shouldn't use poisoned SourceUnits"); + MOZ_ASSERT(*ptr == c, "consuming the wrong code unit"); + ptr++; + } + /** * Unget the '\n' (CR) that precedes a '\n' (LF), when ungetting a line * terminator that's a full "\r\n" sequence. If the prior code unit isn't @@ -1218,16 +1224,14 @@ class TokenStreamCharsBase return MOZ_LIKELY(!sourceUnits.atEnd()) ? CodeUnitValue(sourceUnits.peekCodeUnit()) : EOF; } - void consumeKnownCodeUnit(int32_t unit) { - MOZ_ASSERT(unit != EOF, "shouldn't be matching EOF"); - MOZ_ASSERT(!sourceUnits.atEnd(), "must have units to consume"); -#ifdef DEBUG - CharT next = -#endif - sourceUnits.getCodeUnit(); - MOZ_ASSERT(CodeUnitValue(next) == unit, - "must be consuming the correct unit"); - } + /** Consume a known, non-EOF code unit. */ + inline void consumeKnownCodeUnit(int32_t unit); + + // Forbid accidental calls to consumeKnownCodeUnit *not* with the single + // unit-or-EOF type. CharT should use SourceUnits::consumeKnownCodeUnit; + // CodeUnitValue() results should go through toCharT(), or better yet just + // use the original CharT. + template inline void consumeKnownCodeUnit(T) = delete; MOZ_MUST_USE inline bool fillCharBufferWithTemplateStringContents(const CharT* cur, const CharT* end); @@ -1253,6 +1257,13 @@ TokenStreamCharsBase::toCharT(int32_t value) return mozilla::Utf8Unit(static_cast(value)); } +template +inline void +TokenStreamCharsBase::consumeKnownCodeUnit(int32_t unit) +{ + sourceUnits.consumeKnownCodeUnit(toCharT(unit)); +} + template<> /* static */ MOZ_ALWAYS_INLINE JSAtom* TokenStreamCharsBase::atomizeSourceChars(JSContext* cx, const char16_t* chars, @@ -1307,6 +1318,24 @@ class SpecializedTokenStreamCharsBase using CharsBase = TokenStreamCharsBase; protected: + // Deliberately don't |using| |sourceUnits| because of bug 1472569. :-( + + using typename CharsBase::SourceUnits; + + protected: + // These APIs are only usable by UTF-16-specific code. + + /** + * Consume the rest of a single-line comment (but not the EOL/EOF that + * terminates it) -- infallibly because no 16-bit code unit sequence in a + * comment is an error. + */ + void infallibleConsumeRestOfSingleLineComment(); + + protected: + // These APIs are in both SpecializedTokenStreamCharsBase specializations + // and so are usable in subclasses no matter what CharT is. + using CharsBase::CharsBase; }; @@ -1317,6 +1346,15 @@ class SpecializedTokenStreamCharsBase using CharsBase = TokenStreamCharsBase; protected: + // Deliberately don't |using| |sourceUnits| because of bug 1472569. :-( + + protected: + // These APIs are only usable by UTF-8-specific code. + + protected: + // These APIs are in both SpecializedTokenStreamCharsBase specializations + // and so are usable in subclasses no matter what CharT is. + using CharsBase::CharsBase; }; @@ -1459,12 +1497,6 @@ class GeneralTokenStreamChars CharsBase::ungetCodeUnit(c); } - /** - * Consume code units til EOL/EOF following the start of a single-line - * comment, without consuming the EOL/EOF. - */ - void consumeRestOfSingleLineComment(); - MOZ_MUST_USE MOZ_ALWAYS_INLINE bool updateLineInfoForEOL() { return anyCharsAccess().internalUpdateLineInfoForEOL(this->sourceUnits.offset()); } @@ -1481,6 +1513,7 @@ class TokenStreamChars { private: using CharsBase = TokenStreamCharsBase; + using SpecializedCharsBase = SpecializedTokenStreamCharsBase; using GeneralCharsBase = GeneralTokenStreamChars; using Self = TokenStreamChars; @@ -1491,6 +1524,7 @@ class TokenStreamChars protected: using GeneralCharsBase::anyCharsAccess; using GeneralCharsBase::getCodeUnit; + using SpecializedCharsBase::infallibleConsumeRestOfSingleLineComment; using TokenStreamCharsShared::isAsciiCodePoint; // Deliberately don't |using| |sourceUnits| because of bug 1472569. :-( using GeneralCharsBase::ungetCodeUnit; @@ -1593,6 +1627,17 @@ class TokenStreamChars * changes reflecting that LineTerminator. */ void ungetLineTerminator(); + + /** + * Consume code points til EOL/EOF following the start of a single-line + * comment, without consuming the EOL/EOF. + */ + MOZ_MUST_USE bool consumeRestOfSingleLineComment() { + // This operation is infallible for UTF-16 -- and this implementation + // approach lets the compiler boil away call-side fallibility handling. + infallibleConsumeRestOfSingleLineComment(); + return true; + } }; // TokenStream is the lexical scanner for JavaScript source text. @@ -1644,8 +1689,9 @@ class MOZ_STACK_CLASS TokenStreamSpecific { public: using CharsBase = TokenStreamCharsBase; + using SpecializedCharsBase = SpecializedTokenStreamCharsBase; using GeneralCharsBase = GeneralTokenStreamChars; - using SpecializedCharsBase = TokenStreamChars; + using SpecializedChars = TokenStreamChars; using Position = TokenStreamPosition; @@ -1672,14 +1718,14 @@ class MOZ_STACK_CLASS TokenStreamSpecific using GeneralCharsBase::badToken; // Deliberately don't |using| |charBuffer| because of bug 1472569. :-( using CharsBase::consumeKnownCodeUnit; - using GeneralCharsBase::consumeRestOfSingleLineComment; + using SpecializedChars::consumeRestOfSingleLineComment; using TokenStreamCharsShared::copyCharBufferTo; using TokenStreamCharsShared::drainCharBufferIntoAtom; using CharsBase::fillCharBufferWithTemplateStringContents; - using SpecializedCharsBase::getCodePoint; + using SpecializedChars::getCodePoint; using GeneralCharsBase::getCodeUnit; - using SpecializedCharsBase::getFullAsciiCodePoint; - using SpecializedCharsBase::getNonAsciiCodePoint; + using SpecializedChars::getFullAsciiCodePoint; + using SpecializedChars::getNonAsciiCodePoint; using TokenStreamCharsShared::isAsciiCodePoint; using CharsBase::matchCodeUnit; using GeneralCharsBase::matchUnicodeEscapeIdent; @@ -1691,9 +1737,9 @@ class MOZ_STACK_CLASS TokenStreamSpecific using GeneralCharsBase::newSimpleToken; using CharsBase::peekCodeUnit; // Deliberately don't |using| |sourceUnits| because of bug 1472569. :-( - using SpecializedCharsBase::ungetCodePointIgnoreEOL; + using SpecializedChars::ungetCodePointIgnoreEOL; using GeneralCharsBase::ungetCodeUnit; - using SpecializedCharsBase::ungetNonAsciiNormalizedCodePoint; + using SpecializedChars::ungetNonAsciiNormalizedCodePoint; using GeneralCharsBase::updateLineInfoForEOL; template friend class TokenStreamPosition; From b9da885e9ef50c1df7362cb0807b8dfa5b6bcf64 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Mon, 9 Jul 2018 14:38:16 -0700 Subject: [PATCH 25/37] Bug 1476866 - Add a getNonAsciiCodePointDontNormalize for use in situations that demand such. r=arai --HG-- extra : rebase_source : 07c2e14cd88238ce6421c37260f04b3b8ed59972 --- js/src/frontend/TokenStream.cpp | 17 ++++++--------- js/src/frontend/TokenStream.h | 37 +++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/js/src/frontend/TokenStream.cpp b/js/src/frontend/TokenStream.cpp index a326b7dc0636..7a9cf1d07b4a 100644 --- a/js/src/frontend/TokenStream.cpp +++ b/js/src/frontend/TokenStream.cpp @@ -1329,20 +1329,15 @@ TokenStreamSpecific::putIdentInCharBuffer(const CharT* id if (unit != '\\' || !matchUnicodeEscapeIdent(&codePoint)) break; } else { - int32_t cp; - if (!getNonAsciiCodePoint(unit, &cp)) + // |restoreNextRawCharAddress| undoes all gets, and this function + // doesn't update line/column info. + char32_t cp; + if (!getNonAsciiCodePointDontNormalize(unit, &cp)) return false; - codePoint = AssertedCast(cp); - - if (!unicode::IsIdentifierPart(codePoint)) { - if (MOZ_UNLIKELY(codePoint == '\n')) { - // |restoreNextRawCharAddress| will undo all gets, but we - // have to revert a line/column update manually. - anyCharsAccess().undoInternalUpdateLineInfoForEOL(); - } + codePoint = cp; + if (!unicode::IsIdentifierPart(codePoint)) break; - } } if (!appendCodePointToCharBuffer(codePoint)) diff --git a/js/src/frontend/TokenStream.h b/js/src/frontend/TokenStream.h index c04065e31a52..26c4a9f73adc 100644 --- a/js/src/frontend/TokenStream.h +++ b/js/src/frontend/TokenStream.h @@ -1318,6 +1318,7 @@ class SpecializedTokenStreamCharsBase using CharsBase = TokenStreamCharsBase; protected: + using TokenStreamCharsShared::isAsciiCodePoint; // Deliberately don't |using| |sourceUnits| because of bug 1472569. :-( using typename CharsBase::SourceUnits; @@ -1332,6 +1333,28 @@ class SpecializedTokenStreamCharsBase */ void infallibleConsumeRestOfSingleLineComment(); + /** + * Given |lead| already consumed, consume and return the code point encoded + * starting from it. Infallible because lone surrogates in JS encode a + * "code point" of the same value. + */ + char32_t infallibleGetNonAsciiCodePointDontNormalize(char16_t lead) { + MOZ_ASSERT(!isAsciiCodePoint(lead)); + MOZ_ASSERT(this->sourceUnits.previousCodeUnit() == lead); + + // Handle single-unit code points and lone trailing surrogates. + if (MOZ_LIKELY(!unicode::IsLeadSurrogate(lead)) || + // Or handle lead surrogates not paired with trailing surrogates. + MOZ_UNLIKELY(this->sourceUnits.atEnd() || + !unicode::IsTrailSurrogate(this->sourceUnits.peekCodeUnit()))) + { + return lead; + } + + // Otherwise it's a multi-unit code point. + return unicode::UTF16Decode(lead, this->sourceUnits.getCodeUnit()); + } + protected: // These APIs are in both SpecializedTokenStreamCharsBase specializations // and so are usable in subclasses no matter what CharT is. @@ -1525,6 +1548,7 @@ class TokenStreamChars using GeneralCharsBase::anyCharsAccess; using GeneralCharsBase::getCodeUnit; using SpecializedCharsBase::infallibleConsumeRestOfSingleLineComment; + using SpecializedCharsBase::infallibleGetNonAsciiCodePointDontNormalize; using TokenStreamCharsShared::isAsciiCodePoint; // Deliberately don't |using| |sourceUnits| because of bug 1472569. :-( using GeneralCharsBase::ungetCodeUnit; @@ -1535,6 +1559,18 @@ class TokenStreamChars protected: using GeneralCharsBase::GeneralCharsBase; + /** + * Given the non-ASCII |lead| code unit just consumed, consume and return a + * complete non-ASCII code point. Line/column updates are not performed, + * and line breaks are returned as-is without normalization. + */ + MOZ_MUST_USE bool getNonAsciiCodePointDontNormalize(char16_t lead, char32_t* codePoint) { + // There are no encoding errors in 16-bit JS, so implement this so that + // the compiler knows it, too. + *codePoint = infallibleGetNonAsciiCodePointDontNormalize(lead); + return true; + } + // Try to get the next code point, normalizing '\r', '\r\n', '\n', and the // Unicode line/paragraph separators into '\n'. Also updates internal // line-counter state. Return true on success and store the code point in @@ -1726,6 +1762,7 @@ class MOZ_STACK_CLASS TokenStreamSpecific using GeneralCharsBase::getCodeUnit; using SpecializedChars::getFullAsciiCodePoint; using SpecializedChars::getNonAsciiCodePoint; + using SpecializedChars::getNonAsciiCodePointDontNormalize; using TokenStreamCharsShared::isAsciiCodePoint; using CharsBase::matchCodeUnit; using GeneralCharsBase::matchUnicodeEscapeIdent; From 820f2dc35e41cf370a125c61e56eef2c958f7000 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Mon, 9 Jul 2018 16:22:50 -0700 Subject: [PATCH 26/37] Bug 1476866 - Remove ungetLineTerminator, used only to unget Unicode separators, and replace it with a SourceUnits::ungetLineOrParagraphSeparator. r=arai --HG-- extra : rebase_source : 33a09c9cc1830af41cd621288f8a7be76e42bd3d --- js/src/frontend/TokenStream.cpp | 26 +++++++---------------- js/src/frontend/TokenStream.h | 37 ++++++++++++++++++++++++++------- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/js/src/frontend/TokenStream.cpp b/js/src/frontend/TokenStream.cpp index 7a9cf1d07b4a..673fea3fdc43 100644 --- a/js/src/frontend/TokenStream.cpp +++ b/js/src/frontend/TokenStream.cpp @@ -601,21 +601,6 @@ TokenStreamChars::ungetCodePointIgnoreEOL(uint32_t cod ungetCodeUnit(units[numUnits]); } -template -void -TokenStreamChars::ungetLineTerminator() -{ - this->sourceUnits.ungetCodeUnit(); - - char16_t last = this->sourceUnits.peekCodeUnit(); - MOZ_ASSERT(SourceUnits::isRawEOLChar(last)); - - if (last == '\n') - this->sourceUnits.ungetOptionalCRBeforeLF(); - - anyCharsAccess().undoInternalUpdateLineInfoForEOL(); -} - template size_t SourceUnits::findEOLMax(size_t start, size_t max) @@ -1623,13 +1608,16 @@ TokenStreamSpecific::regexpLiteral(TokenStart start, Toke auto ProcessNonAsciiCodePoint = [this](int32_t lead) { MOZ_ASSERT(lead != EOF); + MOZ_ASSERT(!this->isAsciiCodePoint(lead)); - int32_t codePoint; - if (!this->getNonAsciiCodePoint(lead, &codePoint)) + char32_t codePoint; + if (!this->getNonAsciiCodePointDontNormalize(lead, &codePoint)) return false; - if (codePoint == '\n') { - this->ungetLineTerminator(); + if (MOZ_UNLIKELY(codePoint == unicode::LINE_SEPARATOR || + codePoint == unicode::PARA_SEPARATOR)) + { + this->sourceUnits.ungetLineOrParagraphSeparator(); this->reportError(JSMSG_UNTERMINATED_REGEXP); return false; } diff --git a/js/src/frontend/TokenStream.h b/js/src/frontend/TokenStream.h index 26c4a9f73adc..8bc812814c19 100644 --- a/js/src/frontend/TokenStream.h +++ b/js/src/frontend/TokenStream.h @@ -1097,6 +1097,9 @@ class SourceUnits ptr--; } + /** Unget U+2028 LINE SEPARATOR or U+2029 PARAGRAPH SEPARATOR. */ + inline void ungetLineOrParagraphSeparator(); + void ungetCodeUnit() { MOZ_ASSERT(!atStart(), "can't unget if currently at start"); MOZ_ASSERT(ptr); // make sure it hasn't been poisoned @@ -1146,6 +1149,33 @@ class SourceUnits const CharT* ptr; }; +template<> +inline void +SourceUnits::ungetLineOrParagraphSeparator() +{ +#ifdef DEBUG + char16_t prev = previousCodeUnit(); +#endif + MOZ_ASSERT(prev == unicode::LINE_SEPARATOR || prev == unicode::PARA_SEPARATOR); + + ungetCodeUnit(); +} + +template<> +inline void +SourceUnits::ungetLineOrParagraphSeparator() +{ + unskipCodeUnits(3); + + MOZ_ASSERT(ptr[0].toUint8() == 0xE2); + MOZ_ASSERT(ptr[1].toUint8() == 0x80); + +#ifdef DEBUG + uint8_t last = ptr[2].toUint8(); +#endif + MOZ_ASSERT(last == 0xA8 || last == 0xA9); +} + class TokenStreamCharsShared { // Using char16_t (not CharT) is a simplifying decision that hopefully @@ -1657,13 +1687,6 @@ class TokenStreamChars anyCharsAccess().undoInternalUpdateLineInfoForEOL(); } - /** - * Unget a just-gotten LineTerminator sequence: '\r', '\n', '\r\n', or - * a Unicode line/paragraph separator, also undoing line/column information - * changes reflecting that LineTerminator. - */ - void ungetLineTerminator(); - /** * Consume code points til EOL/EOF following the start of a single-line * comment, without consuming the EOL/EOF. From 5434732353c4e18fb284a10ada5c61a3b9b4dba3 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Tue, 17 Jul 2018 21:00:56 -0700 Subject: [PATCH 27/37] Bug 1476866 - Reduce code indentation in the loop to accumulate regular expression source text by handling the non-ASCII case in an early block-with-continue. r=arai --HG-- extra : rebase_source : 41994cf4a198ac7c1e7a3a6334c367847c835af2 --- js/src/frontend/TokenStream.cpp | 70 ++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/js/src/frontend/TokenStream.cpp b/js/src/frontend/TokenStream.cpp index 673fea3fdc43..62d4cf053122 100644 --- a/js/src/frontend/TokenStream.cpp +++ b/js/src/frontend/TokenStream.cpp @@ -1638,45 +1638,53 @@ TokenStreamSpecific::regexpLiteral(TokenStart start, Toke return badToken(); } - if (MOZ_LIKELY(isAsciiCodePoint(unit))) { - if (unit == '\\') { - if (!this->charBuffer.append(unit)) - return badToken(); + if (MOZ_UNLIKELY(!isAsciiCodePoint(unit))) { + if (!ProcessNonAsciiCodePoint(unit)) + return badToken(); - unit = getCodeUnit(); - if (unit == EOF) { - ReportUnterminatedRegExp(unit); - return badToken(); - } + continue; + } - // Fallthrough only handles ASCII code points, so - // deal with non-ASCII and skip everything else. - if (MOZ_UNLIKELY(!isAsciiCodePoint(unit))) { - if (!ProcessNonAsciiCodePoint(unit)) - return badToken(); + if (unit == '\\') { + if (!this->charBuffer.append(unit)) + return badToken(); - continue; - } - } else if (unit == '[') { - inCharClass = true; - } else if (unit == ']') { - inCharClass = false; - } else if (unit == '/' && !inCharClass) { - // For IE compat, allow unescaped / in char classes. - break; - } - - if (unit == '\r' || unit == '\n') { + unit = getCodeUnit(); + if (unit == EOF) { ReportUnterminatedRegExp(unit); return badToken(); } - if (!this->charBuffer.append(unit)) - return badToken(); - } else { - if (!ProcessNonAsciiCodePoint(unit)) - return badToken(); + // Fallthrough only handles ASCII code points, so + // deal with non-ASCII and skip everything else. + if (MOZ_UNLIKELY(!isAsciiCodePoint(unit))) { + if (!ProcessNonAsciiCodePoint(unit)) + return badToken(); + + continue; + } + } else if (unit == '[') { + inCharClass = true; + } else if (unit == ']') { + inCharClass = false; + } else if (unit == '/' && !inCharClass) { + // For IE compat, allow unescaped / in char classes. + break; } + + if (unit == '\r' || unit == '\n') { + ReportUnterminatedRegExp(unit); + return badToken(); + } + + // We're accumulating regular expression *source* text here: source + // text matching a line break will appear as U+005C REVERSE SOLIDUS + // U+006E LATIN SMALL LETTER N, and |unit| here would be the latter + // code point. + MOZ_ASSERT(!SourceUnits::isRawEOLChar(unit)); + + if (!this->charBuffer.append(unit)) + return badToken(); } while (true); int32_t unit; From c323009c54f17ff769223dfd4cb2597ed95a11cf Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Tue, 17 Jul 2018 21:00:57 -0700 Subject: [PATCH 28/37] Bug 1476866 - Make matchCodeUnit only accept ASCII, and split matchLineTerminator (for '\r' and '\n') out of it. r=arai --HG-- extra : rebase_source : b4ebb077110249c197bbf461b00b89c4ba48b6bb --- js/src/frontend/TokenStream.cpp | 21 +++------- js/src/frontend/TokenStream.h | 68 +++++++++++++++++++++------------ 2 files changed, 48 insertions(+), 41 deletions(-) diff --git a/js/src/frontend/TokenStream.cpp b/js/src/frontend/TokenStream.cpp index 62d4cf053122..a60e362b644b 100644 --- a/js/src/frontend/TokenStream.cpp +++ b/js/src/frontend/TokenStream.cpp @@ -487,9 +487,6 @@ TokenStreamAnyChars::undoInternalUpdateLineInfoForEOL() lineno--; } -// This gets a full code point, starting from an already-consumed leading code -// unit, normalizing EOL sequences to '\n', also updating line/column info as -// needed. template bool TokenStreamChars::getCodePoint(int32_t* cp) @@ -510,10 +507,7 @@ TokenStreamChars::getCodePoint(int32_t* cp) break; if (MOZ_UNLIKELY(c == '\r')) { - // If it's a \r\n sequence: treat as a single EOL, skip over the \n. - if (MOZ_LIKELY(!this->sourceUnits.atEnd())) - this->sourceUnits.matchCodeUnit('\n'); - + matchLineTerminator('\n'); break; } @@ -1846,9 +1840,8 @@ TokenStreamSpecific::getTokenInternal(TokenKind* const tt // Skip over EOL chars, updating line state along the way. // if (c1kind == EOL) { - // If it's a \r\n sequence, consume it as a single EOL. - if (unit == '\r' && !this->sourceUnits.atEnd()) - this->sourceUnits.matchCodeUnit('\n'); + if (unit == '\r') + matchLineTerminator('\n'); if (!updateLineInfoForEOL()) return badToken(); @@ -2339,8 +2332,7 @@ TokenStreamSpecific::getStringOrTemplateToken(char untilC case 'v': unit = '\v'; break; case '\r': - if (MOZ_LIKELY(!this->sourceUnits.atEnd())) - this->sourceUnits.matchCodeUnit('\n'); + matchLineTerminator('\n'); MOZ_FALLTHROUGH; case '\n': { // LineContinuation represents no code points. We're manually @@ -2546,10 +2538,7 @@ TokenStreamSpecific::getStringOrTemplateToken(char untilC if (unit == '\r') { unit = '\n'; - - // If it's a \r\n sequence: treat as a single EOL, skip over the \n. - if (!this->sourceUnits.atEnd()) - this->sourceUnits.matchCodeUnit('\n'); + matchLineTerminator('\n'); } if (!updateLineInfoForEOL()) diff --git a/js/src/frontend/TokenStream.h b/js/src/frontend/TokenStream.h index 8bc812814c19..d8f0c7c155ce 100644 --- a/js/src/frontend/TokenStream.h +++ b/js/src/frontend/TokenStream.h @@ -946,6 +946,9 @@ CodeUnitValue(mozilla::Utf8Unit unit) return unit.toUint8(); } +template +class TokenStreamCharsBase; + // This is the low-level interface to the JS source code buffer. It just gets // raw Unicode code units -- 16-bit char16_t units of source text that are not // (always) full code points, and 8-bit units of UTF-8 source text soon. @@ -1068,14 +1071,19 @@ class SourceUnits ptr -= n; } - bool matchCodeUnit(CharT c) { - if (*ptr == c) { // this will nullptr-crash if poisoned + private: + friend class TokenStreamCharsBase; + + bool internalMatchCodeUnit(CharT c) { + MOZ_ASSERT(ptr, "shouldn't use poisoned SourceUnits"); + if (MOZ_LIKELY(!atEnd()) && *ptr == c) { ptr++; return true; } return false; } + public: void consumeKnownCodeUnit(CharT c) { MOZ_ASSERT(ptr, "shouldn't use poisoned SourceUnits"); MOZ_ASSERT(*ptr == c, "consuming the wrong code unit"); @@ -1225,6 +1233,8 @@ class TokenStreamCharsBase : public TokenStreamCharsShared { protected: + TokenStreamCharsBase(JSContext* cx, const CharT* chars, size_t length, size_t startOffset); + /** * Convert a non-EOF code unit returned by |getCodeUnit()| or * |peekCodeUnit()| to a CharT code unit. @@ -1238,18 +1248,34 @@ class TokenStreamCharsBase sourceUnits.ungetCodeUnit(); } - public: - TokenStreamCharsBase(JSContext* cx, const CharT* chars, size_t length, size_t startOffset); - static MOZ_ALWAYS_INLINE JSAtom* atomizeSourceChars(JSContext* cx, const CharT* chars, size_t length); using SourceUnits = frontend::SourceUnits; - /** Match a non-EOL, non-EOF code unit; return true iff it was matched. */ - inline bool matchCodeUnit(int32_t expect); + /** + * Try to match a non-LineTerminator ASCII code point. Return true iff it + * was matched. + */ + bool matchCodeUnit(char expect) { + MOZ_ASSERT(mozilla::IsAscii(expect)); + MOZ_ASSERT(expect != '\r'); + MOZ_ASSERT(expect != '\n'); + return this->sourceUnits.internalMatchCodeUnit(CharT(expect)); + } + + /** + * Try to match an ASCII LineTerminator code point. Return true iff it was + * matched. + */ + bool matchLineTerminator(char expect) { + MOZ_ASSERT(expect == '\r' || expect == '\n'); + return this->sourceUnits.internalMatchCodeUnit(CharT(expect)); + } + + template bool matchCodeUnit(T) = delete; + template bool matchLineTerminator(T) = delete; - protected: int32_t peekCodeUnit() { return MOZ_LIKELY(!sourceUnits.atEnd()) ? CodeUnitValue(sourceUnits.peekCodeUnit()) : EOF; } @@ -1302,16 +1328,6 @@ TokenStreamCharsBase::atomizeSourceChars(JSContext* cx, const char16_t return AtomizeChars(cx, chars, length); } -template -inline bool -TokenStreamCharsBase::matchCodeUnit(int32_t expect) -{ - MOZ_ASSERT(expect != EOF, "shouldn't be matching EOFs"); - MOZ_ASSERT(!SourceUnits::isRawEOLChar(expect)); - return MOZ_LIKELY(!this->sourceUnits.atEnd()) && - this->sourceUnits.matchCodeUnit(toCharT(expect)); -} - template<> MOZ_MUST_USE inline bool TokenStreamCharsBase::fillCharBufferWithTemplateStringContents(const char16_t* cur, @@ -1580,6 +1596,7 @@ class TokenStreamChars using SpecializedCharsBase::infallibleConsumeRestOfSingleLineComment; using SpecializedCharsBase::infallibleGetNonAsciiCodePointDontNormalize; using TokenStreamCharsShared::isAsciiCodePoint; + using CharsBase::matchLineTerminator; // Deliberately don't |using| |sourceUnits| because of bug 1472569. :-( using GeneralCharsBase::ungetCodeUnit; using GeneralCharsBase::updateLineInfoForEOL; @@ -1601,10 +1618,12 @@ class TokenStreamChars return true; } - // Try to get the next code point, normalizing '\r', '\r\n', '\n', and the - // Unicode line/paragraph separators into '\n'. Also updates internal - // line-counter state. Return true on success and store the code point in - // |*c|. Return false and leave |*c| undefined on failure. + /** + * Get the next code point, converting LineTerminatorSequences to '\n' and + * updating internal line-counter state if needed. Return true on success + * and store the code point in |*c|. Return false and leave |*c| undefined + * on failure. + */ MOZ_MUST_USE bool getCodePoint(int32_t* cp); /** @@ -1625,9 +1644,7 @@ class TokenStreamChars "getFullAsciiCodePoint called incorrectly"); if (MOZ_UNLIKELY(lead == '\r')) { - // NOTE: |this->|-qualify to avoid a gcc bug: see bug 1472569. - if (MOZ_LIKELY(!this->sourceUnits.atEnd())) - this->sourceUnits.matchCodeUnit('\n'); + matchLineTerminator('\n'); } else if (MOZ_LIKELY(lead != '\n')) { *codePoint = lead; return true; @@ -1788,6 +1805,7 @@ class MOZ_STACK_CLASS TokenStreamSpecific using SpecializedChars::getNonAsciiCodePointDontNormalize; using TokenStreamCharsShared::isAsciiCodePoint; using CharsBase::matchCodeUnit; + using CharsBase::matchLineTerminator; using GeneralCharsBase::matchUnicodeEscapeIdent; using GeneralCharsBase::matchUnicodeEscapeIdStart; using GeneralCharsBase::newAtomToken; From 8040464690ff0c6927545026e41bcea3d36186e5 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Fri, 29 Jun 2018 13:46:09 -0700 Subject: [PATCH 29/37] Bug 1476866 - Move fillCharBufferWithTemplateStringContents into TokenStreamCharsBase so that a UTF-8 specialization can eventually be defined for it. r=arai --HG-- extra : rebase_source : 8d4ff04bfe5d7bd8cd092e690b472655c7d1c836 --- js/src/frontend/TokenStream.cpp | 26 +++++++++++ js/src/frontend/TokenStream.h | 81 +++++++++++++-------------------- 2 files changed, 58 insertions(+), 49 deletions(-) diff --git a/js/src/frontend/TokenStream.cpp b/js/src/frontend/TokenStream.cpp index a60e362b644b..a02b22fc7850 100644 --- a/js/src/frontend/TokenStream.cpp +++ b/js/src/frontend/TokenStream.cpp @@ -440,6 +440,32 @@ TokenStreamCharsBase::TokenStreamCharsBase(JSContext* cx, const CharT* ch sourceUnits(chars, length, startOffset) {} +template<> +MOZ_MUST_USE bool +TokenStreamCharsBase::fillCharBufferWithTemplateStringContents(const char16_t* cur, + const char16_t* end) +{ + MOZ_ASSERT(this->charBuffer.length() == 0); + + while (cur < end) { + // Template literals normalize only '\r' and "\r\n" to '\n'. The + // Unicode separators need no special handling here. + // https://tc39.github.io/ecma262/#sec-static-semantics-tv-and-trv + char16_t ch = *cur++; + if (ch == '\r') { + ch = '\n'; + if (cur < end && *cur == '\n') + cur++; + } + + if (!this->charBuffer.append(ch)) + return false; + } + + MOZ_ASSERT(cur == end); + return true; +} + template TokenStreamSpecific::TokenStreamSpecific(JSContext* cx, const ReadOnlyCompileOptions& options, diff --git a/js/src/frontend/TokenStream.h b/js/src/frontend/TokenStream.h index d8f0c7c155ce..5c49fc031f02 100644 --- a/js/src/frontend/TokenStream.h +++ b/js/src/frontend/TokenStream.h @@ -1289,8 +1289,13 @@ class TokenStreamCharsBase // use the original CharT. template inline void consumeKnownCodeUnit(T) = delete; - MOZ_MUST_USE inline bool - fillCharBufferWithTemplateStringContents(const CharT* cur, const CharT* end); + /** + * Accumulate the provided range of already-validated (i.e. valid UTF-8, or + * anything if CharT is char16_t because JS permits lone and mispaired + * surrogates) raw template literal text (i.e. containing no escapes or + * substitutions) into |charBuffer|. + */ + MOZ_MUST_USE bool fillCharBufferWithTemplateStringContents(const CharT* cur, const CharT* end); protected: /** Code units in the source code being tokenized. */ @@ -1328,32 +1333,6 @@ TokenStreamCharsBase::atomizeSourceChars(JSContext* cx, const char16_t return AtomizeChars(cx, chars, length); } -template<> -MOZ_MUST_USE inline bool -TokenStreamCharsBase::fillCharBufferWithTemplateStringContents(const char16_t* cur, - const char16_t* end) -{ - MOZ_ASSERT(this->charBuffer.length() == 0); - - while (cur < end) { - // U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR are - // interpreted literally inside template literal contents; only - // literal CRLF sequences are normalized to '\n'. See - // . - char16_t ch = *cur++; - if (ch == '\r') { - ch = '\n'; - if (cur < end && *cur == '\n') - cur++; - } - - if (!this->charBuffer.append(ch)) - return false; - } - - return true; -} - template class SpecializedTokenStreamCharsBase; @@ -1481,6 +1460,9 @@ class GeneralTokenStreamChars uint32_t matchExtendedUnicodeEscape(uint32_t* codePoint); protected: + using TokenStreamCharsShared::drainCharBufferIntoAtom; + using CharsBase::fillCharBufferWithTemplateStringContents; + using typename CharsBase::SourceUnits; protected: @@ -1572,6 +1554,28 @@ class GeneralTokenStreamChars uint32_t matchUnicodeEscapeIdStart(uint32_t* codePoint); bool matchUnicodeEscapeIdent(uint32_t* codePoint); + + public: + JSAtom* getRawTemplateStringAtom() { + TokenStreamAnyChars& anyChars = anyCharsAccess(); + + MOZ_ASSERT(anyChars.currentToken().type == TokenKind::TemplateHead || + anyChars.currentToken().type == TokenKind::NoSubsTemplate); + const CharT* cur = this->sourceUnits.codeUnitPtrAt(anyChars.currentToken().pos.begin + 1); + const CharT* end; + if (anyChars.currentToken().type == TokenKind::TemplateHead) { + // Of the form |`...${| or |}...${| + end = this->sourceUnits.codeUnitPtrAt(anyChars.currentToken().pos.end - 2); + } else { + // NO_SUBS_TEMPLATE is of the form |`...`| or |}...`| + end = this->sourceUnits.codeUnitPtrAt(anyChars.currentToken().pos.end - 1); + } + + if (!fillCharBufferWithTemplateStringContents(cur, end)) + return nullptr; + + return drainCharBufferIntoAtom(anyChars.cx); + } }; template class TokenStreamChars; @@ -1905,27 +1909,6 @@ class MOZ_STACK_CLASS TokenStreamSpecific bool reportExtraWarningErrorNumberVA(UniquePtr notes, uint32_t offset, unsigned errorNumber, va_list* args); - JSAtom* getRawTemplateStringAtom() { - TokenStreamAnyChars& anyChars = anyCharsAccess(); - - MOZ_ASSERT(anyChars.currentToken().type == TokenKind::TemplateHead || - anyChars.currentToken().type == TokenKind::NoSubsTemplate); - const CharT* cur = this->sourceUnits.codeUnitPtrAt(anyChars.currentToken().pos.begin + 1); - const CharT* end; - if (anyChars.currentToken().type == TokenKind::TemplateHead) { - // Of the form |`...${| or |}...${| - end = this->sourceUnits.codeUnitPtrAt(anyChars.currentToken().pos.end - 2); - } else { - // NO_SUBS_TEMPLATE is of the form |`...`| or |}...`| - end = this->sourceUnits.codeUnitPtrAt(anyChars.currentToken().pos.end - 1); - } - - if (!fillCharBufferWithTemplateStringContents(cur, end)) - return nullptr; - - return drainCharBufferIntoAtom(anyChars.cx); - } - private: // This is private because it should only be called by the tokenizer while // tokenizing not by, for example, BytecodeEmitter. From d243be25b143b543b3cc69e8f31152eb9cc1194d Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Thu, 12 Jul 2018 17:41:31 -0700 Subject: [PATCH 30/37] Bug 1426909 - Abstract out mozilla::DecodeOneUtf8CodePoint for decoding a UTF-8 code point after having consumed a non-ASCII lead unit, with configurable error notification through optional user-provided functors. r=froydnj --HG-- extra : rebase_source : 25836018b00b545a60969abccf40ce313d4da1af --- mfbt/Utf8.cpp | 58 +--- mfbt/Utf8.h | 215 +++++++++++++ mfbt/tests/TestUtf8.cpp | 691 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 902 insertions(+), 62 deletions(-) diff --git a/mfbt/Utf8.cpp b/mfbt/Utf8.cpp index a183abd7a553..e0ddee016f11 100644 --- a/mfbt/Utf8.cpp +++ b/mfbt/Utf8.cpp @@ -4,6 +4,8 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include "mozilla/Maybe.h" +#include "mozilla/TextUtils.h" #include "mozilla/Types.h" #include "mozilla/Utf8.h" @@ -14,66 +16,24 @@ MFBT_API bool mozilla::IsValidUtf8(const void* aCodeUnits, size_t aCount) { const auto* s = static_cast(aCodeUnits); - const auto* limit = s + aCount; + const auto* const limit = s + aCount; while (s < limit) { - uint32_t n = *s++; + unsigned char c = *s++; // If the first byte is ASCII, it's the only one in the code point. Have a // fast path that avoids all the rest of the work and looping in that case. - if ((n & 0x80) == 0) { + if (IsAscii(c)) { continue; } - // The leading code unit determines the length of the next code point and - // the number of bits of the leading code unit that contribute to the code - // point's value. - uint_fast8_t remaining; - uint32_t min; - if ((n & 0xE0) == 0xC0) { - remaining = 1; - min = 0x80; - n &= 0x1F; - } else if ((n & 0xF0) == 0xE0) { - remaining = 2; - min = 0x800; - n &= 0x0F; - } else if ((n & 0xF8) == 0xF0) { - remaining = 3; - min = 0x10000; - n &= 0x07; - } else { - // UTF-8 used to have a hyper-long encoding form, but it's been removed - // for years now. So in this case, the string is not valid UTF-8. + Maybe maybeCodePoint = + DecodeOneUtf8CodePoint(Utf8Unit(c), &s, limit); + if (maybeCodePoint.isNothing()) { return false; } - - // If the code point would require more code units than remain, the encoding - // is invalid. - if (s + remaining > limit) { - return false; - } - - for (uint_fast8_t i = 0; i < remaining; i++) { - // Every non-leading code unit in properly encoded UTF-8 has its high bit - // set and the next-highest bit unset. - if ((s[i] & 0xC0) != 0x80) { - return false; - } - - // The code point being encoded is the concatenation of all the - // unconstrained bits. - n = (n << 6) | (s[i] & 0x3F); - } - - // Don't consider code points that are overlong, UTF-16 surrogates, or - // exceed the maximum code point to be valid. - if (n < min || (0xD800 <= n && n < 0xE000) || n >= 0x110000) { - return false; - } - - s += remaining; } + MOZ_ASSERT(s == limit); return true; } diff --git a/mfbt/Utf8.h b/mfbt/Utf8.h index 32e0a97743fc..172e0c319b7f 100644 --- a/mfbt/Utf8.h +++ b/mfbt/Utf8.h @@ -12,6 +12,10 @@ #ifndef mozilla_Utf8_h #define mozilla_Utf8_h +#include "mozilla/Casting.h" // for mozilla::AssertedCast +#include "mozilla/Likely.h" // for MOZ_UNLIKELY +#include "mozilla/Maybe.h" // for mozilla::Maybe +#include "mozilla/TextUtils.h" // for mozilla::IsAscii #include "mozilla/Types.h" // for MFBT_API #include // for CHAR_BIT @@ -205,6 +209,217 @@ public: extern MFBT_API bool IsValidUtf8(const void* aCodeUnits, size_t aCount); +/** + * Given |aLeadUnit| that is a non-ASCII code unit, a pointer to an |Iter aIter| + * that (initially) itself points one unit past |aLeadUnit|, and + * |const EndIter aEnd| that denotes the end of the UTF-8 data when compared + * against |*aIter| using |aEnd - *aIter|: + * + * If |aLeadUnit| and subsequent code units computed using |*aIter| (up to + * |aEnd|) encode a valid code point -- not exceeding Unicode's range, not a + * surrogate, in shortest form -- then return Some(that code point) and advance + * |*aIter| past those code units. + * + * Otherwise decrement |*aIter| (so that it points at |aLeadUnit|) and return + * Nothing(). + * + * |Iter| and |EndIter| are generalized concepts most easily understood as if + * they were |const char*|, |const unsigned char*|, or |const Utf8Unit*|: + * iterators that when dereferenced can be used to construct a |Utf8Unit| and + * that can be compared and modified in certain limited ways. (Carefully note + * that this function mutates |*aIter|.) |Iter| and |EndIter| are template + * parameters to support more-complicated adaptor iterators. + * + * The template parameters after |Iter| allow users to implement custom handling + * for various forms of invalid UTF-8. A version of this function that defaults + * all such handling to no-ops is defined below this function. To learn how to + * define your own custom handling, consult the implementation of that function, + * which documents exactly how custom handler functors are invoked. + * + * This function is MOZ_ALWAYS_INLINE: if you don't need that, use the version + * of this function without the "Inline" suffix on the name. + */ +template +MOZ_ALWAYS_INLINE Maybe +DecodeOneUtf8CodePointInline(const Utf8Unit aLeadUnit, + Iter* aIter, const EndIter aEnd, + OnBadLeadUnit aOnBadLeadUnit, + OnNotEnoughUnits aOnNotEnoughUnits, + OnBadTrailingUnit aOnBadTrailingUnit, + OnBadCodePoint aOnBadCodePoint, + OnNotShortestForm aOnNotShortestForm) +{ + MOZ_ASSERT(Utf8Unit((*aIter)[-1]) == aLeadUnit); + + char32_t n = aLeadUnit.toUint8(); + MOZ_ASSERT(!IsAscii(n)); + + // |aLeadUnit| determines the number of trailing code units in the code point + // and the bits of |aLeadUnit| that contribute to the code point's value. + uint8_t remaining; + uint32_t min; + if ((n & 0b1110'0000) == 0b1100'0000) { + remaining = 1; + min = 0x80; + n &= 0b0001'1111; + } else if ((n & 0b1111'0000) == 0b1110'0000) { + remaining = 2; + min = 0x800; + n &= 0b0000'1111; + } else if ((n & 0b1111'1000) == 0b1111'0000) { + remaining = 3; + min = 0x10000; + n &= 0b0000'0111; + } else { + *aIter -= 1; + aOnBadLeadUnit(); + return Nothing(); + } + + // If the code point would require more code units than remain, the encoding + // is invalid. + auto actual = aEnd - *aIter; + if (MOZ_UNLIKELY(actual < remaining)) { + *aIter -= 1; + aOnNotEnoughUnits(AssertedCast(actual + 1), remaining + 1); + return Nothing(); + } + + for (uint8_t i = 0; i < remaining; i++) { + uint8_t unit = Utf8Unit(*(*aIter)++).toUint8(); + + // Every non-leading code unit in properly encoded UTF-8 has its high + // bit set and the next-highest bit unset. + if (MOZ_UNLIKELY((unit & 0b1100'0000) != 0b1000'0000)) { + uint8_t unitsObserved = i + 1 + 1; + *aIter -= unitsObserved; + aOnBadTrailingUnit(unitsObserved); + return Nothing(); + } + + // The code point being encoded is the concatenation of all the + // unconstrained bits. + n = (n << 6) | (unit & 0b0011'1111); + } + + // UTF-16 surrogates and values outside the Unicode range are invalid. + if (MOZ_UNLIKELY(n > 0x10FFFF || (0xD800 <= n && n <= 0xDFFF))) { + uint8_t unitsObserved = remaining + 1; + *aIter -= unitsObserved; + aOnBadCodePoint(n, unitsObserved); + return Nothing(); + } + + // Overlong code points are also invalid. + if (MOZ_UNLIKELY(n < min)) { + uint8_t unitsObserved = remaining + 1; + *aIter -= unitsObserved; + aOnNotShortestForm(n, unitsObserved); + return Nothing(); + } + + return Some(n); +} + +/** + * Identical to the above function, but not forced to be instantiated inline -- + * the compiler is permitted to common up separate invocations if it chooses. + */ +template +inline Maybe +DecodeOneUtf8CodePoint(const Utf8Unit aLeadUnit, + Iter* aIter, const EndIter aEnd, + OnBadLeadUnit aOnBadLeadUnit, + OnNotEnoughUnits aOnNotEnoughUnits, + OnBadTrailingUnit aOnBadTrailingUnit, + OnBadCodePoint aOnBadCodePoint, + OnNotShortestForm aOnNotShortestForm) +{ + return DecodeOneUtf8CodePointInline(aLeadUnit, aIter, aEnd, + aOnBadLeadUnit, aOnNotEnoughUnits, + aOnBadTrailingUnit, aOnBadCodePoint, + aOnNotShortestForm); +} + +/** + * Like the always-inlined function above, but with no-op behavior from all + * trailing if-invalid notifier functors. + * + * This function is MOZ_ALWAYS_INLINE: if you don't need that, use the version + * of this function without the "Inline" suffix on the name. + */ +template +MOZ_ALWAYS_INLINE Maybe +DecodeOneUtf8CodePointInline(const Utf8Unit aLeadUnit, + Iter* aIter, const EndIter aEnd) +{ + // aOnBadLeadUnit is called when |aLeadUnit| itself is an invalid lead unit in + // a multi-unit code point. It is passed no arguments: the caller already has + // |aLeadUnit| on hand, so no need to provide it again. + auto onBadLeadUnit = []() {}; + + // aOnNotEnoughUnits is called when |aLeadUnit| properly indicates a code + // point length, but there aren't enough units from |*aIter| to |aEnd| to + // satisfy that length. It is passed the number of code units actually + // available (according to |aEnd - *aIter|) and the number of code units that + // |aLeadUnit| indicates are needed. Both numbers include the contribution + // of |aLeadUnit| itself: so |aUnitsAvailable <= 3|, |aUnitsNeeded <= 4|, and + // |aUnitsAvailable < aUnitsNeeded|. As above, it also is not passed the lead + // code unit. + auto onNotEnoughUnits = [](uint8_t aUnitsAvailable, uint8_t aUnitsNeeded) {}; + + // aOnBadTrailingUnit is called when one of the trailing code units implied by + // |aLeadUnit| doesn't match the 0b10xx'xxxx bit pattern that all UTF-8 + // trailing code units must satisfy. It is passed the total count of units + // observed (including |aLeadUnit|). The bad trailing code unit will + // conceptually be at |(*aIter)[aUnitsObserved - 1]| if this functor is + // called, and so |aUnitsObserved <= 4|. + auto onBadTrailingUnit = [](uint8_t aUnitsObserved) {}; + + // aOnBadCodePoint is called when a structurally-correct code point encoding + // is found, but the *value* that is encoded is not a valid code point: either + // because it exceeded the U+10FFFF Unicode maximum code point, or because it + // was a UTF-16 surrogate. It is passed the non-code point value and the + // number of code units used to encode it. + auto onBadCodePoint = [](char32_t aBadCodePoint, uint8_t aUnitsObserved) {}; + + // aOnNotShortestForm is called when structurally-correct encoding is found, + // but the encoded value should have been encoded in fewer code units (e.g. + // mis-encoding U+0000 as 0b1100'0000 0b1000'0000 in two code units instead of + // as 0b0000'0000). It is passed the mis-encoded code point (which will be + // valid and not a surrogate) and the count of code units that mis-encoded it. + auto onNotShortestForm = [](char32_t aBadCodePoint, uint8_t aUnitsObserved) {}; + + return DecodeOneUtf8CodePointInline(aLeadUnit, aIter, aEnd, + onBadLeadUnit, onNotEnoughUnits, + onBadTrailingUnit, onBadCodePoint, + onNotShortestForm); +} + +/** + * Identical to the above function, but not forced to be instantiated inline -- + * the compiler/linker are allowed to common up separate invocations. + */ +template +inline Maybe +DecodeOneUtf8CodePoint(const Utf8Unit aLeadUnit, + Iter* aIter, const EndIter aEnd) +{ + return DecodeOneUtf8CodePointInline(aLeadUnit, aIter, aEnd); +} + } // namespace mozilla #endif /* mozilla_Utf8_h */ diff --git a/mfbt/tests/TestUtf8.cpp b/mfbt/tests/TestUtf8.cpp index 3eeb881b38c1..e7a2829da004 100644 --- a/mfbt/tests/TestUtf8.cpp +++ b/mfbt/tests/TestUtf8.cpp @@ -8,8 +8,15 @@ #include "mozilla/ArrayUtils.h" #include "mozilla/Assertions.h" +#include "mozilla/EnumSet.h" +#include "mozilla/IntegerRange.h" +#include "mozilla/TextUtils.h" using mozilla::ArrayLength; +using mozilla::DecodeOneUtf8CodePoint; +using mozilla::EnumSet; +using mozilla::IntegerRange; +using mozilla::IsAscii; using mozilla::IsValidUtf8; using mozilla::Utf8Unit; @@ -35,6 +42,242 @@ TestUtf8Unit() MOZ_RELEASE_ASSERT(first == second); } +template +struct ToUtf8Units +{ +public: + explicit ToUtf8Units(const Char* aStart, const Char* aEnd) + : lead(Utf8Unit(aStart[0])) + , iter(aStart + 1) + , end(aEnd) + { + MOZ_RELEASE_ASSERT(!IsAscii(aStart[0])); + } + + const Utf8Unit lead; + const Char* iter; + const Char* const end; +}; + +class AssertIfCalled +{ +public: + template + void operator()(Args&&... aArgs) { + MOZ_RELEASE_ASSERT(false, "AssertIfCalled instance was called"); + } +}; + +// NOTE: For simplicity in treating |aCharN| identically regardless whether it's +// a string literal or a more-generalized array, we require |aCharN| be +// null-terminated. + +template +static void +ExpectValidCodePoint(const Char (&aCharN)[N], + char32_t aExpectedCodePoint) +{ + MOZ_RELEASE_ASSERT(aCharN[N - 1] == 0, + "array must be null-terminated for |aCharN + N - 1| to " + "compute the value of |aIter| as altered by " + "DecodeOneUtf8CodePoint"); + + ToUtf8Units simpleUnit(aCharN, aCharN + N - 1); + auto simple = + DecodeOneUtf8CodePoint(simpleUnit.lead, &simpleUnit.iter, simpleUnit.end); + MOZ_RELEASE_ASSERT(simple.isSome()); + MOZ_RELEASE_ASSERT(*simple == aExpectedCodePoint); + MOZ_RELEASE_ASSERT(simpleUnit.iter == simpleUnit.end); + + ToUtf8Units complexUnit(aCharN, aCharN + N - 1); + auto complex = + DecodeOneUtf8CodePoint(complexUnit.lead, &complexUnit.iter, complexUnit.end, + AssertIfCalled(), + AssertIfCalled(), + AssertIfCalled(), + AssertIfCalled(), + AssertIfCalled()); + MOZ_RELEASE_ASSERT(complex.isSome()); + MOZ_RELEASE_ASSERT(*complex == aExpectedCodePoint); + MOZ_RELEASE_ASSERT(complexUnit.iter == complexUnit.end); +} + +enum class InvalidUtf8Reason +{ + BadLeadUnit, + NotEnoughUnits, + BadTrailingUnit, + BadCodePoint, + NotShortestForm, +}; + +template +static void +ExpectInvalidCodePointHelper(const Char (&aCharN)[N], + InvalidUtf8Reason aExpectedReason, + uint8_t aExpectedUnitsAvailable, + uint8_t aExpectedUnitsNeeded, + char32_t aExpectedBadCodePoint, + uint8_t aExpectedUnitsObserved) +{ + MOZ_RELEASE_ASSERT(aCharN[N - 1] == 0, + "array must be null-terminated for |aCharN + N - 1| to " + "compute the value of |aIter| as altered by " + "DecodeOneUtf8CodePoint"); + + ToUtf8Units simpleUnit(aCharN, aCharN + N - 1); + auto simple = + DecodeOneUtf8CodePoint(simpleUnit.lead, &simpleUnit.iter, simpleUnit.end); + MOZ_RELEASE_ASSERT(simple.isNothing()); + MOZ_RELEASE_ASSERT(static_cast(simpleUnit.iter) == aCharN); + + EnumSet reasons; + uint8_t unitsAvailable; + uint8_t unitsNeeded; + char32_t badCodePoint; + uint8_t unitsObserved; + + struct OnNotShortestForm + { + EnumSet& reasons; + char32_t& badCodePoint; + uint8_t& unitsObserved; + + void operator()(char32_t aBadCodePoint, uint8_t aUnitsObserved) { + reasons += InvalidUtf8Reason::NotShortestForm; + badCodePoint = aBadCodePoint; + unitsObserved = aUnitsObserved; + } + }; + + ToUtf8Units complexUnit(aCharN, aCharN + N - 1); + auto complex = + DecodeOneUtf8CodePoint(complexUnit.lead, &complexUnit.iter, complexUnit.end, + [&reasons]() { + reasons += InvalidUtf8Reason::BadLeadUnit; + }, + [&reasons, &unitsAvailable, &unitsNeeded](uint8_t aUnitsAvailable, + uint8_t aUnitsNeeded) + { + reasons += InvalidUtf8Reason::NotEnoughUnits; + unitsAvailable = aUnitsAvailable; + unitsNeeded = aUnitsNeeded; + }, + [&reasons, &unitsObserved](uint8_t aUnitsObserved) + { + reasons += InvalidUtf8Reason::BadTrailingUnit; + unitsObserved = aUnitsObserved; + }, + [&reasons, &badCodePoint, &unitsObserved](char32_t aBadCodePoint, + uint8_t aUnitsObserved) + { + reasons += InvalidUtf8Reason::BadCodePoint; + badCodePoint = aBadCodePoint; + unitsObserved = aUnitsObserved; + }, + [&reasons, &badCodePoint, &unitsObserved](char32_t aBadCodePoint, + uint8_t aUnitsObserved) + { + reasons += InvalidUtf8Reason::NotShortestForm; + badCodePoint = aBadCodePoint; + unitsObserved = aUnitsObserved; + }); + MOZ_RELEASE_ASSERT(complex.isNothing()); + MOZ_RELEASE_ASSERT(static_cast(complexUnit.iter) == aCharN); + + bool alreadyIterated = false; + for (InvalidUtf8Reason reason : reasons) { + MOZ_RELEASE_ASSERT(!alreadyIterated); + alreadyIterated = true; + + switch (reason) { + case InvalidUtf8Reason::BadLeadUnit: + break; + + case InvalidUtf8Reason::NotEnoughUnits: + MOZ_RELEASE_ASSERT(unitsAvailable == aExpectedUnitsAvailable); + MOZ_RELEASE_ASSERT(unitsNeeded == aExpectedUnitsNeeded); + break; + + case InvalidUtf8Reason::BadTrailingUnit: + MOZ_RELEASE_ASSERT(unitsObserved == aExpectedUnitsObserved); + break; + + case InvalidUtf8Reason::BadCodePoint: + MOZ_RELEASE_ASSERT(badCodePoint == aExpectedBadCodePoint); + MOZ_RELEASE_ASSERT(unitsObserved == aExpectedUnitsObserved); + break; + + case InvalidUtf8Reason::NotShortestForm: + MOZ_RELEASE_ASSERT(badCodePoint == aExpectedBadCodePoint); + MOZ_RELEASE_ASSERT(unitsObserved == aExpectedUnitsObserved); + break; + } + } +} + +// NOTE: For simplicity in treating |aCharN| identically regardless whether it's +// a string literal or a more-generalized array, we require |aCharN| be +// null-terminated in all these functions. + +template +static void +ExpectBadLeadUnit(const Char (&aCharN)[N]) +{ + ExpectInvalidCodePointHelper(aCharN, + InvalidUtf8Reason::BadLeadUnit, + 0xFF, 0xFF, 0xFFFFFFFF, 0xFF); +} + +template +static void +ExpectNotEnoughUnits(const Char (&aCharN)[N], + uint8_t aExpectedUnitsAvailable, + uint8_t aExpectedUnitsNeeded) +{ + ExpectInvalidCodePointHelper(aCharN, + InvalidUtf8Reason::NotEnoughUnits, + aExpectedUnitsAvailable, aExpectedUnitsNeeded, + 0xFFFFFFFF, 0xFF); +} + +template +static void +ExpectBadTrailingUnit(const Char (&aCharN)[N], + uint8_t aExpectedUnitsObserved) +{ + ExpectInvalidCodePointHelper(aCharN, + InvalidUtf8Reason::BadTrailingUnit, + 0xFF, 0xFF, 0xFFFFFFFF, + aExpectedUnitsObserved); +} + +template +static void +ExpectNotShortestForm(const Char (&aCharN)[N], + char32_t aExpectedBadCodePoint, + uint8_t aExpectedUnitsObserved) +{ + ExpectInvalidCodePointHelper(aCharN, + InvalidUtf8Reason::NotShortestForm, + 0xFF, 0xFF, + aExpectedBadCodePoint, + aExpectedUnitsObserved); +} + +template +static void +ExpectBadCodePoint(const Char (&aCharN)[N], + char32_t aExpectedBadCodePoint, + uint8_t aExpectedUnitsObserved) +{ + ExpectInvalidCodePointHelper(aCharN, + InvalidUtf8Reason::BadCodePoint, + 0xFF, 0xFF, + aExpectedBadCodePoint, + aExpectedUnitsObserved); +} + static void TestIsValidUtf8() { @@ -62,48 +305,469 @@ TestIsValidUtf8() static_assert(twoBytesLen == 3, "U+0606 in two bytes plus nul"); MOZ_RELEASE_ASSERT(IsValidUtf8(twoBytes, twoBytesLen)); + ExpectValidCodePoint(twoBytes, 0x0606); + // 3 static const char threeBytes[] = u8"᨞"; // U+1A1E BUGINESE PALLAWA constexpr size_t threeBytesLen = ArrayLength(threeBytes); static_assert(threeBytesLen == 4, "U+1A1E in three bytes plus nul"); MOZ_RELEASE_ASSERT(IsValidUtf8(threeBytes, threeBytesLen)); + ExpectValidCodePoint(threeBytes, 0x1A1E); + // 4 static const char fourBytes[] = u8"🁡"; // U+1F061 DOMINO TILE HORIZONTAL-06-06 constexpr size_t fourBytesLen = ArrayLength(fourBytes); static_assert(fourBytesLen == 5, "U+1F061 in four bytes plus nul"); MOZ_RELEASE_ASSERT(IsValidUtf8(fourBytes, fourBytesLen)); + ExpectValidCodePoint(fourBytes, 0x1F061); + // Max code point static const char maxCodePoint[] = u8"􏿿"; // U+10FFFF constexpr size_t maxCodePointLen = ArrayLength(maxCodePoint); static_assert(maxCodePointLen == 5, "U+10FFFF in four bytes plus nul"); MOZ_RELEASE_ASSERT(IsValidUtf8(maxCodePoint, maxCodePointLen)); + ExpectValidCodePoint(maxCodePoint, 0x10FFFF); + // One past max code point - static unsigned const char onePastMaxCodePoint[] = { 0xF4, 0x90, 0x80, 0x80 }; + static const unsigned char onePastMaxCodePoint[] = { 0xF4, 0x90, 0x80, 0x80, 0x0 }; constexpr size_t onePastMaxCodePointLen = ArrayLength(onePastMaxCodePoint); MOZ_RELEASE_ASSERT(!IsValidUtf8(onePastMaxCodePoint, onePastMaxCodePointLen)); + ExpectBadCodePoint(onePastMaxCodePoint, 0x110000, 4); + // Surrogate-related testing - static const unsigned char justBeforeSurrogates[] = { 0xED, 0x9F, 0xBF }; - MOZ_RELEASE_ASSERT(IsValidUtf8(justBeforeSurrogates, ArrayLength(justBeforeSurrogates))); + // (Note that the various code unit sequences here are null-terminated to + // simplify life for ExpectValidCodePoint, which presumes null termination.) - static const unsigned char leastSurrogate[] = { 0xED, 0xA0, 0x80 }; - MOZ_RELEASE_ASSERT(!IsValidUtf8(leastSurrogate, ArrayLength(leastSurrogate))); + static const unsigned char justBeforeSurrogates[] = { 0xED, 0x9F, 0xBF, 0x0 }; + constexpr size_t justBeforeSurrogatesLen = ArrayLength(justBeforeSurrogates) - 1; + MOZ_RELEASE_ASSERT(IsValidUtf8(justBeforeSurrogates, justBeforeSurrogatesLen)); - static const unsigned char arbitraryHighSurrogate[] = { 0xED, 0xA2, 0x87 }; - MOZ_RELEASE_ASSERT(!IsValidUtf8(arbitraryHighSurrogate, ArrayLength(arbitraryHighSurrogate))); + ExpectValidCodePoint(justBeforeSurrogates, 0xD7FF); - static const unsigned char arbitraryLowSurrogate[] = { 0xED, 0xB7, 0xAF }; - MOZ_RELEASE_ASSERT(!IsValidUtf8(arbitraryLowSurrogate, ArrayLength(arbitraryLowSurrogate))); + static const unsigned char leastSurrogate[] = { 0xED, 0xA0, 0x80, 0x0 }; + constexpr size_t leastSurrogateLen = ArrayLength(leastSurrogate) - 1; + MOZ_RELEASE_ASSERT(!IsValidUtf8(leastSurrogate, leastSurrogateLen)); - static const unsigned char greatestSurrogate[] = { 0xED, 0xBF, 0xBF }; - MOZ_RELEASE_ASSERT(!IsValidUtf8(greatestSurrogate, ArrayLength(greatestSurrogate))); + ExpectBadCodePoint(leastSurrogate, 0xD800, 3); - static const unsigned char justAfterSurrogates[] = { 0xEE, 0x80, 0x80 }; - MOZ_RELEASE_ASSERT(IsValidUtf8(justAfterSurrogates, ArrayLength(justAfterSurrogates))); + static const unsigned char arbitraryHighSurrogate[] = { 0xED, 0xA2, 0x87, 0x0 }; + constexpr size_t arbitraryHighSurrogateLen = ArrayLength(arbitraryHighSurrogate) - 1; + MOZ_RELEASE_ASSERT(!IsValidUtf8(arbitraryHighSurrogate, arbitraryHighSurrogateLen)); + + ExpectBadCodePoint(arbitraryHighSurrogate, 0xD887, 3); + + static const unsigned char arbitraryLowSurrogate[] = { 0xED, 0xB7, 0xAF, 0x0 }; + constexpr size_t arbitraryLowSurrogateLen = ArrayLength(arbitraryLowSurrogate) - 1; + MOZ_RELEASE_ASSERT(!IsValidUtf8(arbitraryLowSurrogate, arbitraryLowSurrogateLen)); + + ExpectBadCodePoint(arbitraryLowSurrogate, 0xDDEF, 3); + + static const unsigned char greatestSurrogate[] = { 0xED, 0xBF, 0xBF, 0x0 }; + constexpr size_t greatestSurrogateLen = ArrayLength(greatestSurrogate) - 1; + MOZ_RELEASE_ASSERT(!IsValidUtf8(greatestSurrogate, greatestSurrogateLen)); + + ExpectBadCodePoint(greatestSurrogate, 0xDFFF, 3); + + static const unsigned char justAfterSurrogates[] = { 0xEE, 0x80, 0x80, 0x0 }; + constexpr size_t justAfterSurrogatesLen = ArrayLength(justAfterSurrogates) - 1; + MOZ_RELEASE_ASSERT(IsValidUtf8(justAfterSurrogates, justAfterSurrogatesLen)); + + ExpectValidCodePoint(justAfterSurrogates, 0xE000); +} + +static void +TestDecodeOneValidUtf8CodePoint() +{ + // NOTE: DecodeOneUtf8CodePoint decodes only *non*-ASCII code points that + // consist of multiple code units, so there are no ASCII tests below. + + // Length two. + + ExpectValidCodePoint(u8"€", 0x80); // + ExpectValidCodePoint(u8"©", 0xA9); // COPYRIGHT SIGN + ExpectValidCodePoint(u8"¶", 0xB6); // PILCROW SIGN + ExpectValidCodePoint(u8"¾", 0xBE); // VULGAR FRACTION THREE QUARTERS + ExpectValidCodePoint(u8"÷", 0xF7); // DIVISION SIGN + ExpectValidCodePoint(u8"ÿ", 0xFF); // LATIN SMALL LETTER Y WITH DIAERESIS + ExpectValidCodePoint(u8"Ā", 0x100); // LATIN CAPITAL LETTER A WITH MACRON + ExpectValidCodePoint(u8"IJ", 0x132); // LATIN CAPITAL LETTER LIGATURE IJ + ExpectValidCodePoint(u8"ͼ", 0x37C); // GREEK SMALL DOTTED LUNATE SIGMA SYMBOL + ExpectValidCodePoint(u8"Ӝ", 0x4DC); // CYRILLIC CAPITAL LETTER ZHE WITTH DIAERESIS + ExpectValidCodePoint(u8"۩", 0x6E9); // ARABIC PLACE OF SAJDAH + ExpectValidCodePoint(u8"߿", 0x7FF); // + + // Length three. + + ExpectValidCodePoint(u8"ࠀ", 0x800); // SAMARITAN LETTER ALAF + ExpectValidCodePoint(u8"ࡁ", 0x841); // MANDAIC LETTER AB + ExpectValidCodePoint(u8"ࣿ", 0x8FF); // ARABIC MARK SIDEWAYS NOON GHUNNA + ExpectValidCodePoint(u8"ஆ", 0xB86); // TAMIL LETTER AA + ExpectValidCodePoint(u8"༃", 0xF03); // TIBETAN MARK GTER YIG MGO -UM GTER TSHEG MA + ExpectValidCodePoint(u8"࿉", 0xFC9); // TIBETAN SYMBOL NOR BU (but on my system it really looks like SOFT-SERVE ICE CREAM FROM ABOVE THE PLANE if you ask me) + ExpectValidCodePoint(u8"ဪ", 0x102A); // MYANMAR LETTER AU + ExpectValidCodePoint(u8"ᚏ", 0x168F); // OGHAM LETTER RUIS + ExpectValidCodePoint("\xE2\x80\xA8", 0x2028); // (the hated) LINE SEPARATOR + ExpectValidCodePoint("\xE2\x80\xA9", 0x2029); // (the hated) PARAGRAPH SEPARATOR + ExpectValidCodePoint(u8"☬", 0x262C); // ADI SHAKTI + ExpectValidCodePoint(u8"㊮", 0x32AE); // CIRCLED IDEOGRAPH RESOURCE + ExpectValidCodePoint(u8"㏖", 0x33D6); // SQUARE MOL + ExpectValidCodePoint(u8"ꔄ", 0xA504); // VAI SYLLABLE WEEN + ExpectValidCodePoint(u8"ퟕ", 0xD7D5); // HANGUL JONGSEONG RIEUL-SSANGKIYEOK + ExpectValidCodePoint(u8"퟿", 0xD7FF); // + ExpectValidCodePoint(u8"", 0xE000); // + ExpectValidCodePoint(u8"鱗", 0xF9F2); // CJK COMPATIBILITY IDEOGRAPH-F9F + ExpectValidCodePoint(u8"﷽", 0xFDFD); // ARABIC LIGATURE BISMILLAH AR-RAHMAN AR-RAHHHEEEEM + ExpectValidCodePoint(u8"￿", 0xFFFF); // + + // Length four. + ExpectValidCodePoint(u8"𐀀", 0x10000); // LINEAR B SYLLABLE B008 A + ExpectValidCodePoint(u8"𔑀", 0x14440); // ANATOLIAN HIEROGLYPH A058 + ExpectValidCodePoint(u8"𝛗", 0x1D6D7); // MATHEMATICAL BOLD SMALL PHI + ExpectValidCodePoint(u8"💩", 0x1F4A9); // PILE OF POO + ExpectValidCodePoint(u8"🔫", 0x1F52B); // PISTOL + ExpectValidCodePoint(u8"🥌", 0x1F94C); // CURLING STONE + ExpectValidCodePoint(u8"🥏", 0x1F94F); // FLYING DISC + ExpectValidCodePoint(u8"𠍆", 0x20346); // CJK UNIFIED IDEOGRAPH-20346 + ExpectValidCodePoint(u8"𡠺", 0x2183A); // CJK UNIFIED IDEOGRAPH-2183A + ExpectValidCodePoint(u8"񁟶", 0x417F6); // + ExpectValidCodePoint(u8"񾠶", 0x7E836); // + ExpectValidCodePoint(u8"󾽧", 0xFEF67); // + ExpectValidCodePoint(u8"􏿿", 0x10FFFF); // +} + +static void +TestDecodeBadLeadUnit() +{ + // These tests are actually exhaustive. + + unsigned char badLead[] = { '\0', '\0' }; + + for (uint8_t lead : IntegerRange(0b1000'0000, 0b1100'0000)) { + badLead[0] = lead; + ExpectBadLeadUnit(badLead); + } + + { + uint8_t lead = 0b1111'1000; + do { + badLead[0] = lead; + ExpectBadLeadUnit(badLead); + if (lead == 0b1111'1111) { + break; + } + + lead++; + } while (true); + } +} + +static void +TestTooFewOrBadTrailingUnits() +{ + // Lead unit indicates a two-byte code point. + + char truncatedTwo[] = { '\0', '\0' }; + char badTrailTwo[] = { '\0', '\0', '\0' }; + + for (uint8_t lead : IntegerRange(0b1100'0000, 0b1110'0000)) { + truncatedTwo[0] = lead; + ExpectNotEnoughUnits(truncatedTwo, 1, 2); + + badTrailTwo[0] = lead; + for (uint8_t trail : IntegerRange(0b0000'0000, 0b1000'0000)) { + badTrailTwo[1] = trail; + ExpectBadTrailingUnit(badTrailTwo, 2); + } + + for (uint8_t trail : IntegerRange(0b1100'0000, 0b1111'1111)) { + badTrailTwo[1] = trail; + ExpectBadTrailingUnit(badTrailTwo, 2); + } + } + + // Lead unit indicates a three-byte code point. + + char truncatedThreeOne[] = { '\0', '\0' }; + char truncatedThreeTwo[] = { '\0', '\0', '\0' }; + unsigned char badTrailThree[] = { '\0', '\0', '\0', '\0' }; + + for (uint8_t lead : IntegerRange(0b1110'0000, 0b1111'0000)) { + truncatedThreeOne[0] = lead; + ExpectNotEnoughUnits(truncatedThreeOne, 1, 3); + + truncatedThreeTwo[0] = lead; + ExpectNotEnoughUnits(truncatedThreeTwo, 2, 3); + + badTrailThree[0] = lead; + badTrailThree[2] = 0b1011'1111; // make valid to test overreads + for (uint8_t mid : IntegerRange(0b0000'0000, 0b1000'0000)) { + badTrailThree[1] = mid; + ExpectBadTrailingUnit(badTrailThree, 2); + } + { + uint8_t mid = 0b1100'0000; + do { + badTrailThree[1] = mid; + ExpectBadTrailingUnit(badTrailThree, 2); + if (mid == 0b1111'1111) { + break; + } + + mid++; + } while (true); + } + + badTrailThree[1] = 0b1011'1111; + for (uint8_t last : IntegerRange(0b0000'0000, 0b1000'0000)) { + badTrailThree[2] = last; + ExpectBadTrailingUnit(badTrailThree, 3); + } + { + uint8_t last = 0b1100'0000; + do { + badTrailThree[2] = last; + ExpectBadTrailingUnit(badTrailThree, 3); + if (last == 0b1111'1111) { + break; + } + + last++; + } while (true); + } + } + + // Lead unit indicates a four-byte code point. + + char truncatedFourOne[] = { '\0', '\0' }; + char truncatedFourTwo[] = { '\0', '\0', '\0' }; + char truncatedFourThree[] = { '\0', '\0', '\0', '\0' }; + + unsigned char badTrailFour[] = { '\0', '\0', '\0', '\0', '\0' }; + + for (uint8_t lead : IntegerRange(0b1111'0000, 0b1111'1000)) { + truncatedFourOne[0] = lead; + ExpectNotEnoughUnits(truncatedFourOne, 1, 4); + + truncatedFourTwo[0] = lead; + ExpectNotEnoughUnits(truncatedFourTwo, 2, 4); + + truncatedFourThree[0] = lead; + ExpectNotEnoughUnits(truncatedFourThree, 3, 4); + + badTrailFour[0] = lead; + badTrailFour[2] = badTrailFour[3] = 0b1011'1111; // test for overreads + for (uint8_t second : IntegerRange(0b0000'0000, 0b1000'0000)) { + badTrailFour[1] = second; + ExpectBadTrailingUnit(badTrailFour, 2); + } + { + uint8_t second = 0b1100'0000; + do { + badTrailFour[1] = second; + ExpectBadTrailingUnit(badTrailFour, 2); + if (second == 0b1111'1111) { + break; + } + + second++; + } while (true); + } + + badTrailFour[1] = badTrailFour[3] = 0b1011'1111; // test for overreads + for (uint8_t third : IntegerRange(0b0000'0000, 0b1000'0000)) { + badTrailFour[2] = third; + ExpectBadTrailingUnit(badTrailFour, 3); + } + { + uint8_t third = 0b1100'0000; + do { + badTrailFour[2] = third; + ExpectBadTrailingUnit(badTrailFour, 3); + if (third == 0b1111'1111) { + break; + } + + third++; + } while (true); + } + + badTrailFour[2] = 0b1011'1111; + for (uint8_t fourth : IntegerRange(0b0000'0000, 0b1000'0000)) { + badTrailFour[3] = fourth; + ExpectBadTrailingUnit(badTrailFour, 4); + } + { + uint8_t fourth = 0b1100'0000; + do { + badTrailFour[3] = fourth; + ExpectBadTrailingUnit(badTrailFour, 4); + if (fourth == 0b1111'1111) { + break; + } + + fourth++; + } while (true); + } + } +} + +static void +TestBadSurrogate() +{ + // These tests are actually exhaustive. + + ExpectValidCodePoint("\xED\x9F\xBF", 0xD7FF); // last before surrogates + ExpectValidCodePoint("\xEE\x80\x80", 0xE000); // first after surrogates + + // First invalid surrogate encoding is { 0xED, 0xA0, 0x80 }. Last invalid + // surrogate encoding is { 0xED, 0xBF, 0xBF }. + + char badSurrogate[] = { '\xED', '\0', '\0', '\0' }; + + for (char32_t c = 0xD800; c < 0xE000; c++) { + badSurrogate[1] = 0b1000'0000 ^ ((c & 0b1111'1100'0000) >> 6); + badSurrogate[2] = 0b1000'0000 ^ ((c & 0b0000'0011'1111)); + + ExpectBadCodePoint(badSurrogate, c, 3); + } +} + +static void +TestBadTooBig() +{ + // These tests are actually exhaustive. + + ExpectValidCodePoint("\xF4\x8F\xBF\xBF", 0x10'FFFF); // last code point + + // Four-byte code points are + // + // 0b1111'0xxx 0b10xx'xxxx 0b10xx'xxxx 0b10xx'xxxx + // + // with 3 + 6 + 6 + 6 == 21 unconstrained bytes, so the structurally + // representable limit (exclusive) is 2**21 - 1 == 2097152. + + char tooLargeCodePoint[] = { '\0', '\0', '\0', '\0', '\0' }; + + for (char32_t c = 0x11'0000; c < (1 << 21); c++) { + tooLargeCodePoint[0] = 0b1111'0000 ^ ((c & 0b1'1100'0000'0000'0000'0000) >> 18); + tooLargeCodePoint[1] = 0b1000'0000 ^ ((c & 0b0'0011'1111'0000'0000'0000) >> 12); + tooLargeCodePoint[2] = 0b1000'0000 ^ ((c & 0b0'0000'0000'1111'1100'0000) >> 6); + tooLargeCodePoint[3] = 0b1000'0000 ^ ((c & 0b0'0000'0000'0000'0011'1111)); + + ExpectBadCodePoint(tooLargeCodePoint, c, 4); + } +} + +static void +TestBadCodePoint() +{ + TestBadSurrogate(); + TestBadTooBig(); +} + +static void +TestNotShortestForm() +{ + { + // One-byte in two-byte. + + char oneInTwo[] = { '\0', '\0', '\0' }; + + for (char32_t c = '\0'; c < 0x80; c++) { + oneInTwo[0] = 0b1100'0000 ^ ((c & 0b0111'1100'0000) >> 6); + oneInTwo[1] = 0b1000'0000 ^ ((c & 0b0000'0011'1111)); + + ExpectNotShortestForm(oneInTwo, c, 2); + } + + // One-byte in three-byte. + + char oneInThree[] = { '\0', '\0', '\0', '\0' }; + + for (char32_t c = '\0'; c < 0x80; c++) { + oneInThree[0] = 0b1110'0000 ^ ((c & 0b1111'0000'0000'0000) >> 12); + oneInThree[1] = 0b1000'0000 ^ ((c & 0b0000'1111'1100'0000) >> 6); + oneInThree[2] = 0b1000'0000 ^ ((c & 0b0000'0000'0011'1111)); + + ExpectNotShortestForm(oneInThree, c, 3); + } + + // One-byte in four-byte. + + char oneInFour[] = { '\0', '\0', '\0', '\0', '\0' }; + + for (char32_t c = '\0'; c < 0x80; c++) { + oneInFour[0] = 0b1111'0000 ^ ((c & 0b1'1100'0000'0000'0000'0000) >> 18); + oneInFour[1] = 0b1000'0000 ^ ((c & 0b0'0011'1111'0000'0000'0000) >> 12); + oneInFour[2] = 0b1000'0000 ^ ((c & 0b0'0000'0000'1111'1100'0000) >> 6); + oneInFour[3] = 0b1000'0000 ^ ((c & 0b0'0000'0000'0000'0011'1111)); + + ExpectNotShortestForm(oneInFour, c, 4); + } + } + + { + // Two-byte in three-byte. + + char twoInThree[] = { '\0', '\0', '\0', '\0' }; + + for (char32_t c = 0x80; c < 0x800; c++) { + twoInThree[0] = 0b1110'0000 ^ ((c & 0b1111'0000'0000'0000) >> 12); + twoInThree[1] = 0b1000'0000 ^ ((c & 0b0000'1111'1100'0000) >> 6); + twoInThree[2] = 0b1000'0000 ^ ((c & 0b0000'0000'0011'1111)); + + ExpectNotShortestForm(twoInThree, c, 3); + } + + // Two-byte in four-byte. + + char twoInFour[] = { '\0', '\0', '\0', '\0', '\0' }; + + for (char32_t c = 0x80; c < 0x800; c++) { + twoInFour[0] = 0b1111'0000 ^ ((c & 0b1'1100'0000'0000'0000'0000) >> 18); + twoInFour[1] = 0b1000'0000 ^ ((c & 0b0'0011'1111'0000'0000'0000) >> 12); + twoInFour[2] = 0b1000'0000 ^ ((c & 0b0'0000'0000'1111'1100'0000) >> 6); + twoInFour[3] = 0b1000'0000 ^ ((c & 0b0'0000'0000'0000'0011'1111)); + + ExpectNotShortestForm(twoInFour, c, 4); + } + } + + { + // Three-byte in four-byte. + + char threeInFour[] = { '\0', '\0', '\0', '\0', '\0' }; + + for (char32_t c = 0x800; c < 0x1'0000; c++) { + threeInFour[0] = 0b1111'0000 ^ ((c & 0b1'1100'0000'0000'0000'0000) >> 18); + threeInFour[1] = 0b1000'0000 ^ ((c & 0b0'0011'1111'0000'0000'0000) >> 12); + threeInFour[2] = 0b1000'0000 ^ ((c & 0b0'0000'0000'1111'1100'0000) >> 6); + threeInFour[3] = 0b1000'0000 ^ ((c & 0b0'0000'0000'0000'0011'1111)); + + ExpectNotShortestForm(threeInFour, c, 4); + } + } +} + +static void +TestDecodeOneInvalidUtf8CodePoint() +{ + TestDecodeBadLeadUnit(); + TestTooFewOrBadTrailingUnits(); + TestBadCodePoint(); + TestNotShortestForm(); +} + +static void +TestDecodeOneUtf8CodePoint() +{ + TestDecodeOneValidUtf8CodePoint(); + TestDecodeOneInvalidUtf8CodePoint(); } int @@ -111,5 +775,6 @@ main() { TestUtf8Unit(); TestIsValidUtf8(); + TestDecodeOneUtf8CodePoint(); return 0; } From 856781b0bb4959475a3d6cd6112d0952dbbf2a49 Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Thu, 19 Jul 2018 13:16:29 +1000 Subject: [PATCH 31/37] Bug 1476500 - Fix some prose in the MacroAssembler class comment r=tcampbell,nbp Also fix an out-of-date reference to check-masm --HG-- extra : rebase_source : f7a23870d844eabd8af82aa87a49895e0443e3c6 --- js/src/jit/MacroAssembler.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/js/src/jit/MacroAssembler.h b/js/src/jit/MacroAssembler.h index bd5c3f2fe0bc..054d1feeeb01 100644 --- a/js/src/jit/MacroAssembler.h +++ b/js/src/jit/MacroAssembler.h @@ -60,11 +60,11 @@ // - If the declaration is "inline", then the method definition(s) would be in // the "-inl.h" variant of the same file(s). // -// The script check_macroassembler_style.py (check-masm target of the Makefile) -// is used to verify that method definitions are matching the annotation added -// to the method declarations. If there is any difference, then you either -// forgot to define the method in one of the macro assembler, or you forgot to -// update the annotation of the macro assembler declaration. +// The script check_macroassembler_style.py (which runs on every build) is +// used to verify that method definitions match the annotation on the method +// declarations. If there is any difference, then you either forgot to define +// the method in one of the macro assembler, or you forgot to update the +// annotation of the macro assembler declaration. // // Some convenient short-cuts are used to avoid repeating the same list of // architectures on each method declaration, such as PER_ARCH and @@ -79,7 +79,7 @@ // inline uint32_t framePushed() const OOL_IN_HEADER; // // Such functions should then be defined immediately after MacroAssembler's -// definition, for example like so: +// definition, for example: // // //{{{ check_macroassembler_style // inline uint32_t @@ -100,10 +100,10 @@ // // For each architecture, we have a macro named DEFINED_ON_arch. This macro is // empty if this is not the current architecture. Otherwise it must be either -// set to "define" or "crash" (only use for the none target so-far). +// set to "define" or "crash" (only used for the none target so far). // -// The DEFINED_ON macro maps the list of architecture names given as argument to -// a list of macro names. For example, +// The DEFINED_ON macro maps the list of architecture names given as arguments +// to a list of macro names. For example, // // DEFINED_ON(arm, x86_shared) // @@ -126,7 +126,7 @@ // architecture if it is listed in the arguments of DEFINED_ON. // // This result is appended to DEFINED_ON_RESULT_ before expanding the macro, -// which result is either no annotation, a MOZ_CRASH(), or a "= delete" +// which results in either no annotation, a MOZ_CRASH(), or a "= delete" // annotation on the method declaration. # define DEFINED_ON_x86 From 1a917abbc8fe024ad660ad9bfeefec7351a1df12 Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Fri, 20 Jul 2018 12:42:33 +1000 Subject: [PATCH 32/37] Bug 1476500 - Add extra assignments and make some types more specific r=nbp --HG-- extra : rebase_source : af6c06d44b181095c15fe34d8529aae72c486ec8 --- js/src/gc/ArenaList.h | 4 ++-- js/src/gc/GCRuntime.h | 6 ++++-- js/src/gc/Nursery.h | 8 +++++--- js/src/jit/CodeGenerator.cpp | 8 +++++--- js/src/jit/CompileWrappers.cpp | 12 ++++++------ js/src/jit/CompileWrappers.h | 13 +++++++------ js/src/jit/MacroAssembler-inl.h | 4 ++-- js/src/jit/MacroAssembler.cpp | 23 ++++++++++++++--------- js/src/vm/Realm.h | 3 ++- 9 files changed, 47 insertions(+), 34 deletions(-) diff --git a/js/src/gc/ArenaList.h b/js/src/gc/ArenaList.h index 1b61d36a5435..5cd8e05ae97c 100644 --- a/js/src/gc/ArenaList.h +++ b/js/src/gc/ArenaList.h @@ -241,7 +241,7 @@ class FreeLists inline void unmarkPreMarkedFreeCells(AllocKind kind); - const void* addressOfFreeList(AllocKind thingKind) const { + FreeSpan** addressOfFreeList(AllocKind thingKind) { return &freeLists_[thingKind]; } }; @@ -298,7 +298,7 @@ class ArenaLists FreeLists& freeLists() { return freeLists_.ref(); } const FreeLists& freeLists() const { return freeLists_.ref(); } - const void* addressOfFreeList(AllocKind thingKind) const { + FreeSpan** addressOfFreeList(AllocKind thingKind) { return freeLists_.refNoCheck().addressOfFreeList(thingKind); } diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h index 7ec86590369d..37cbac26e3fb 100644 --- a/js/src/gc/GCRuntime.h +++ b/js/src/gc/GCRuntime.h @@ -293,7 +293,9 @@ class GCRuntime void onOutOfMallocMemory(const AutoLockGC& lock); #ifdef JS_GC_ZEAL - const void* addressOfZealModeBits() { return &zealModeBits; } + const uint32_t* addressOfZealModeBits() { + return &zealModeBits.refNoCheck(); + } void getZealBits(uint32_t* zealBits, uint32_t* frequency, uint32_t* nextScheduled); void setZeal(uint8_t zeal, uint32_t frequency); bool parseAndSetZeal(const char* str); @@ -1005,7 +1007,7 @@ class GCRuntime // after minor GC. MainThreadData blocksToFreeAfterMinorGC; - const void* addressOfNurseryPosition() { + void* addressOfNurseryPosition() { return nursery_.refNoCheck().addressOfPosition(); } const void* addressOfNurseryCurrentEnd() { diff --git a/js/src/gc/Nursery.h b/js/src/gc/Nursery.h index c64e262bd694..86437511df51 100644 --- a/js/src/gc/Nursery.h +++ b/js/src/gc/Nursery.h @@ -324,9 +324,11 @@ class Nursery /* Print total profile times on shutdown. */ void printTotalProfileTimes(); - void* addressOfCurrentEnd() const { return (void*)¤tEnd_; } - void* addressOfPosition() const { return (void*)&position_; } - void* addressOfCurrentStringEnd() const { return (void*)¤tStringEnd_; } + void* addressOfPosition() const { return (void**)&position_; } + const void* addressOfCurrentEnd() const { return (void**)¤tEnd_; } + const void* addressOfCurrentStringEnd() const { + return (void*)¤tStringEnd_; + } void requestMinorGC(JS::gcreason::Reason reason) const; diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index bc069b71111f..7821795b53ab 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -4049,8 +4049,9 @@ CodeGenerator::maybeEmitGlobalBarrierCheck(const LAllocation* maybeGlobal, OutOf if (gen->realm->maybeGlobal() != obj) return; - auto addr = AbsoluteAddress(gen->realm->addressOfGlobalWriteBarriered()); - masm.branch32(Assembler::NotEqual, addr, Imm32(0), ool->rejoin()); + const uint32_t* addr = gen->realm->addressOfGlobalWriteBarriered(); + masm.branch32(Assembler::NotEqual, AbsoluteAddress(addr), Imm32(0), + ool->rejoin()); } template @@ -13286,7 +13287,8 @@ CodeGenerator::visitRandom(LRandom* ins) Register64 s1Reg(ToRegister(ins->temp3()), ToRegister(ins->temp4())); #endif - const void* rng = gen->realm->addressOfRandomNumberGenerator(); + const XorShift128PlusRNG* rng = + gen->realm->addressOfRandomNumberGenerator(); masm.movePtr(ImmPtr(rng), tempReg); static_assert(sizeof(XorShift128PlusRNG) == 2 * sizeof(uint64_t), diff --git a/js/src/jit/CompileWrappers.cpp b/js/src/jit/CompileWrappers.cpp index 0cffda9942d0..47d8829a8861 100644 --- a/js/src/jit/CompileWrappers.cpp +++ b/js/src/jit/CompileWrappers.cpp @@ -28,7 +28,7 @@ CompileRuntime::get(JSRuntime* rt) } #ifdef JS_GC_ZEAL -const void* +const uint32_t* CompileRuntime::addressOfGCZealModeBits() { return runtime()->gc.addressOfZealModeBits(); @@ -171,25 +171,25 @@ CompileZone::addressOfIonBailAfter() } #endif -const void* +const uint32_t* CompileZone::addressOfNeedsIncrementalBarrier() { return zone()->addressOfNeedsIncrementalBarrier(); } -const void* +gc::FreeSpan** CompileZone::addressOfFreeList(gc::AllocKind allocKind) { return zone()->arenas.addressOfFreeList(allocKind); } -const void* +void* CompileZone::addressOfNurseryPosition() { return zone()->runtimeFromAnyThread()->gc.addressOfNurseryPosition(); } -const void* +void* CompileZone::addressOfStringNurseryPosition() { // Objects and strings share a nursery, for now at least. @@ -254,7 +254,7 @@ CompileRealm::runtime() return CompileRuntime::get(realm()->runtimeFromAnyThread()); } -const void* +const mozilla::non_crypto::XorShift128PlusRNG* CompileRealm::addressOfRandomNumberGenerator() { return realm()->addressOfRandomNumberGenerator(); diff --git a/js/src/jit/CompileWrappers.h b/js/src/jit/CompileWrappers.h index 6fce06991091..b349e4ea90e3 100644 --- a/js/src/jit/CompileWrappers.h +++ b/js/src/jit/CompileWrappers.h @@ -28,7 +28,7 @@ class CompileRuntime static CompileRuntime* get(JSRuntime* rt); #ifdef JS_GC_ZEAL - const void* addressOfGCZealModeBits(); + const uint32_t* addressOfGCZealModeBits(); #endif const JitRuntime* jitRuntime(); @@ -75,10 +75,10 @@ class CompileZone const void* addressOfIonBailAfter(); #endif - const void* addressOfNeedsIncrementalBarrier(); - const void* addressOfFreeList(gc::AllocKind allocKind); - const void* addressOfNurseryPosition(); - const void* addressOfStringNurseryPosition(); + const uint32_t* addressOfNeedsIncrementalBarrier(); + gc::FreeSpan** addressOfFreeList(gc::AllocKind allocKind); + void* addressOfNurseryPosition(); + void* addressOfStringNurseryPosition(); const void* addressOfNurseryCurrentEnd(); const void* addressOfStringNurseryCurrentEnd(); @@ -103,7 +103,8 @@ class CompileRealm return realm(); } - const void* addressOfRandomNumberGenerator(); + const mozilla::non_crypto::XorShift128PlusRNG* + addressOfRandomNumberGenerator(); const JitRealm* jitRealm(); diff --git a/js/src/jit/MacroAssembler-inl.h b/js/src/jit/MacroAssembler-inl.h index c064aadfdd36..004069842c89 100644 --- a/js/src/jit/MacroAssembler-inl.h +++ b/js/src/jit/MacroAssembler-inl.h @@ -637,8 +637,8 @@ MacroAssembler::branchTestNeedsIncrementalBarrier(Condition cond, Label* label) { MOZ_ASSERT(cond == Zero || cond == NonZero); CompileZone* zone = GetJitContext()->realm->zone(); - AbsoluteAddress needsBarrierAddr(zone->addressOfNeedsIncrementalBarrier()); - branchTest32(cond, needsBarrierAddr, Imm32(0x1), label); + const uint32_t* needsBarrierAddr = zone->addressOfNeedsIncrementalBarrier(); + branchTest32(cond, AbsoluteAddress(needsBarrierAddr), Imm32(0x1), label); } void diff --git a/js/src/jit/MacroAssembler.cpp b/js/src/jit/MacroAssembler.cpp index b8301bb556e3..bc7483f9d678 100644 --- a/js/src/jit/MacroAssembler.cpp +++ b/js/src/jit/MacroAssembler.cpp @@ -818,9 +818,10 @@ MacroAssembler::checkAllocatorState(Label* fail) #ifdef JS_GC_ZEAL // Don't execute the inline path if gc zeal or tracing are active. - branch32(Assembler::NotEqual, - AbsoluteAddress(GetJitContext()->runtime->addressOfGCZealModeBits()), Imm32(0), - fail); + const uint32_t *ptrZealModeBits = + GetJitContext()->runtime->addressOfGCZealModeBits(); + branch32(Assembler::NotEqual, AbsoluteAddress(ptrZealModeBits), Imm32(0), + fail); #endif // Don't execute the inline path if the realm has an object metadata callback, @@ -863,10 +864,13 @@ MacroAssembler::nurseryAllocateObject(Register result, Register temp, gc::AllocK int thingSize = int(gc::Arena::thingSize(allocKind)); int totalSize = thingSize + nDynamicSlots * sizeof(HeapSlot); MOZ_ASSERT(totalSize % gc::CellAlignBytes == 0); - loadPtr(AbsoluteAddress(zone->addressOfNurseryPosition()), result); + void *ptrNurseryPosition = zone->addressOfNurseryPosition(); + loadPtr(AbsoluteAddress(ptrNurseryPosition), result); computeEffectiveAddress(Address(result, totalSize), temp); - branchPtr(Assembler::Below, AbsoluteAddress(zone->addressOfNurseryCurrentEnd()), temp, fail); - storePtr(temp, AbsoluteAddress(zone->addressOfNurseryPosition())); + const void *ptrNurseryCurrentEnd = zone->addressOfNurseryCurrentEnd(); + branchPtr(Assembler::Below, AbsoluteAddress(ptrNurseryCurrentEnd), temp, + fail); + storePtr(temp, AbsoluteAddress(ptrNurseryPosition)); if (nDynamicSlots) { computeEffectiveAddress(Address(result, thingSize), temp); @@ -886,14 +890,15 @@ MacroAssembler::freeListAllocate(Register result, Register temp, gc::AllocKind a // Load the first and last offsets of |zone|'s free list for |allocKind|. // If there is no room remaining in the span, fall back to get the next one. - loadPtr(AbsoluteAddress(zone->addressOfFreeList(allocKind)), temp); + gc::FreeSpan **ptrFreeList = zone->addressOfFreeList(allocKind); + loadPtr(AbsoluteAddress(ptrFreeList), temp); load16ZeroExtend(Address(temp, js::gc::FreeSpan::offsetOfFirst()), result); load16ZeroExtend(Address(temp, js::gc::FreeSpan::offsetOfLast()), temp); branch32(Assembler::AboveOrEqual, result, temp, &fallback); // Bump the offset for the next allocation. add32(Imm32(thingSize), result); - loadPtr(AbsoluteAddress(zone->addressOfFreeList(allocKind)), temp); + loadPtr(AbsoluteAddress(ptrFreeList), temp); store16(result, Address(temp, js::gc::FreeSpan::offsetOfFirst())); sub32(Imm32(thingSize), result); addPtr(temp, result); // Turn the offset into a pointer. @@ -904,7 +909,7 @@ MacroAssembler::freeListAllocate(Register result, Register temp, gc::AllocKind a // interpreter will call the GC allocator to set up a new arena to allocate // from, after which we can resume allocating in the jit. branchTest32(Assembler::Zero, result, result, fail); - loadPtr(AbsoluteAddress(zone->addressOfFreeList(allocKind)), temp); + loadPtr(AbsoluteAddress(ptrFreeList), temp); addPtr(temp, result); // Turn the offset into a pointer. Push(result); // Update the free list to point to the next span (which may be empty). diff --git a/js/src/vm/Realm.h b/js/src/vm/Realm.h index 4ad2485c9456..d4322f68d7a9 100644 --- a/js/src/vm/Realm.h +++ b/js/src/vm/Realm.h @@ -812,7 +812,8 @@ class JS::Realm : public JS::shadow::Realm // Initializes randomNumberGenerator if needed. mozilla::non_crypto::XorShift128PlusRNG& getOrCreateRandomNumberGenerator(); - const void* addressOfRandomNumberGenerator() const { + const mozilla::non_crypto::XorShift128PlusRNG* + addressOfRandomNumberGenerator() const { return randomNumberGenerator_.ptr(); } From 81bdf394fbbe9c3e208a97740109ea5d1a1cf65f Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Wed, 18 Jul 2018 16:01:51 +1000 Subject: [PATCH 33/37] Bug 1476500 - Add a comment explaining why we need this value r=sfink It was unclear why we need addressOfStringNurseryCurrentEnd but not addressOfStringNurseryPosition. Add a comment explaining this in this context. --HG-- extra : rebase_source : 4fb1907c6fe133223c9d323f8062470e716df4a5 --- js/src/jit/CompileWrappers.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/js/src/jit/CompileWrappers.cpp b/js/src/jit/CompileWrappers.cpp index 47d8829a8861..2a2363f4b6fa 100644 --- a/js/src/jit/CompileWrappers.cpp +++ b/js/src/jit/CompileWrappers.cpp @@ -205,6 +205,12 @@ CompileZone::addressOfNurseryCurrentEnd() const void* CompileZone::addressOfStringNurseryCurrentEnd() { + // Although objects and strings share a nursery (and this may change) + // there is still a separate string end address. The only time it + // is different from the regular end address, is when nursery strings are + // disabled (it will be NULL). + // + // This function returns _a pointer to_ that end address. return zone()->runtimeFromAnyThread()->gc.addressOfStringNurseryCurrentEnd(); } From 43a7c80ca281b59ab4c98ee887f777c02e6f5e98 Mon Sep 17 00:00:00 2001 From: Bob Clary Date: Fri, 20 Jul 2018 00:12:45 -0700 Subject: [PATCH 34/37] Bug 1402406 - do not format log message if super MochitestFormatter returns None, r=gbrown --- testing/mochitest/runtests.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/testing/mochitest/runtests.py b/testing/mochitest/runtests.py index ed9875254890..7c5718186c12 100644 --- a/testing/mochitest/runtests.py +++ b/testing/mochitest/runtests.py @@ -131,6 +131,8 @@ class MochitestFormatter(TbplFormatter): def __call__(self, data): output = super(MochitestFormatter, self).__call__(data) + if not output: + return None log_level = data.get('level', 'info').upper() if 'js_source' in data or log_level == 'ERROR': From d9e4da676c4956efaf9f2883fb7d102c880ac74a Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Fri, 20 Jul 2018 16:13:56 +0900 Subject: [PATCH 35/37] Bug 1456006 - Part 0: Add reportError and reportExtraWarning variants to receive offset instead of ParseNode. r=jwalden --- js/src/frontend/BytecodeEmitter.cpp | 30 +++++++++++++++++++++++++++++ js/src/frontend/BytecodeEmitter.h | 4 ++++ 2 files changed, 34 insertions(+) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index f8c6c6035020..18b676d551d1 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -1484,6 +1484,20 @@ BytecodeEmitter::reportError(ParseNode* pn, unsigned errorNumber, ...) va_end(args); } +void +BytecodeEmitter::reportError(const Maybe& maybeOffset, unsigned errorNumber, ...) +{ + MOZ_ASSERT_IF(!maybeOffset, this->scriptStartOffsetSet); + uint32_t offset = maybeOffset ? *maybeOffset : this->scriptStartOffset; + + va_list args; + va_start(args, errorNumber); + + parser->errorReporter().errorAtVA(offset, errorNumber, &args); + + va_end(args); +} + bool BytecodeEmitter::reportExtraWarning(ParseNode* pn, unsigned errorNumber, ...) { @@ -1499,6 +1513,22 @@ BytecodeEmitter::reportExtraWarning(ParseNode* pn, unsigned errorNumber, ...) return result; } +bool +BytecodeEmitter::reportExtraWarning(const Maybe& maybeOffset, + unsigned errorNumber, ...) +{ + MOZ_ASSERT_IF(!maybeOffset, this->scriptStartOffsetSet); + uint32_t offset = maybeOffset ? *maybeOffset : this->scriptStartOffset; + + va_list args; + va_start(args, errorNumber); + + bool result = parser->errorReporter().reportExtraWarningErrorNumberVA(nullptr, offset, errorNumber, &args); + + va_end(args); + return result; +} + bool BytecodeEmitter::emitNewInit(JSProtoKey key) { diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 3336a0e2bb51..2db42fb50247 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -419,7 +419,11 @@ struct MOZ_STACK_CLASS BytecodeEmitter } void reportError(ParseNode* pn, unsigned errorNumber, ...); + void reportError(const mozilla::Maybe& maybeOffset, + unsigned errorNumber, ...); bool reportExtraWarning(ParseNode* pn, unsigned errorNumber, ...); + bool reportExtraWarning(const mozilla::Maybe& maybeOffset, + unsigned errorNumber, ...); // If pn contains a useful expression, return true with *answer set to true. // If pn contains a useless expression, return true with *answer set to From d9086904d4ac474c022e5f20f52faa6948231c48 Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Fri, 20 Jul 2018 16:13:56 +0900 Subject: [PATCH 36/37] Bug 1456006 - Part 1: Add SwitchEmitter. r=jwalden --- js/src/frontend/BytecodeEmitter.cpp | 331 ++++---------------- js/src/frontend/ParseNode.h | 14 +- js/src/frontend/SwitchEmitter.cpp | 425 +++++++++++++++++++++++++ js/src/frontend/SwitchEmitter.h | 469 ++++++++++++++++++++++++++++ js/src/moz.build | 1 + 5 files changed, 958 insertions(+), 282 deletions(-) create mode 100644 js/src/frontend/SwitchEmitter.cpp create mode 100644 js/src/frontend/SwitchEmitter.h diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 18b676d551d1..1726b78480ac 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -29,6 +29,7 @@ #include "frontend/ForOfLoopControl.h" #include "frontend/IfEmitter.h" #include "frontend/Parser.h" +#include "frontend/SwitchEmitter.h" #include "frontend/TDZCheckCache.h" #include "frontend/TryEmitter.h" #include "vm/BytecodeUtil.h" @@ -2341,38 +2342,27 @@ BytecodeEmitter::emitNumberOp(double dval) MOZ_NEVER_INLINE bool BytecodeEmitter::emitSwitch(ParseNode* pn) { - ParseNode* cases = pn->pn_right; - MOZ_ASSERT(cases->isKind(ParseNodeKind::LexicalScope) || - cases->isKind(ParseNodeKind::StatementList)); + ParseNode* lexical = pn->pn_right; + MOZ_ASSERT(lexical->isKind(ParseNodeKind::LexicalScope)); + ParseNode* cases = lexical->scopeBody(); + MOZ_ASSERT(cases->isKind(ParseNodeKind::StatementList)); - // Ensure that the column of the switch statement is set properly. - if (!updateSourceCoordNotes(pn->pn_pos.begin)) + SwitchEmitter se(this); + if (!se.emitDiscriminant(Some(pn->pn_pos.begin))) return false; - - // Emit code for the discriminant. if (!emitTree(pn->pn_left)) return false; // Enter the scope before pushing the switch BreakableControl since all // breaks are under this scope. - Maybe tdzCache; - Maybe emitterScope; - if (cases->isKind(ParseNodeKind::LexicalScope)) { - if (!cases->isEmptyScope()) { - tdzCache.emplace(this); - emitterScope.emplace(this); - if (!emitterScope->enterLexical(this, ScopeKind::Lexical, cases->scopeBindings())) - return false; - } - - // Advance |cases| to refer to the switch case list. - cases = cases->scopeBody(); + if (!lexical->isEmptyScope()) { + if (!se.emitLexical(lexical->scopeBindings())) + return false; // A switch statement may contain hoisted functions inside its // cases. The PNX_FUNCDEFS flag is propagated from the STATEMENTLIST // bodies of the cases to the case list. if (cases->pn_xflags & PNX_FUNCDEFS) { - MOZ_ASSERT(emitterScope); for (ParseNode* caseNode = cases->pn_head; caseNode; caseNode = caseNode->pn_next) { if (caseNode->pn_right->pn_xflags & PNX_FUNCDEFS) { if (!emitHoistedFunctionsInList(caseNode->pn_right)) @@ -2380,305 +2370,104 @@ BytecodeEmitter::emitSwitch(ParseNode* pn) } } } - } - - // After entering the scope, push the switch control. - BreakableControl controlInfo(this, StatementKind::Switch); - - ptrdiff_t top = offset(); - - // Switch bytecodes run from here till end of final case. - uint32_t caseCount = cases->pn_count; - if (caseCount > JS_BIT(16)) { - reportError(pn, JSMSG_TOO_MANY_CASES); - return false; - } - - // Try for most optimal, fall back if not dense ints. - JSOp switchOp = JSOP_TABLESWITCH; - uint32_t tableLength = 0; - int32_t low, high; - bool hasDefault = false; - CaseClause* firstCase = cases->pn_head ? &cases->pn_head->as() : nullptr; - if (caseCount == 0 || - (caseCount == 1 && (hasDefault = firstCase->isDefault()))) - { - low = 0; - high = -1; } else { - Vector intmap; - int32_t intmapBitLength = 0; - - low = JSVAL_INT_MAX; - high = JSVAL_INT_MIN; + MOZ_ASSERT(!(cases->pn_xflags & PNX_FUNCDEFS)); + } + SwitchEmitter::TableGenerator tableGen(this); + uint32_t caseCount = cases->pn_count; + CaseClause* firstCase = cases->pn_head ? &cases->pn_head->as() : nullptr; + if (caseCount == 0) { + tableGen.finish(0); + } else if (caseCount == 1 && firstCase->isDefault()) { + caseCount = 0; + tableGen.finish(0); + } else { for (CaseClause* caseNode = firstCase; caseNode; caseNode = caseNode->next()) { if (caseNode->isDefault()) { - hasDefault = true; caseCount--; // one of the "cases" was the default continue; } - if (switchOp == JSOP_CONDSWITCH) + if (tableGen.isInvalid()) continue; - MOZ_ASSERT(switchOp == JSOP_TABLESWITCH); - ParseNode* caseValue = caseNode->caseExpression(); if (caseValue->getKind() != ParseNodeKind::Number) { - switchOp = JSOP_CONDSWITCH; + tableGen.setInvalid(); continue; } int32_t i; if (!NumberEqualsInt32(caseValue->pn_dval, &i)) { - switchOp = JSOP_CONDSWITCH; + tableGen.setInvalid(); continue; } - if (unsigned(i + int(JS_BIT(15))) >= unsigned(JS_BIT(16))) { - switchOp = JSOP_CONDSWITCH; - continue; - } - if (i < low) - low = i; - if (i > high) - high = i; - - // Check for duplicates, which require a JSOP_CONDSWITCH. - // We bias i by 65536 if it's negative, and hope that's a rare - // case (because it requires a malloc'd bitmap). - if (i < 0) - i += JS_BIT(16); - if (i >= intmapBitLength) { - size_t newLength = NumWordsForBitArrayOfLength(i + 1); - if (!intmap.resize(newLength)) { - ReportOutOfMemory(cx); - return false; - } - intmapBitLength = newLength * BitArrayElementBits; - } - if (IsBitArrayElementSet(intmap.begin(), intmap.length(), i)) { - switchOp = JSOP_CONDSWITCH; - continue; - } - SetBitArrayElement(intmap.begin(), intmap.length(), i); + if (!tableGen.addNumber(i)) + return false; } - // Compute table length and select condswitch instead if overlarge or - // more than half-sparse. - if (switchOp == JSOP_TABLESWITCH) { - tableLength = uint32_t(high - low + 1); - if (tableLength >= JS_BIT(16) || tableLength > 2 * caseCount) - switchOp = JSOP_CONDSWITCH; - } + tableGen.finish(caseCount); } - // The note has one or two offsets: first tells total switch code length; - // second (if condswitch) tells offset to first JSOP_CASE. - unsigned noteIndex; - size_t switchSize; - if (switchOp == JSOP_CONDSWITCH) { - // 0 bytes of immediate for unoptimized switch. - switchSize = 0; - if (!newSrcNote3(SRC_CONDSWITCH, 0, 0, ¬eIndex)) - return false; - } else { - MOZ_ASSERT(switchOp == JSOP_TABLESWITCH); - - // 3 offsets (len, low, high) before the table, 1 per entry. - switchSize = size_t(JUMP_OFFSET_LEN * (3 + tableLength)); - if (!newSrcNote2(SRC_TABLESWITCH, 0, ¬eIndex)) - return false; - } - - // Emit switchOp followed by switchSize bytes of jump or lookup table. - MOZ_ASSERT(top == offset()); - if (!emitN(switchOp, switchSize)) + if (!se.validateCaseCount(caseCount)) return false; - Vector table; - - JumpList condSwitchDefaultOff; - if (switchOp == JSOP_CONDSWITCH) { - unsigned caseNoteIndex; - bool beforeCases = true; - ptrdiff_t lastCaseOffset = -1; - - // The case conditions need their own TDZ cache since they might not - // all execute. - TDZCheckCache tdzCache(this); + bool isTableSwitch = tableGen.isValid(); + if (isTableSwitch) { + if (!se.emitTable(tableGen)) + return false; + } else { + if (!se.emitCond()) + return false; // Emit code for evaluating cases and jumping to case statements. for (CaseClause* caseNode = firstCase; caseNode; caseNode = caseNode->next()) { - ParseNode* caseValue = caseNode->caseExpression(); + if (caseNode->isDefault()) + continue; + ParseNode* caseValue = caseNode->caseExpression(); // If the expression is a literal, suppress line number emission so // that debugging works more naturally. - if (caseValue) { - if (!emitTree(caseValue, ValueUsage::WantValue, - caseValue->isLiteral() ? SUPPRESS_LINENOTE : EMIT_LINENOTE)) - { - return false; - } - } - - if (!beforeCases) { - // prevCase is the previous JSOP_CASE's bytecode offset. - if (!setSrcNoteOffset(caseNoteIndex, 0, offset() - lastCaseOffset)) - return false; - } - if (!caseValue) { - // This is the default clause. - continue; - } - - if (!newSrcNote2(SRC_NEXTCASE, 0, &caseNoteIndex)) - return false; - - // The case clauses are produced before any of the case body. The - // JumpList is saved on the parsed tree, then later restored and - // patched when generating the cases body. - JumpList caseJump; - if (!emitJump(JSOP_CASE, &caseJump)) - return false; - caseNode->setOffset(caseJump.offset); - lastCaseOffset = caseJump.offset; - - if (beforeCases) { - // Switch note's second offset is to first JSOP_CASE. - unsigned noteCount = notes().length(); - if (!setSrcNoteOffset(noteIndex, 1, lastCaseOffset - top)) - return false; - unsigned noteCountDelta = notes().length() - noteCount; - if (noteCountDelta != 0) - caseNoteIndex += noteCountDelta; - beforeCases = false; - } - } - - // If we didn't have an explicit default (which could fall in between - // cases, preventing us from fusing this setSrcNoteOffset with the call - // in the loop above), link the last case to the implicit default for - // the benefit of IonBuilder. - if (!hasDefault && - !beforeCases && - !setSrcNoteOffset(caseNoteIndex, 0, offset() - lastCaseOffset)) - { - return false; - } - - // Emit default even if no explicit default statement. - if (!emitJump(JSOP_DEFAULT, &condSwitchDefaultOff)) - return false; - } else { - MOZ_ASSERT(switchOp == JSOP_TABLESWITCH); - - // skip default offset. - jsbytecode* pc = code(top + JUMP_OFFSET_LEN); - - // Fill in switch bounds, which we know fit in 16-bit offsets. - SET_JUMP_OFFSET(pc, low); - pc += JUMP_OFFSET_LEN; - SET_JUMP_OFFSET(pc, high); - pc += JUMP_OFFSET_LEN; - - if (tableLength != 0) { - if (!table.growBy(tableLength)) { - ReportOutOfMemory(cx); + if (!emitTree(caseValue, ValueUsage::WantValue, + caseValue->isLiteral() ? SUPPRESS_LINENOTE : EMIT_LINENOTE)) + { return false; } - for (CaseClause* caseNode = firstCase; caseNode; caseNode = caseNode->next()) { - if (ParseNode* caseValue = caseNode->caseExpression()) { - MOZ_ASSERT(caseValue->isKind(ParseNodeKind::Number)); - - int32_t i = int32_t(caseValue->pn_dval); - MOZ_ASSERT(double(i) == caseValue->pn_dval); - - i -= low; - MOZ_ASSERT(uint32_t(i) < tableLength); - MOZ_ASSERT(!table[i]); - table[i] = caseNode; - } - } + if (!se.emitCaseJump()) + return false; } } - JumpTarget defaultOffset{ -1 }; - // Emit code for each case's statements. for (CaseClause* caseNode = firstCase; caseNode; caseNode = caseNode->next()) { - if (switchOp == JSOP_CONDSWITCH && !caseNode->isDefault()) { - // The case offset got saved in the caseNode structure after - // emitting the JSOP_CASE jump instruction above. - JumpList caseCond; - caseCond.offset = caseNode->offset(); - if (!emitJumpTargetAndPatch(caseCond)) + if (caseNode->isDefault()) { + if (!se.emitDefaultBody()) return false; + } else { + if (isTableSwitch) { + ParseNode* caseValue = caseNode->caseExpression(); + MOZ_ASSERT(caseValue->isKind(ParseNodeKind::Number)); + + int32_t i = int32_t(caseValue->pn_dval); + MOZ_ASSERT(double(i) == caseValue->pn_dval); + + if (!se.emitCaseBody(i, tableGen)) + return false; + } else { + if (!se.emitCaseBody()) + return false; + } } - JumpTarget here; - if (!emitJumpTarget(&here)) - return false; - if (caseNode->isDefault()) - defaultOffset = here; - - // If this is emitted as a TABLESWITCH, we'll need to know this case's - // offset later when emitting the table. Store it in the node's - // pn_offset (giving the field a different meaning vs. how we used it - // on the immediately preceding line of code). - caseNode->setOffset(here.offset); - - TDZCheckCache tdzCache(this); - if (!emitTree(caseNode->statementList())) return false; } - if (!hasDefault) { - // If no default case, offset for default is to end of switch. - if (!emitJumpTarget(&defaultOffset)) - return false; - } - MOZ_ASSERT(defaultOffset.offset != -1); - - // Set the default offset (to end of switch if no default). - jsbytecode* pc; - if (switchOp == JSOP_CONDSWITCH) { - pc = nullptr; - patchJumpsToTarget(condSwitchDefaultOff, defaultOffset); - } else { - MOZ_ASSERT(switchOp == JSOP_TABLESWITCH); - pc = code(top); - SET_JUMP_OFFSET(pc, defaultOffset.offset - top); - pc += JUMP_OFFSET_LEN; - } - - // Set the SRC_SWITCH note's offset operand to tell end of switch. - if (!setSrcNoteOffset(noteIndex, 0, lastNonJumpTargetOffset() - top)) - return false; - - if (switchOp == JSOP_TABLESWITCH) { - // Skip over the already-initialized switch bounds. - pc += 2 * JUMP_OFFSET_LEN; - - // Fill in the jump table, if there is one. - for (uint32_t i = 0; i < tableLength; i++) { - CaseClause* caseNode = table[i]; - ptrdiff_t off = caseNode ? caseNode->offset() - top : 0; - SET_JUMP_OFFSET(pc, off); - pc += JUMP_OFFSET_LEN; - } - } - - // Patch breaks before leaving the scope, as all breaks are under the - // lexical scope if it exists. - if (!controlInfo.patchBreaks(this)) - return false; - - if (emitterScope && !emitterScope->leave(this)) + if (!se.emitEnd()) return false; return true; diff --git a/js/src/frontend/ParseNode.h b/js/src/frontend/ParseNode.h index e371c32fe735..9010f5bb5083 100644 --- a/js/src/frontend/ParseNode.h +++ b/js/src/frontend/ParseNode.h @@ -255,16 +255,13 @@ IsTypeofKind(ParseNodeKind kind) * StatementList list pn_head: list of pn_count statements * If ternary pn_kid1: cond, pn_kid2: then, pn_kid3: else or null. * Switch binary pn_left: discriminant - * pn_right: list of Case nodes, with at most one - * default node, or if there are let bindings - * in the top level of the switch body's cases, a - * LexicalScope node that contains the list of - * Case nodes. + * pn_right: LexicalScope node that contains the list + * of Case nodes, with at most one + * default node. * Case binary pn_left: case-expression if CaseClause, or * null if DefaultClause * pn_right: StatementList node for this case's * statements - * pn_u.binary.offset: scratch space for the emitter * While binary pn_left: cond, pn_right: body * DoWhile binary pn_left: body, pn_right: cond * For binary pn_left: either ForIn (for-in statement), @@ -558,7 +555,6 @@ class ParseNode union { unsigned iflags; /* JSITER_* flags for ParseNodeKind::For node */ bool isStatic; /* only for ParseNodeKind::ClassMethod */ - uint32_t offset; /* for the emitter's use on ParseNodeKind::Case nodes */ }; } binary; struct { /* one kid if unary */ @@ -1022,10 +1018,6 @@ class CaseClause : public BinaryNode // The next CaseClause in the same switch statement. CaseClause* next() const { return pn_next ? &pn_next->as() : nullptr; } - // Scratch space used by the emitter. - uint32_t offset() const { return pn_u.binary.offset; } - void setOffset(uint32_t u) { pn_u.binary.offset = u; } - static bool test(const ParseNode& node) { bool match = node.isKind(ParseNodeKind::Case); MOZ_ASSERT_IF(match, node.isArity(PN_BINARY)); diff --git a/js/src/frontend/SwitchEmitter.cpp b/js/src/frontend/SwitchEmitter.cpp new file mode 100644 index 000000000000..8c1c2f9aa01a --- /dev/null +++ b/js/src/frontend/SwitchEmitter.cpp @@ -0,0 +1,425 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- + * vim: set ts=8 sts=4 et sw=4 tw=99: + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "frontend/SwitchEmitter.h" + +#include "jsutil.h" + +#include "frontend/BytecodeEmitter.h" +#include "frontend/SharedContext.h" +#include "frontend/SourceNotes.h" +#include "vm/BytecodeUtil.h" +#include "vm/Opcodes.h" +#include "vm/Runtime.h" + +using namespace js; +using namespace js::frontend; + +using mozilla::Maybe; + +bool +SwitchEmitter::TableGenerator::addNumber(int32_t caseValue) +{ + if (isInvalid()) + return true; + + if (unsigned(caseValue + int(JS_BIT(15))) >= unsigned(JS_BIT(16))) { + setInvalid(); + return true; + } + + if (intmap_.isNothing()) + intmap_.emplace(); + + low_ = std::min(low_, caseValue); + high_ = std::max(high_, caseValue); + + // Check for duplicates, which require a JSOP_CONDSWITCH. + // We bias caseValue by 65536 if it's negative, and hope that's a rare case + // (because it requires a malloc'd bitmap). + if (caseValue < 0) + caseValue += JS_BIT(16); + if (caseValue >= intmapBitLength_) { + size_t newLength = NumWordsForBitArrayOfLength(caseValue + 1); + if (!intmap_->resize(newLength)) { + ReportOutOfMemory(bce_->cx); + return false; + } + intmapBitLength_ = newLength * BitArrayElementBits; + } + if (IsBitArrayElementSet(intmap_->begin(), intmap_->length(), caseValue)) { + // Duplicate entry is not supported in table switch. + setInvalid(); + return true; + } + SetBitArrayElement(intmap_->begin(), intmap_->length(), caseValue); + return true; +} + +void +SwitchEmitter::TableGenerator::finish(uint32_t caseCount) +{ + intmap_.reset(); + +#ifdef DEBUG + finished_ = true; +#endif + + if (isInvalid()) + return; + + if (caseCount == 0) { + low_ = 0; + high_ = -1; + return; + } + + // Compute table length and select condswitch instead if overlarge + // or more than half-sparse. + tableLength_ = uint32_t(high_ - low_ + 1); + if (tableLength_ >= JS_BIT(16) || tableLength_ > 2 * caseCount) + setInvalid(); +} + +uint32_t +SwitchEmitter::TableGenerator::toCaseIndex(int32_t caseValue) const +{ + MOZ_ASSERT(finished_); + MOZ_ASSERT(isValid()); + uint32_t caseIndex = uint32_t(caseValue - low_); + MOZ_ASSERT(caseIndex < tableLength_); + return caseIndex; +} + +uint32_t +SwitchEmitter::TableGenerator::tableLength() const +{ + MOZ_ASSERT(finished_); + MOZ_ASSERT(isValid()); + return tableLength_; +} + +SwitchEmitter::SwitchEmitter(BytecodeEmitter* bce) + : bce_(bce) +{} + +bool +SwitchEmitter::emitDiscriminant(const Maybe& switchPos) +{ + MOZ_ASSERT(state_ == State::Start); + switchPos_ = switchPos; + + if (switchPos_) { + // Ensure that the column of the switch statement is set properly. + if (!bce_->updateSourceCoordNotes(*switchPos_)) + return false; + } + + state_ = State::Discriminant; + return true; +} + +bool +SwitchEmitter::emitLexical(Handle bindings) +{ + MOZ_ASSERT(state_ == State::Discriminant); + MOZ_ASSERT(bindings); + + tdzCacheLexical_.emplace(bce_); + emitterScope_.emplace(bce_); + if (!emitterScope_->enterLexical(bce_, ScopeKind::Lexical, bindings)) + return false; + + state_ = State::Lexical; + return true; +} + +bool +SwitchEmitter::validateCaseCount(uint32_t caseCount) +{ + MOZ_ASSERT(state_ == State::Discriminant || state_ == State::Lexical); + if (caseCount > JS_BIT(16)) { + bce_->reportError(switchPos_, JSMSG_TOO_MANY_CASES); + return false; + } + caseCount_ = caseCount; + + state_ = State::CaseCount; + return true; +} + +bool +SwitchEmitter::emitCond() +{ + MOZ_ASSERT(state_ == State::CaseCount); + + kind_ = Kind::Cond; + + // After entering the scope if necessary, push the switch control. + controlInfo_.emplace(bce_, StatementKind::Switch); + top_ = bce_->offset(); + + if (!caseOffsets_.resize(caseCount_)) { + ReportOutOfMemory(bce_->cx); + return false; + } + + // The note has two offsets: first tells total switch code length; + // second tells offset to first JSOP_CASE. + if (!bce_->newSrcNote3(SRC_CONDSWITCH, 0, 0, ¬eIndex_)) + return false; + + MOZ_ASSERT(top_ == bce_->offset()); + if (!bce_->emitN(JSOP_CONDSWITCH, 0)) + return false; + + tdzCacheCaseAndBody_.emplace(bce_); + + state_ = State::Cond; + return true; +} + +bool +SwitchEmitter::emitTable(const TableGenerator& tableGen) +{ + MOZ_ASSERT(state_ == State::CaseCount); + kind_ = Kind::Table; + + // After entering the scope if necessary, push the switch control. + controlInfo_.emplace(bce_, StatementKind::Switch); + top_ = bce_->offset(); + + // The note has one offset that tells total switch code length. + + // 3 offsets (len, low, high) before the table, 1 per entry. + size_t switchSize = size_t(JUMP_OFFSET_LEN * (3 + tableGen.tableLength())); + if (!bce_->newSrcNote2(SRC_TABLESWITCH, 0, ¬eIndex_)) + return false; + + if (!caseOffsets_.resize(tableGen.tableLength())) { + ReportOutOfMemory(bce_->cx); + return false; + } + + MOZ_ASSERT(top_ == bce_->offset()); + if (!bce_->emitN(JSOP_TABLESWITCH, switchSize)) + return false; + + // Skip default offset. + jsbytecode* pc = bce_->code(top_ + JUMP_OFFSET_LEN); + + // Fill in switch bounds, which we know fit in 16-bit offsets. + SET_JUMP_OFFSET(pc, tableGen.low()); + SET_JUMP_OFFSET(pc + JUMP_OFFSET_LEN, tableGen.high()); + + state_ = State::Table; + return true; +} + +bool +SwitchEmitter::emitCaseOrDefaultJump(uint32_t caseIndex, bool isDefault) +{ + MOZ_ASSERT(kind_ == Kind::Cond); + + if (state_ == State::Case) { + // Link the last JSOP_CASE's SRC_NEXTCASE to current JSOP_CASE or + // JSOP_DEFAULT for the benefit of IonBuilder. + if (!bce_->setSrcNoteOffset(caseNoteIndex_, 0, bce_->offset() - lastCaseOffset_)) + return false; + } + + if (isDefault) { + if (!bce_->emitJump(JSOP_DEFAULT, &condSwitchDefaultOffset_)) + return false; + return true; + } + + if (!bce_->newSrcNote2(SRC_NEXTCASE, 0, &caseNoteIndex_)) + return false; + + JumpList caseJump; + if (!bce_->emitJump(JSOP_CASE, &caseJump)) + return false; + caseOffsets_[caseIndex] = caseJump.offset; + lastCaseOffset_ = caseJump.offset; + + if (state_ == State::Cond) { + // Switch note's second offset is to first JSOP_CASE. + unsigned noteCount = bce_->notes().length(); + if (!bce_->setSrcNoteOffset(noteIndex_, 1, lastCaseOffset_ - top_)) + return false; + unsigned noteCountDelta = bce_->notes().length() - noteCount; + if (noteCountDelta != 0) + caseNoteIndex_ += noteCountDelta; + } + + return true; +} + +bool +SwitchEmitter::emitCaseJump() +{ + MOZ_ASSERT(kind_ == Kind::Cond); + MOZ_ASSERT(state_ == State::Cond || state_ == State::Case); + if (!emitCaseOrDefaultJump(caseIndex_, false)) + return false; + caseIndex_++; + + state_ = State::Case; + return true; +} + +bool +SwitchEmitter::emitImplicitDefault() +{ + MOZ_ASSERT(kind_ == Kind::Cond); + MOZ_ASSERT(state_ == State::Cond || state_ == State::Case); + if (!emitCaseOrDefaultJump(0, true)) + return false; + + caseIndex_ = 0; + + // No internal state after emitting default jump. + return true; +} + +bool +SwitchEmitter::emitCaseBody() +{ + MOZ_ASSERT(kind_ == Kind::Cond); + MOZ_ASSERT(state_ == State::Cond || state_ == State::Case || + state_ == State::CaseBody || state_ == State::DefaultBody); + + tdzCacheCaseAndBody_.reset(); + + if (state_ == State::Cond || state_ == State::Case) { + // For cond switch, JSOP_DEFAULT is always emitted. + if (!emitImplicitDefault()) + return false; + } + + JumpList caseJump; + caseJump.offset = caseOffsets_[caseIndex_]; + if (!bce_->emitJumpTargetAndPatch(caseJump)) + return false; + + JumpTarget here; + if (!bce_->emitJumpTarget(&here)) + return false; + caseIndex_++; + + tdzCacheCaseAndBody_.emplace(bce_); + + state_ = State::CaseBody; + return true; +} + +bool +SwitchEmitter::emitCaseBody(int32_t caseValue, const TableGenerator& tableGen) +{ + MOZ_ASSERT(kind_ == Kind::Table); + MOZ_ASSERT(state_ == State::Table || + state_ == State::CaseBody || state_ == State::DefaultBody); + + tdzCacheCaseAndBody_.reset(); + + JumpTarget here; + if (!bce_->emitJumpTarget(&here)) + return false; + caseOffsets_[tableGen.toCaseIndex(caseValue)] = here.offset; + + tdzCacheCaseAndBody_.emplace(bce_); + + state_ = State::CaseBody; + return true; +} + +bool +SwitchEmitter::emitDefaultBody() +{ + MOZ_ASSERT(state_ == State::Cond || state_ == State::Table || + state_ == State::Case || + state_ == State::CaseBody); + MOZ_ASSERT(!hasDefault_); + + tdzCacheCaseAndBody_.reset(); + + if (state_ == State::Cond || state_ == State::Case) { + // For cond switch, JSOP_DEFAULT is always emitted. + if (!emitImplicitDefault()) + return false; + } + JumpTarget here; + if (!bce_->emitJumpTarget(&here)) + return false; + defaultJumpTargetOffset_ = here; + + tdzCacheCaseAndBody_.emplace(bce_); + + hasDefault_ = true; + state_ = State::DefaultBody; + return true; +} + +bool +SwitchEmitter::emitEnd() +{ + MOZ_ASSERT(state_ == State::Cond || state_ == State::Table || + state_ == State::CaseBody || state_ == State::DefaultBody); + + tdzCacheCaseAndBody_.reset(); + + if (!hasDefault_) { + // If no default case, offset for default is to end of switch. + if (!bce_->emitJumpTarget(&defaultJumpTargetOffset_)) + return false; + } + MOZ_ASSERT(defaultJumpTargetOffset_.offset != -1); + + // Set the default offset (to end of switch if no default). + jsbytecode* pc; + if (kind_ == Kind::Cond) { + pc = nullptr; + bce_->patchJumpsToTarget(condSwitchDefaultOffset_, defaultJumpTargetOffset_); + } else { + // Fill in the default jump target. + pc = bce_->code(top_); + SET_JUMP_OFFSET(pc, defaultJumpTargetOffset_.offset - top_); + pc += JUMP_OFFSET_LEN; + } + + // Set the SRC_SWITCH note's offset operand to tell end of switch. + if (!bce_->setSrcNoteOffset(noteIndex_, 0, bce_->lastNonJumpTargetOffset() - top_)) + return false; + + if (kind_ == Kind::Table) { + // Skip over the already-initialized switch bounds. + pc += 2 * JUMP_OFFSET_LEN; + + // Fill in the jump table, if there is one. + for (uint32_t i = 0, length = caseOffsets_.length(); i < length; i++) { + ptrdiff_t off = caseOffsets_[i]; + SET_JUMP_OFFSET(pc, off == 0 ? 0 : off - top_); + pc += JUMP_OFFSET_LEN; + } + } + + // Patch breaks before leaving the scope, as all breaks are under the + // lexical scope if it exists. + if (!controlInfo_->patchBreaks(bce_)) + return false; + + if (emitterScope_ && !emitterScope_->leave(bce_)) + return false; + + emitterScope_.reset(); + tdzCacheLexical_.reset(); + + controlInfo_.reset(); + + state_ = State::End; + return true; +} diff --git a/js/src/frontend/SwitchEmitter.h b/js/src/frontend/SwitchEmitter.h new file mode 100644 index 000000000000..92061a045890 --- /dev/null +++ b/js/src/frontend/SwitchEmitter.h @@ -0,0 +1,469 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- + * vim: set ts=8 sts=4 et sw=4 tw=99: + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef frontend_SwitchEmitter_h +#define frontend_SwitchEmitter_h + +#include "mozilla/Attributes.h" +#include "mozilla/Maybe.h" + +#include +#include + +#include "frontend/BytecodeControlStructures.h" +#include "frontend/EmitterScope.h" +#include "frontend/JumpList.h" +#include "frontend/TDZCheckCache.h" +#include "gc/Rooting.h" +#include "js/AllocPolicy.h" +#include "js/Value.h" +#include "js/Vector.h" +#include "vm/Scope.h" + +namespace js { +namespace frontend { + +struct BytecodeEmitter; + +// Class for emitting bytecode for switch-case-default block. +// +// Usage: (check for the return value is omitted for simplicity) +// +// `switch (discriminant) { case c1_expr: c1_body; }` +// SwitchEmitter se(this); +// se.emitDiscriminant(Some(offset_of_switch)); +// emit(discriminant); +// +// se.validateCaseCount(1); +// se.emitCond(); +// +// emit(c1_expr); +// se.emitCaseJump(); +// +// se.emitCaseBody(); +// emit(c1_body); +// +// se.emitEnd(); +// +// `switch (discriminant) { case c1_expr: c1_body; case c2_expr: c2_body; +// default: def_body; }` +// SwitchEmitter se(this); +// se.emitDiscriminant(Some(offset_of_switch)); +// emit(discriminant); +// +// se.validateCaseCount(2); +// se.emitCond(); +// +// emit(c1_expr); +// se.emitCaseJump(); +// +// emit(c2_expr); +// se.emitCaseJump(); +// +// se.emitCaseBody(); +// emit(c1_body); +// +// se.emitCaseBody(); +// emit(c2_body); +// +// se.emitDefaultBody(); +// emit(def_body); +// +// se.emitEnd(); +// +// `switch (discriminant) { case c1_expr: c1_body; case c2_expr: c2_body; }` +// with Table Switch +// SwitchEmitter::TableGenerator tableGen(this); +// tableGen.addNumber(c1_expr_value); +// tableGen.addNumber(c2_expr_value); +// tableGen.finish(2); +// +// // If `!tableGen.isValid()` here, `emitCond` should be used instead. +// +// SwitchEmitter se(this); +// se.emitDiscriminant(Some(offset_of_switch)); +// emit(discriminant); +// se.validateCaseCount(2); +// se.emitTable(tableGen); +// +// se.emitCaseBody(c1_expr_value, tableGen); +// emit(c1_body); +// +// se.emitCaseBody(c2_expr_value, tableGen); +// emit(c2_body); +// +// se.emitEnd(); +// +// `switch (discriminant) { case c1_expr: c1_body; case c2_expr: c2_body; +// default: def_body; }` +// with Table Switch +// SwitchEmitter::TableGenerator tableGen(bce); +// tableGen.addNumber(c1_expr_value); +// tableGen.addNumber(c2_expr_value); +// tableGen.finish(2); +// +// // If `!tableGen.isValid()` here, `emitCond` should be used instead. +// +// SwitchEmitter se(this); +// se.emitDiscriminant(Some(offset_of_switch)); +// emit(discriminant); +// se.validateCaseCount(2); +// se.emitTable(tableGen); +// +// se.emitCaseBody(c1_expr_value, tableGen); +// emit(c1_body); +// +// se.emitCaseBody(c2_expr_value, tableGen); +// emit(c2_body); +// +// se.emitDefaultBody(); +// emit(def_body); +// +// se.emitEnd(); +// +// `switch (discriminant) { case c1_expr: c1_body; }` +// in case c1_body contains lexical bindings +// SwitchEmitter se(this); +// se.emitDiscriminant(Some(offset_of_switch)); +// emit(discriminant); +// +// se.validateCaseCount(1); +// +// se.emitLexical(bindings); +// +// se.emitCond(); +// +// emit(c1_expr); +// se.emitCaseJump(); +// +// se.emitCaseBody(); +// emit(c1_body); +// +// se.emitEnd(); +// +// `switch (discriminant) { case c1_expr: c1_body; }` +// in case c1_body contains hosted functions +// SwitchEmitter se(this); +// se.emitDiscriminant(Some(offset_of_switch)); +// emit(discriminant); +// +// se.validateCaseCount(1); +// +// se.emitLexical(bindings); +// emit(hosted functions); +// +// se.emitCond(); +// +// emit(c1_expr); +// se.emitCaseJump(); +// +// se.emitCaseBody(); +// emit(c1_body); +// +// se.emitEnd(); +// +class MOZ_STACK_CLASS SwitchEmitter +{ + // Bytecode for each case. + // + // Cond Switch + // {discriminant} + // JSOP_CONDSWITCH + // + // {c1_expr} + // JSOP_CASE c1 + // + // JSOP_JUMPTARGET + // {c2_expr} + // JSOP_CASE c2 + // + // ... + // + // JSOP_JUMPTARGET + // JSOP_DEFAULT default + // + // c1: + // JSOP_JUMPTARGET + // {c1_body} + // JSOP_GOTO end + // + // c2: + // JSOP_JUMPTARGET + // {c2_body} + // JSOP_GOTO end + // + // default: + // end: + // JSOP_JUMPTARGET + // + // Table Switch + // {discriminant} + // JSOP_TABLESWITCH c1, c2, ... + // + // c1: + // JSOP_JUMPTARGET + // {c1_body} + // JSOP_GOTO end + // + // c2: + // JSOP_JUMPTARGET + // {c2_body} + // JSOP_GOTO end + // + // ... + // + // end: + // JSOP_JUMPTARGET + + public: + enum class Kind { + Table, + Cond + }; + + // Class for generating optimized table switch data. + class MOZ_STACK_CLASS TableGenerator + { + BytecodeEmitter* bce_; + + // Bit array for given numbers. + mozilla::Maybe> intmap_; + + // The length of the intmap_. + int32_t intmapBitLength_ = 0; + + // The length of the table. + uint32_t tableLength_ = 0; + + // The lower and higher bounds of the table. + int32_t low_ = JSVAL_INT_MAX, high_ = JSVAL_INT_MIN; + + // Whether the table is still valid. + bool valid_= true; + +#ifdef DEBUG + bool finished_ = false; +#endif + + public: + explicit TableGenerator(BytecodeEmitter* bce) + : bce_(bce) + {} + + void setInvalid() { + valid_ = false; + } + MOZ_MUST_USE bool isValid() const { + return valid_; + } + MOZ_MUST_USE bool isInvalid() const { + return !valid_; + } + + // Add the given number to the table. The number is the value of + // `expr` for `case expr:` syntax. + MOZ_MUST_USE bool addNumber(int32_t caseValue); + + // Finish generating the table. + // `caseCount` should be the number of cases in the switch statement, + // excluding the default case. + void finish(uint32_t caseCount); + + private: + friend SwitchEmitter; + + // The following methods can be used only after calling `finish`. + + // Returns the lower bound of the added numbers. + int32_t low() const { + MOZ_ASSERT(finished_); + return low_; + } + + // Returns the higher bound of the numbers. + int32_t high() const { + MOZ_ASSERT(finished_); + return high_; + } + + // Returns the index in SwitchEmitter.caseOffsets_ for table switch. + uint32_t toCaseIndex(int32_t caseValue) const; + + // Returns the length of the table. + // This method can be called only if `isValid()` is true. + uint32_t tableLength() const; + }; + + private: + BytecodeEmitter* bce_; + + // `kind_` should be set to the correct value in emitCond/emitTable. + Kind kind_ = Kind::Cond; + + // True if there's explicit default case. + bool hasDefault_ = false; + + // The source note index for SRC_CONDSWITCH. + unsigned noteIndex_ = 0; + + // Source note index of the previous SRC_NEXTCASE. + unsigned caseNoteIndex_ = 0; + + // The number of cases in the switch statement, excluding the default case. + uint32_t caseCount_ = 0; + + // Internal index for case jump and case body, used by cond switch. + uint32_t caseIndex_ = 0; + + // Bytecode offset after emitting `discriminant`. + ptrdiff_t top_ = 0; + + // Bytecode offset of the previous JSOP_CASE. + ptrdiff_t lastCaseOffset_ = 0; + + // Bytecode offset of the JSOP_JUMPTARGET for default body. + JumpTarget defaultJumpTargetOffset_ = { -1 }; + + // Bytecode offset of the JSOP_DEFAULT. + JumpList condSwitchDefaultOffset_; + + // Instantiated when there's lexical scope for entire switch. + mozilla::Maybe tdzCacheLexical_; + mozilla::Maybe emitterScope_; + + // Instantiated while emitting case expression and case/default body. + mozilla::Maybe tdzCacheCaseAndBody_; + + // Control for switch. + mozilla::Maybe controlInfo_; + + mozilla::Maybe switchPos_; + + // Cond Switch: + // Offset of each JSOP_CASE. + // Table Switch: + // Offset of each JSOP_JUMPTARGET for case. + js::Vector caseOffsets_; + + // The state of this emitter. + // + // +-------+ emitDiscriminant +--------------+ + // | Start |----------------->| Discriminant |-+ + // +-------+ +--------------+ | + // | + // +-------------------------------------------+ + // | + // | validateCaseCount +-----------+ + // +->+------------------------>+------------------>| CaseCount |-+ + // | ^ +-----------+ | + // | emitLexical +---------+ | | + // +------------>| Lexical |-+ | + // +---------+ | + // | + // +--------------------------------------------------------------+ + // | + // | emitTable +-------+ + // +---------->| Table |---------------------------->+-+ + // | +-------+ ^ | + // | | | + // | emitCond +------+ | | + // +---------->| Cond |-+------------------------>+->+ | + // +------+ | ^ | + // | | | + // | emitCase +------+ | | + // +->+--------->| Case |->+-+ | + // ^ +------+ | | + // | | | + // +--------------------+ | + // | + // +---------------------------------------------------+ + // | + // | emitEnd +-----+ + // +-+----------------------------------------->+-------->| End | + // | ^ +-----+ + // | emitCaseBody +----------+ | + // +->+-+---------------->| CaseBody |--->+-+-+ + // ^ | +----------+ ^ | + // | | | | + // | | emitDefaultBody +-------------+ | | + // | +---------------->| DefaultBody |-+ | + // | +-------------+ | + // | | + // +-------------------------------------+ + // + enum class State { + // The initial state. + Start, + + // After calling emitDiscriminant. + Discriminant, + + // After calling validateCaseCount. + CaseCount, + + // After calling emitLexical. + Lexical, + + // After calling emitCond. + Cond, + + // After calling emitTable. + Table, + + // After calling emitCase. + Case, + + // After calling emitCaseBody. + CaseBody, + + // After calling emitDefaultBody. + DefaultBody, + + // After calling emitEnd. + End + }; + State state_ = State::Start; + + public: + explicit SwitchEmitter(BytecodeEmitter* bce); + + // `switchPos` is the offset in the source code for the character below: + // + // switch ( cond ) { ... } + // ^ + // | + // switchPos + // + // Can be Nothing() if not available. + MOZ_MUST_USE bool emitDiscriminant(const mozilla::Maybe& switchPos); + + // `caseCount` should be the number of cases in the switch statement, + // excluding the default case. + MOZ_MUST_USE bool validateCaseCount(uint32_t caseCount); + + // `bindings` is a lexical scope for the entire switch, in case there's + // let/const effectively directly under case or default blocks. + MOZ_MUST_USE bool emitLexical(Handle bindings); + + MOZ_MUST_USE bool emitCond(); + MOZ_MUST_USE bool emitTable(const TableGenerator& tableGen); + + MOZ_MUST_USE bool emitCaseJump(); + + MOZ_MUST_USE bool emitCaseBody(); + MOZ_MUST_USE bool emitCaseBody(int32_t caseValue, const TableGenerator& tableGen); + MOZ_MUST_USE bool emitDefaultBody(); + MOZ_MUST_USE bool emitEnd(); + + private: + MOZ_MUST_USE bool emitCaseOrDefaultJump(uint32_t caseIndex, bool isDefault); + MOZ_MUST_USE bool emitImplicitDefault(); +}; + +} /* namespace frontend */ +} /* namespace js */ + +#endif /* frontend_SwitchEmitter_h */ diff --git a/js/src/moz.build b/js/src/moz.build index 9b9a180be726..210c5389d041 100755 --- a/js/src/moz.build +++ b/js/src/moz.build @@ -219,6 +219,7 @@ UNIFIED_SOURCES += [ 'frontend/JumpList.cpp', 'frontend/NameFunctions.cpp', 'frontend/ParseNode.cpp', + 'frontend/SwitchEmitter.cpp', 'frontend/TDZCheckCache.cpp', 'frontend/TokenStream.cpp', 'frontend/TryEmitter.cpp', From 050c18ab846b7908e255c8f36170b04f13c80aa6 Mon Sep 17 00:00:00 2001 From: Sebastian Hengst Date: Fri, 20 Jul 2018 10:44:59 +0300 Subject: [PATCH 37/37] Bug 1471704 - Reftest annotation changes for tiling on linux with webrender: set background-position tests as passing. r=me --- .../meta/css/CSS2/backgrounds/background-position-001.xht.ini | 2 +- .../meta/css/CSS2/backgrounds/background-position-002.xht.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/web-platform/meta/css/CSS2/backgrounds/background-position-001.xht.ini b/testing/web-platform/meta/css/CSS2/backgrounds/background-position-001.xht.ini index dcf7b229a325..0f77a0ac8025 100644 --- a/testing/web-platform/meta/css/CSS2/backgrounds/background-position-001.xht.ini +++ b/testing/web-platform/meta/css/CSS2/backgrounds/background-position-001.xht.ini @@ -1,3 +1,3 @@ [background-position-001.xht] expected: - if os == "linux": FAIL + if os == "linux" and not webrender: FAIL diff --git a/testing/web-platform/meta/css/CSS2/backgrounds/background-position-002.xht.ini b/testing/web-platform/meta/css/CSS2/backgrounds/background-position-002.xht.ini index b1346e953498..6572a389f4a5 100644 --- a/testing/web-platform/meta/css/CSS2/backgrounds/background-position-002.xht.ini +++ b/testing/web-platform/meta/css/CSS2/backgrounds/background-position-002.xht.ini @@ -1,3 +1,3 @@ [background-position-002.xht] expected: - if os == "linux": FAIL + if os == "linux" and not webrender: FAIL