Covers test case proposed during the review

Author: mbaluda

c/cert/src/rules/ERR32-C/DoNotRelyOnIndeterminateValuesOfErrno.ql

@@ -37,18 +37,22 @@ class SignalCheckOperation extends EqualityOperation, GuardCondition {
     )
   }
 
+  ControlFlowNode getCheckedSuccessor() {
+    result != errorSuccessor and result = this.getASuccessor()
+  }
+
   ControlFlowNode getErrorSuccessor() { result = errorSuccessor }
 }
 
 /**
  * Models signal handlers that call signal() and return
  */
 class SignalCallingHandler extends Function {
-  SignalCall sh;
+  SignalCall registration;
 
   SignalCallingHandler() {
     // is a signal handler
-    this = sh.getArgument(1).(FunctionAccess).getTarget() and
+    this = registration.getArgument(1).(FunctionAccess).getTarget() and
     // calls signal() on the handled signal
     exists(SignalCall sCall |
       sCall.getEnclosingFunction() = this and
@@ -63,18 +67,35 @@ class SignalCallingHandler extends Function {
     )
   }
 
-  SignalCall getHandler() { result = sh }
+  SignalCall getCall() { result = registration }
+}
+
+/**
+ * CFG nodes preceeding `ErrnoRead`
+ */
+ControlFlowNode preceedErrnoRead(ErrnoRead er) {
+  result = er
+  or
+  exists(ControlFlowNode mid |
+    result = mid.getAPredecessor() and
+    mid = preceedErrnoRead(er) and
+    // stop recursion on calls to `abort` and `_Exit`
+    not result.(FunctionCall).getTarget().hasGlobalName(["abort", "_Exit"]) and
+    // stop recursion on successful `SignalCheckOperation`
+    not result = any(SignalCheckOperation o).getCheckedSuccessor()
+  )
 }
 
-from ErrnoRead errno, SignalCall h
+from ErrnoRead errno, SignalCall signal
 where
   not isExcluded(errno, Contracts5Package::doNotRelyOnIndeterminateValuesOfErrnoQuery()) and
-  exists(SignalCallingHandler sc | sc.getHandler() = h |
-    // errno read in the handler
-    sc.calls*(errno.getEnclosingFunction())
+  exists(SignalCallingHandler handler |
+    // errno read after the handler returns
+    handler.getCall() = signal
     or
-    // errno is read after the handle returns
-    sc.getHandler() = h and
-    errno.getAPredecessor+() = h
+    // errno read inside the handler
+    signal.getEnclosingFunction() = handler
+  |
+    signal = preceedErrnoRead(errno)
   )
-select errno, "`errno` has indeterminate value after this $@.", h, h.toString()
+select errno, "`errno` has indeterminate value after this $@.", signal, signal.toString()

c/cert/test/rules/ERR32-C/DoNotRelyOnIndeterminateValuesOfErrno.expected

@@ -1,5 +1,5 @@
-| test.c:12:5:12:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:35:21:35:26 | call to signal | call to signal |
-| test.c:37:5:37:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:35:21:35:26 | call to signal | call to signal |
-| test.c:42:5:42:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:35:21:35:26 | call to signal | call to signal |
-| test.c:51:5:51:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:35:21:35:26 | call to signal | call to signal |
-| test.c:51:5:51:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:45:17:45:22 | call to signal | call to signal |
+| test.c:12:5:12:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:10:21:10:26 | call to signal | call to signal |
+| test.c:39:5:39:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:35:21:35:26 | call to signal | call to signal |
+| test.c:46:5:46:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:44:21:44:26 | call to signal | call to signal |
+| test.c:62:5:62:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:54:17:54:22 | call to signal | call to signal |
+| test.c:62:5:62:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:58:17:58:22 | call to signal | call to signal |

c/cert/test/rules/ERR32-C/test.c

@@ -31,6 +31,15 @@ void handler4(int signum) {
   }
 }
 
+void handler5(int signum) {
+  pfv old_handler = signal(signum, SIG_DFL);
+  if (old_handler != SIG_ERR) {
+    perror(""); // COMPLIANT
+  } else {
+    perror(""); // NON_COMPLIANT
+  }
+}
+
 int main(void) {
   pfv old_handler = signal(SIGINT, handler1);
   if (old_handler == SIG_ERR) {
@@ -39,13 +48,15 @@ int main(void) {
 
   old_handler = signal(SIGINT, handler2);
   if (old_handler == SIG_ERR) {
-    perror(""); // NON_COMPLIANT
+    perror(""); // COMPLIANT
   }
 
   old_handler = signal(SIGINT, handler3);
 
   old_handler = signal(SIGINT, handler4);
 
+  old_handler = signal(SIGINT, handler5);
+
   FILE *fp = fopen("something", "r");
   if (fp == NULL) {
     perror("Error: "); // NON_COMPLIANT