Merge pull request #21929 from owen-mc/go/no-ret-functions

Go: Recognize more non-returning logging functions

Author: owen-mc

go/ql/lib/change-notes/2026-06-01-non-returning-functions.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* More logging functions are now recognized as not returning or panicking.

go/ql/lib/semmle/go/Concepts.qll

@@ -413,17 +413,13 @@ private class ExternalLoggerCall extends LoggerCall::Range, DataFlow::CallNode {
   }
 }
 
-/**
- * A call to an interface that looks like a logger. It is common to use a
- * locally-defined interface for logging to make it easy to changing logging
- * library.
- */
-private class HeuristicLoggerCall extends LoggerCall::Range, DataFlow::CallNode {
-  HeuristicLoggerCall() {
-    exists(Method m, string tp, string logFunctionPrefix, string name |
-      m = this.getTarget() and
-      m.hasQualifiedName(_, tp, name) and
-      m.getReceiverBaseType().getUnderlyingType() instanceof InterfaceType
+private class HeuristicLoggerFunction extends Method {
+  string logFunctionPrefix;
+
+  HeuristicLoggerFunction() {
+    exists(string tp, string name |
+      this.hasQualifiedName(_, tp, name) and
+      this.getReceiverBaseType().getUnderlyingType() instanceof InterfaceType
     |
       tp.regexpMatch(".*[lL]ogger") and
       logFunctionPrefix =
@@ -435,6 +431,19 @@ private class HeuristicLoggerCall extends LoggerCall::Range, DataFlow::CallNode
     )
   }
 
+  override predicate mayReturnNormally() { logFunctionPrefix != "Fatal" }
+
+  override predicate mustPanic() { logFunctionPrefix = "Panic" }
+}
+
+/**
+ * A call to an interface that looks like a logger. It is common to use a
+ * locally-defined interface for logging to make it easy to change logging
+ * library.
+ */
+private class HeuristicLoggerCall extends LoggerCall::Range, DataFlow::CallNode {
+  HeuristicLoggerCall() { this.getTarget() instanceof HeuristicLoggerFunction }
+
   override DataFlow::Node getAMessageComponent() { result = this.getASyntacticArgument() }
 }
 

go/ql/lib/semmle/go/frameworks/Glog.qll

@@ -12,17 +12,37 @@ import go
  * forks.
  */
 module Glog {
+  /** Gets a package name for `glog` or `klog` (which is a fork). */
+  string packagePath() {
+    result =
+      package([
+          "github.com/golang/glog", "gopkg.in/glog", "k8s.io/klog", "github.com/barakmich/glog"
+        ], "")
+  }
+
   private class GlogFunction extends Function {
     int firstPrintedArg;
+    string format;
+    string level;
 
     GlogFunction() {
-      exists(string pkg, string fn, string level |
-        pkg = package(["github.com/golang/glog", "gopkg.in/glog", "k8s.io/klog"], "") and
+      exists(string pkg, string context, int nContextArgs, string depth, int nDepthArgs, string fn |
+        pkg = packagePath() and
         level = ["Error", "Exit", "Fatal", "Info", "Warning"] and
         (
-          fn = level + ["", "f", "ln"] and firstPrintedArg = 0
+          context = "" and nContextArgs = 0
+          or
+          context = "Context" and nContextArgs = 1
+        ) and
+        (
+          depth = "" and nDepthArgs = 0
           or
-          fn = level + "Depth" and firstPrintedArg = 1
+          depth = "Depth" and nDepthArgs = 1
+        ) and
+        format = ["", "f", "ln"] and
+        (
+          fn = level + context + depth + format and
+          firstPrintedArg = nContextArgs + nDepthArgs
         )
       |
         this.hasQualifiedName(pkg, fn)
@@ -35,10 +55,15 @@ module Glog {
      * Gets the index of the first argument that may be output, including a format string if one is present.
      */
     int getFirstPrintedArg() { result = firstPrintedArg }
+
+    /** Holds if this function takes a format string. */
+    predicate formatter() { format = "f" }
+
+    override predicate mayReturnNormally() { level != "Fatal" and level != "Exit" }
   }
 
   private class StringFormatter extends StringOps::Formatting::Range instanceof GlogFunction {
-    StringFormatter() { this.getName().matches("%f") }
+    StringFormatter() { this.formatter() }
 
     override int getFormatStringIndex() { result = super.getFirstPrintedArg() }
   }

go/ql/lib/semmle/go/frameworks/Logrus.qll

@@ -28,6 +28,12 @@ module Logrus {
         this.(Method).hasQualifiedName(packagePath(), ["Entry", "Logger"], name)
       )
     }
+
+    override predicate mayReturnNormally() {
+      not exists(string level, string suffix | level = ["Fatal", "Panic"] |
+        this.getName() = level + suffix
+      )
+    }
   }
 
   private class StringFormatters extends StringOps::Formatting::Range instanceof LogFunction {

go/ql/lib/semmle/go/frameworks/Zap.qll

@@ -47,7 +47,7 @@ module Zap {
   }
 
   /** A Zap logging function which always panics. */
-  private class FatalLogMethod extends Method {
+  private class FatalLogMethod extends ZapFunction {
     FatalLogMethod() {
       this.hasQualifiedName(packagePath(), "Logger", "Fatal")
       or
@@ -58,7 +58,7 @@ module Zap {
   }
 
   /** A Zap logging function which always panics. */
-  private class MustPanicLogMethod extends Method {
+  private class MustPanicLogMethod extends ZapFunction {
     MustPanicLogMethod() {
       this.hasQualifiedName(packagePath(), "Logger", "Panic")
       or

go/ql/lib/semmle/go/frameworks/stdlib/Log.qll

@@ -29,18 +29,37 @@ module Log {
   }
 
   private class LogFormatter extends StringOps::Formatting::Range instanceof LogFunction {
-    LogFormatter() { this.getName() = ["Fatalf", "Panicf", "Printf"] }
+    LogFormatter() { this.getName() = ["Fatalf", "Panicf", "Printf", "Panic", "Panicf", "Panicln"] }
 
     override int getFormatStringIndex() { result = 0 }
   }
 
   /** A fatal log function, which calls `os.Exit`. */
   private class FatalLogFunction extends Function {
-    FatalLogFunction() { this.hasQualifiedName("log", ["Fatal", "Fatalf", "Fatalln"]) }
+    FatalLogFunction() {
+      exists(string fn | fn = ["Fatal", "Fatalf", "Fatalln"] |
+        this.hasQualifiedName("log", fn)
+        or
+        this.(Method).hasQualifiedName("log", "Logger", fn)
+      )
+    }
 
     override predicate mayReturnNormally() { none() }
   }
 
+  /** A log function which must panic. */
+  private class PanicLogFunction extends Function {
+    PanicLogFunction() {
+      exists(string fn | fn = ["Panic", "Panicf", "Panicln"] |
+        this.hasQualifiedName("log", fn)
+        or
+        this.(Method).hasQualifiedName("log", "Logger", fn)
+      )
+    }
+
+    override predicate mustPanic() { any() }
+  }
+
   // These models are not implemented using Models-as-Data because they represent reverse flow.
   private class FunctionModels extends TaintTracking::FunctionModel {
     FunctionInput inp;
@@ -63,30 +82,6 @@ module Log {
     FunctionOutput outp;
 
     MethodModels() {
-      // signature: func (*Logger) Fatal(v ...interface{})
-      this.hasQualifiedName("log", "Logger", "Fatal") and
-      (inp.isParameter(_) and outp.isReceiver())
-      or
-      // signature: func (*Logger) Fatalf(format string, v ...interface{})
-      this.hasQualifiedName("log", "Logger", "Fatalf") and
-      (inp.isParameter(_) and outp.isReceiver())
-      or
-      // signature: func (*Logger) Fatalln(v ...interface{})
-      this.hasQualifiedName("log", "Logger", "Fatalln") and
-      (inp.isParameter(_) and outp.isReceiver())
-      or
-      // signature: func (*Logger) Panic(v ...interface{})
-      this.hasQualifiedName("log", "Logger", "Panic") and
-      (inp.isParameter(_) and outp.isReceiver())
-      or
-      // signature: func (*Logger) Panicf(format string, v ...interface{})
-      this.hasQualifiedName("log", "Logger", "Panicf") and
-      (inp.isParameter(_) and outp.isReceiver())
-      or
-      // signature: func (*Logger) Panicln(v ...interface{})
-      this.hasQualifiedName("log", "Logger", "Panicln") and
-      (inp.isParameter(_) and outp.isReceiver())
-      or
       // signature: func (*Logger) Print(v ...interface{})
       this.hasQualifiedName("log", "Logger", "Print") and
       (inp.isParameter(_) and outp.isReceiver())