checkAPIs.pl: check for return/goto in TRY/CATCH blocks
authorPeter Wu <peter@lekensteyn.nl>
Tue, 9 Oct 2018 15:39:05 +0000 (17:39 +0200)
committerAnders Broman <a.broman58@gmail.com>
Wed, 10 Oct 2018 04:07:20 +0000 (04:07 +0000)
As documented in epan/exceptions.h, return/goto should never be used in
a TRY/CATCH/FINALLY block as ENDTRY must be executed first. Additionally
clamp the exit code since values larger than 255 will wrap around. Use a
small value as shells typically use 128+signal for termination signals.

Verified against packet-t125.c and ftype-protocol.c while they suffered
from the return bug. Tested against packet-gssapi.c for lack of false
positives (goto with labels within the function) and against:

    int main() {
        TRY {
            goto bar;
            goto omg;
            goto bar;
            goto barrie;
    barrie: ;
        } ENDTRY;
    bar: meh;
    }

Change-Id: I44484add34e238e07a84fc2c74b69f50ba6dc3f3
Ping-Bug: 15189
Reviewed-on: https://code.wireshark.org/review/30097
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
tools/checkAPIs.pl

index 5f3864a63880ff1b5d0d2523a7f6e01a2d0e8992..6caec1df53ad87b71ac29220bada60f8a93db6f9 100755 (executable)
@@ -917,6 +917,38 @@ sub check_pref_var_dupes($$)
         return $errorcount;
 }
 
+# Check for forbidden control flow changes, see epan/exceptions.h
+sub check_try_catch($$)
+{
+        my ($fileContentsRef, $filename) = @_;
+        my $errorCount = 0;
+
+        # Match TRY { ... } ENDTRY (with an optional '\' in case of a macro).
+        my @items = (${$fileContentsRef} =~ m/ \bTRY\s*\{ (.+?) \}\s* \\? \s*ENDTRY\b /xsg);
+        for my $block (@items) {
+                if ($block =~ m/ \breturn\b /x) {
+                        print STDERR "Error: return is forbidden in TRY/CATCH in $filename\n";
+                        $errorCount++;
+                }
+
+                my @gotoLabels = $block =~ m/ \bgoto\s+ (\w+) /xsg;
+                my %seen = ();
+                for my $gotoLabel (@gotoLabels) {
+                        if ($seen{$gotoLabel}) {
+                                next;
+                        }
+                        $seen{$gotoLabel} = 1;
+
+                        if ($block !~ /^ \s* $gotoLabel \s* :/xsgm) {
+                                print STDERR "Error: goto to label '$gotoLabel' outside TRY/CATCH is forbidden in $filename\n";
+                                $errorCount++;
+                        }
+                }
+        }
+
+        return $errorCount;
+}
+
 sub print_usage
 {
         print "Usage: checkAPIs.pl [-M] [-h] [-g group1[:count]] [-g group2] ... \n";
@@ -1240,6 +1272,8 @@ while ($_ = pop @filelist)
 
         $errorCount += check_proto_tree_add_XXX(\$fileContents, $filename);
 
+        $errorCount += check_try_catch(\$fileContents, $filename);
+
 
         # Check and count APIs
         for my $groupArg (@apiGroups) {
@@ -1304,7 +1338,7 @@ if (scalar @apiSummaryGroups > 0) {
         }
 }
 
-exit($errorCount);
+exit($errorCount > 120 ? 120 : $errorCount);
 
 #
 # Editor modelines  -  http://www.wireshark.org/tools/modelines.html