{"action":"created","issue":{"url":"https://api.github.com/repos/dotnet/coreclr/issues/3199","repository_url":"https://api.github.com/repos/dotnet/coreclr","labels_url":"https://api.github.com/repos/dotnet/coreclr/issues/3199/labels{/name}","comments_url":"https://api.github.com/repos/dotnet/coreclr/issues/3199/comments","events_url":"https://api.github.com/repos/dotnet/coreclr/issues/3199/events","html_url":"https://github.com/dotnet/coreclr/issues/3199","id":134140540,"number":3199,"title":"Stack corruption with snprintf(3) and format %hu","user":{"login":"krytarowski","id":6665730,"avatar_url":"https://avatars.githubusercontent.com/u/6665730?v=3","gravatar_id":"","url":"https://api.github.com/users/krytarowski","html_url":"https://github.com/krytarowski","followers_url":"https://api.github.com/users/krytarowski/followers","following_url":"https://api.github.com/users/krytarowski/following{/other_user}","gists_url":"https://api.github.com/users/krytarowski/gists{/gist_id}","starred_url":"https://api.github.com/users/krytarowski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/krytarowski/subscriptions","organizations_url":"https://api.github.com/users/krytarowski/orgs","repos_url":"https://api.github.com/users/krytarowski/repos","events_url":"https://api.github.com/users/krytarowski/events{/privacy}","received_events_url":"https://api.github.com/users/krytarowski/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":44,"created_at":"2016-02-17T00:48:06Z","updated_at":"2016-02-18T11:56:06Z","closed_at":null,"body":"PAL tests fall down with\r\n\r\n`src/pal/tests/palsuite/c_runtime/vsprintf/test11/test11.c`\r\n\r\nwith the following example call:\r\n\r\n```\r\nDoNumTest(\"foo %hu\", 0xFFFF, \"foo 65535\");\r\n```\r\n\r\nThere is stack corruption here:\r\nhttps://github.com/dotnet/coreclr/blob/master/src/pal/src/cruntime/printfcpp.cpp#L1822\r\n\r\n```\r\n                else if (Type == PFF_TYPE_INT && Prefix == PFF_PREFIX_SHORT)\r\n                {\r\n                    // Convert explicitly from int to short to get\r\n                    // correct sign extension for shorts on all systems.\r\n                    int n;\r\n                    short s;\r\n\r\n                    n = va_arg(ap, int);\r\n                    s = (short) n;\r\n\r\n                    TempInt = snprintf(BufferPtr, TempCount, TempBuff, s); // <--- here\r\n                }\r\n```\r\n\r\n```\r\n(gdb) list\r\n1814                        // Convert explicitly from int to short to get\r\n1815                        // correct sign extension for shorts on all systems.\r\n1816                        int n;\r\n1817                        short s;\r\n1818    \r\n1819                        n = va_arg(ap, int);\r\n1820                        s = (short) n;\r\n1821    \r\n1822                        TempInt = snprintf(BufferPtr, TempCount, TempBuff, s);\r\n1823                    }\r\n(gdb) n\r\n1820                        s = (short) n;\r\n(gdb) p n\r\n$8 = 65535\r\n(gdb) n\r\n1822                        TempInt = snprintf(BufferPtr, TempCount, TempBuff, s);\r\n(gdb) p s\r\n$9 = -1\r\n(gdb) p BufferPtr\r\n$10 = (LPSTR) 0x7f7fffffd924 \"\"\r\n(gdb) p BufferPtr-1\r\n$11 = (CHAR *) 0x7f7fffffd923 \" \"\r\n(gdb) p BufferPtr-2\r\n$12 = (CHAR *) 0x7f7fffffd922 \"o \"\r\n(gdb) p BufferPtr-3\r\n$13 = (CHAR *) 0x7f7fffffd921 \"oo \"\r\n(gdb) p BufferPtr-4\r\n$14 = (CHAR *) 0x7f7fffffd920 \"foo \"\r\n(gdb) p TempBuff\r\n$15 = \"%hu\\000\\177\\177\\000\\000\\006\\362E\\000\\000\\000\\000\\000\\000\\222\\377\\367\\177\\177\\000\\000\\000P�\\367\\177\\177\\000\\000�\\322\\377\\377\\177\\177\\000\\000\\006\\362E\\000\\000\\000\\000\\000�\\322\\377\\377\\177\\177\\000\\000\\000P�\\367\\177\\177\\000\\000\\020\\323\\377\\377\\177\\177\\000\\000\\006\\362E\\000\\000\\000\\000\\000��D\\000\\000\\000\\000\\000\\377\\232N\\000\\000\\000\\000\\000\\000X\\377\\367\\016\\000\\000\\000\\000P�\\367\\177\\177\\000\\000@\\323\\377\\377\\177\\177\\000\\000\\006\\362E\\000\\000\\000\\000\\000\\004\\323\\377\\377\\177\\177\\000\\000L2�\\367\\177\\177\\000\\000\\000X\\377\\367\\177\\177\\000\\000��N\\000\\000\\000\\000\\000\\000X\\377\\367\\177\\177\\000\\000L2�\\367\\177\\177\\000\\000\\020\\323\\377\\377\\001\\000\\000\\000�a�\\367\\177\\177\\000\\000 \\323\\377\\377\\177\\177\\000\\000\"...\r\n(gdb) n\r\n\r\nBreakpoint 2, CoreVsnprintf (pthrCurrent=0x7f7ff7b05000, Buffer=0x7f7fffffd924 \"\", Count=2147483643, Format=0x7f7fffffd240 \"%hu\", aparg=0x7f7fffffcdb8)\r\n    at /tmp/pkgsrc-tmp/wip/coreclr-git/work/coreclr/src/pal/src/cruntime/printfcpp.cpp:1819\r\n1819                        n = va_arg(ap, int);\r\n(gdb) \r\n```\r\n\r\nI'm trying to determine what's going on."},"comment":{"url":"https://api.github.com/repos/dotnet/coreclr/issues/comments/185681180","html_url":"https://github.com/dotnet/coreclr/issues/3199#issuecomment-185681180","issue_url":"https://api.github.com/repos/dotnet/coreclr/issues/3199","id":185681180,"user":{"login":"jasonwilliams200OK","id":2031765,"avatar_url":"https://avatars.githubusercontent.com/u/2031765?v=3","gravatar_id":"","url":"https://api.github.com/users/jasonwilliams200OK","html_url":"https://github.com/jasonwilliams200OK","followers_url":"https://api.github.com/users/jasonwilliams200OK/followers","following_url":"https://api.github.com/users/jasonwilliams200OK/following{/other_user}","gists_url":"https://api.github.com/users/jasonwilliams200OK/gists{/gist_id}","starred_url":"https://api.github.com/users/jasonwilliams200OK/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasonwilliams200OK/subscriptions","organizations_url":"https://api.github.com/users/jasonwilliams200OK/orgs","repos_url":"https://api.github.com/users/jasonwilliams200OK/repos","events_url":"https://api.github.com/users/jasonwilliams200OK/events{/privacy}","received_events_url":"https://api.github.com/users/jasonwilliams200OK/received_events","type":"User","site_admin":false},"created_at":"2016-02-18T11:56:06Z","updated_at":"2016-02-18T11:56:06Z","body":"This is a good approach to have PAL_token defined in headers for abstraction purposes.\r\n\r\nAnother approach could be something like this: https://github.com/libuv/libuv/pull/532/files <- i.e. the external hook approach.\r\n\r\n---\r\n\r\nExample Problem:\r\n\r\nMSVCR does not provide std snprintf before MSC_VER<1900 (aka VS2015 which astoundingly and finally conforms with C99 standards). Many people think `_snprintf()` in VS2013 and below is the suitable candidate for snprintf. Unfortunately, this is not true as there is one gotcha there that `_snprintf()` doesn't zero-terminate the buffer on overflow. So it was implemented manually using vsnprintf_s followed by adjustment of the null terminator accordingly. Since libuv has many many dependents and is a backbone to Google Chrome (v8 JavaScript engine, node.js and aspnet/KestrelHttpServer), it needs to be absolutely conformant behavior cross-platform.\r\n\r\nSolution:\r\n\r\nAgain, the most traditional approach was to define it in some common header (like PAL.h; which libuv was also using) and then include that header everywhere where the feature is required. Another approach which is used in https://github.com/libuv/libuv/pull/532 is by adding a `snprintf.c` file with the fallback functionality and then build system (libuv uses gyp instead of cmake) is configured in a way that it will not compile this c file the compiler is VS2013 and below. Note that the implementation in sprintf.c is also wrapped with `defined(_MSC_VER) && _MSC_VER < 1900` condition (so external build systems of downstream projects if include the file unconditionally in the list of to-be-compiled files, they don't compile), and then at call sites:\r\n\r\n```c\r\n#if !defined(snprintf) && defined(_MSC_VER) && _MSC_VER < 1900\r\n// ^ call sites need to be wrapped in the same preprocessor condition\r\nextern int snprintf(char*, size_t, const char*, ...);\r\n#endif\r\n```\r\n---\r\n\r\nWe can extend and configure this shimming out approach for every inconsistent external library function.\r\n\r\n*Footnote: this is just me highlighting a potential alternative approach, I acknowledge the fact that there is nothing wrong with either approaches and both provide degree of convenience in their own ways. Most of the world is using the \"common header with varied behaviors\" approach.*"}}