Address review comments 6

Author: owen-mc

ql/src/Security/CWE-681/IncorrectIntegerConversion.ql

@@ -33,12 +33,10 @@ float getMaxIntValue(int bitSize, boolean isSigned) {
  * architecture-specific.
  */
 int getIntTypeBitSize(File file) {
-  if file.hasConstrainedIntBitSize(32)
-  then result = 32
-  else
-    if file.hasConstrainedIntBitSize(64)
-    then result = 64
-    else result = 0
+  file.constrainsIntBitSize(result)
+  or
+  not file.constrainsIntBitSize(_) and
+  result = 0
 }
 
 /**

ql/src/go.qll

@@ -3,6 +3,7 @@
  */
 
 import Customizations
+import semmle.go.Architectures
 import semmle.go.AST
 import semmle.go.Comments
 import semmle.go.Concepts

ql/src/semmle/go/Architectures.qll

@@ -0,0 +1,27 @@
+/** Provides classes for working with architectures. */
+
+import go
+
+/**
+ * An architecture that is valid in a build constraint.
+ *
+ * Information obtained from
+ * https://github.com/golang/go/blob/98cbf45cfc6a5a50cc6ac2367f9572cb198b57c7/src/go/types/gccgosizes.go
+ * where the first field of the struct is 4 for 32-bit architectures
+ * and 8 for 64-bit architectures.
+ */
+class Architecture extends string {
+  int bitSize;
+
+  Architecture() {
+    this in ["386", "amd64p32", "arm", "armbe", "mips", "mipsle", "mips64p32", "mips64p32le", "ppc",
+          "s390", "sparc"] and
+    bitSize = 32
+    or
+    this in ["amd64", "arm64", "arm64be", "ppc64", "ppc64le", "mips64", "mips64le", "s390x",
+          "sparc64"] and
+    bitSize = 64
+  }
+
+  int getBitSize() { result = bitSize }
+}

ql/src/semmle/go/Comments.qll

@@ -211,4 +211,10 @@ class BuildConstraintComment extends LineComment {
   }
 
   override string getAPrimaryQlClass() { result = "BuildConstraintComment" }
+
+  /** Gets the body of this build constraint. */
+  string getConstraintBody() { result = getText().splitAt("+build ", 1) }
+
+  /** Gets a disjunct of this build constraint. */
+  string getADisjunct() { result = getConstraintBody().splitAt(" ") }
 }

ql/src/semmle/go/Files.qll

@@ -203,55 +203,40 @@ class File extends Container, @file, Documentable, ExprParent, GoModExprParent,
   pragma[noinline]
   predicate hasBuildConstraints() { exists(BuildConstraintComment bc | this = bc.getFile()) }
 
-  /**
-   * Gets an architecture that is valid in a build constraint with bit
-   * size `bitSize`.
-   *
-   * Information obtained from
-   * https://github.com/golang/go/blob/98cbf45cfc6a5a50cc6ac2367f9572cb198b57c7/src/go/types/gccgosizes.go
-   * where the first field of the struct is 4 for 32-bit architectures
-   * and 8 for 64-bit architectures.
-   */
-  private string getAnArchitecture(int bitSize) {
-    bitSize = 32 and
-    result in ["386", "amd64p32", "arm", "armbe", "mips", "mipsle", "mips64p32", "mips64p32le",
-          "ppc", "s390", "sparc"]
-    or
-    bitSize = 64 and
-    result in ["amd64", "arm64", "arm64be", "ppc64", "ppc64le", "mips64", "mips64le", "s390x",
-          "sparc64"]
-  }
-
   /**
    * Holds if this file contains build constraints that ensure that it
-   * is only built on architectures of bit size `bitSize`.
+   * is only built on architectures of bit size `bitSize`, which can be
+   * 32 or 64.
    */
-  predicate hasConstrainedIntBitSize(int bitSize) {
-    hasExplicitBuildConstraintsForArchitectures(bitSize) or
-    hasImplicitBuildConstraintForAnArchitecture(bitSize)
+  predicate constrainsIntBitSize(int bitSize) {
+    explicitlyConstrainsIntBitSize(bitSize) or
+    implicitlyConstrainsIntBitSize(bitSize)
   }
 
   /**
    * Holds if this file contains explicit build constraints that ensure
-   * that it is only built on an architecture of bit size `bitSize`.
+   * that it is only built on an architecture of bit size `bitSize`,
+   * which can be 32 or 64.
    */
-  predicate hasExplicitBuildConstraintsForArchitectures(int bitSize) {
+  predicate explicitlyConstrainsIntBitSize(int bitSize) {
     exists(BuildConstraintComment bcc, string bc |
       this = bcc.getFile() and bc = bcc.getText().splitAt("+build ", 1)
     |
-      forex(string disjunct | disjunct = bc.splitAt(" ") |
-        disjunct.splitAt(",").matches(getAnArchitecture(bitSize))
+      forex(string disjunct | disjunct = bcc.getADisjunct() |
+        disjunct.splitAt(",").(Architecture).getBitSize() = bitSize
       )
     )
   }
 
   /**
    * Holds if this file has a name which acts as an implicit build
    * constraint that ensures that it is only built on an
-   * architecture of bit size `bitSize`.
+   * architecture of bit size `bitSize`, which can be 32 or 64.
    */
-  predicate hasImplicitBuildConstraintForAnArchitecture(int bitSize) {
-    this.getStem().regexpMatch(".*_" + getAnArchitecture(bitSize) + "(_test)?")
+  predicate implicitlyConstrainsIntBitSize(int bitSize) {
+    this
+        .getStem()
+        .regexpMatch(".*_" + any(Architecture arch | arch.getBitSize() = bitSize) + "(_test)?")
   }
 
   override string toString() { result = Container.super.toString() }