Skip to content

disassembler: don't force precision for dumpDouble#59

Open
Mythie wants to merge 1 commit intoCyberShadow:masterfrom
Mythie:dont-force-precision-for-dump-double
Open

disassembler: don't force precision for dumpDouble#59
Mythie wants to merge 1 commit intoCyberShadow:masterfrom
Mythie:dont-force-precision-for-dump-double

Conversation

@Mythie
Copy link

@Mythie Mythie commented Mar 31, 2023

Forcing precision would cause the parsing MAX_DOUBLE to fail which would result in the .asasm output being blank, disabling the forced precision allows it to continue without any regressions as far as I can tell.

Forcing precision would cause the parsing MAX_DOUBLE to fail which would result in the `.asasm` output being blank, disabling the forced precision allows it to continue without any regressions as far as I can tell.
@CyberShadow
Copy link
Owner

Yes, unfortunately just %g will cause loss of precision on a roundtrip.

import std.stdio;
void main()
{
    double N = 0x123456789ABCDp0;
    writefln("%g", N);
    writefln("%.18g", N);
}

Output:

3.20256e+14
320255973501901

Using %a (full hex notation) avoids this but makes the numbers not very human readable.

Forcing precision would cause the parsing MAX_DOUBLE to fail

It doesn't fail here. I am testing with double.max, which is 1.79769313486231571e+308 (0x1.fffffffffffffp+1023). Do you have an example?

@Mythie
Copy link
Author

Mythie commented Apr 8, 2023

Yep so that's the one I'm having an issue on, running rabcdasm on the .abc file throws an error in dumpDouble with the erroneous value being the following:

value=1.79769e+308 string_to_write=1.79769313486231571e+308

Exception seems to be thrown when we force the value we've written to the buffer back to a double for comparison:

if (forceDouble(to!double(s)) == v)

I say seem's as I'm on Apple Silicon and thus have to use ldmd2 but I can confirm the same happens with other compilers.

@CyberShadow
Copy link
Owner

Yes, that definitely sounds like something ARM-specific. forceDouble exists so that the compiler does not reuse the 80-bit value in the x86 FPU registers, but casts it to a 64-bit value (which we do by putting it in conventional memory).

@CyberShadow
Copy link
Owner

value=1.79769e+308 string_to_write=1.79769313486231571e+308

Where is this coming from? There is no string_to_write in RABCDAsm or the D source code.

@Mythie
Copy link
Author

Mythie commented Apr 8, 2023

value=1.79769e+308 string_to_write=1.79769313486231571e+308

Where is this coming from? There is no string_to_write in RABCDAsm or the D source code.

These are the v and s values within the dumpDouble method.

Yes, that definitely sounds like something ARM-specific. forceDouble exists so that the compiler does not reuse the 80-bit value in the x86 FPU registers, but casts it to a 64-bit value (which we do by putting it in conventional memory).

I recall having this issue on an x64 machine as well using dmd2 but I'd need to double check. Give me a day or two and I'll get back to you!

@CyberShadow
Copy link
Owner

CyberShadow commented Apr 8, 2023

Okay, got it.

to!double calls the libc sscanf under the hood. It's possible that the libc implementation can't parse this value for whatever reason, perhaps a bug.

One way to test that theory would be to try it in a C program:

#include <stdio.h>

int main()
{
    double v = 0;
    int res = sscanf("1.79769313486231571e+308", "%lf", &v);
    printf("Scanned %d values (%a)\n", res, v);
    return 0;
}

For me this prints Scanned 1 values (0x1.fffffffffffffp+1023) on Linux/x86_64.

@Mythie
Copy link
Author

Mythie commented Apr 8, 2023

Can confirm the same happens with Clang and GCC on Apple Silicon:

→ for i in *.out; do echo $i; ./$i; done
a-with-clang.out
Scanned 1 values (0x1.fffffffffffffp+1023)
a-with-gcc.out
Scanned 1 values (0x1.fffffffffffffp+1023)

Compiled with:

gcc -O3 test.c -o a-with-gcc.out
clang -O3 test.c -o a-with-clang.out

@Pyogenics
Copy link

I encountered an swf which had errors disassembling, had to use this pr to get most of the abcs to disassemble on my x86 linux system.

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.

3 participants