Skip to content

Why is the supplied code behaving like explaint? #2131

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

Closed
Dudeplayz opened this issue Nov 9, 2021 · 12 comments
Closed

Why is the supplied code behaving like explaint? #2131

Dudeplayz opened this issue Nov 9, 2021 · 12 comments

Comments

@Dudeplayz
Copy link

Hi,
I have the following function:

export function compute(pc : u32, opcode : u16) : u32{
  return pc + (((opcode & 0x1f8) >> 3) - (opcode & 0x200 ? 0x40 : 0));
}

The function is called from JS site with the parameters compute(65, 63457).
The result of this function call is 65597. If I change the opcode to u32 the result is 61, which is the result I expect to get. For me, it looks like some type-overflow. I can also fix it by doing the following things:

return pc + (((opcode as u32 & 0x1f8) >> 3) - (opcode & 0x200 ? 0x40 : 0));
return pc + (((opcode & 0x1f8 as u32) >> 3) - (opcode & 0x200 ? 0x40 : 0));
return pc + (((opcode & 0x1f8) >> 3 as u32) - (opcode & 0x200 ? 0x40 : 0));
return pc + (((opcode & 0x1f8) >> 3) as u32 - (opcode & 0x200 ? 0x40 : 0));

Modifying the right statement with as u32 does nothing.

The strange thing is if I do this:

export function compute() : u32{
  const pc : u32 = 65;
  const opcode : u16 = 63457
  return pc + (((opcode & 0x1f8) >> 3) - (opcode & 0x200 ? 0x40 : 0));
}

... it also works.

So can someone explain to me what happens here?

@dcodeIO
Copy link
Member

dcodeIO commented Nov 9, 2021

Execution order there is (opcode & 0x1f8) >> 3 = 60, opcode & 0x200 ? 0x40 : 0 = 64, then computing 60 - 64 in an u16 context, wrapping around to 65532. Then, in u32 context, 65 + 65532 = 65597.

The difference in the second sample results from the compiler inlining const opcode : u16 = 63457, where something seems to go wrong. Need to investigate. Changing the const to a let produces the same result as the former.

@MaxGraey
Copy link
Member

MaxGraey commented Nov 9, 2021

That's because your code with u16 basically equal to this:

pc + ((((opcode & 0x1f8) >> 3) - (opcode & 0x200 ? 0x40 : 0)) & 0xFFFF);

let's simplify this to

pc + (a - b);

if a and b are u16 all operations associated with this vars also will be u16. In this case it's -.

It little bit different from C/C++ which always course (ucast) small types to 32-bit. AssemblyScript and Rust always preserve type for all operations.

@MaxGraey
Copy link
Member

MaxGraey commented Nov 9, 2021

If you want to take same effect without change input type you could use this:

export function compute(pc: u32, opcode: u16) : u32 {
  return pc + (((opcode & 0x1f8) >> 3) as u32 - (opcode & 0x200 ? 0x40 : 0) as u32);
  // or
  // return pc + (((opcode as u32 & 0x1f8) >> 3) - (opcode as u32 & 0x200 ? 0x40 : 0));
}

@MaxGraey
Copy link
Member

MaxGraey commented Nov 9, 2021

However issue with constant case is real issue.

@Dudeplayz
Copy link
Author

Dudeplayz commented Nov 9, 2021

That's because your code with u16 basically equal to this:

pc + ((((opcode & 0x1f8) >> 3) - (opcode & 0x200 ? 0x40 : 0)) & 0xFFFF);

let's simplify this to

pc + (a - b);

if a and b are u16 all operations associated with this vars also will be u16. In this case it's -.

It little bit different from C/C++ which always course (ucast) small types to 32-bit. AssemblyScript and Rust always preserve type for all operations.

Hello @dcodeIO and @MaxGraey
thanks for the explanation, but I have a few questions left:

  1. Why does this overflow does not happen if the opcode is u32? Then it is still pc + (a-b) in that case (a-b) should also get a turnaround?
  2. Why works
return pc + (((opcode & 0x1f8) >> 3) - (opcode & 0x200 ? 0x40 : 0 as u32))

but not

return pc + (((opcode & 0x1f8) >> 3) - (opcode & 0x200 ? 0x40 : 0) as u32)

Adding additional braces fixes the above:

return pc + (((opcode & 0x1f8) >> 3) - ((opcode & 0x200 ? 0x40 : 0) as u32))
  1. How are these statements evaluated? From right to left always using the left type?

@MaxGraey
Copy link
Member

MaxGraey commented Nov 9, 2021

but not
return pc + (((opcode & 0x1f8) >> 3) - (opcode & 0x200 ? 0x40 : 0) as u32)

Becouse wrap to u16 make sense only for ((opcode & 0x1f8) >> 3) in this sample.
(opcode & 0x200 ? 0x40 : 0) as u32 or (opcode & 0x200 ? 0x40 : 0) as u16 doesn't matter. In both cases it will be same result (but only for this specific case). Because 0x40 and 0 fit even to u8.

while ((opcode & 0x1f8) >> 3) expression still implicitly wrapping by 0xFFFF which required for u16 type

@Dudeplayz
Copy link
Author

@MaxGraey and what is different between the first statement and the third (added later)?

@MaxGraey
Copy link
Member

MaxGraey commented Nov 9, 2021

However return pc + (((opcode & 0x1f8) >> 3) - (opcode & 0x200 ? 0x40 : 0 as u32)) case is interesting. That's should behave the same as below one.

@dcodeIO
Copy link
Member

dcodeIO commented Nov 9, 2021

The as u32 case wraps around twice, but on u32 range:

65 + (60 - 64) = 65 + 4294967292 = 61
        ^^^        ^^^

@Dudeplayz
Copy link
Author

@dcodeIO now I got it. Thanks!

Next question:
What is the difference between

return pc + (((opcode & 0x1f8) >> 3) - (opcode & 0x200 ? 0x40 : 0) as u32)

and

return pc + (((opcode & 0x1f8) >> 3) - <u32>(opcode & 0x200 ? 0x40 : 0));

?

@Dudeplayz
Copy link
Author

However return pc + (((opcode & 0x1f8) >> 3) - (opcode & 0x200 ? 0x40 : 0 as u32)) case is interesting. That's should behave the same as below one.

And why behave the third:

return pc + (((opcode & 0x1f8) >> 3) - ((opcode & 0x200 ? 0x40 : 0) as u32))

also like the first one?

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in one week if no further activity occurs. Thank you for your contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants