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

Added Sub_Ovh implementation. #64

Merged
merged 2 commits into from
Nov 28, 2018

Conversation

Geramy
Copy link

@Geramy Geramy commented Jul 25, 2018

I added the Sub_Ovh Implementation next is the Sub_Ovh_Un Implementation this is for ticket: #9

I added the Sub_Ovh Implementation next is the Sub_Ovh_Un Implementation this is for ticket: CosmosOS#9
@mterwoord
Copy link
Contributor

Can you please include a test which proves this change works as expected?

Had to use a hack to fix stack problem with exceptions

Added Sub_Ovf_Un IL Opcode
}
// Let's check if we add overflow and if so throw OverflowException
XS.Jump(ConditionalTestEnum.NoOverflow, xSuccessLabel);
if (xSize > 4) // Hack to stop stack corruption
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something we will have to consider on exception handling. When throwing in the middle of an opcode, this will probably work for most of the simple cases, but not sure about more complex ones. I'm not sure about what the CLI spec defines for the stack, still have to read that, but if the following is valid IL:

try
{
    ldloc.0
    ldloc.1
    ldloc.2
    add.ovf
    add.ovf
}
catch
{
    // do something
}

And we throw at the first add.ovf, the stack is not in a good state to jump to the catch block (still has the other local on the stack, which was expected to be consumed by the second add.ovf). Roslyn seems to emit good IL for this (not sure if by spec, still have to check), so this should work for now.

Also, I think we should file an issue about this or add this info to #16.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to ECMA-335, the throw opcode description is this:

The throw instruction throws the exception object (type O) on the stack and empties the stack.

So I think we should always empty the stack when throwing an exception, we should add some common infrastructure to throw exceptions, probably part of #16, I'll add a note there.

@mterwoord mterwoord merged commit ecab282 into CosmosOS:master Nov 28, 2018
@jp2masa jp2masa mentioned this pull request Nov 28, 2018
2 tasks
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