Bug 1178772 - Add check_macroassembler_style.py: Verify that each MacroAssembler declaration maps to all its definitions. r=h4writer

config/check_macroassembler_style.py

@@ -0,0 +1,264 @@
+# 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/.
+
+#----------------------------------------------------------------------------
+# This script checks that SpiderMonkey MacroAssembler methods are properly
+# annotated.
+#
+# The MacroAssembler has one interface for all platforms, but it might have one
+# definition per platform. The code of the MacroAssembler use a macro to
+# annotate the method declarations, in order to delete the function if it is not
+# present on the current platform, and also to locate the files in which the
+# methods are defined.
+#
+# This script scans the MacroAssembler.h header, for method declarations.
+# It also scans MacroAssembler-/arch/.cpp, MacroAssembler-/arch/-inl.h, and
+# MacroAssembler-inl.h for method definitions. The result of both scans are
+# uniformized, and compared, to determine if the MacroAssembler.h header as
+# proper methods annotations.
+#----------------------------------------------------------------------------
+
+from __future__ import print_function
+
+import difflib
+import os
+import re
+import subprocess
+import sys
+from check_utils import get_all_toplevel_filenames
+
+architecture_independent = set([ 'generic' ])
+all_architecture_names = set([ 'x86', 'x64', 'arm', 'arm64', 'mips' ])
+all_shared_architecture_names = set([ 'x86_shared', 'arm', 'arm64', 'mips' ])
+
+def get_normalized_signatures(signature, fileAnnot = None):
+    # Remove semicolon.
+    signature = signature.replace(';', ' ')
+    # Normalize spaces.
+    signature = re.sub(r'\s+', ' ', signature).strip()
+    # Remove argument names.
+    signature = re.sub(r'(?P<type>(?:[(]|,\s)[\w\s:*&]+)(?P<name>\s\w+)(?=[,)])', '\g<type>', signature)
+    # Remove class name
+    signature = signature.replace('MacroAssembler::', '')
+
+    # Extract list of architectures
+    archs = ['generic']
+    if fileAnnot:
+        archs = [fileAnnot['arch']]
+
+    if 'DEFINED_ON(' in signature:
+        archs = re.sub(r'.*DEFINED_ON\((?P<archs>[^()]*)\).*', '\g<archs>', signature).split(',')
+        archs = [a.strip() for a in archs]
+        signature = re.sub(r'\s+DEFINED_ON\([^()]*\)', '', signature)
+
+    elif 'PER_ARCH' in signature:
+        archs = all_architecture_names
+        signature = re.sub(r'\s+PER_ARCH', '', signature)
+
+    elif 'PER_SHARED_ARCH' in signature:
+        archs = all_shared_architecture_names
+        signature = re.sub(r'\s+PER_SHARED_ARCH', '', signature)
+
+    else:
+        # No signature annotation, the list of architectures remains unchanged.
+        pass
+
+    # Extract inline annotation
+    inline = False
+    if fileAnnot:
+        inline = fileAnnot['inline']
+
+    if 'inline ' in signature:
+        signature = re.sub(r'inline\s+', '', signature)
+        inline = True
+
+    return [
+        { 'arch': a, 'sig': 'inline ' + signature }
+        for a in archs
+    ]
+
+file_suffixes = set([
+    a.replace('_', '-') for a in
+    all_architecture_names.union(all_shared_architecture_names)
+])
+def get_file_annotation(filename):
+    origFilename = filename
+    filename = filename.split('/')[-1]
+
+    inline = False
+    if filename.endswith('.cpp'):
+        filename = filename[:-len('.cpp')]
+    elif filename.endswith('-inl.h'):
+        inline = True
+        filename = filename[:-len('-inl.h')]
+    else:
+        raise Exception('unknown file name', origFilename)
+
+    arch = 'generic'
+    for suffix in file_suffixes:
+        if filename == 'MacroAssembler-' + suffix:
+            arch = suffix
+            break
+
+    return {
+        'inline': inline,
+        'arch': arch.replace('-', '_')
+    }
+
+def get_macroassembler_definitions(filename):
+    try:
+        fileAnnot = get_file_annotation(filename)
+    except:
+        return []
+
+    style_section = False
+    code_section = False
+    lines = ''
+    signatures = []
+    with open(os.path.join('../..', filename)) as f:
+        for line in f:
+            if '//{{{ check_macroassembler_style' in line:
+                style_section = True
+            elif '//}}} check_macroassembler_style' in line:
+                style_section = False
+            if not style_section:
+                continue
+
+            line = re.sub(r'//.*', '', line)
+            if line.startswith('{'):
+                if 'MacroAssembler::' in lines:
+                    signatures.extend(get_normalized_signatures(lines, fileAnnot))
+                code_section = True
+                continue
+            if line.startswith('}'):
+                code_section = False
+                lines = ''
+                continue
+            if code_section:
+                continue
+
+            if len(line.strip()) == 0:
+                lines = ''
+                continue
+            lines = lines + line
+            # Continue until we have a complete declaration
+            if '{' not in lines:
+                continue
+            # Skip variable declarations
+            if ')' not in lines:
+                lines = ''
+                continue
+
+    return signatures
+
+def get_macroassembler_declaration(filename):
+    style_section = False
+    lines = ''
+    signatures = []
+    with open(os.path.join('../..', filename)) as f:
+        for line in f:
+            if '//{{{ check_macroassembler_style' in line:
+                style_section = True
+            elif '//}}} check_macroassembler_style' in line:
+                style_section = False
+            if not style_section:
+                continue
+
+            line = re.sub(r'//.*', '', line)
+            if len(line.strip()) == 0:
+                lines = ''
+                continue
+            lines = lines + line
+            # Continue until we have a complete declaration
+            if ';' not in lines:
+                continue
+            # Skip variable declarations
+            if ')' not in lines:
+                lines = ''
+                continue
+
+            signatures.extend(get_normalized_signatures(lines))
+            lines = ''
+
+    return signatures
+
+def append_signatures(d, sigs):
+    for s in sigs:
+        if s['sig'] not in d:
+            d[s['sig']] = []
+        d[s['sig']].append(s['arch']);
+    return d
+
+def generate_file_content(signatures):
+    output = []
+    for s in sorted(signatures.keys()):
+        archs = set(signatures[s])
+        if len(archs.symmetric_difference(architecture_independent)) == 0:
+            output.append(s + ';\n')
+            if s.startswith('inline'):
+                output.append('    is defined in MacroAssembler-inl.h\n')
+            else:
+                output.append('    is defined in MacroAssembler.cpp\n')
+        else:
+            if len(archs.symmetric_difference(all_architecture_names)) == 0:
+                output.append(s + ' PER_ARCH;\n')
+            elif len(archs.symmetric_difference(all_shared_architecture_names)) == 0:
+                output.append(s + ' PER_SHARED_ARCH;\n')
+            else:
+                output.append(s + ' DEFINED_ON(' + ', '.join(archs) + ');\n')
+            for a in archs:
+                a = a.replace('_', '-')
+                masm = '%s/MacroAssembler-%s' % (a, a)
+                if s.startswith('inline'):
+                    output.append('    is defined in %s-inl.h\n' % masm)
+                else:
+                    output.append('    is defined in %s.cpp\n' % masm)
+    return output
+
+def check_style():
+    # We read from the header file the signature of each function.
+    decls = dict()      # type: dict(signature => ['x86', 'x64'])
+
+    # We infer from each file the signature of each MacroAssembler function.
+    defs = dict()       # type: dict(signature => ['x86', 'x64'])
+
+    # Select the appropriate files.
+    for filename in get_all_toplevel_filenames():
+        if not filename.startswith('js/src/jit/'):
+            continue
+        if 'MacroAssembler' not in filename:
+            continue
+
+        if filename.endswith('MacroAssembler.h'):
+            decls = append_signatures(decls, get_macroassembler_declaration(filename))
+        else:
+            defs = append_signatures(defs, get_macroassembler_definitions(filename))
+
+    # Compare declarations and definitions output.
+    difflines = difflib.unified_diff(generate_file_content(decls),
+                                     generate_file_content(defs),
+                                     fromfile='check_macroassembler_style.py declared syntax',
+                                     tofile='check_macroassembler_style.py found definitions')
+    ok = True
+    for diffline in difflines:
+        ok = False
+        print(diffline, end='')
+
+    return ok
+
+
+def main():
+    ok = check_style()
+
+    if ok:
+        print('TEST-PASS | check_macroassembler_style.py | ok')
+    else:
+        print('TEST-UNEXPECTED-FAIL | check_macroassembler_style.py | actual output does not match expected output;  diff is above')
+
+    sys.exit(0 if ok else 1)
+
+
+if __name__ == '__main__':
+    main()

