{"action":"created","issue":{"url":"https://api.github.com/repos/dotnet/coreclr/issues/20865","repository_url":"https://api.github.com/repos/dotnet/coreclr","labels_url":"https://api.github.com/repos/dotnet/coreclr/issues/20865/labels{/name}","comments_url":"https://api.github.com/repos/dotnet/coreclr/issues/20865/comments","events_url":"https://api.github.com/repos/dotnet/coreclr/issues/20865/events","html_url":"https://github.com/dotnet/coreclr/pull/20865","id":378496202,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI5MTg5Mjk2","number":20865,"title":"[WIP] Fix handling of ECMA-355 file format corner case","user":{"login":"MichalStrehovsky","id":13110571,"node_id":"MDQ6VXNlcjEzMTEwNTcx","avatar_url":"https://avatars0.githubusercontent.com/u/13110571?v=4","gravatar_id":"","url":"https://api.github.com/users/MichalStrehovsky","html_url":"https://github.com/MichalStrehovsky","followers_url":"https://api.github.com/users/MichalStrehovsky/followers","following_url":"https://api.github.com/users/MichalStrehovsky/following{/other_user}","gists_url":"https://api.github.com/users/MichalStrehovsky/gists{/gist_id}","starred_url":"https://api.github.com/users/MichalStrehovsky/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MichalStrehovsky/subscriptions","organizations_url":"https://api.github.com/users/MichalStrehovsky/orgs","repos_url":"https://api.github.com/users/MichalStrehovsky/repos","events_url":"https://api.github.com/users/MichalStrehovsky/events{/privacy}","received_events_url":"https://api.github.com/users/MichalStrehovsky/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-11-07T22:27:04Z","updated_at":"2018-11-08T11:45:15Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/dotnet/coreclr/pulls/20865","html_url":"https://github.com/dotnet/coreclr/pull/20865","diff_url":"https://github.com/dotnet/coreclr/pull/20865.diff","patch_url":"https://github.com/dotnet/coreclr/pull/20865.patch"},"body":"This fixes the handling of a corner case in the ECMA-355 spec in which a module that has 0xFFFF methods *and* the last type in the module has no methods. (I'm going to refer to methods, but this also impacts fields, and to a lesser extent properties and events.)\r\n\r\nThe file format represents \"last type with no methods\" as a type whose methods start at the index of Number_Of_Methods in the assembly. For a module with exactly 0xFFFF methods, this index would be 0x10000 (index zero is reserved and not assigned to an actual method). The file format also says that the width of the database column that specifies method indices is 2 bytes if the number of methods is less or equal to 0xFFFF. You probably see where things go wrong.\r\n\r\nThe spec also says that the \"type with no methods\" situation can be expressed as \"methods of this type start at index 0\" (the invalid index), but CLR didn't implement it this way, so it's unlikely anyone emits them like that.\r\n\r\nThis was famously hit in the Windows build lab where we were forced to ask people to add a dummy method as a workaround (https://twitter.com/h0x0d/status/1012176973433409536). After giving the workaround, I thought - \"what a great corner case!\", filed a bug to fix this, checked with the rest of the team where nobody remembered anyone hitting this since the v1 of the CLR, and moved on with life. A couple months later someone else hit this. Since this is too big of a coincidence, here's my attempt to fix the corner case.\r\n\r\nThis pull request has 3 parts, with increasing levels of complexity:\r\n\r\n* Don't stomp over HRESULT if metadata saving operation failed. This was causing ILASM to generate broken assemblies when this situation is hit, without an error. This is the least problematic part of this pull request and on it's own, will at least make ILASM fail.\r\n* Support in the metadata writer to emit index zero if we're in the corner case situation (this hopefully avoids tooling breakage - we could emit index zero for other last-type-has-no-methods situations, but since nobody can handle that, we limit it to the case where the output would be broken anyways)\r\n* Support in the metadata reader for types whose method list starts at index zero.\r\n\r\nIf we go with fixing this, we'll also need to have a look at other tooling, e.g. dotnet/corefx#29554 that impacts Roslyn and a bunch of other tools."},"comment":{"url":"https://api.github.com/repos/dotnet/coreclr/issues/comments/436967468","html_url":"https://github.com/dotnet/coreclr/pull/20865#issuecomment-436967468","issue_url":"https://api.github.com/repos/dotnet/coreclr/issues/20865","id":436967468,"node_id":"MDEyOklzc3VlQ29tbWVudDQzNjk2NzQ2OA==","user":{"login":"MichalStrehovsky","id":13110571,"node_id":"MDQ6VXNlcjEzMTEwNTcx","avatar_url":"https://avatars0.githubusercontent.com/u/13110571?v=4","gravatar_id":"","url":"https://api.github.com/users/MichalStrehovsky","html_url":"https://github.com/MichalStrehovsky","followers_url":"https://api.github.com/users/MichalStrehovsky/followers","following_url":"https://api.github.com/users/MichalStrehovsky/following{/other_user}","gists_url":"https://api.github.com/users/MichalStrehovsky/gists{/gist_id}","starred_url":"https://api.github.com/users/MichalStrehovsky/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MichalStrehovsky/subscriptions","organizations_url":"https://api.github.com/users/MichalStrehovsky/orgs","repos_url":"https://api.github.com/users/MichalStrehovsky/repos","events_url":"https://api.github.com/users/MichalStrehovsky/events{/privacy}","received_events_url":"https://api.github.com/users/MichalStrehovsky/received_events","type":"User","site_admin":false},"created_at":"2018-11-08T11:45:15Z","updated_at":"2018-11-08T11:45:15Z","author_association":"MEMBER","body":"@davidwr What do you think about this?\r\n\r\nCc @sergiy-k "}}
{"actor":{"display_login":"MichalStrehovsky"}}