Merge pull request #9811 from smowton/smowton/feature/kotlin-jvmoverloads-annotation

Kotlin: Implement JvmOverloads annotation

Author: smowton

java/kotlin-extractor/src/main/kotlin/ExternalDeclExtractor.kt

@@ -84,7 +84,7 @@ class ExternalDeclExtractor(val logger: FileLogger, val invocationTrapFile: Stri
                                     // file information if needed:
                                     val ftw = tw.makeFileTrapWriter(binaryPath, irDecl is IrClass)
 
-                                    val fileExtractor = KotlinFileExtractor(logger, ftw, binaryPath, manager, this, primitiveTypeMapping, pluginContext, globalExtensionState)
+                                    val fileExtractor = KotlinFileExtractor(logger, ftw, binaryPath, manager, this, primitiveTypeMapping, pluginContext, KotlinFileExtractor.DeclarationStack(), globalExtensionState)
 
                                     if (irDecl is IrClass) {
                                         // Populate a location and compilation-unit package for the file. This is similar to

java/kotlin-extractor/src/main/kotlin/KotlinExtractorExtension.kt

@@ -322,7 +322,7 @@ private fun doFile(
                 // file information
                 val sftw = tw.makeSourceFileTrapWriter(srcFile, true)
                 val externalDeclExtractor = ExternalDeclExtractor(logger, invocationTrapFile, srcFilePath, primitiveTypeMapping, pluginContext, globalExtensionState, fileTrapWriter)
-                val fileExtractor = KotlinFileExtractor(logger, sftw, srcFilePath, null, externalDeclExtractor, primitiveTypeMapping, pluginContext, globalExtensionState)
+                val fileExtractor = KotlinFileExtractor(logger, sftw, srcFilePath, null, externalDeclExtractor, primitiveTypeMapping, pluginContext, KotlinFileExtractor.DeclarationStack(), globalExtensionState)
 
                 fileExtractor.extractFileContents(srcFile, sftw.fileId)
                 externalDeclExtractor.extractExternalClasses()

java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt

@@ -38,7 +38,6 @@ open class KotlinUsesExtractor(
     val pluginContext: IrPluginContext,
     val globalExtensionState: KotlinExtractorGlobalState
 ) {
-
     val javaLangObject by lazy {
         val result = pluginContext.referenceClass(FqName("java.lang.Object"))?.owner
         result?.let { extractExternalClassLater(it) }
@@ -128,18 +127,24 @@ open class KotlinUsesExtractor(
             return this
         }
 
+        val newDeclarationStack =
+            if (this is KotlinFileExtractor)
+                this.declarationStack
+            else
+                KotlinFileExtractor.DeclarationStack()
+
         if (clsFile == null || isExternalDeclaration(cls)) {
             val filePath = getIrClassBinaryPath(cls)
             val newTrapWriter = tw.makeFileTrapWriter(filePath, true)
             val newLoggerTrapWriter = logger.tw.makeFileTrapWriter(filePath, false)
             val newLogger = FileLogger(logger.loggerBase, newLoggerTrapWriter)
-            return KotlinFileExtractor(newLogger, newTrapWriter, filePath, dependencyCollector, externalClassExtractor, primitiveTypeMapping, pluginContext, globalExtensionState)
+            return KotlinFileExtractor(newLogger, newTrapWriter, filePath, dependencyCollector, externalClassExtractor, primitiveTypeMapping, pluginContext, newDeclarationStack, globalExtensionState)
         }
 
         val newTrapWriter = tw.makeSourceFileTrapWriter(clsFile, true)
         val newLoggerTrapWriter = logger.tw.makeSourceFileTrapWriter(clsFile, false)
         val newLogger = FileLogger(logger.loggerBase, newLoggerTrapWriter)
-        return KotlinFileExtractor(newLogger, newTrapWriter, clsFile.path, dependencyCollector, externalClassExtractor, primitiveTypeMapping, pluginContext, globalExtensionState)
+        return KotlinFileExtractor(newLogger, newTrapWriter, clsFile.path, dependencyCollector, externalClassExtractor, primitiveTypeMapping, pluginContext, newDeclarationStack, globalExtensionState)
     }
 
     // The Kotlin compiler internal representation of Outer<T>.Inner<S>.InnerInner<R> is InnerInner<R, S, T>. This function returns just `R`.
@@ -1027,14 +1032,18 @@ open class KotlinUsesExtractor(
      * looked up the parent ID ourselves, we would get as ID for
      * `java.lang.Throwable`, which isn't what we want. So we have to
      * allow it to be passed in.
-    */
+     *
+     * `maybeParameterList` can be supplied to override the function's
+     * value parameters; this is used for generating labels of overloads
+     * that omit one or more parameters that has a default value specified.
+     */
     @OptIn(ObsoleteDescriptorBasedAPI::class)
-    fun getFunctionLabel(f: IrFunction, maybeParentId: Label<out DbElement>?, classTypeArgsIncludingOuterClasses: List<IrTypeArgument>?) =
+    fun getFunctionLabel(f: IrFunction, maybeParentId: Label<out DbElement>?, classTypeArgsIncludingOuterClasses: List<IrTypeArgument>?, maybeParameterList: List<IrValueParameter>? = null) =
         getFunctionLabel(
             f.parent,
             maybeParentId,
             getFunctionShortName(f).nameInDB,
-            f.valueParameters,
+            maybeParameterList ?: f.valueParameters,
             getAdjustedReturnType(f),
             f.extensionReceiverParameter,
             getFunctionTypeParameters(f),
@@ -1437,6 +1446,12 @@ open class KotlinUsesExtractor(
 
     fun getTypeParameterParentLabel(param: IrTypeParameter) =
         param.parent.let {
+            (it as? IrFunction)?.let { fn ->
+                if (this is KotlinFileExtractor)
+                    this.declarationStack.findOverriddenAttributes(fn)?.id
+                else
+                    null
+            } ?:
             when (it) {
                 is IrClass -> useClassSource(it)
                 is IrFunction -> useFunction(it, noReplace = true)
@@ -1574,13 +1589,23 @@ open class KotlinUsesExtractor(
      */
     fun getValueParameterLabel(vp: IrValueParameter, parent: Label<out DbCallable>?): String {
         val declarationParent = vp.parent
-        val parentId = parent ?: useDeclarationParent(declarationParent, false)
+        val overriddenParentAttributes = (declarationParent as? IrFunction)?.let {
+            // Note the check 'vp.fileOrNull?.path == this.filePath' should never actually do anything, since references
+            // to a value parameter should always come from within the same .kt file.
+            if (this is KotlinFileExtractor && vp.fileOrNull?.path == this.filePath)
+                this.declarationStack.findOverriddenAttributes(it)
+            else
+                null
+        }
+        val parentId = parent ?: overriddenParentAttributes?.id ?: useDeclarationParent(declarationParent, false)
 
-        val idx = if (declarationParent is IrFunction && declarationParent.extensionReceiverParameter != null)
+        val idxBase = overriddenParentAttributes?.valueParameters?.indexOf(vp) ?: vp.index
+        val idxOffset = if (declarationParent is IrFunction && declarationParent.extensionReceiverParameter != null)
             // For extension functions increase the index to match what the java extractor sees:
-            vp.index + 1
+            1
         else
-            vp.index
+            0
+        val idx = idxBase + idxOffset
 
         if (idx < 0) {
             // We're not extracting this and this@TYPE parameters of functions:

java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt

@@ -0,0 +1,39 @@
+public class User {
+
+  public static String source() { return "taint"; }
+
+  public static void test(Test2 t2, GenericTest<Integer> gt) {
+
+    Test.taintSuppliedAsDefault(1, "no taint", 2);
+    Test.taintSuppliedAsDefault(1, 2);
+    Test.noTaintByDefault(1, source(), 2, 3);
+    Test.noTaintByDefault(1, source(), 2);
+
+    Test2.taintSuppliedAsDefaultStatic(1, "no taint", 2);
+    Test2.taintSuppliedAsDefaultStatic(1, 2);
+    Test2.noTaintByDefaultStatic(1, source(), 2, 3);
+    Test2.noTaintByDefaultStatic(1, source(), 2);
+
+    t2.taintSuppliedAsDefault(1, "no taint", 2);
+    t2.taintSuppliedAsDefault(1, 2);
+    t2.noTaintByDefault(1, source(), 2, 3);
+    t2.noTaintByDefault(1, source(), 2);
+
+    gt.taintSuppliedAsDefault(1, "no taint", 2);
+    gt.taintSuppliedAsDefault(1, 2);
+    gt.noTaintByDefault(1, source(), 2, 3);
+    gt.noTaintByDefault(1, source(), 2);
+
+    new ConstructorTaintsByDefault(1, "no taint", 2);
+    new ConstructorTaintsByDefault(1, 2);
+    new ConstructorDoesNotTaintByDefault(1, source(), 2, 3);
+    new ConstructorDoesNotTaintByDefault(1, source(), 2);
+
+    new GenericConstructorTaintsByDefault<Integer>(1, "no taint", 2);
+    new GenericConstructorTaintsByDefault<Integer>(1, 2);
+    new GenericConstructorDoesNotTaintByDefault<Integer>(1, source(), 2, 3);
+    new GenericConstructorDoesNotTaintByDefault<Integer>(1, source(), 2);
+
+  }
+
+}

java/ql/integration-tests/posix-only/kotlin/jvmoverloads_flow/User.java

@@ -0,0 +1,19 @@
+| User.java:9:30:9:37 | source(...) | test.kt:13:97:13:97 | s |
+| User.java:10:30:10:37 | source(...) | test.kt:13:97:13:97 | s |
+| User.java:14:37:14:44 | source(...) | test.kt:25:105:25:105 | s |
+| User.java:15:37:15:44 | source(...) | test.kt:25:105:25:105 | s |
+| User.java:19:28:19:35 | source(...) | test.kt:33:97:33:97 | s |
+| User.java:20:28:20:35 | source(...) | test.kt:33:97:33:97 | s |
+| User.java:24:28:24:35 | source(...) | test.kt:43:93:43:93 | s |
+| User.java:25:28:25:35 | source(...) | test.kt:43:93:43:93 | s |
+| User.java:29:45:29:52 | source(...) | test.kt:58:10:58:10 | s |
+| User.java:30:45:30:52 | source(...) | test.kt:58:10:58:10 | s |
+| User.java:34:61:34:68 | source(...) | test.kt:74:10:74:10 | s |
+| User.java:35:61:35:68 | source(...) | test.kt:74:10:74:10 | s |
+| test.kt:10:55:10:62 | source(...) | test.kt:10:84:10:84 | s |
+| test.kt:22:63:22:70 | source(...) | test.kt:22:92:22:92 | s |
+| test.kt:22:63:22:70 | source(...) | test.kt:22:92:22:92 | s |
+| test.kt:30:55:30:62 | source(...) | test.kt:30:84:30:84 | s |
+| test.kt:40:53:40:60 | source(...) | test.kt:40:80:40:80 | s |
+| test.kt:47:92:47:99 | source(...) | test.kt:50:10:50:10 | s |
+| test.kt:63:100:63:107 | source(...) | test.kt:66:10:66:10 | s |

java/ql/integration-tests/posix-only/kotlin/jvmoverloads_flow/test.expected

@@ -0,0 +1,78 @@
+fun getString() = "Hello world"
+
+fun source() = "tainted"
+
+fun sink(s: String) { }
+
+object Test {
+
+  @JvmOverloads @JvmStatic
+  fun taintSuppliedAsDefault(before: Int, s: String = source(), after: Int) { sink(s) }
+  
+  @JvmOverloads @JvmStatic
+  fun noTaintByDefault(before: Int, s: String = "no taint", after: Int, after2: Int = 1) { sink(s) }
+
+}
+
+public class Test2 {
+
+  companion object {
+
+    @JvmOverloads @JvmStatic
+    fun taintSuppliedAsDefaultStatic(before: Int, s: String = source(), after: Int) { sink(s) }
+
+    @JvmOverloads @JvmStatic
+    fun noTaintByDefaultStatic(before: Int, s: String = "no taint", after: Int, after2: Int = 1) { sink(s) }
+
+  }
+
+  @JvmOverloads
+  fun taintSuppliedAsDefault(before: Int, s: String = source(), after: Int) { sink(s) }
+
+  @JvmOverloads
+  fun noTaintByDefault(before: Int, s: String = "no taint", after: Int, after2: Int = 1) { sink(s) }
+
+}
+
+public class GenericTest<T> {
+
+  @JvmOverloads
+  fun taintSuppliedAsDefault(before: T, s: String = source(), after: T) { sink(s) }
+
+  @JvmOverloads
+  fun noTaintByDefault(before: T, s: String = "no taint", after: T, after2: Int = 1) { sink(s) }
+
+}
+
+public class ConstructorTaintsByDefault @JvmOverloads constructor(before: Int, s: String = source(), after: Int) {
+
+  init {
+    sink(s)
+  }
+
+}
+
+public class ConstructorDoesNotTaintByDefault @JvmOverloads constructor(before: Int, s: String = "no taint", after: Int, after2: Int = 1) {
+
+  init {
+    sink(s)
+  }
+   
+}
+
+public class GenericConstructorTaintsByDefault<T> @JvmOverloads constructor(before: T, s: String = source(), after: T) {  
+
+  init {
+    sink(s)
+  }
+   
+}
+ 
+public class GenericConstructorDoesNotTaintByDefault<T> @JvmOverloads constructor(before: T, s: String = "no taint", after: T, after2: T? = null) { 
+
+  init {
+    sink(s)
+  }
+
+}
+

java/ql/integration-tests/posix-only/kotlin/jvmoverloads_flow/test.kt

@@ -0,0 +1,4 @@
+from create_database_utils import *
+
+os.mkdir('kbuild')
+run_codeql_database_create(["kotlinc test.kt -d kbuild", "javac User.java -cp kbuild"], lang="java")

java/ql/integration-tests/posix-only/kotlin/jvmoverloads_flow/test.py

@@ -0,0 +1,18 @@
+import java
+import semmle.code.java.dataflow.DataFlow
+
+class Config extends DataFlow::Configuration {
+  Config() { this = "config" }
+
+  override predicate isSource(DataFlow::Node n) {
+    n.asExpr().(MethodAccess).getCallee().getName() = "source"
+  }
+
+  override predicate isSink(DataFlow::Node n) {
+    n.asExpr().(Argument).getCall().getCallee().getName() = "sink"
+  }
+}
+
+from Config c, DataFlow::Node source, DataFlow::Node sink
+where c.hasFlow(source, sink)
+select source, sink

java/ql/integration-tests/posix-only/kotlin/jvmoverloads_flow/test.ql

@@ -63,6 +63,8 @@ class Element extends @element, Top {
       i = 7 and result = "Setter for a Kotlin delegated property"
       or
       i = 8 and result = "Proxy static method for a @JvmStatic-annotated function or property"
+      or
+      i = 9 and result = "Forwarder for a @JvmOverloads-annotated function"
     )
   }
 }