{"action":"created","issue":{"url":"https://api.github.com/repos/mono/mono/issues/3085","repository_url":"https://api.github.com/repos/mono/mono","labels_url":"https://api.github.com/repos/mono/mono/issues/3085/labels{/name}","comments_url":"https://api.github.com/repos/mono/mono/issues/3085/comments","events_url":"https://api.github.com/repos/mono/mono/issues/3085/events","html_url":"https://github.com/mono/mono/pull/3085","id":157851553,"number":3085,"title":"Fix for non GCC win x64 calling convention when passing empty structs over pinvoke.","user":{"login":"lateralusX","id":11529140,"avatar_url":"https://avatars.githubusercontent.com/u/11529140?v=3","gravatar_id":"","url":"https://api.github.com/users/lateralusX","html_url":"https://github.com/lateralusX","followers_url":"https://api.github.com/users/lateralusX/followers","following_url":"https://api.github.com/users/lateralusX/following{/other_user}","gists_url":"https://api.github.com/users/lateralusX/gists{/gist_id}","starred_url":"https://api.github.com/users/lateralusX/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lateralusX/subscriptions","organizations_url":"https://api.github.com/users/lateralusX/orgs","repos_url":"https://api.github.com/users/lateralusX/repos","events_url":"https://api.github.com/users/lateralusX/events{/privacy}","received_events_url":"https://api.github.com/users/lateralusX/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/mono/mono/labels/cla-already-signed","name":"cla-already-signed","color":"009800"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2016-06-01T08:35:44Z","updated_at":"2016-06-07T09:37:29Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mono/mono/pulls/3085","html_url":"https://github.com/mono/mono/pull/3085","diff_url":"https://github.com/mono/mono/pull/3085.diff","patch_url":"https://github.com/mono/mono/pull/3085.patch"},"body":"Standard C and C++ doesn't allow empty structs, empty structs will always have a\r\nsize of 1 byte. GCC have an extension to allow empty structs,\r\nhttps://gcc.gnu.org/onlinedocs/gcc/Empty-Structures.html. This cause a\r\nlittle dilemma since runtime build using none GCC compiler will not be\r\ncompatible with GCC build C libraries and the other way around. On platforms\r\nwhere empty structs has size of 1 byte it must be represented in call and cannot\r\nbe dropped.\r\n\r\nLibraries build using for example MSVC failed pinvoke call when empty\r\nstructs where used in pinvoke2.cs tests. The problem was that current\r\nimplementation dropped value types of size 0, eliminating them from the\r\ncall. Since MSVC implements C/C++ standard regarding empty structs (no extension used),\r\nthe function expected the parameter to be present, even if was not used.\r\nThis became a problem if the empty struct ended up in one of the 4 first\r\nparameters (backed by registers) since if excluded, wrong parameter gets\r\ninto wrong registers (the same will apply to stack slots as well). For example,\r\na C signature like this, (int a, EmptyStruct es, int b) will get lowered to the\r\nfollowing on MSVC win x64, a -> rcx, es -> rdx and b -> r8, but since caller\r\neliminated es from the call, the register assignment from the caller was\r\na -> rcx and b -> rdx causing a violation of the calling convention and\r\nerror in callee.\r\n\r\nThe fix is to make sure that empty structs are still represented on none GCC\r\ncompilers on windows x64 (MSVC), since they must be represented in the call\r\nto pinvoked methods. Fix will also use a const value of 0 to represent the value\r\npassed as the struct instead of loading a value from stack into register/stack slot,\r\nthis is inline with the value the .NET runtime passes to pinvoked method. The reason\r\nwhy we use a 0 instead of loading from stack is because a struct of size 0 is not explicitly\r\nrepresented on stack and will have garbage data if loaded. Since it’s an empty struct value\r\ntype there is no need to load anything, just represent it as 0 in the register/stack slot passed\r\nto caller.\r\n\r\nThere is also a case when returning an empty struct from a pinvoked method. Result will be returned\r\nas 0 in RAX by MSVC compiled libraries (or whatever value stored in the 1 byte representation).\r\nSince we have similar issues as above with empty structs of size 0 not explicitly represented on the\r\nstack, the store will be eliminated and not written back to stack by caller since an empty struct doesn’t\r\nrepresent any unique data.\r\n\r\nCalling convention for managed->managed code is kept intact since it is working as expected\r\nhandling the case with empty structs as input and return values.\r\n\r\nWith fix all pinvoke2.cs tests now pass running on Windows x64 JIT."},"comment":{"url":"https://api.github.com/repos/mono/mono/issues/comments/224229854","html_url":"https://github.com/mono/mono/pull/3085#issuecomment-224229854","issue_url":"https://api.github.com/repos/mono/mono/issues/3085","id":224229854,"user":{"login":"lateralusX","id":11529140,"avatar_url":"https://avatars.githubusercontent.com/u/11529140?v=3","gravatar_id":"","url":"https://api.github.com/users/lateralusX","html_url":"https://github.com/lateralusX","followers_url":"https://api.github.com/users/lateralusX/followers","following_url":"https://api.github.com/users/lateralusX/following{/other_user}","gists_url":"https://api.github.com/users/lateralusX/gists{/gist_id}","starred_url":"https://api.github.com/users/lateralusX/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lateralusX/subscriptions","organizations_url":"https://api.github.com/users/lateralusX/orgs","repos_url":"https://api.github.com/users/lateralusX/repos","events_url":"https://api.github.com/users/lateralusX/events{/privacy}","received_events_url":"https://api.github.com/users/lateralusX/received_events","type":"User","site_admin":false},"created_at":"2016-06-07T09:37:29Z","updated_at":"2016-06-07T09:37:29Z","body":"Changes based on feedback and discussions done. OK to merge?"}}
{"actor":{"display_login":"lateralusX"}}