Skip to content

Conversation

@Hecate2
Copy link
Contributor

@Hecate2 Hecate2 commented Jan 22, 2025

Fixed return in multiple layers of nested try context. Still a dangerous case is not solved. Commented in tests.

def internal_():
  try:
    try:
      return a
    finally:
      throw
  finally:
    ...

def external_():
  b = 1
  try:
    a = internal_()
    # internal_ generates a return value in evaluation stack,
    # but never stored into local variables here
  catch:
    return b

throw is executed and nothing should be returned in this case, but the value of a is remained in the evaluation stack, without being handled by the external context.

Also the assembly is not the most optimal. There is a JMP to an ENDTRY. The JMP is unnecessary.

@Hecate2
Copy link
Contributor Author

Hecate2 commented Jan 22, 2025

Should this be solved in Neo VM? Evaluation stack should be cleared when invocation stack is popped with exception

shargon
shargon previously approved these changes Jan 22, 2025
@Hecate2
Copy link
Contributor Author

Hecate2 commented Jan 22, 2025

I think the VM should be responsible for clearing the evaluation stack when exception is thrown.

http://lampwww.epfl.ch/~magarcia/ScalaCompilerCornerReloaded/2010Q2/ExceptionHandling.pdf

1.4 Some rules about stack-emptiness

  • endfinally empties the evaluation stack as a side-effect
  • The evaluation stack shall be empty after the return value is popped by a ret instruction.
  • Exception handlers can access the local variables and the local memory pool of the routine that catches the exception, but any intermediate results on the evaluation stack at the time the exception was thrown are lost.
  • An exception object describing the exception is automatically created by the CLI and pushed onto the evaluation stack as the first item upon entry of a filter or catch clause.

@nan01ab
Copy link
Contributor

nan01ab commented Jan 22, 2025

I think the VM should be responsible for clearing the evaluation stack when exception is thrown.

http://lampwww.epfl.ch/~magarcia/ScalaCompilerCornerReloaded/2010Q2/ExceptionHandling.pdf

1.4 Some rules about stack-emptiness

  • endfinally empties the evaluation stack as a side-effect
  • The evaluation stack shall be empty after the return value is popped by a ret instruction.
  • Exception handlers can access the local variables and the local memory pool of the routine that catches the exception, but any intermediate results on the evaluation stack at the time the exception was thrown are lost.
  • An exception object describing the exception is automatically created by the CLI and pushed onto the evaluation stack as the first item upon entry of a filter or catch clause.

Great proposal.

nan01ab
nan01ab previously approved these changes Jan 22, 2025
…to return-in-try

# Conflicts:
#	tests/Neo.Compiler.CSharp.UnitTests/TestingArtifacts/Contract_Returns.cs
@Hecate2 Hecate2 dismissed stale reviews from nan01ab and shargon via be4c40e January 23, 2025 03:10
@Hecate2 Hecate2 changed the title partially fix return in try partially fix return in try; optimizer replaces JMP->ENDTRY with only ENDTRY Jan 23, 2025
@Hecate2 Hecate2 requested review from nan01ab and shargon January 23, 2025 07:14
@Hecate2 Hecate2 merged commit 3eb6230 into neo-project:master Jan 23, 2025
3 checks passed
@newera-001
Copy link

tightly strong leader!

Jim8y added a commit that referenced this pull request Feb 5, 2025
* master:
  fix: always check division overflow for int32 and int64 (#1287)
  Fix: some compiler warnings (#1288)
  use INC DEC (#1286)
  Update neo (#1285)
  Update: keep same sdk version with 'neo' (#1284)
  partially fix return in try; optimizer replaces JMP->ENDTRY with only ENDTRY (#1283)
  update to dotnet 9 (#1257)
  Added some styles (#1280)
  [`Fix`]: `checked(-x)` if x is int or long (#1281)
  Fix continue and goto (#1282)

# Conflicts:
#	tests/Neo.Compiler.CSharp.UnitTests/TestingArtifacts/Contract_Types_BigInteger.cs
Jim8y pushed a commit that referenced this pull request Aug 3, 2025
… ENDTRY (#1283)

* partially fix return in try
* optimizer replaces JMP-ENDTRY with only ENDTRY
* Add { }
---------
Co-authored-by: Shargon <[email protected]>
Jim8y pushed a commit that referenced this pull request Aug 18, 2025
… ENDTRY (#1283)

* partially fix return in try

* optimizer replaces JMP-ENDTRY with only ENDTRY

* Add { }

---------

Co-authored-by: Shargon <[email protected]>
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.

4 participants