Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Expose missing members of Array to prep for dotnet/corefx#9998. #6230

Merged
danmoseley merged 6 commits into
dotnet:masterfrom
danmoseley:master
Jul 12, 2016
Merged

Expose missing members of Array to prep for dotnet/corefx#9998. #6230
danmoseley merged 6 commits into
dotnet:masterfrom
danmoseley:master

Conversation

@danmoseley
Copy link
Copy Markdown
Member

modelgen.exe is helpful.

@stephentoub

@stephentoub
Copy link
Copy Markdown
Member

stephentoub commented Jul 12, 2016

The build failures are legitimate. At least one of the members you're trying to add back is missing an implementation for CoreCLR, e.g.

coreclr/src/vm/ecalllist.h

Lines 1522 to 1524 in 08786f2

#ifndef FEATURE_CORECLR
FCFuncElement("get_LongLength", ArrayNative::GetLongLengthNoRank)
#endif

That's resulting in the failure:

Generating native image for System.Private.CoreLib.

Assert failure(PID 43633 [0x0000aa71], Thread: 43633 [0xaa71]): Consistency check failed: No method entry found for System.Array::get_LongLength.
FAILED: 0 != id
    File: /mnt/resource/j/workspace/dotnet_coreclr/master/debug_centos7.1_prtest/src/vm/ecall.cpp Line: 270
    Image: /mnt/resource/j/workspace/dotnet_coreclr/master/debug_centos7.1_prtest/bin/Product/Linux.x64.Debug/crossgen

Microsoft (R) CoreCLR Native Image Generator - Version 4.5.22220.0
Copyright (c) Microsoft Corporation.  All rights reserved.

./build.sh: line 268: 43633 Aborted                 $__BinDir/crossgen $__BinDir/System.Private.CoreLib.dll
Failed to generate native image for System.Private.CoreLib.

@danmoseley
Copy link
Copy Markdown
Member Author

Ah. I naively believed I only needed to build mscorlib locally. Let me see..

@danmoseley
Copy link
Copy Markdown
Member Author

@stephentoub Good to go?

@stephentoub
Copy link
Copy Markdown
Member

It looks ok to me, but it'd be good for @jkotas to confirm we're ok with changing the LongLength implementation like this. To my knowledge, the longest possible array length in the runtime is less than int.MaxValue (https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Array.cs#L614), but we seem to have gone out of our way here to add this to the runtime, so it'd be good to get his perspective before it's deleted.

@danmoseley
Copy link
Copy Markdown
Member Author

In the apicatalog note about removing it, it says "this was added for standardization purposes but is not useful because the CLR does not support it". Good to get Jan's confirmation..

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Jul 12, 2016

In full framework, we do have option that allows creating multi-dimensional arrays with more than Int32.MaxValue elements (https://www.bing.com/search?q=gcallowverylargeobjects).

You should leave the implementation as is, and just remove the FEATURE_CORECLR ifdef around ArrayNative::GetLongLengthNoRank in vm\ecalllist.h.

@mikedn
Copy link
Copy Markdown

mikedn commented Jul 12, 2016

In full framework, we do have option that allows creating multi-dimensional arrays with more than Int32.MaxValue elements (https://www.bing.com/search?q=gcallowverylargeobjects).

I don't think that's the case. gcallowverylargeobjects allow arrays greater than 2GB, not arrays with length > Int32.MaxValue.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Jul 12, 2016

@mikedn You are right ... I forgot that we ended up not enabling the 2GB+ elements case. Thanks!

@danmosemsft My other comment about leaving the Length implementation as is, and just removing the ifdef in ecalllist.h still applies.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Jul 12, 2016

LGTM

@stephentoub
Copy link
Copy Markdown
Member

LGTM2 as long as it's squashed when merging.

Is this going to cause internal build failures due to the asmmeta file needing to be updated?

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Jul 12, 2016

asmmeta file needing to be updated?

asmmeta file needs to be updated only for changes that affect full framework. This one is not affecting full framework.

@danmoseley danmoseley merged commit 3e9d463 into dotnet:master Jul 12, 2016
@danmoseley danmoseley deleted the master branch April 11, 2021 23:55
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…lr#9998. (dotnet/coreclr#6230)

* Expose missing members of Array to prep for dotnet/corefxdotnet/coreclr#9998. modelgen.exe is helpful.

* Remove dead code

* Implement Array.LongLength.

* Revert "Implement Array.LongLength."

This reverts commit dotnet/coreclr@fdf7e96.

* Revert "Remove dead code"

This reverts commit dotnet/coreclr@432087c.

* Expose Array.GetLongLength from VM


Commit migrated from dotnet/coreclr@3e9d463
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants