Scan SCons source code for uncaught KeyboardInterrupts. (Christoph Wiedemann)

9 files changed, 145 insertions, 11 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 7776757..40d8454 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -143,6 +143,9 @@ RELEASE 0.97 - XXX
     dictionary to map user-specified values to legal values from the list
     (like EnumOption() already doee).
 
+  - Add specific exceptions to try:-except: blocks without any listed,
+    so that they won't catch and mask keyboard interrupts.
+
   From Wayne Lee:
 
   - Avoid "maximum recursion limit" errors when removing $(-$) pairs
@@ -364,6 +367,10 @@ RELEASE 0.97 - XXX
   - Have the Qt Builder make uic-generated files dependent on the .ui.h
     file, if one exists.
 
+  - Add a test to make sure that SCons source code does not contain
+    try:-except: blocks that catch all errors, which potentially catch
+    and mask keyboard interrupts.
+
   From Russell Yanofsky:
 
   - Add support for the Metrowerks Codewarrior compiler and linker
diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py
index 7de6307..ca543bd 100644
--- a/src/engine/SCons/Environment.py
+++ b/src/engine/SCons/Environment.py
@@ -836,7 +836,11 @@ class Base:
                 continue
             try:
                 target, depends = string.split(line, ':', 1)
-            except:
+            except (AttributeError, TypeError, ValueError):
+                # Python 1.5.2 throws TypeError if line isn't a string,
+                # Python 2.x throws AttributeError because it tries
+                # to call line.splite().  Either can throw ValueError
+                # if the line doesn't split into two or more elements.
                 pass
             else:
                 self.Depends(string.split(target), string.split(depends))
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index c5d10bc..c381b8a 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -1452,6 +1452,9 @@ class File(Base):
     def get_stored_info(self):
         try:
             stored = self.dir.sconsign().get_entry(self.name)
+        except (KeyError, OSError):
+            return BuildInfo()
+        else:
             if isinstance(stored, BuildInfo):
                 return stored
             # The stored build information isn't a BuildInfo object.
@@ -1463,8 +1466,6 @@ class File(Base):
             for key, val in stored.__dict__.items():
                 setattr(binfo, key, val)
             return binfo
-        except:
-            return BuildInfo()
 
     def get_stored_implicit(self):
         binfo = self.get_stored_info()
@@ -1744,7 +1745,7 @@ class File(Base):
 
         try:
             mtime = self.get_timestamp()
-        except:
+        except OSError:
             mtime = 0
             raise SCons.Errors.UserError, "no such %s" % self
 
