Bug 1434765 - Properly reject invalid variables in #if{,n}def. r=froydnj,nalexander

The invalid variable test for #if{,n}def was only checking that the
first character in the variable was alphanumeric or underscore, not
the other characters.

More generally, preprocessor instructions were also cut out such that
whitespaces before and after arguments were part of the arguments.
Subtly, some legitimate strings end with what, in ISO-8859-1, is
considered as whitespaces, and because the preprocessor largely works
on byte strings (str), and because the regexps are using re.U, those
characters (e.g. 0xa0) that can legitimately appear in byte strings of
UTF-8 encoding, are treated as whitespaces. So we remove the re.U from
the instruction regexp, so that only plain ascii whitespaces only are
stripped out.

There's one place in layout/tools/reftest/manifest.jsm that was using
a broken pattern, making the test never true, which, once fixed, unveils
broken tests, so the branch that was never used is removed.

--HG--
extra : rebase_source : b695dec025c55aee0e50f2a0047278fe9c849c9e

layout/tools/reftest/manifest.jsm

@@ -512,12 +512,7 @@ sandbox.compareRetainedDisplayLists = g.compareRetainedDisplayLists;
     sandbox.styloVsGecko = false;
 #endif
 
-// Printing via Skia PDF is only supported on Mac for now.
-#ifdef XP_MACOSX && MOZ_ENABLE_SKIA_PDF
-    sandbox.skiaPdf = true;
-#else
     sandbox.skiaPdf = false;
-#endif
 
 #ifdef RELEASE_OR_BETA
     sandbox.release_or_beta = true;

python/mozbuild/mozbuild/preprocessor.py

@@ -339,9 +339,8 @@ class Preprocessor:
         """
         self.marker = aMarker
         if aMarker:
-            self.instruction = re.compile('{0}(?P<cmd>[a-z]+)(?:\s(?P<args>.*))?$'
+            self.instruction = re.compile('{0}(?P<cmd>[a-z]+)(?:\s+(?P<args>.*?))?\s*$'
-                                          .format(aMarker),
+                                          .format(aMarker))
-                                          re.U)
             self.comment = re.compile(aMarker, re.U)
         else:
             class NoMatch(object):
@@ -606,7 +605,7 @@ class Preprocessor:
         if self.disableLevel and not replace:
             self.disableLevel += 1
             return
-        if re.match('\W', args, re.U):
+        if re.search('\W', args, re.U):
             raise Preprocessor.Error(self, 'INVALID_VAR', args)
         if args not in self.context:
             self.disableLevel = 1
@@ -621,7 +620,7 @@ class Preprocessor:
         if self.disableLevel and not replace:
             self.disableLevel += 1
             return
-        if re.match('\W', args, re.U):
+        if re.search('\W', args, re.U):
             raise Preprocessor.Error(self, 'INVALID_VAR', args)
         if args in self.context:
             self.disableLevel = 1

python/mozbuild/mozbuild/test/test_preprocessor.py

@@ -642,5 +642,24 @@ class TestPreprocessor(unittest.TestCase):
             self.pp.handleCommandLine(['-Fsubstitution', '-Dfoo=foobarbaz', '@foo@.in'])
             self.assertEqual(self.pp.out.getvalue(), 'foobarbaz\n')
 
+    def test_invalid_ifdef(self):
+        with MockedOpen({'dummy': '#ifdef FOO == BAR\nPASS\n#endif'}):
+            with self.assertRaises(Preprocessor.Error) as e:
+                self.pp.do_include('dummy')
+            self.assertEqual(e.exception.key, 'INVALID_VAR')
+
+        with MockedOpen({'dummy': '#ifndef FOO == BAR\nPASS\n#endif'}):
+            with self.assertRaises(Preprocessor.Error) as e:
+                self.pp.do_include('dummy')
+            self.assertEqual(e.exception.key, 'INVALID_VAR')
+
+        # Trailing whitespaces, while not nice, shouldn't be an error.
+        self.do_include_pass([
+            '#ifndef  FOO ',
+            'PASS',
+            '#endif',
+        ])
+
+
 if __name__ == '__main__':
     main()