config/check_spidermonkey_style.py

@@ -43,6 +43,7 @@ import re
 import subprocess
 import sys
 import traceback
+from check_utils import get_all_toplevel_filenames
 
 # We don't bother checking files in these directories, because they're (a) auxiliary or (b)
 # imported code that doesn't follow our coding style.
@@ -217,20 +218,6 @@ class FileKind(object):
         error(filename, None, 'unknown file kind')
 
 
-def get_all_filenames():
-    '''Get a list of all the files in the (Mercurial or Git) repository.'''
-    cmds = [['hg', 'manifest', '-q'], ['git', 'ls-files', '--full-name', '../..']]
-    for cmd in cmds:
-        try:
-            all_filenames = subprocess.check_output(cmd, universal_newlines=True,
-                                                    stderr=subprocess.PIPE).split('\n')
-            return all_filenames
-        except:
-            continue
-    else:
-        raise Exception('failed to run any of the repo manifest commands', cmds)
-
-
 def check_style():
     # We deal with two kinds of name.
     # - A "filename" is a full path to a file from the repository root.
@@ -247,7 +234,7 @@ def check_style():
     js_names = dict()           # type: dict(filename, inclname)
 
     # Select the appropriate files.
-    for filename in get_all_filenames():
+    for filename in get_all_toplevel_filenames():
         if filename.startswith('mfbt/') and filename.endswith('.h'):
             inclname = 'mozilla/' + filename.split('/')[-1]
             mfbt_inclnames.add(inclname)

config/check_utils.py

@@ -0,0 +1,30 @@
+# 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/.
+
+import subprocess
+
+def get_all_toplevel_filenames():
+    '''Get a list of all the files in the (Mercurial or Git) repository.'''
+    try:
+        cmd = ['hg', 'manifest', '-q']
+        all_filenames = subprocess.check_output(cmd, universal_newlines=True,
+                                                stderr=subprocess.PIPE).split('\n')
+        return all_filenames
+    except:
+        pass
+
+    try:
+        # Get the relative path to the top-level directory.
+        cmd = ['git', 'rev-parse', '--show-cdup']
+        top_level = subprocess.check_output(cmd, universal_newlines=True,
+                                                stderr=subprocess.PIPE).split('\n')[0]
+        cmd = ['git', 'ls-files', '--full-name', top_level]
+        all_filenames = subprocess.check_output(cmd, universal_newlines=True,
+                                                stderr=subprocess.PIPE).split('\n')
+        return all_filenames
+    except:
+        pass
+
+    raise Exception('failed to run any of the repo manifest commands', cmds)

js/src/Makefile.in

@@ -127,6 +127,9 @@ endif
 check-style::
 	(cd $(srcdir) && $(PYTHON) $(topsrcdir)/config/check_spidermonkey_style.py);
 
+check-masm::
+	(cd $(srcdir) && $(PYTHON) $(topsrcdir)/config/check_macroassembler_style.py);
+
 check-jit-test::
 	$(JITTEST_ASAN_ENV) $(wildcard $(RUN_TEST_PROGRAM)) $(PYTHON) -u $(srcdir)/jit-test/jit_test.py \
 	        --no-slow --no-progress --format=automation --jitflags=all \
@@ -134,7 +137,7 @@ check-jit-test::
 			$(JITTEST_EXTRA_ARGS) \
 	        $(DIST)/bin/$(JS_SHELL_NAME)$(BIN_SUFFIX)
 
-check:: check-style
+check:: check-style check-masm
 
 check-jstests:
 	$(wildcard $(RUN_TEST_PROGRAM)) $(PYTHON) -u $(srcdir)/tests/jstests.py \

js/src/jit/MacroAssembler-inl.h

