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

Likely bug in Mac code gen #5798

Closed
tmat opened this issue May 5, 2016 · 8 comments
Closed

Likely bug in Mac code gen #5798

tmat opened this issue May 5, 2016 · 8 comments

Comments

@tmat
Copy link
Member

tmat commented May 5, 2016

dotnet/roslyn#11042 failed on Mac due to what seems to be a bad code gen.

The PR updates from crossgen version to 1.0.2-rc3-24011-00 to 1.0.2-rc3-24102-00.

The CI script first builds Roslyn and then builds Roslyn again with the Roslyn built in previous step. The second build fails with an assertion. When the following property:

http://source.roslyn.io/#Microsoft.CodeAnalysis/InternalUtilities/MultiDictionary.cs,98

is rewritten to

            public int Count
            {
                get
                {
                    if (_value == null)
                    {
                        return 0;
                    }

                    var set = _value as ImmutableHashSet<V>;
                    if (set == null)
                    {
                        return 1;
                    }

                    return set.Count;
                }
            }

The build passes.

So it seems that the code gen doesn't generate code for Nullable<int> in generic struct (the containing type).

Repro:

  • Sync to SRM-24102 Roslyn branch from fork https://github.com/tmat/roslyn.git.
  • Run
.\cibuild.sh

Expected: the build should report no errors.
Actual: the build report errors. Find the assert failure in Binaries\Debug\build.log.

I'll try to get a smaller repo.

@LLITCHEV
Copy link
Contributor

LLITCHEV commented May 8, 2016

The code I have looks fine (in terms of CodeGen). (I evaluated the Linux code, but there is no difference between Linux/MacOS Codegen anyway). This seems to be a crossgen issue (R2R) that is being worked on - generic dictionary support in crossgen R2R. @tmat was gracious to verify that without crossgen (ni image) there is no problem. The failure seems to be only reproducible if crossgen image is used.

@LLITCHEV
Copy link
Contributor

LLITCHEV commented May 8, 2016

@fadimounir has a PR that will mopst likely address this - support for generic dictionary lookup in R2R.)

@LLITCHEV LLITCHEV assigned fadimounir and unassigned LLITCHEV May 8, 2016
@jkotas
Copy link
Member

jkotas commented May 8, 2016

@fadimounir change is unlikely going to fix this. It will allow code for more methods to be saved into R2R images. It is not expected to change any of the code that is being saved today to change.

@LLITCHEV Do you have any details why it is failing? (e.g. dump of the incorrect code, etc.)

@jkotas jkotas assigned LLITCHEV and unassigned fadimounir May 8, 2016
@jkotas
Copy link
Member

jkotas commented May 8, 2016

Also, @tmat mentioned that the bug repros on Mac only. It does not repro on Linux.

@LLITCHEV
Copy link
Contributor

LLITCHEV commented May 8, 2016

Yes... That is right. It doesn't repro on Linux. I have dumped the method and everything on Linux and everything looks right. Need to get a Mac and do the same there - get the dump that is. But there should not be differences since the codegen on Linux and Mac is the same.

@gkhanna79 gkhanna79 assigned JohnChen0 and unassigned LLITCHEV May 9, 2016
@gkhanna79
Copy link
Member

@JohnChen0 PTAL

@JohnChen0
Copy link

@tmat I wasn't able to repro this on Mac. Looks like branch SRM-24102 already has a workaround for this issue, so I checked out commit f6554c8 from fork https://github.com/tmat/roslyn.git, but ./cibuild.sh completed without any errors. Could you help with verifying repro steps, or provide a repro machine?

JohnChen0 referenced this issue in JohnChen0/coreclr May 10, 2016
@JohnChen0
Copy link

Closed dotnet/coreclr#4801 via dotnet/coreclr#4883.

@JohnChen0 JohnChen0 removed their assignment May 10, 2016
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 30, 2020
@msftgits msftgits added this to the 1.0.0-rtm milestone Jan 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants