-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
ILDasm regression in relation to double (float64) and float (float32) values #111014
Comments
Tagging subscribers to this area: @dotnet/area-system-numerics |
|
Tagging subscribers to this area: @JulieLeeMSFT |
Notably this isn't a cast, but rather is interpreting it as the raw bitwise representation, that is |
https://godbolt.org/z/exzocbrzM It seems that it doesn't work on 8.0.0? |
Maybe I used the wrong wording. As I said, I'm not a C++ expert. runtime/src/coreclr/ilasm/asmparse.y Lines 270 to 271 in 6045e29
Then there must be something wrong with gobolt, here is the IL code I just generated with the 8.0.0 version. (https://www.nuget.org/packages/runtime.win-x64.Microsoft.NETCore.ILDAsm/8.0.0) // .NET IL Disassembler. Version 8.0.0
// Metadata version: v4.0.30319
.assembly extern System.Runtime
{
.ver 0:0:0:0
}
.module Program.dll
// MVID: {21758ade-0f42-4b59-8c4a-af87a71559ae}
.imagebase 0x00400000
.file alignment 0x00000200
.stackreserve 0x00100000
.subsystem 0x0003 // WINDOWS_CUI
.corflags 0x00000001 // ILONLY
// Image base: 0x000001E44FB80000
// =============== CLASS MEMBERS DECLARATION ===================
.class public abstract auto ansi sealed beforefieldinit Program
extends [System.Runtime]System.Object
{
.field public static literal float64 ValueDoubleOne = float64(1.)
.field public static literal float64 ValueDoubleOnePointTwo = float64(1.2)
.field public static literal float64 ValueDoubleMinusOne = float64(-1.)
.field public static literal float64 ValueDoubleMinusZero = float64(-0.)
.field public static literal float32 ValueFloatOne = float32(1.)
.field public static literal float32 ValueFloatOnePointTwo = float32(1.2)
.field public static literal float32 ValueFloatMinusOne = float32(-1.)
.field public static literal float32 ValueFloatMinusZero = float32(-0.)
} // end of class Program
// =============================================================
// *********** DISASSEMBLY COMPLETE *********************** |
Description
ILDasm (9.0) outputs the wrong format in
float64(...)
andfloat32(...)
for double and float values without decimal places. The decimal point is missing.ILAsm interprets these as
int64
and therefore casts them todouble
orfloat
values, which leads to incorrect values.Reproduction Steps
ildasm Program.dll /out=Program.il
ilasm Program.il /DLL
ildasm Program.dll /out=Program.il
(to see the effect)or
Expected behavior
Output should be the same as input:
Actual behavior
The IL code from step 2 will be (note the missing
.
s):After a roundtrip in step 4 the IL code will be:
Regression?
Yes, it is a regression, the following versions of ILDasm work:
The first version that no longer works is: 9.0.0-preview.4.24266.19
I think the behavior was changed with this PR: #98336
Known Workarounds
As a workaround, you can only correct the output using appropriate regex.
Configuration
Other information
I am not an expert in C++, but I think the problem should be fixed by adding this code snippet after the two places?
runtime/src/coreclr/ildasm/dasm.cpp
Line 2611 in 163b59e
runtime/src/coreclr/ildasm/dasm.cpp
Line 2625 in 163b59e
Although these places were also affected by the change, they do not seem to make any difference for ILAsm:
runtime/src/coreclr/ildasm/dasm.cpp
Line 1917 in 163b59e
runtime/src/coreclr/ildasm/dasm.cpp
Line 1936 in 163b59e
The text was updated successfully, but these errors were encountered: