Skip to content
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

Support for newer versions of Java #171

Open
rohanpadhye opened this issue Jul 8, 2022 · 17 comments
Open

Support for newer versions of Java #171

rohanpadhye opened this issue Jul 8, 2022 · 17 comments

Comments

@rohanpadhye
Copy link

My group has found some nice use cases for Phosphor, and we're hoping to use it in an upcoming project that requires at least Java 11.

The master branch README says that support for Java 9+ is on the roadmap, and I can see a java14 branch with WIP commits. What is that status of this feature? What are the main blockers for using Phosphor with newer versions of Java? Are there workarounds that will let us use Phosphor (with possibly reduced functionality) in say Java 14?

Thanks!

@jon-bell
Copy link
Collaborator

jon-bell commented Jul 8, 2022

There are some architectural issues with the way that Phosphor pushes taint tags around that became very problematic with some of the code generation in newer versions of Java. There are no easy workarounds.

With that said... 🤣 great timing on reporting this issue: I, minutes ago, completed a refactoring that should resolve those conflicts (7879f79). My next step will be to merge the java14 branch into this one, and am hopeful that once I resolve the syntactic conflicts, this refactoring should solve all of the remaining issues and Phosphor will soon support all versions of Java >= 7. I'll update here as I find out more - it is hard to predict how long these things will take, because there's a chance that this will solve everything, and also a chance that it will just allow me to find other problems that I didn't imagine :)

@rohanpadhye
Copy link
Author

Awesome! That's great to know :-) We'll await the merge then.

@jon-bell
Copy link
Collaborator

This is moving forward, and I now have a version that is passing our complete integration test suite on Java 16 (using Adoptium/OpenJDK). I'd be curious to know if this version (in phosphor-0.1.0-dev) works for your application, and if not, what kinds of errors you see. I just added some hurried documentation, please note that compared to the prior version of Phosphor, there are now 3 jars: one to use as a javaagent (phosphor-jigsaw-javaagent), one to use to generate instrumented JVMs (phosphor-instrument-jigsaw), and one to use for instrumenting everything else (the regular Phosphor jar). No need to use -Xbootclasspath flags when running the JVM.

@rohanpadhye
Copy link
Author

Woo-hoo! This is very exciting :-) Thanks for the short turnaround (or perhaps we were lucky in our timing).

Pinging @keltono @vasumv @aoli-al as interested parties.

@jon-bell We'll have some feedback on this soon.

@aoli-al
Copy link
Contributor

aoli-al commented Jul 26, 2022

Thank you for maintaining such a great tool @jon-bell ! I've tried to use phosphor with Java 11 and I got following exceptions:

Error occurred during initialization of VM
java.lang.NoSuchMethodError: 'void jdk.internal.misc.Unsafe.putReferenceVolatile(java.lang.Object, long, java.lang.Object)'
        at edu.columbia.cs.psl.phosphor.runtime.RuntimeJDKInternalUnsafePropagator.compareAndSetInt(java.base/RuntimeJDKInternalUnsafePropagator.java:764)
        at java.util.concurrent.ConcurrentHashMap.initTable(java.base/ConcurrentHashMap.java:2288)
        at java.util.concurrent.ConcurrentHashMap.putVal(java.base/ConcurrentHashMap.java:1017)
        at java.util.concurrent.ConcurrentHashMap.put(java.base/ConcurrentHashMap.java:1006)
        at java.util.Properties.put(java.base/Properties.java:1340)
        at java.lang.System.initProperties(java.base/Native Method)
        at java.lang.System.initPhase1(java.base/System.java:1948)

It seems that putReferenceVolatile is not implemented in Java 11. https://github.com/openjdk/jdk/blob/jdk-11%2B0/src/java.base/share/classes/jdk/internal/misc/Unsafe.java.

@jon-bell
Copy link
Collaborator

@aoli-al Is exactly Java 11 a requirement? Would you please try with 16?

@aoli-al
Copy link
Contributor

aoli-al commented Jul 26, 2022

Unfortunately, I'm trying to instrument some upstream software, and I can only compile it using Java 11. I'll let you know if the dependency can be fixed easily.

@jon-bell
Copy link
Collaborator

OK. Feel free to muck around with the unsafe wrappers. I am working on some more significant changes for performance that might also solve this problem eventually.

@jon-bell
Copy link
Collaborator

Also to be clear: you can still compile your upstream software using Java 11 - but then run it using a 16 VM.

@aoli-al
Copy link
Contributor

aoli-al commented Jul 26, 2022

Right, that would solve my problem. Thank you!

BTW, mvn 3.6.3 (the latest version on Ubuntu 20.04) does not support Java 16. Maybe we can add a maven wrapper to make it easier to compile?

@rohanpadhye
Copy link
Author

rohanpadhye commented Jul 26, 2022

Btw @jon-bell another student working with me (@keltono) did get Phosphor to work with Java 16. Great stuff! Thanks again :-)

We are ironing out some other kinks (e.g. running a maven plugin with a Phosphor-instrumented IDE seems to run into some other issues, but we can workaround it).

@jon-bell
Copy link
Collaborator

@aoli-al You should be able to compile Phosphor with Java 11 - it just needs to be run using version 16 right now.

@aoli-al
Copy link
Contributor

aoli-al commented Jul 28, 2022

Hi @jon-bell , I've tested phosphor against some complex applications and I saw some crashes. Here is the minimal reproducible program I have. Shall I move this discussion to a new issue to make it more trackable?

public class TaintTest {
    public void call(Object[] a, Object[] b) {
        if (a != null) {
            System.out.println(a);
        }
    }

    /*
    Exception in thread "main" java.lang.NoSuchMethodError: 'long edu.columbia.cs.psl.phosphor.runtime.RuntimeBoxUnboxPropagator.parseUnsignedLong(java.lang.CharSequence, int, int, int, edu.columbia.cs.psl.phosphor.runtime.PhosphorStackFrame)'
        at al.aoli.exchain.demo.TaintTest.crash(TaintTest.java:15)
        at al.aoli.exchain.demo.TaintTest.test(TaintTest.java:29)
        at al.aoli.exchain.demo.Main.main(Main.java:35)
     */
    public void crash() {
        Long l = Long.parseUnsignedLong("123456", 0, 5, 10);
    }

    /*
    output: edu.columbia.cs.psl.phosphor.struct.TaggedReferenceArray@467aecef
     */
    public void wrongOutput() {
        String[] obj = new String[] {"123", "456"};
        call( obj, obj);
    }
}

@jon-bell
Copy link
Collaborator

Feel free to open a new issue for that; even better would be a PR :) The missing method should be effectively the same as public static long parseLong(CharSequence s, int beginIndex, int endIndex, int radix, PhosphorStackFrame phosphorStackFrame) {, but with the first argument declared as a CharSequence.

@keltono
Copy link

keltono commented Jul 28, 2022

Hi, I haven't had any issues with the data tracking (which has been great!), but I get the following error when I try to instrument any class with control tracking enabled. I have a minimal example here:

Class:

package com.example;
public class Test {
    public static void main(String[] args) {
        return;
    }
}

Error:

Data flow tracking: enabled
Control flow tracking: enabled
Branch not taken: enabled
Exception occurred while instrumenting com/example/Test:
java.lang.IndexOutOfBoundsException
        at edu.columbia.cs.psl.phosphor.struct.harmony.util.ArrayList.add(ArrayList.java:109)
        at edu.columbia.cs.psl.phosphor.instrumenter.LocalVariableManager.createPermanentLocalVariable(LocalVariableManager.java:189)
        at edu.columbia.cs.psl.phosphor.instrumenter.LocalVariableManager.createMasterControlStackLV(LocalVariableManager.java:175)
        at edu.columbia.cs.psl.phosphor.control.ControlStackInitializingMV.visitCode(ControlStackInitializingMV.java:40)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.MethodVisitor.visitCode(MethodVisitor.java:232)
        at edu.columbia.cs.psl.phosphor.instrumenter.ReflectionHidingMV.visitCode(ReflectionHidingMV.java:439)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.MethodVisitor.visitCode(MethodVisitor.java:232)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.MethodVisitor.visitCode(MethodVisitor.java:232)
        at edu.columbia.cs.psl.phosphor.instrumenter.TaintPassingMV.visitCode(TaintPassingMV.java:80)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.MethodVisitor.visitCode(MethodVisitor.java:232)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.MethodVisitor.visitCode(MethodVisitor.java:232)
        at edu.columbia.cs.psl.phosphor.instrumenter.LocalVariableManager.visitCode(LocalVariableManager.java:360)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.MethodVisitor.visitCode(MethodVisitor.java:232)
        at edu.columbia.cs.psl.phosphor.instrumenter.LocalVariableAdder.visitCode(LocalVariableAdder.java:83)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.tree.MethodNode.accept(MethodNode.java:742)
        at edu.columbia.cs.psl.phosphor.instrumenter.TaintLoadCoercer$UninstTaintLoadCoercerMN.visitEnd(TaintLoadCoercer.java:241)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.MethodVisitor.visitEnd(MethodVisitor.java:783)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.tree.MethodNode.accept(MethodNode.java:772)
        at edu.columbia.cs.psl.phosphor.instrumenter.PrimitiveArrayAnalyzer$PrimitiveArrayAnalyzerMN.visitEnd(PrimitiveArrayAnalyzer.java:289)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.MethodVisitor.visitEnd(MethodVisitor.java:783)
        at edu.columbia.cs.psl.phosphor.instrumenter.PrimitiveArrayAnalyzer.visitEnd(PrimitiveArrayAnalyzer.java:110)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.MethodVisitor.visitEnd(MethodVisitor.java:783)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.tree.MethodNode.accept(MethodNode.java:772)
        at edu.columbia.cs.psl.phosphor.instrumenter.TaintTrackingClassVisitor$1.visitEnd(TaintTrackingClassVisitor.java:350)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.ClassReader.readMethod(ClassReader.java:1495)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.ClassReader.accept(ClassReader.java:721)
        at edu.columbia.cs.psl.phosphor.org.objectweb.asm.ClassReader.accept(ClassReader.java:401)
        at edu.columbia.cs.psl.phosphor.PreMain$PCLoggingTransformer.instrumentWithRetry(PreMain.java:265)
        at edu.columbia.cs.psl.phosphor.PreMain$PCLoggingTransformer.transform(PreMain.java:171)
        at edu.columbia.cs.psl.phosphor.Instrumenter.instrumentClass(Instrumenter.java:149)
        at edu.columbia.cs.psl.phosphor.Instrumenter$3.call(Instrumenter.java:437)
        at edu.columbia.cs.psl.phosphor.Instrumenter$3.call(Instrumenter.java:432)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
        at java.base/java.lang.Thread.run(Thread.java:831)
Saving com/example/Test to debug-preinst/

(This repeats several times)

Should I make a new issue for this?

@jon-bell
Copy link
Collaborator

@keltono This is a known issue as stated on the readme for the branch. It is entirely unimplemented. My development cycles for this refactoring are limited, and my primary goal right now is to resolve performance regressions.

@keltono
Copy link

keltono commented Jul 28, 2022

ah, understood. Not sure how I missed that. Thanks!

@jon-bell jon-bell mentioned this issue Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants