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

Missing check for STATIC method when executing #250

Open
ilovejpf opened this issue Jul 31, 2020 · 19 comments
Open

Missing check for STATIC method when executing #250

ilovejpf opened this issue Jul 31, 2020 · 19 comments
Labels
bug first issue Good issue for first commit

Comments

@ilovejpf
Copy link

Hi! I find that jpf is missing check static method, which can be severe! Let me show you an example.
First, you write a class C, which will later be compiled to a method call to use INVOKESTATIC.

cat > C.java <<EOF
class C {
   public static void main(String[] a) {
     int i = 123;
     D.m("foobar");
     System.out.println(i);
   }
}
EOF

Then you write the static method m in class D.

cat > D.java <<EOF
class D {
   public static String m(String s) { return s; }
}
EOF

Then you can run javac C.java D.java to compile, and both java C and java -jar build/RunJPF.jar +classpath=. C should work fine.

Then you remove that static in class D by running sed -i s/static// D.java; javac D.java, then java C will throw IncompatibleClassChangeError as expected.

java.lang.IncompatibleClassChangeError: Expected static method D.m(Ljava/lang/String;)Ljava/lang/String;
	at C.main(C.java:4)

However, java -jar build/RunJPF.jar +classpath=. C will print out 248 (please note that this number should be 123! ) and end up with no errors detected. Missing the check can lead to executing something unexpected and destroy certain variables in the program.
The fix may be adding a check for whether the method to be invoked is actually static or not when executing INVOKESTATIC.
Thanks!

@cyrille-artho
Copy link
Member

Hi,
Thank you very much for noticing this. This is an issue that we hope won't be too hard to fix, but it may be difficult to test.
We would need the following:

  1. C.java and D.java, which are both compiled as usual during gradle's testCompile phase.
  2. Before the tests run, we'd need a task that (a) creates a modified version of D.java in a temporary directory, (b) compiles it, and (c) moves D.class (the compiled file) to the location of the other compiled files.
  3. Now the tests can run.

This special task in (2) can be implemented using doFirst:
https://docs.gradle.org/current/userguide/build_lifecycle.html

The actions inside that task could be implemented as shell tasks, as in:
https://discuss.gradle.org/t/run-a-process-before-tests/25690/2

However, this would make this particular gradle task less portable and more failure-prone; it should at the very least not prevent the other tests from running if it cannot execute. Perhaps it's also possible to use a gradle-native task to compile the modified D.java.

What do you think?

@alokprince
Copy link

can I try to solve this issue?
I am a beginner and I am willing to learn and work on this issue as my first issue.

@cyrille-artho
Copy link
Member

You are welcome to work on this, as I'm not aware of anyone else doing so. As I've written earlier, testing this issue is non-trivial.

@alokprince
Copy link

I will try my best for this issue.

@Venkatesh-24
Copy link

Hi I am a beginner. Can I try to solve this issue??

@cyrille-artho
Copy link
Member

Yes, it seems alokprince is not working on this issue right now, so please give it a try.

@Venkatesh-24
Copy link

@cyrille-artho can you please point me to related documentation and entry point of the codebase.I am new to this repository.

@cyrille-artho
Copy link
Member

See the history of this issue and the JPF wiki to get started.
A good way to resolve this issue would be to have a test for it first, which (as described above) is not so simple because code has to be compiled twice.

@Venkatesh-24
Copy link

Okay I will try my best to solve it.

@shoyebmd424
Copy link

I can try solve this issue please assign me for this issue

@quadhier quadhier added the bug label Jul 28, 2023
@cyrille-artho cyrille-artho added the first issue Good issue for first commit label Jul 31, 2023
@CrypticMessenger
Copy link

Hi @cyrille-artho , can I work on this issue? I have identified the changes and tested locally, it resolves the problem, I am currently working on writing Unit test (the way you suggested in this thread).

@cyrille-artho
Copy link
Member

Yes, there is no recent comment from anyone working on this, so feel free to go ahead.

@sshaikshoaib
Copy link

Hi I am a beginner. can I work for this issue

@cyrille-artho
Copy link
Member

Yes, please give it a try. Manually reproducing the existing incorrect behavior is not very hard, but automating this with a special build target is a bit more work. Try one step first and let us know how far you get.

@siddhesh0705
Copy link

Hi,
I’m Siddhesh Sangale, and I’m new to open-source contributions. I’m eager to get started by tackling some beginner-friendly tasks that can help me build confidence and gain experience. While I’m just beginning my open-source journey, I’m excited to grow my skills and contribute to impactful projects.

I’m looking forward to working on something meaningful and gradually taking on bigger challenges as I learn. I would appreciate any guidance or suggestions for good starting points.

can you appoint me with this issue ?

@cyrille-artho
Copy link
Member

Yes, a fix for this is fairly easy, and as long as the issue remains open and there has not been any activity for a while, it's safe to assume that it has not been resolved yet.
Feel free to try to fix this in the code, and I can change the issue later to ask for an automated test for this.

@siddhesh0705
Copy link

@cyrille-artho can you provide me with the inital steps that how can start with this issue and on which branch I can find this issue ?

@siddhesh0705
Copy link

@cyrille-artho
I have submitted a Pull Request to address this issue. Could you please review it and provide feedback or suggest any changes needed to meet your requirements?

@siddhesh0705
Copy link

siddhesh0705 commented Nov 19, 2024

Hi @cyrille-artho ,
I hope this message finds you well. I wanted to sincerely apologize for my recent lack of focus on the issue we were working on. My college practical exams have been taking up much of my time and attention, and I regret that I was unable to dedicate the effort required to make meaningful progress.

Now that my exams are behind me, I am committed to refocusing on the problem and catching up. Thank you for your understanding, and I truly appreciate your guidance and patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug first issue Good issue for first commit
Projects
None yet
Development

No branches or pull requests

9 participants