@@ -12,6 +12,7 @@
 namespace js {
 namespace jit {
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Frame manipulation functions.
 
@@ -75,6 +76,7 @@ MacroAssembler::call(const CallSiteDesc& desc, Label* label)
     append(desc, currentOffset(), framePushed());
 }
 
+//}}} check_macroassembler_style
 // ===============================================================
 
 void

js/src/jit/MacroAssembler.cpp

@@ -2554,6 +2554,7 @@ MacroAssembler::icRestoreLive(LiveRegisterSet& liveRegs, AfterICSaveLive& aic)
     PopRegsInMask(liveRegs);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
@@ -2718,3 +2719,5 @@ MacroAssembler::freeStack(Register amount)
 {
     addToStackPtr(amount);
 }
+
+//}}} check_macroassembler_style

js/src/jit/MacroAssembler.h

@@ -7,6 +7,7 @@
 #ifndef jit_MacroAssembler_h
 #define jit_MacroAssembler_h
 
+#include "mozilla/MacroForEach.h"
 #include "mozilla/MathAlgorithms.h"
 
 #include "jscompartment.h"
@@ -34,28 +35,134 @@
 #include "vm/Shape.h"
 #include "vm/UnboxedObject.h"
 
-// This Macro is only a hint for code readers that the function is defined in a
-// specific macro assembler, and not in the generic macro assembler.  Thus, when
-// looking for the implementation one might find if the function is implemented
-// and where it is supposed to be implemented.
-# define PER_ARCH
+// * How to read/write MacroAssembler method declarations:
+//
+// The following macros are made to avoid #ifdef around each method declarations
+// of the Macro Assembler, and they are also used as an hint on the location of
+// the implementations of each method.  For example, the following declaration
+//
+//   void Pop(FloatRegister t) DEFINED_ON(x86_shared, arm);
+//
+// suggests the MacroAssembler::Pop(FloatRegister) method is implemented in
+// x86-shared/MacroAssembler-x86-shared.h, and also in arm/MacroAssembler-arm.h.
+//
+// - If there is no annotation, then there is only one generic definition in
+//   MacroAssembler.cpp.
+//
+// - 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.
+//
+// Some convenient short-cuts are used to avoid repeating the same list of
+// architectures on each method declaration, such as PER_ARCH and
+// PER_SHARED_ARCH.
 
+# define ALL_ARCH mips, arm, arm64, x86, x64
+# define ALL_SHARED_ARCH mips, arm, arm64, x86_shared
+
+// * How this macro works:
+//
+// DEFINED_ON is a macro which check if, for the current architecture, the
+// method is defined on the macro assembler or not.
+//
+// For each architecutre, 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).
+//
+// The DEFINED_ON macro maps the list of architecture names given as argument to
+// a list of macro names.  For example,
+//
+//   DEFINED_ON(arm, x86_shared)
+//
+// is expanded to
+//
+//   DEFINED_ON_none DEFINED_ON_arm DEFINED_ON_x86_shared
+//
+// which are later expanded on ARM, x86, x64 by DEFINED_ON_EXPAND_ARCH_RESULTS
+// to
+//
+//   define
+//
+// or if the JIT is disabled or set to no architecture to
+//
+//   crash
+//
+// or to nothing, if the current architecture is not lsited in the list of
+// arguments of DEFINED_ON.  Note, only one of the DEFINED_ON_arch macro
+// contributes to the non-empty result, which is the macro of the current
+// 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"
+// annotation on the method declaration.
+
+# define DEFINED_ON_x86
+# define DEFINED_ON_x64
+# define DEFINED_ON_x86_shared
+# define DEFINED_ON_arm
+# define DEFINED_ON_arm64
+# define DEFINED_ON_mips
+# define DEFINED_ON_none
+
+// Specialize for each architecture.
 #if defined(JS_CODEGEN_X86)
-# define ONLY_X86_X64
+# undef DEFINED_ON_x86
+# define DEFINED_ON_x86 define
+# undef DEFINED_ON_x86_shared
+# define DEFINED_ON_x86_shared define
 #elif defined(JS_CODEGEN_X64)
-# define ONLY_X86_X64
+# undef DEFINED_ON_x64
+# define DEFINED_ON_x64 define
+# undef DEFINED_ON_x86_shared
+# define DEFINED_ON_x86_shared define
 #elif defined(JS_CODEGEN_ARM)
-# define ONLY_X86_X64 = delete
+# undef DEFINED_ON_arm
+# define DEFINED_ON_arm define
 #elif defined(JS_CODEGEN_ARM64)
-# define ONLY_X86_X64 = delete
+# undef DEFINED_ON_arm64
+# define DEFINED_ON_arm64 define
 #elif defined(JS_CODEGEN_MIPS)
-# define ONLY_X86_X64 = delete
+# undef DEFINED_ON_mips
+# define DEFINED_ON_mips define
 #elif defined(JS_CODEGEN_NONE)
-# define ONLY_X86_X64 = delete
+# undef DEFINED_ON_none
+# define DEFINED_ON_none crash
 #else
 # error "Unknown architecture!"
 #endif
 
+# define DEFINED_ON_RESULT_crash   { MOZ_CRASH(); }
+# define DEFINED_ON_RESULT_define
+# define DEFINED_ON_RESULT_        = delete
+
+# define DEFINED_ON_DISPATCH_RESULT(Result)     \
+    DEFINED_ON_RESULT_ ## Result
+
+// We need to let the evaluation of MOZ_FOR_EACH terminates.
+# define DEFINED_ON_EXPAND_ARCH_RESULTS_3(ParenResult)  \
+    DEFINED_ON_DISPATCH_RESULT ParenResult
+# define DEFINED_ON_EXPAND_ARCH_RESULTS_2(ParenResult)  \
+    DEFINED_ON_EXPAND_ARCH_RESULTS_3 (ParenResult)
+# define DEFINED_ON_EXPAND_ARCH_RESULTS(ParenResult)    \
+    DEFINED_ON_EXPAND_ARCH_RESULTS_2 (ParenResult)
+
+# define DEFINED_ON_FWDARCH(Arch) DEFINED_ON_ ## Arch
+# define DEFINED_ON_MAP_ON_ARCHS(ArchList)              \
+    DEFINED_ON_EXPAND_ARCH_RESULTS(                     \
+      (MOZ_FOR_EACH(DEFINED_ON_FWDARCH, (), ArchList)))
+
+# define DEFINED_ON(...)                                \
+    DEFINED_ON_MAP_ON_ARCHS((none, __VA_ARGS__))
+
+# define PER_ARCH DEFINED_ON(ALL_ARCH)
+# define PER_SHARED_ARCH DEFINED_ON(ALL_SHARED_ARCH)
+
+
 #ifdef IS_LITTLE_ENDIAN
 #define IMM32_16ADJ(X) X << 16
 #else
@@ -285,6 +392,7 @@ class MacroAssembler : public MacroAssemblerSpecific
         return size();
     }
 
+    //{{{ check_macroassembler_style
   public:
     // ===============================================================
     // Frame manipulation functions.
@@ -309,20 +417,20 @@ class MacroAssembler : public MacroAssemblerSpecific
     // ===============================================================
     // Stack manipulation functions.
 
-    void PushRegsInMask(LiveRegisterSet set) PER_ARCH;
+    void PushRegsInMask(LiveRegisterSet set) PER_SHARED_ARCH;
     void PushRegsInMask(LiveGeneralRegisterSet set);
 
     void PopRegsInMask(LiveRegisterSet set);
     void PopRegsInMask(LiveGeneralRegisterSet set);
-    void PopRegsInMaskIgnore(LiveRegisterSet set, LiveRegisterSet ignore) PER_ARCH;
+    void PopRegsInMaskIgnore(LiveRegisterSet set, LiveRegisterSet ignore) PER_SHARED_ARCH;
 
-    void Push(const Operand op) PER_ARCH ONLY_X86_X64;
-    void Push(Register reg) PER_ARCH;
-    void Push(const Imm32 imm) PER_ARCH;
-    void Push(const ImmWord imm) PER_ARCH;
-    void Push(const ImmPtr imm) PER_ARCH;
-    void Push(const ImmGCPtr ptr) PER_ARCH;
-    void Push(FloatRegister reg) PER_ARCH;
+    void Push(const Operand op) DEFINED_ON(x86_shared);
+    void Push(Register reg) PER_SHARED_ARCH;
+    void Push(const Imm32 imm) PER_SHARED_ARCH;
+    void Push(const ImmWord imm) PER_SHARED_ARCH;
+    void Push(const ImmPtr imm) PER_SHARED_ARCH;
+    void Push(const ImmGCPtr ptr) PER_SHARED_ARCH;
+    void Push(FloatRegister reg) PER_SHARED_ARCH;
     void Push(jsid id, Register scratchReg);
     void Push(TypedOrValueRegister v);
     void Push(ConstantOrRegister v);
@@ -334,10 +442,10 @@ class MacroAssembler : public MacroAssemblerSpecific
     inline CodeOffsetLabel PushWithPatch(ImmWord word);
     inline CodeOffsetLabel PushWithPatch(ImmPtr imm);
 
-    void Pop(const Operand op) PER_ARCH ONLY_X86_X64;
-    void Pop(Register reg) PER_ARCH;
-    void Pop(FloatRegister t) PER_ARCH ONLY_X86_X64;
-    void Pop(const ValueOperand& val) PER_ARCH;
+    void Pop(const Operand op) DEFINED_ON(x86_shared);
+    void Pop(Register reg) PER_SHARED_ARCH;
+    void Pop(FloatRegister t) DEFINED_ON(x86_shared);
+    void Pop(const ValueOperand& val) PER_SHARED_ARCH;
     void popRooted(VMFunction::RootType rootType, Register cellReg, const ValueOperand& valueReg);
 
     // Move the stack pointer based on the requested amount.
@@ -352,19 +460,20 @@ class MacroAssembler : public MacroAssemblerSpecific
     // ===============================================================
     // Simple call functions.
 
-    void call(Register reg) PER_ARCH;
-    void call(const Address& addr) PER_ARCH ONLY_X86_X64;
-    void call(Label* label) PER_ARCH;
-    void call(ImmWord imm) PER_ARCH;
+    void call(Register reg) PER_SHARED_ARCH;
+    void call(const Address& addr) DEFINED_ON(x86_shared);
+    void call(Label* label) PER_SHARED_ARCH;
+    void call(ImmWord imm) PER_SHARED_ARCH;
     // Call a target native function, which is neither traceable nor movable.
-    void call(ImmPtr imm) PER_ARCH;
-    void call(AsmJSImmPtr imm) PER_ARCH;
+    void call(ImmPtr imm) PER_SHARED_ARCH;
+    void call(AsmJSImmPtr imm) PER_SHARED_ARCH;
     // Call a target JitCode, which must be traceable, and may be movable.
-    void call(JitCode* c) PER_ARCH;
+    void call(JitCode* c) PER_SHARED_ARCH;
 
     inline void call(const CallSiteDesc& desc, const Register reg);
     inline void call(const CallSiteDesc& desc, Label* label);
 
+    //}}} check_macroassembler_style
   public:
 
     // Emits a test of a value against all types in a TypeSet. A scratch

js/src/jit/arm/MacroAssembler-arm.cpp

@@ -5110,6 +5110,7 @@ MacroAssemblerARMCompat::asMasm() const
     return *static_cast<const MacroAssembler*>(this);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
@@ -5300,3 +5301,5 @@ MacroAssembler::call(JitCode* c)
     ma_movPatchable(ImmPtr(c->raw()), ScratchRegister, Always, rs);
     ma_callJitHalfPush(ScratchRegister);
 }
+
+//}}} check_macroassembler_style

js/src/jit/arm64/MacroAssembler-arm64.cpp

@@ -552,18 +552,10 @@ MacroAssemblerCompat::breakpoint()
     Brk((code++) & 0xffff);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
-void
-MacroAssembler::reserveStack(uint32_t amount)
-{
-    // TODO: This bumps |sp| every time we reserve using a second register.
-    // It would save some instructions if we had a fixed frame size.
-    vixl::MacroAssembler::Claim(Operand(amount));
-    adjustFrame(amount);
-}
-
 void
 MacroAssembler::PushRegsInMask(LiveRegisterSet set)
 {
@@ -688,7 +680,7 @@ MacroAssembler::Push(FloatRegister f)
 }
 
 void
-MacroAssembler::Pop(const Register reg)
+MacroAssembler::Pop(Register reg)
 {
     pop(reg);
     adjustFrame(-1 * int64_t(sizeof(int64_t)));
@@ -701,6 +693,15 @@ MacroAssembler::Pop(const ValueOperand& val)
     adjustFrame(-1 * int64_t(sizeof(int64_t)));
 }
 
+void
+MacroAssembler::reserveStack(uint32_t amount)
+{
+    // TODO: This bumps |sp| every time we reserve using a second register.
+    // It would save some instructions if we had a fixed frame size.
+    vixl::MacroAssembler::Claim(Operand(amount));
+    adjustFrame(amount);
+}
+
 // ===============================================================
 // Simple call functions.
 
@@ -753,5 +754,7 @@ MacroAssembler::call(JitCode* c)
     blr(scratch64);
 }
 
+//}}} check_macroassembler_style
+
 } // namespace jit
 } // namespace js

js/src/jit/mips/MacroAssembler-mips.cpp

@@ -3651,6 +3651,7 @@ MacroAssemblerMIPSCompat::asMasm() const
     return *static_cast<const MacroAssembler*>(this);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
