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

C# Debugger does not understand DateTime #753

Closed
jchannon opened this issue Sep 8, 2016 · 33 comments
Closed

C# Debugger does not understand DateTime #753

jchannon opened this issue Sep 8, 2016 · 33 comments

Comments

@jchannon
Copy link
Contributor

jchannon commented Sep 8, 2016

Environment data

dotnet --info output:

.NET Command Line Tools (1.0.0-preview2-003121)

Product Information:
 Version:            1.0.0-preview2-003121
 Commit SHA-1 hash:  1e9d529bc5

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  10.11
 OS Platform: Darwin
 RID:         osx.10.11-x64

VS Code version:1.4.0
C# Extension version:1.4.1

Steps to reproduce

Debug a C# app and look at a DateTime object in Variables/Debug Console window

Expected behavior

To show a DateTime object at correct locale and when in Debug Console doing something like (dateA - dateB).TotalSeconds to show correct results

Actual behavior

@gregg-miskelly
Copy link
Contributor

gregg-miskelly commented Sep 8, 2016

@jchannon thanks for the report! Good bug.

Here is a slightly simpler repro:

    public class Program
    {
        public static DateTime ReturnInput(DateTime input)
        {
            return input; 
        }

        public static void Main(string[] args)
        {
            DateTime now = DateTime.Now;
            Console.WriteLine(now); // set breakpoint here
        }
    }
  1. Set a breakpoint on the call to Console.WriteLine
  2. Compare the value of now to ReturnInput(now)

Result:
now
{09/08/2016 12:34:49}
ReturnInput(now)
{06/24/0001 02:59:04}

Expected:
Two values should be the same

@gregg-miskelly
Copy link
Contributor

@mikem8361 I am guessing (though I don't have anything to back this up other than this seems similar to the previous bug with decimal) that this is a runtime bug with marshalling large struct input arguments.

Would you mind trying this (or emailing me instructions on how we can try this) with mdbg?

@jchannon
Copy link
Contributor Author

jchannon commented Sep 8, 2016

One thing it emphasise is if you look in the variable window the DateTime is in US format but when you expand the DateTime object it's shown in UK locale (where I am located) so it shows 09/08/2016 10:54:18 but then expanded shows that the Date property is 08/09/2016

@gregg-miskelly
Copy link
Contributor

@jchannon I don't think this matters - the issue is that when we pass the DateTime as an input argument to a function (subtraction operator in your example) the value is being corrupted as it passed to the function.

@jchannon
Copy link
Contributor Author

jchannon commented Sep 8, 2016

Maybe we have 2 issues here

On Thursday, 8 September 2016, Gregg Miskelly [email protected]
wrote:

@jchannon https://github.com/jchannon I don't think this matters - the
issue is that when we pass the DateTime as an input argument to a function
(subtraction operator in your example) the value is being corrupted as it
passed to the function.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#753 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGaphfCBUq2IrzRAUv0f4lig2oGteEbks5qoGedgaJpZM4J4WOv
.

@mikem8361
Copy link
Member

The bug was with any 16 byte value type. The mdbg syntax is awkward something like "funceval namespace.class.get_Value instance".

Sent from my Verizon, Samsung Galaxy smartphone

-------- Original message --------
From: Jonathan Channon [email protected]
Date: 9/8/16 12:54 PM (GMT-08:00)
To: OmniSharp/omnisharp-vscode [email protected]
Cc: Mike McLaughlin [email protected], Mention [email protected]
Subject: Re: [OmniSharp/omnisharp-vscode] C# Debugger does not understand DateTime (#753)

Maybe we have 2 issues here

On Thursday, 8 September 2016, Gregg Miskelly [email protected]
wrote:

@jchannon https://github.com/jchannon I don't think this matters - the
issue is that when we pass the DateTime as an input argument to a function
(subtraction operator in your example) the value is being corrupted as it
passed to the function.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#753 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGaphfCBUq2IrzRAUv0f4lig2oGteEbks5qoGedgaJpZM4J4WOv
.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//issues/753#issuecomment-245719616, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AJ-Fa0EWx-uLux2ZSSAaqqZkYo_nnmzTks5qoGfvgaJpZM4J4WOv.

@gregg-miskelly
Copy link
Contributor

@mikem8361 would the bug apply non-this input arguments?

@mikem8361
Copy link
Member

No, only return values. On Linux and OS X, the calling convention has 16 byte values being returned in RCX:RDX unlike windows where a return value pointer is passed to method.

The funceval code wasn’t dealing with that case even though the runtime did.

mikem

From: Gregg Miskelly [mailto:[email protected]]
Sent: Thursday, September 8, 2016 2:08 PM
To: OmniSharp/omnisharp-vscode [email protected]
Cc: Mike McLaughlin [email protected]; Mention [email protected]
Subject: Re: [OmniSharp/omnisharp-vscode] C# Debugger does not understand DateTime (#753)

@mikem8361https://github.com/mikem8361 would the bug apply non-this input arguments?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//issues/753#issuecomment-245741344, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AJ-Fa7T3ioy5Y_aZE5dEslCbl02wGbRFks5qoHktgaJpZM4J4WOv.

@gregg-miskelly
Copy link
Contributor

@mikem8361 Yes, that is what I thought. I think this might be a new bug that affects input arguments of large structs. Its possible that it is VIL that is somehow setting up the func eval wrong, but this works correctly on Windows, and this isn't specific to DateTime -- I see the same issue if I make my own 16-byte struct.

@mikem8361
Copy link
Member

@jkotas do we pass in 16 bytes into a method in registers on Linux/OS X like return values?

From: Gregg Miskelly [mailto:[email protected]]
Sent: Thursday, September 8, 2016 2:28 PM
To: OmniSharp/omnisharp-vscode [email protected]
Cc: Mike McLaughlin [email protected]; Mention [email protected]
Subject: Re: [OmniSharp/omnisharp-vscode] C# Debugger does not understand DateTime (#753)

@mikem8361https://github.com/mikem8361 Yes, that is what I thought. I think this might be a new bug that affects input arguments of large structs. Its possible that it is VIL that is somehow setting up the func eval wrong, but this works correctly on Windows, and this isn't specific to DateTime -- I see the same issue if I make my own 16-byte struct.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//issues/753#issuecomment-245747159, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AJ-FaxfuaI0Kl8JPRK7Mq8rJfx6x6CECks5qoH38gaJpZM4J4WOv.

@jkotas
Copy link
Member

jkotas commented Sep 8, 2016

@jkotas do we pass in 16 bytes into a method in registers on Linux/OS X like return values?

Correct.

@mikem8361
Copy link
Member

Is the ENREGISTERED_PARAMTYPE_MAXSIZE the correct define? It seems to be 8 bytes on all x64 platforms (16 bytes on ARM64).

From vm\amd64\cgencpu.h:

#define ENREGISTERED_RETURNTYPE_INTEGER_MAXSIZE 8 // bytes
#define ENREGISTERED_PARAMTYPE_MAXSIZE 8 // bytes

#ifdef UNIX_AMD64_ABI
#define ENREGISTERED_RETURNTYPE_MAXSIZE 16 // bytes
#define CALLDESCR_ARGREGS 1 // CallDescrWorker has ArgumentRegister parameter
#define CALLDESCR_FPARGREGS 1 // CallDescrWorker has FloatArgumentRegisters parameter
#else

From: Jan Kotas [mailto:[email protected]]
Sent: Thursday, September 8, 2016 4:25 PM
To: OmniSharp/omnisharp-vscode [email protected]
Cc: Mike McLaughlin [email protected]; Mention [email protected]
Subject: Re: [OmniSharp/omnisharp-vscode] C# Debugger does not understand DateTime (#753)

@jkotashttps://github.com/jkotas do we pass in 16 bytes into a method in registers on Linux/OS X like return values?

Correct.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//issues/753#issuecomment-245772977, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AJ-Fax5omSwL6lahJ6odhV4aMSuEGzvnks5qoJk1gaJpZM4J4WOv.

@jkotas
Copy link
Member

jkotas commented Sep 8, 2016

The code for this is under FEATURE_UNIX_AMD64_STRUCT_PASSING. It was done by @janvorli .

@mikem8361
Copy link
Member

Looks like the bug repros under mdbg.

[p#:0, t#:0] mdbg> print now
now=System.DateTime <9/8/16 4:57:23 PM>
DaysToMonth365=array [13]
DaysToMonth366=array [13]
MinValue=System.DateTime
MaxValue=System.DateTime
dateData=9859461543285131878
[p#:0, t#:0] mdbg> funceval ConsoleApplication.Program.ReturnInput now
result = System.DateTime <6/24/01 2:59:04 AM>
DaysToMonth365=array [13]
DaysToMonth366=array [13]
MinValue=System.DateTime
MaxValue=System.DateTime
dateData=150443443958818
results saved to $result

@janvorli
Copy link
Member

janvorli commented Sep 9, 2016

@mikem8361 - the ENREGISTERED_PARAMTYPE_MAXSIZE is defined as 16 for UNIX_AMD64_ABI (you may have misread it as ARM64)

@janvorli
Copy link
Member

janvorli commented Sep 9, 2016

@mikem8361 Ah, sorry, that was the ENREGISTERED_RETURNTYPE_MAXSIZE. The ENREGISTERED_PARAMTYPE_MAXSIZE is set to 8 which is wrong, the return and param sizes the same.
Let me check when did it get changed to the wrong value.

@janvorli
Copy link
Member

janvorli commented Sep 9, 2016

@mikem8361 Interesting - the value was always wrong - never updated since the initial commit.
And I guess I can see why it was never discovered before - its value is never used for UNIX AMD64 compilation - we only check if the define is present at all places - with the only exception - the MarshalInfo::SetupArgumentSizes in mlinfo.cpp. And I guess we've never hit that code path for struct over 8 bytes in our tests.

@janvorli
Copy link
Member

janvorli commented Sep 9, 2016

The same issue is for ENREGISTERED_RETURNTYPE_INTEGER_MAXSIZE. That should also be 16 for Unix AMD64. That one influences two places. One in the arg iterator the case when the parameter is typedbyref when it would incorrectly set flag indicating that a return buffer is needed for structs over 8 bytes. And the other in IsUnmanagedValueTypeReturnedByRef which is used by marshalling.

@janvorli
Copy link
Member

janvorli commented Sep 9, 2016

Let me fix the constants, I'll send a PR in a minute.

@gregg-miskelly
Copy link
Contributor

Cool. I created https://github.com/dotnet/coreclr/issues/7115 to track this. I will resolve this as External.

@janvorli
Copy link
Member

janvorli commented Sep 9, 2016

@gregg-miskelly - I am not sure if fixing the constants will fix the problem in the debugger. So maybe it would be better to leave this open and close it after it is verified that it has fixed the issue. The point is that the structs of upto 16 bytes in registers are passed correctly at most places as parameters, we have a test for that in CoreCLR.

@gregg-miskelly
Copy link
Contributor

@janvorli Its possible that there will be other CoreCLR bugs too, but since Mike found this repos in mdbg, I am pretty sure that all the issues are on the CoreCLR side. So I would suggest we leave https://github.com/dotnet/coreclr/issues/7115 open until we have verified the fix in mdbg.

@gregg-miskelly
Copy link
Contributor

@jchannon for the weird switching between US date time format (mm/dd/yyyy) and UK, do you get the same behavior if you just evaluate the functions normally?

In other words, try adding this code:

string lastmodifiedStr = lastmodified.ToString();
string lastmodifiedDateStr = lastmodified.Date.ToString();

And evaluate those strings under the debugger.

@jchannon
Copy link
Contributor Author

jchannon commented Sep 9, 2016

Adding that code and looking in the Variables window the string representation is the UK format.

@gregg-miskelly
Copy link
Contributor

The two known bugs in CoreCLR that were effecting this scenario are now set to be fixed for .NET Core 1.1.

@jchannon
Copy link
Contributor Author

Is that Q1 2017?

On Thursday, 15 September 2016, Gregg Miskelly [email protected]
wrote:

The two known bugs in CoreCLR that were effecting this scenario are now
set to be fixed for .NET Core 1.1.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#753 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGaptid8QnGOHAVfYlw-RNZD5jaQsSKks5qqY7ogaJpZM4J4WOv
.

@gregg-miskelly
Copy link
Contributor

@jchannon
Copy link
Contributor Author

Aha. Thanks

On Thursday, 15 September 2016, Gregg Miskelly [email protected]
wrote:

Fall 2016 according to https://github.com/dotnet/
core/blob/master/roadmap.md#ship-dates


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#753 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGappvIlvjaVyLpUQuIxuXD5MLPSsdpks5qqZmkgaJpZM4J4WOv
.

@jchannon
Copy link
Contributor Author

jchannon commented Nov 17, 2016

Hi, just installed netcore 1.1 and this doesn't seem to be fixed

I ran the same repro as suggested by @gregg-miskelly here #753 (comment)

screen shot 2016-11-17 at 11 37 39

This is also visible in Jetbrains Rider

@jchannon jchannon reopened this Nov 17, 2016
@gregg-miskelly
Copy link
Contributor

@jchannon unfortunately @mikem8361 said that https://github.com/dotnet/coreclr/issues/7115 didn't actually make 1.1. @mikem8361 can @jchannon install a preview3 CLI to check if this repos? Or will preview3 also be missing the fix?

@jchannon
Copy link
Contributor Author

The milestones on the issue/PR says 1.1 so you might want to change that,
not sure how you know what makes it into what release otherwise :)

On 17 November 2016 at 16:32, Gregg Miskelly [email protected]
wrote:

@jchannon https://github.com/jchannon unfortunately @mikem8361
https://github.com/mikem8361 said that dotnet/coreclr#7115
https://github.com/dotnet/coreclr/issues/7115 didn't actually make 1.1.
@mikem8361 https://github.com/mikem8361 can @jchannon
https://github.com/jchannon install a preview3 CLI to check if this
repos? Or will preview3 also be missing the fix?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#753 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGaplPHK-BwWcIO-Bpc5ZnztgGNOkQRks5q_IGMgaJpZM4J4WOv
.

@gregg-miskelly
Copy link
Contributor

Agreed :)

@mikem8361
Copy link
Member

I’m not sure if the fix is in the dotnet cli review3 (I don’t know how to figure that out), but if you add "Microsoft.NETCore.Runtime.CoreCLR": "1.2.0-beta-24609-02" to your test app’s project.json it will have the fix.

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

6 participants