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

hevm: tty: skip null src maps when jumping with shift+n #832

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

d-xo
Copy link
Contributor

@d-xo d-xo commented Oct 7, 2021

Description

I think this fixes #686, but since the source map format and parser are very mysterious to me I'm not really sure that I've done the right thing.

Afaict the issue was that we sometimes get a sourcemap back from currentPosition that looks like this:

Just (SM {srcMapOffset = -1, srcMapLength = -1, srcMapFile = -1, srcMapJump = JumpRegular, srcMapModifierDepth = 0})

Which is treated as distinct by the check in isNextSourcePosition. I'm assuming the -1 in this case is a sentinel value meaning the given source pc does not have a corresponding position in the provided source map? Perhaps there is a cleaner fix here where we make this position unrepresentable? @MrChico thoughts?

cc @transmissions11 for testing.

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

@transmissions11
Copy link
Contributor

yayy tysm for the fix! doesn't seem to work for me yet sadly

Screen Shot 2021-10-07 at 5 24 23 PM

@transmissions11
Copy link
Contributor

btw just to clarify the reproduction cases a bit more:

  • it comes up with shift + N and shift + P
  • it does not come up with control + n but does come up sometimes with control + p

@transmissions11
Copy link
Contributor

transmissions11 commented Oct 8, 2021

if you want a repo to try it on, can follow the getting started instructions on the README of the this repo and debug the first test the menu shows you: https://github.com/rari-Capital/vaults

@transmissions11
Copy link
Contributor

transmissions11 commented Oct 8, 2021

the issue is super pronounced in that repo because hevm doesn't seem to recognize the Vault contract at all (hence why they have no label in the debugger), so no sourcemaps can be pulled for it. will file a separate issue for that as well (still think this is a fix we should implement nonetheless)

Screen Shot 2021-10-07 at 5 30 05 PM

Screen Shot 2021-10-07 at 5 29 05 PM

@d-xo
Copy link
Contributor Author

d-xo commented Oct 8, 2021

yayy tysm for the fix! doesn't seem to work for me yet sadly

sad, I was testing in dss and I thought I'd fixed this. will take a look at vaults and see if I can figure out what's going on.

the issue is super pronounced in that repo because hevm doesn't seem to recognize the Vault contract at all (hence why they have no label the debugger), so no sourcemaps can be pulled for it. will file a separate issue for that as well

hmmmm, yeah this is bad, issue would be great 👍

still think this is a fix we should implement nonetheless

agree

@MrChico
Copy link
Member

MrChico commented Oct 8, 2021

c-N uses the isNextSourcePositionWithoutEntering function, the fix here touches only isNextSourcePosition.

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.

Debugger shows <unknown>:Nothing (?) while debugging frequently
3 participants