diff --git a/src/engine/SCons/Node/Python.py b/src/engine/SCons/Node/Python.py
index 28ff674..cabc162 100644
--- a/src/engine/SCons/Node/Python.py
+++ b/src/engine/SCons/Node/Python.py
@@ -68,7 +68,7 @@ class Value(SCons.Node.Node):
         value contents."""
         try:
             binfo = self.binfo
-        except:
+        except AttributeError:
             binfo = self.binfo = self.new_binfo()
         try:
             return binfo.csig
diff --git a/src/engine/SCons/Tool/intelc.py b/src/engine/SCons/Tool/intelc.py
index ded33e9..454998a 100644
--- a/src/engine/SCons/Tool/intelc.py
+++ b/src/engine/SCons/Tool/intelc.py
@@ -189,10 +189,11 @@ def generate(env, version=None, abi=None, topdir=None, verbose=1):
             for p in paths:
                 try:
                     path=get_intel_registry_value(p[1], version, abi)
+                except SCons.Errors.InternalError:
+                    env.PrependENVPath(p[0], os.path.join(topdir, p[2]))
+                else:
                     env.PrependENVPath(p[0], ';'.split(path))
                     # print "ICL %s: %s, final=%s"%(p[0], path, str(env['ENV'][p[0]]))
-                except:
-                    env.PrependENVPath(p[0], os.path.join(topdir, p[2]))
 
     if is_win32:
         env['CC']        = 'icl'
diff --git a/src/engine/SCons/Util.py b/src/engine/SCons/Util.py
index 87aba96..982fdec 100644
--- a/src/engine/SCons/Util.py
+++ b/src/engine/SCons/Util.py
@@ -181,7 +181,7 @@ else:
 def to_String_for_signature(obj):
     try:
         f = obj.for_signature
-    except:
+    except AttributeError:
         return to_String(obj)
     else:
         return f()
diff --git a/src/engine/SCons/dblite.py b/src/engine/SCons/dblite.py
index 8bd28e9..00f8274 100644
--- a/src/engine/SCons/dblite.py
+++ b/src/engine/SCons/dblite.py
@@ -61,7 +61,7 @@ class dblite:
         if (len(p) > 0):
           try:
             self._dict = cPickle.loads(p)
-          except:
+          except cPickle.UnpicklingError:
             if (ignore_corrupt_dbfiles == 0): raise
             if (ignore_corrupt_dbfiles == 1):
               print "Warning: Discarding corrupt database:", self._file_name
diff --git a/src/setup.py b/src/setup.py
index 525d000..d2b76c0 100644
--- a/src/setup.py
+++ b/src/setup.py
@@ -164,9 +164,11 @@ class install_lib(_install_lib):
                 # ...and they didn't explicitly ask for the standard
                 # directory, so guess based on what's out there.
                 try:
-                    e = filter(lambda x: x[:6] == "scons-", os.listdir(prefix))
-                except:
+                    l = os.listdir(prefix)
+                except OSError:
                     e = None
+                else:
+                    e = filter(lambda x: x[:6] == "scons-", l)
                 if e:
                     # We found a path name (e.g.) /usr/lib/scons-XXX,
                     # so pick the version-specific directory.
diff --git a/src/test_interrupts.py b/src/test_interrupts.py
new file mode 100644
index 0000000..d18bece
--- /dev/null
+++ b/src/test_interrupts.py
@@ -0,0 +1,119 @@
+#!/usr/bin/env python
+#
+# __COPYRIGHT__
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
+# KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+# WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+#
+
+__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
+
+"""
+Verify that the SCons source code contains only correct handling of
+keyboard interrupts (e.g. Ctrl-C).
+"""
+
+import os
+import os.path
+import re
+import string
+import time
+
+import TestSCons
+
+test = TestSCons.TestSCons()
+
+# We do not want statements of the form:
+# try:
+#   # do something, e.g.
+#   return a['x']
+# except:
+#   # do the exception handling
+#   a['x'] = getx()
+#   return a['x']
+#
+# The code above may catch a KeyboardInterrupt exception, which was not
+# intended by the programmer. We check for these situations in all python
+# source files.
+
+try:
+    cwd = os.environ['SCONS_CWD']
+except KeyError:
+    scons_lib_dir = os.environ['SCONS_LIB_DIR']
+    MANIFEST = os.path.join(scons_lib_dir, 'MANIFEST.in')
+else:
+    #cwd = os.getcwd()
+    scons_lib_dir = os.path.join(cwd, 'build', 'scons')
+    MANIFEST = os.path.join(scons_lib_dir, 'MANIFEST')
+
+files = string.split(open(MANIFEST).read())
+files = filter(lambda f: f[-3:] == '.py', files)
+
+# some regexps to parse the python files
+tryexc_pat = re.compile(
+r'^(?P<try_or_except>(?P<indent> *)(try|except)( [^\n]*)?:.*)',re.MULTILINE)
+keyboardint_pat = re.compile(r' *except +([^,],)*KeyboardInterrupt([ ,][^\n]*)?:[^\n]*')
+exceptall_pat   = re.compile(r' *except *:[^\n]*')
+
+uncaughtKeyboardInterrupt = 0
+for f in files:
+    contents = open(os.path.join(scons_lib_dir, f)).read()
+    try_except_lines = {}
+    lastend = 0
+    while 1:
+        match = tryexc_pat.search( contents, lastend )
+        if match is None:
+            break
+        #print match.groups()
+        lastend = match.end()
+        try:
+            indent_list = try_except_lines[match.group('indent')]
+        except:
+            indent_list = []
+        line_num = 1 + string.count(contents[:match.start()], '\n')
+        indent_list.append( (line_num, match.group('try_or_except') ) )
+        try_except_lines[match.group('indent')] = indent_list
+    for indent in try_except_lines.keys():
+        exc_keyboardint_seen = 0
+        exc_all_seen = 0
+        for (l,statement) in try_except_lines[indent] + [(-1,indent + 'try')]:
+            #print "%4d %s" % (l,statement),
+            m1 = keyboardint_pat.match(statement)
+            m2 = exceptall_pat.match(statement)
+            if string.find(statement, indent + 'try') == 0:
+                if exc_all_seen and not exc_keyboardint_seen:
+                    uncaughtKeyboardInterrupt = 1
+                    print "File %s:%d: Uncaught KeyboardInterrupt!" % (f,line)
+                exc_keyboardint_seen = 0
+                exc_all_seen = 0
+                line = l
+                #print " -> reset"
+            elif not m1 is None:
+                exc_keyboardint_seen = 1
+                #print " -> keyboard -> ", m1.groups()
+            elif not m2 is None:
+                exc_all_seen = 1
+                #print " -> all -> ", m2.groups()
+            else:
+                pass
+                #print "Warning: unknown statement %s" % statement
+
+test.fail_test(uncaughtKeyboardInterrupt)
+
+test.pass_test()