@@ -3822,3 +3823,5 @@ MacroAssembler::call(JitCode* c)
     ma_liPatchable(ScratchRegister, Imm32((uint32_t)c->raw()));
     ma_callJitHalfPush(ScratchRegister);
 }
+
+//}}} check_macroassembler_style

js/src/jit/none/Trampoline-none.cpp

@@ -53,26 +53,3 @@ bool ICCompare_Double::Compiler::generateStubCode(MacroAssembler&) { MOZ_CRASH()
 bool ICBinaryArith_Int32::Compiler::generateStubCode(MacroAssembler&) { MOZ_CRASH(); }
 bool ICUnaryArith_Int32::Compiler::generateStubCode(MacroAssembler&) { MOZ_CRASH(); }
 JitCode* JitRuntime::generateProfilerExitFrameTailStub(JSContext*) { MOZ_CRASH(); }
-
-void MacroAssembler::alignFrameForICArguments(AfterICSaveLive& aic) { MOZ_CRASH(); }
-void MacroAssembler::restoreFrameAlignmentForICArguments(AfterICSaveLive& aic) { MOZ_CRASH(); }
-
-void MacroAssembler::clampDoubleToUint8(FloatRegister input, Register output) { MOZ_CRASH(); }
-
-// ===============================================================
-// Stack manipulation functions.
-
-void MacroAssembler::PushRegsInMask(LiveRegisterSet) { MOZ_CRASH(); }
-void MacroAssembler::PopRegsInMaskIgnore(LiveRegisterSet, LiveRegisterSet) { MOZ_CRASH(); }
-
-void MacroAssembler::Push(Register reg) { MOZ_CRASH(); }
-void MacroAssembler::Push(const Imm32 imm) { MOZ_CRASH(); }
-void MacroAssembler::Push(const ImmWord imm) { MOZ_CRASH(); }
-void MacroAssembler::Push(const ImmPtr imm) { MOZ_CRASH(); }
-void MacroAssembler::Push(const ImmGCPtr ptr) { MOZ_CRASH(); }
-void MacroAssembler::Push(FloatRegister reg) { MOZ_CRASH(); }
-
-void MacroAssembler::Pop(Register reg) { MOZ_CRASH(); }
-void MacroAssembler::Pop(const ValueOperand& val) { MOZ_CRASH(); }
-
-void MacroAssembler::reserveStack(uint32_t amount) { MOZ_CRASH(); }

js/src/jit/x64/MacroAssembler-x64.cpp

@@ -578,6 +578,7 @@ MacroAssemblerX64::asMasm() const
     return *static_cast<const MacroAssembler*>(this);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
@@ -600,3 +601,5 @@ MacroAssembler::reserveStack(uint32_t amount)
     }
     framePushed_ += amount;
 }
+
+//}}} check_macroassembler_style

js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp

@@ -195,6 +195,7 @@ MacroAssemblerX86Shared::asMasm() const
     return *static_cast<const MacroAssembler*>(this);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
@@ -415,3 +416,5 @@ MacroAssembler::call(JitCode* target)
 {
     Assembler::call(target);
 }
+
+//}}} check_macroassembler_style

js/src/jit/x86/MacroAssembler-x86.cpp

@@ -565,6 +565,7 @@ MacroAssemblerX86::asMasm() const
     return *static_cast<const MacroAssembler*>(this);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
@@ -587,3 +588,5 @@ MacroAssembler::reserveStack(uint32_t amount)
     }
     framePushed_ += amount;
 }
+
+//}}} check_macroassembler_style

testing/mach_commands.py

@@ -352,7 +352,11 @@ class CheckSpiderMonkeyCommand(MachCommandBase):
         check_style_cmd = [sys.executable, os.path.join(self.topsrcdir, 'config', 'check_spidermonkey_style.py')]
         check_style_result = subprocess.call(check_style_cmd, cwd=os.path.join(self.topsrcdir, 'js', 'src'))
 
-        all_passed = jittest_result and jstest_result and jsapi_tests_result and check_style_result
+        print('running check-masm')
+        check_masm_cmd = [sys.executable, os.path.join(self.topsrcdir, 'config', 'check_macroassembler_style.py')]
+        check_masm_result = subprocess.call(check_masm_cmd, cwd=os.path.join(self.topsrcdir, 'js', 'src'))
+
+        all_passed = jittest_result and jstest_result and jsapi_tests_result and check_style_result and check_masm_result
 
         return all_passed