Skip to content

Bad double -> uint32 conversion when using clamp trap mode #5603

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
kumpera opened this issue Sep 26, 2017 · 3 comments · Fixed by #5647
Closed

Bad double -> uint32 conversion when using clamp trap mode #5603

kumpera opened this issue Sep 26, 2017 · 3 comments · Fixed by #5647

Comments

@kumpera
Copy link

kumpera commented Sep 26, 2017

Test case:

#include <stdio.h>
#include <stdlib.h>

static double *bla;

int main (int argc, char *argv[]) {
	int i = 0;
	bla = malloc (8 * 16);
	for (i = 0; i < 16; ++i)
		bla[i] = 4294967295.0;
	unsigned int x = (unsigned int)bla[6];
	printf ("x is %f x is %x\n", bla[6], x);
	return 0;
}

How to compile it:

emcc -s "BINARYEN_TRAP_MODE='clamp'" -Os -g  -s WASM=1 repro.c -o repro.html

It prints x is 4294967295.000000 x is 80000000

The expected value for x is ffffffff and it works as expected if BINARYEN_TRAP_MODE is not passed.

@kripken
Copy link
Member

kripken commented Sep 26, 2017

Thanks, yeah, looks like in clamp mode we currently just clamp to [-2147483648..2147483648), which is indeed odd for an unsigned conversion. We should clamp to unsigned values when the operation is an unsigned one.

@jgravelle-google, this would change the clamping code a little, so it could conflict with WebAssembly/binaryen#1168, should I wait for that to land first?

@jgravelle-google
Copy link
Contributor

I did notice that while working through that. I can add it to that PR if you don't mind

@kripken
Copy link
Member

kripken commented Sep 26, 2017

Sounds good. Meanwhile I did a little experimenting and wrote this patch, so that might help (or feel free to ignore and do it your way).

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 a pull request may close this issue.

3 participants