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

Fixed the error when Empty main runs with VerTracker #494

Merged
merged 1 commit into from
Aug 31, 2024
Merged

Fixed the error when Empty main runs with VerTracker #494

merged 1 commit into from
Aug 31, 2024

Conversation

shreyasCs012
Copy link
Contributor

@shreyasCs012 shreyasCs012 commented Aug 29, 2024

Description :

This PR fixes the error when we run an empty main set to a VerTracker listener.

Issue Fixed :

#380

Testing :

I added a test that calls the empty main method setting the listener to VerTracker.

###Fix :

  • The error was reflected because the addOperandAttr() method was called in the VerTracker without checking if the
    operand stack was empty.

  • The if condition (frame.getTopPos() >= 0) checks for the above undetected condition.

@cyrille-artho
Copy link
Member

Looks good! I think we need the same check in both places where addOperandAttr is called; see my comment.
We unfortunately don't have a test to ascertain this, though.

@shreyasCs012
Copy link
Contributor Author

shreyasCs012 commented Aug 31, 2024

I think Before the call for addOperandAttr(varId); there is a call for the peek() method which from the look of it returns the element in the slots array of index top

 public int peek () {
    return slots[top];
  }

if the top value does not change before the call for the addOperandAttr(varId); method then either an error should hit in that call for the peek() method or if the error does not hit there should be no problem when the call for addOperandAttr(varId);.

Again I am assuming that the top variable is not being changed in between the call for the peek() method and the addOperendAttr()

So, I think a check in the call for peek() would be more suitable than a check in the call for addOperandAttr(varId)

@cyrille-artho
Copy link
Member

Good observation. I noticed now also that the instruction must be ALOAD, which means that there is a local variable slot available (because the bytecode verifier would have rejected the code earlier, when it was verified after loading, if that were not the case).
We can therefore merge this PR as it is. Thank you very much for your work!

@cyrille-artho cyrille-artho merged commit 7d2f7a9 into javapathfinder:master Aug 31, 2024
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants