hm-riscv / vscode-riscv-venus

VS Code extension with the Venus RISC-V simulator
https://marketplace.visualstudio.com/items?itemName=hm.riscv-venus
MIT License
64 stars 15 forks source link

Console Input locks up entire IDE #81

Closed JoJoDeveloping closed 2 years ago

JoJoDeveloping commented 2 years ago

Consider the following program:

addi a0, x0, 0x130 # 0x130 equals parseInt
ecall
jal pollInt
addi a0, x0, 10 # exit
ecall

pollInt:
addi a0, x0, 0x131 # 0x130 equals parseInt
addi a5, x0, 0 # we use a5 as our comparison for branching
addi a1, x0, 0 # we also need to set a1
ecall
beq a1, a5, pollInt # if no input was detected both are 0 and we need to keep polling
addi a0, x0, 1 # If we have input we echo it in the console
ecall   # If we have input we echo it in the console
jr ra   # return

Usually, it runs fine. However, when single-stepping, if you try to step over the call to pollInt in line 3, your IDE partially locks up. In particular, the terminal becomes unresponsive, as well as the stop button. So your program will wait forever since you can not enter anything in the console, and you also can not terminate the debugging. The only way out is to restart VSCode.

m1n1 commented 2 years ago

Thanks for mentioning the issue @JoJoDeveloping

Yes I noticed this issue actually myself already. The problem has actually nothing todo with console input but rather with the stepOver and stepOut Behaviour. When executing a long call the these operations could lock up the event loop in their old implementation.

This issue is already fixed on the master branch see here. The newest release on the marketplace doesn't contain this fix yet. I will create a new release next week. If you don't want to wait you can always build the extension yourself, see the wiki

I will close this issue when the release with the fix is published.

JoJoDeveloping commented 2 years ago

Thanks!

m1n1 commented 2 years ago

v1.6.1 is online which should fix this problem.

JoJoDeveloping commented 2 years ago

You will want to add the line

simulator.driver.timer = null;

at the beginning of VenusRuntime.start(), otherwise the behavior when executing with stopAtEntry == false is glitchy. (I already did in my clone but I'm too lazy to PR since I also modified a bunch of other stuff that may or may not be upstream-worthy)

m1n1 commented 2 years ago

Yeah good find, also we need to clear the timeout.

I reopened this issue because the v1.6.1 release wouldn't start on some linux hosts. I couldn't reproduce this issue on my Linux VM. Does 1.6.1 work for you @JoJoDeveloping ? (Except the timer issue)

Further releases are halted until this issue is resolved.

Also my I ask for what purpose you are using our Venus Simulator @JoJoDeveloping ?

JoJoDeveloping commented 2 years ago

Another issue I had was that the dot matrix x y size values may be undefined, while the "dot matrix" property itself is not, and thus you get an exception during debug initialization.

We're planning on using this for a college class.

JoJoDeveloping commented 2 years ago

https://github.com/hm-riscv/vscode-riscv-venus/blob/master/src/venusDebug.ts#L250

I think changing this to

if (args.ledMatrixSize && args.ledMatrixSize.x && args.ledMatrixSize.y) {

solves this. Quick fix is to add a run configuration where these are defined.

But it might just be that this only affected me. My current local changes work under Linux.

wallento commented 2 years ago

Excellent, thank you. Our own class is about to start next week, so happy to closely work with you on fixing any issues ;)