Skip to content

Kotlin: stub trap .class files when extracting a class from Kotlin source #11510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e34d72a
Kotlin: stub trap .class files when extracting a class from Kotlin so…
smowton Nov 30, 2022
748637c
Tidy and use version 0 for classes extracted from source
smowton Dec 1, 2022
08e3431
Also stub class files relating to file classes and top-level declarat…
smowton Dec 1, 2022
540a2a6
Don't create stub trap files for anonymous or local classes, or unexp…
smowton Dec 2, 2022
910a1f8
Adjust opt-in required to use string-manipulation functions in Kotlin…
smowton Dec 2, 2022
d9dc8e3
Fix binary names for classes declared from source
smowton Dec 2, 2022
82f3c2f
Mark the Companion field as static
smowton Dec 6, 2022
5e023bf
Remove no-longer-applicable diagnostic matches
smowton Dec 6, 2022
f2fded6
Accept jvmstatic-annotation changes
smowton Dec 6, 2022
9f722a7
Disable java_and_kotlin inconsistency test; accept changes
smowton Dec 6, 2022
f5579d5
Accept test changes: classes no longer getting multiple locations
smowton Dec 6, 2022
59eb81b
Accept test changes: a raw class getting extracted solely for use in …
smowton Dec 6, 2022
d37a10e
Accept test changes: methods no longer appearing to be `final`
smowton Dec 6, 2022
c68ac46
Accept test changes: again this is a raw class extracted just for its…
smowton Dec 6, 2022
00f323c
Fix: extract directly exposed fields with `static` modifier
smowton Dec 6, 2022
d2e7797
Rename to writeStubTrapFile
smowton Dec 6, 2022
522a549
Improve debug logging when the external decl extractor handles an IrFile
smowton Dec 6, 2022
ecbb96f
Remove no-longer-needed diagnostic expectations
smowton Dec 7, 2022
c526020
Note TODO re: re-enabling suspend function Java interop testing
smowton Dec 7, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@
import java.util.zip.ZipFile;

import com.github.codeql.Logger;
import static com.github.codeql.ClassNamesKt.getIrDeclBinaryName;
import static com.github.codeql.ClassNamesKt.getIrElementBinaryName;
import static com.github.codeql.ClassNamesKt.getIrClassVirtualFile;

import org.jetbrains.kotlin.ir.IrElement;
import org.jetbrains.kotlin.ir.declarations.IrClass;

import com.intellij.openapi.vfs.VirtualFile;
Expand Down Expand Up @@ -212,20 +213,19 @@ private File trapFileFor(File file) {
PathTransformer.std().fileAsDatabaseString(file) + ".trap.gz");
}

private File getTrapFileForDecl(IrDeclaration sym, String signature) {
private File getTrapFileForDecl(IrElement sym, String signature) {
if (currentSpecFileEntry == null)
return null;
return trapFileForDecl(sym, signature);
}

private File trapFileForDecl(IrDeclaration sym, String signature) {
private File trapFileForDecl(IrElement sym, String signature) {
return FileUtil.fileRelativeTo(currentSpecFileEntry.getTrapFolder(),
trapFilePathForDecl(sym, signature));
}

private String trapFilePathForDecl(IrDeclaration sym, String signature) {
String binaryName = getIrDeclBinaryName(sym);
String binaryNameWithSignature = binaryName + signature;
private String trapFilePathForDecl(IrElement sym, String signature) {
String binaryName = getIrElementBinaryName(sym);
// TODO: Reinstate this?
//if (getTrackClassOrigins())
// classId += "-" + StringDigestor.digest(sym.getSourceFileId());
Expand All @@ -241,7 +241,7 @@ private String trapFilePathForDecl(IrDeclaration sym, String signature) {
* Deletion of existing trap files.
*/

private void deleteTrapFileAndDependencies(IrDeclaration sym, String signature) {
private void deleteTrapFileAndDependencies(IrElement sym, String signature) {
File trap = trapFileForDecl(sym, signature);
if (trap.exists()) {
trap.delete();
Expand Down Expand Up @@ -269,7 +269,7 @@ private void deleteTrapFileAndDependencies(IrDeclaration sym, String signature)
* Any unique suffix needed to distinguish `sym` from other declarations with the same name.
* For functions for example, this means its parameter signature.
*/
private TrapFileManager getMembersWriterForDecl(File trap, File trapFileBase, TrapClassVersion trapFileVersion, IrDeclaration sym, String signature) {
private TrapFileManager getMembersWriterForDecl(File trap, File trapFileBase, TrapClassVersion trapFileVersion, IrElement sym, String signature) {
if (use_trap_locking) {
TrapClassVersion currVersion = TrapClassVersion.fromSymbol(sym, log);
String shortName = sym instanceof IrDeclarationWithName ? ((IrDeclarationWithName)sym).getName().asString() : "(name unknown)";
Expand Down Expand Up @@ -326,7 +326,7 @@ private TrapFileManager getMembersWriterForDecl(File trap, File trapFileBase, Tr
return trapWriter(trap, sym, signature);
}

private TrapFileManager trapWriter(File trapFile, IrDeclaration sym, String signature) {
private TrapFileManager trapWriter(File trapFile, IrElement sym, String signature) {
if (!trapFile.getName().endsWith(".trap.gz"))
throw new CatastrophicError("OdasaOutput only supports writing to compressed trap files");
String relative = FileUtil.relativePath(trapFile, currentSpecFileEntry.getTrapFolder());
Expand All @@ -335,7 +335,7 @@ private TrapFileManager trapWriter(File trapFile, IrDeclaration sym, String sign
return concurrentWriter(trapFile, relative, log, sym, signature);
}

private TrapFileManager concurrentWriter(File trapFile, String relative, Logger log, IrDeclaration sym, String signature) {
private TrapFileManager concurrentWriter(File trapFile, String relative, Logger log, IrElement sym, String signature) {
if (trapFile.exists())
return null;
return new TrapFileManager(trapFile, relative, true, log, sym, signature);
Expand All @@ -345,11 +345,11 @@ public class TrapFileManager implements AutoCloseable {

private TrapDependencies trapDependenciesForClass;
private File trapFile;
private IrDeclaration sym;
private IrElement sym;
private String signature;
private boolean hasError = false;

private TrapFileManager(File trapFile, String relative, boolean concurrentCreation, Logger log, IrDeclaration sym, String signature) {
private TrapFileManager(File trapFile, String relative, boolean concurrentCreation, Logger log, IrElement sym, String signature) {
trapDependenciesForClass = new TrapDependencies(relative);
this.trapFile = trapFile;
this.sym = sym;
Expand All @@ -360,7 +360,7 @@ public File getFile() {
return trapFile;
}

public void addDependency(IrDeclaration dep, String signature) {
public void addDependency(IrElement dep, String signature) {
trapDependenciesForClass.addDependency(trapFilePathForDecl(dep, signature));
}

Expand Down Expand Up @@ -422,7 +422,7 @@ public void setHasError() {
* previously set by a call to {@link OdasaOutput#setCurrentSourceFile(File)}.
*/
public TrapLocker getTrapLockerForCurrentSourceFile() {
return new TrapLocker((IrClass)null, null);
return new TrapLocker((IrClass)null, null, true);
}

/**
Expand Down Expand Up @@ -460,19 +460,19 @@ public TrapLocker getTrapLockerForModule(String moduleName) {
*
* @return a {@link TrapLocker} for the trap file corresponding to the given class symbol.
*/
public TrapLocker getTrapLockerForDecl(IrDeclaration sym, String signature) {
return new TrapLocker(sym, signature);
public TrapLocker getTrapLockerForDecl(IrElement sym, String signature, boolean fromSource) {
return new TrapLocker(sym, signature, fromSource);
}

public class TrapLocker implements AutoCloseable {
private final IrDeclaration sym;
private final IrElement sym;
private final File trapFile;
// trapFileBase is used when doing lockless TRAP file writing.
// It is trapFile without the #metadata.trap.gz suffix.
private File trapFileBase = null;
private TrapClassVersion trapFileVersion = null;
private final String signature;
private TrapLocker(IrDeclaration decl, String signature) {
private TrapLocker(IrElement decl, String signature, boolean fromSource) {
this.sym = decl;
this.signature = signature;
if (sym==null) {
Expand All @@ -485,7 +485,10 @@ private TrapLocker(IrDeclaration decl, String signature) {
} else {
// We encode the metadata into the filename, so that the
// TRAP filenames for different metadatas don't overlap.
trapFileVersion = TrapClassVersion.fromSymbol(sym, log);
if (fromSource)
trapFileVersion = new TrapClassVersion(0, 0, 0, "kotlin");
else
trapFileVersion = TrapClassVersion.fromSymbol(sym, log);
String baseName = normalTrapFile.getName().replace(".trap.gz", "");
// If a class has lots of inner classes, then we get lots of files
// in a single directory. This makes our directory listings later slow.
Expand Down Expand Up @@ -717,11 +720,18 @@ private static long getVirtualFileTimeStamp(VirtualFile vf, Logger log) {
return vf.getTimeStamp();
}

private static TrapClassVersion fromSymbol(IrDeclaration sym, Logger log) {
VirtualFile vf = sym instanceof IrClass ? getIrClassVirtualFile((IrClass)sym) :
sym.getParent() instanceof IrClass ? getIrClassVirtualFile((IrClass)sym.getParent()) :
null;
if(vf == null)
private static VirtualFile getVirtualFileIfClass(IrElement e) {
if (e instanceof IrClass)
return getIrClassVirtualFile((IrClass)e);
else
return null;
}

private static TrapClassVersion fromSymbol(IrElement sym, Logger log) {
VirtualFile vf = getVirtualFileIfClass(sym);
if (vf == null && sym instanceof IrDeclaration)
vf = getVirtualFileIfClass(((IrDeclaration)sym).getParent());
if (vf == null)
return new TrapClassVersion(-1, 0, 0, null);

final int[] versionStore = new int[1];
Expand Down
142 changes: 78 additions & 64 deletions java/kotlin-extractor/src/main/kotlin/ExternalDeclExtractor.kt
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
package com.github.codeql

import com.github.codeql.utils.isExternalDeclaration
import com.github.codeql.utils.isExternalFileClassMember
import com.semmle.extractor.java.OdasaOutput
import com.semmle.util.data.StringDigestor
import org.jetbrains.kotlin.backend.common.extensions.IrPluginContext
import org.jetbrains.kotlin.ir.IrElement
import org.jetbrains.kotlin.ir.declarations.*
import org.jetbrains.kotlin.ir.util.fqNameWhenAvailable
import org.jetbrains.kotlin.ir.util.isFileClass
import org.jetbrains.kotlin.ir.util.packageFqName
import org.jetbrains.kotlin.ir.util.parentClassOrNull
import org.jetbrains.kotlin.name.FqName
import java.io.BufferedWriter
import java.io.File
import java.util.ArrayList
import java.util.HashSet
Expand All @@ -25,87 +23,103 @@ class ExternalDeclExtractor(val logger: FileLogger, val invocationTrapFile: Stri
val propertySignature = ";property"
val fieldSignature = ";field"

val output = OdasaOutput(false, logger).also {
it.setCurrentSourceFile(File(sourceFilePath))
}

fun extractLater(d: IrDeclarationWithName, signature: String): Boolean {
if (d !is IrClass && !isExternalFileClassMember(d)) {
logger.errorElement("External declaration is neither a class, nor a top-level declaration", d)
return false
}
val declBinaryName = declBinaryNames.getOrPut(d) { getIrDeclBinaryName(d) }
val declBinaryName = declBinaryNames.getOrPut(d) { getIrElementBinaryName(d) }
val ret = externalDeclsDone.add(Pair(declBinaryName, signature))
if (ret) externalDeclWorkList.add(Pair(d, signature))
return ret
}
fun extractLater(c: IrClass) = extractLater(c, "")

fun writeStubTrapFile(e: IrElement, signature: String = "") {
extractElement(e, signature, true) { trapFileBW, _, _ ->
trapFileBW.write("// Trap file stubbed because this declaration was extracted from source in $sourceFilePath\n")
trapFileBW.write("// Part of invocation $invocationTrapFile\n")
}
}

private fun extractElement(element: IrElement, possiblyLongSignature: String, fromSource: Boolean, extractorFn: (BufferedWriter, String, OdasaOutput.TrapFileManager) -> Unit) {
// In order to avoid excessively long signatures which can lead to trap file names longer than the filesystem
// limit, we truncate and add a hash to preserve uniqueness if necessary.
val signature = if (possiblyLongSignature.length > 100) {
possiblyLongSignature.substring(0, 92) + "#" + StringDigestor.digest(possiblyLongSignature).substring(0, 8)
} else { possiblyLongSignature }
output.getTrapLockerForDecl(element, signature, fromSource).useAC { locker ->
locker.trapFileManager.useAC { manager ->
val shortName = when(element) {
is IrDeclarationWithName -> element.name.asString()
is IrFile -> element.name
else -> "(unknown name)"
}
if (manager == null) {
logger.info("Skipping extracting external decl $shortName")
} else {
val trapFile = manager.file
val trapTmpFile = File.createTempFile("${trapFile.nameWithoutExtension}.", ".${trapFile.extension}.tmp", trapFile.parentFile)
try {
GZIPOutputStream(trapTmpFile.outputStream()).bufferedWriter().use {
extractorFn(it, signature, manager)
}

if (!trapTmpFile.renameTo(trapFile)) {
logger.error("Failed to rename $trapTmpFile to $trapFile")
}
} catch (e: Exception) {
manager.setHasError()
logger.error("Failed to extract '$shortName'. Partial TRAP file location is $trapTmpFile", e)
}
}
}
}
}

fun extractExternalClasses() {
val output = OdasaOutput(false, logger)
output.setCurrentSourceFile(File(sourceFilePath))
do {
val nextBatch = ArrayList(externalDeclWorkList)
externalDeclWorkList.clear()
nextBatch.forEach { workPair ->
val (irDecl, possiblyLongSignature) = workPair
// In order to avoid excessively long signatures which can lead to trap file names longer than the filesystem
// limit, we truncate and add a hash to preserve uniqueness if necessary.
val signature = if (possiblyLongSignature.length > 100) {
possiblyLongSignature.substring(0, 92) + "#" + StringDigestor.digest(possiblyLongSignature).substring(0, 8)
} else { possiblyLongSignature }
output.getTrapLockerForDecl(irDecl, signature).useAC { locker ->
locker.trapFileManager.useAC { manager ->
val shortName = when(irDecl) {
is IrDeclarationWithName -> irDecl.name.asString()
else -> "(unknown name)"
}
if(manager == null) {
logger.info("Skipping extracting external decl $shortName")
} else {
val trapFile = manager.file
val trapTmpFile = File.createTempFile("${trapFile.nameWithoutExtension}.", ".${trapFile.extension}.tmp", trapFile.parentFile)

val containingClass = getContainingClassOrSelf(irDecl)
if (containingClass == null) {
logger.errorElement("Unable to get containing class", irDecl)
return
}
val binaryPath = getIrClassBinaryPath(containingClass)
try {
GZIPOutputStream(trapTmpFile.outputStream()).bufferedWriter().use { trapFileBW ->
// We want our comments to be the first thing in the file,
// so start off with a mere TrapWriter
val tw = TrapWriter(logger.loggerBase, TrapLabelManager(), trapFileBW, diagnosticTrapWriter)
tw.writeComment("Generated by the CodeQL Kotlin extractor for external dependencies")
tw.writeComment("Part of invocation $invocationTrapFile")
if (signature != possiblyLongSignature) {
tw.writeComment("Function signature abbreviated; full signature is: $possiblyLongSignature")
}
// Now elevate to a SourceFileTrapWriter, and populate the
// file information if needed:
val ftw = tw.makeFileTrapWriter(binaryPath, true)
extractElement(irDecl, possiblyLongSignature, false) { trapFileBW, signature, manager ->
val containingClass = getContainingClassOrSelf(irDecl)
if (containingClass == null) {
logger.errorElement("Unable to get containing class", irDecl)
} else {
val binaryPath = getIrClassBinaryPath(containingClass)

val fileExtractor = KotlinFileExtractor(logger, ftw, null, binaryPath, manager, this, primitiveTypeMapping, pluginContext, KotlinFileExtractor.DeclarationStack(), globalExtensionState)
// We want our comments to be the first thing in the file,
// so start off with a mere TrapWriter
val tw = TrapWriter(logger.loggerBase, TrapLabelManager(), trapFileBW, diagnosticTrapWriter)
tw.writeComment("Generated by the CodeQL Kotlin extractor for external dependencies")
tw.writeComment("Part of invocation $invocationTrapFile")
if (signature != possiblyLongSignature) {
tw.writeComment("Function signature abbreviated; full signature is: $possiblyLongSignature")
}
// Now elevate to a SourceFileTrapWriter, and populate the
// file information if needed:
val ftw = tw.makeFileTrapWriter(binaryPath, true)

if (irDecl is IrClass) {
// Populate a location and compilation-unit package for the file. This is similar to
// the beginning of `KotlinFileExtractor.extractFileContents` but without an `IrFile`
// to start from.
val pkg = irDecl.packageFqName?.asString() ?: ""
val pkgId = fileExtractor.extractPackage(pkg)
ftw.writeHasLocation(ftw.fileId, ftw.getWholeFileLocation())
ftw.writeCupackage(ftw.fileId, pkgId)
val fileExtractor = KotlinFileExtractor(logger, ftw, null, binaryPath, manager, this, primitiveTypeMapping, pluginContext, KotlinFileExtractor.DeclarationStack(), globalExtensionState)

fileExtractor.extractClassSource(irDecl, extractDeclarations = !irDecl.isFileClass, extractStaticInitializer = false, extractPrivateMembers = false, extractFunctionBodies = false)
} else {
fileExtractor.extractDeclaration(irDecl, extractPrivateMembers = false, extractFunctionBodies = false)
}
}
if (irDecl is IrClass) {
// Populate a location and compilation-unit package for the file. This is similar to
// the beginning of `KotlinFileExtractor.extractFileContents` but without an `IrFile`
// to start from.
val pkg = irDecl.packageFqName?.asString() ?: ""
val pkgId = fileExtractor.extractPackage(pkg)
ftw.writeHasLocation(ftw.fileId, ftw.getWholeFileLocation())
ftw.writeCupackage(ftw.fileId, pkgId)

if (!trapTmpFile.renameTo(trapFile)) {
logger.error("Failed to rename $trapTmpFile to $trapFile")
}
} catch (e: Exception) {
manager.setHasError()
logger.error("Failed to extract '$shortName'. Partial TRAP file location is $trapTmpFile", e)
}
fileExtractor.extractClassSource(irDecl, extractDeclarations = !irDecl.isFileClass, extractStaticInitializer = false, extractPrivateMembers = false, extractFunctionBodies = false)
} else {
fileExtractor.extractDeclaration(irDecl, extractPrivateMembers = false, extractFunctionBodies = false)
}
}
}
Expand Down
Loading