compileall used the ctime of bytecode and source to determine if the bytecode

should be recreated. This created a timing hole. Fixed by just doing what
import does; check the mtime and magic number.

Lib/compileall.py

@@ -11,10 +11,11 @@
 See module py_compile for details of the actual byte-compilation.
 
 """
-
 import os
 import sys
 import py_compile
+import struct
+import imp
 
 __all__ = ["compile_dir","compile_path"]
 
@@ -54,11 +55,17 @@ def compile_dir(dir, maxlevels=10, ddir=None,
         if os.path.isfile(fullname):
             head, tail = name[:-3], name[-3:]
             if tail == '.py':
+                if not force:
+                    try:
+                        mtime = os.stat(fullname).st_mtime
+                        expect = struct.pack('<4sl', imp.get_magic(), mtime)
                         cfile = fullname + (__debug__ and 'c' or 'o')
-                ftime = os.stat(fullname).st_mtime
-                try: ctime = os.stat(cfile).st_mtime
-                except os.error: ctime = 0
-                if (ctime > ftime) and not force: continue
+                        with open(cfile, 'rb') as chandle:
+                            actual = chandle.read(8)
+                        if expect == actual:
+                            continue
+                    except IOError:
+                        pass
                 if not quiet:
                     print 'Compiling', fullname, '...'
                 try:
@@ -80,7 +87,8 @@ def compile_dir(dir, maxlevels=10, ddir=None,
              name != os.curdir and name != os.pardir and \
              os.path.isdir(fullname) and \
              not os.path.islink(fullname):
-            if not compile_dir(fullname, maxlevels - 1, dfile, force, rx, quiet):
+            if not compile_dir(fullname, maxlevels - 1, dfile, force, rx,
+                               quiet):
                 success = 0
     return success
 

Lib/test/test_compileall.py

@@ -0,0 +1,63 @@
+import compileall
+import imp
+import os
+import py_compile
+import shutil
+import struct
+import sys
+import tempfile
+import time
+from test import test_support
+import unittest
+
+
+class CompileallTests(unittest.TestCase):
+
+    def setUp(self):
+        self.directory = tempfile.mkdtemp()
+        self.source_path = os.path.join(self.directory, '_test.py')
+        self.bc_path = self.source_path + ('c' if __debug__ else 'o')
+        with open(self.source_path, 'w') as file:
+            file.write('x = 123\n')
+
+    def tearDown(self):
+        shutil.rmtree(self.directory)
+
+    def data(self):
+        with open(self.bc_path, 'rb') as file:
+            data = file.read(8)
+        mtime = int(os.stat(self.source_path).st_mtime)
+        compare = struct.pack('<4sl', imp.get_magic(), mtime)
+        return data, compare
+
+    def recreation_check(self, metadata):
+        """Check that compileall recreates bytecode when the new metadata is
+        used."""
+        if not hasattr(os, 'stat'):
+            return
+        py_compile.compile(self.source_path)
+        self.assertEqual(*self.data())
+        with open(self.bc_path, 'rb') as file:
+            bc = file.read()[len(metadata):]
+        with open(self.bc_path, 'wb') as file:
+            file.write(metadata)
+            file.write(bc)
+        self.assertNotEqual(*self.data())
+        compileall.compile_dir(self.directory, force=False, quiet=True)
+        self.assertTrue(*self.data())
+
+    def test_mtime(self):
+        # Test a change in mtime leads to a new .pyc.
+        self.recreation_check(struct.pack('<4sl', imp.get_magic(), 1))
+
+    def test_magic_number(self):
+        # Test a change in mtime leads to a new .pyc.
+        self.recreation_check(b'\0\0\0\0')
+
+
+def test_main():
+    test_support.run_unittest(CompileallTests)
+
+
+if __name__ == "__main__":
+    test_main()

Misc/ACKS

@@ -233,6 +233,7 @@ Peter Funk
 Geoff Furnish
 Ulisses Furquim
 Achim Gaedke
+Martin von Gagern
 Lele Gaifax
 Santiago Gala
 Yitzchak Gale

Misc/NEWS

@@ -152,6 +152,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #5128: Make compileall properly inspect bytecode to determine if needs
+  to be recreated. This avoids a timing hole thanks to the old reliance on the
+  ctime of the files involved.
+
 - Issue #5122: Synchronize tk load failure check to prevent a potential
   deadlock.