Skip to content

Stack corruption with snprintf(3) and format %hu #5130

Closed
Closed
Stack corruption with snprintf(3) and format %hu#5130
Milestone
Future

Description

@krytarowski
krytarowski
opened on Feb 17, 2016

PAL tests fall down with

src/pal/tests/palsuite/c_runtime/vsprintf/test11/test11.c

with the following example call:

DoNumTest("foo %hu", 0xFFFF, "foo 65535");

There is stack corruption here:
https://github.com/dotnet/coreclr/blob/master/src/pal/src/cruntime/printfcpp.cpp#L1822

                else if (Type == PFF_TYPE_INT && Prefix == PFF_PREFIX_SHORT)
                {
                    // Convert explicitly from int to short to get
                    // correct sign extension for shorts on all systems.
                    int n;
                    short s;

                    n = va_arg(ap, int);
                    s = (short) n;

                    TempInt = snprintf(BufferPtr, TempCount, TempBuff, s); // <--- here
                }

(gdb) list
1814                        // Convert explicitly from int to short to get
1815                        // correct sign extension for shorts on all systems.
1816                        int n;
1817                        short s;
1818    
1819                        n = va_arg(ap, int);
1820                        s = (short) n;
1821    
1822                        TempInt = snprintf(BufferPtr, TempCount, TempBuff, s);
1823                    }
(gdb) n
1820                        s = (short) n;
(gdb) p n
$8 = 65535
(gdb) n
1822                        TempInt = snprintf(BufferPtr, TempCount, TempBuff, s);
(gdb) p s
$9 = -1
(gdb) p BufferPtr
$10 = (LPSTR) 0x7f7fffffd924 ""
(gdb) p BufferPtr-1
$11 = (CHAR *) 0x7f7fffffd923 " "
(gdb) p BufferPtr-2
$12 = (CHAR *) 0x7f7fffffd922 "o "
(gdb) p BufferPtr-3
$13 = (CHAR *) 0x7f7fffffd921 "oo "
(gdb) p BufferPtr-4
$14 = (CHAR *) 0x7f7fffffd920 "foo "
(gdb) p TempBuff
$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"...
(gdb) n

Breakpoint 2, CoreVsnprintf (pthrCurrent=0x7f7ff7b05000, Buffer=0x7f7fffffd924 "", Count=2147483643, Format=0x7f7fffffd240 "%hu", aparg=0x7f7fffffcdb8)
    at /tmp/pkgsrc-tmp/wip/coreclr-git/work/coreclr/src/pal/src/cruntime/printfcpp.cpp:1819
1819                        n = va_arg(ap, int);
(gdb)

I'm trying to determine what's going on.

Activity

  1. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    Debugging on NetBSD 7.99.26

  2. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    With the following patch I reduce the number of falling down tests (alloca(3) fixes for NetBSD locally committed) from ~63 to 7.

    diff --git a/src/pal/src/cruntime/printfcpp.cpp b/src/pal/src/cruntime/printfcpp.cpp
index 84c003b..9c41b7a 100644
--- a/src/pal/src/cruntime/printfcpp.cpp
+++ b/src/pal/src/cruntime/printfcpp.cpp
@@ -1397,85 +1397,113 @@ int CoreVfwprintf(CPalThread *pthrCurrent, PAL_FILE *stream, const wchar_16 *for
                 if (Type == PFF_TYPE_P && Prefix == PFF_PREFIX_SHORT)
                 {
                     // Convert from pointer -> int -> short to avoid warnings.
                     long trunc1;
                     short trunc2;

                     trunc1 = va_arg(ap, LONG);
                     trunc2 = (short)trunc1;
                     trunc1 = trunc2;

                     TempInt = snprintf(TempSprintfStr, TEMP_COUNT, TempBuff, trunc1);

                     if (TempInt < 0 || static_cast<size_t>(TempInt) >= TEMP_COUNT)
                     {
                         if (NULL == (TempSprintfStrPtr = (char*)InternalMalloc(++TempInt)))
                         {
                             ERROR("InternalMalloc failed\n");
                             LOGEXIT("vfwprintf returns int -1\n");
                             PERF_EXIT(vfwprintf);
                             pthrCurrent->SetLastError(ERROR_NOT_ENOUGH_MEMORY);
                             va_end(ap);
                             return -1;
                         }

                         TempSprintfStr = TempSprintfStrPtr;
                         snprintf(TempSprintfStr, TempInt, TempBuff, trunc2);
                     }
                 }
                 else if (Type == PFF_TYPE_INT && Prefix == PFF_PREFIX_SHORT)
                 {
+#if 0
                     // Convert explicitly from int to short to get
                     // correct sign extension for shorts on all systems.
                     int n;
                     short s;

                     n = va_arg(ap, int);
                     s = (short) n;

                     TempInt = snprintf(TempSprintfStr, TEMP_COUNT, TempBuff, s);

                     if (TempInt < 0 || static_cast<size_t>(TempInt) >= TEMP_COUNT)
                     {
                         if (NULL == (TempSprintfStrPtr = (char*)InternalMalloc(++TempInt)))
                         {
                             ERROR("InternalMalloc failed\n");
                             LOGEXIT("vfwprintf returns int -1\n");
                             PERF_EXIT(vfwprintf);
                             pthrCurrent->SetLastError(ERROR_NOT_ENOUGH_MEMORY);
                             va_end(ap);
                             return -1;
                         }

                         TempSprintfStr = TempSprintfStrPtr;
                         snprintf(TempSprintfStr, TempInt, TempBuff, s);
                     }
+#else
+                    va_list apcopy;
+
+                    va_copy(apcopy, ap);
+                    TempInt = vsnprintf(TempSprintfStr, TEMP_COUNT, TempBuff, apcopy);
+                    va_end(apcopy);
+                    PAL_printf_arg_remover(&ap, Width, Precision, Type, Prefix);
+
+                    if (TempInt < 0 || static_cast<size_t>(TempInt) >= TEMP_COUNT)
+                    {
+                        if (NULL == (TempSprintfStrPtr = (char*)InternalMalloc(++TempInt)))
+                        {
+                            ERROR("InternalMalloc failed\n");
+                            LOGEXIT("vfwprintf returns int -1\n");
+                            PERF_EXIT(vfwprintf);
+                            pthrCurrent->SetLastError(ERROR_NOT_ENOUGH_MEMORY);
+                            va_end(ap);
+                            return -1;
+                        }
+                        
+                        TempSprintfStr = TempSprintfStrPtr;
+                        va_copy(apcopy, ap);
+                        vsnprintf(TempSprintfStr, TempInt, TempBuff, apcopy);
+                        va_end(apcopy);
+                        PAL_printf_arg_remover(&ap, Width, Precision, Type, Prefix);
+                    }
+#endif
                 }
                 else
                 {
                     va_list apcopy;

                     va_copy(apcopy, ap);
                     TempInt = vsnprintf(TempSprintfStr, TEMP_COUNT, TempBuff, apcopy);
                     va_end(apcopy);
                     PAL_printf_arg_remover(&ap, Width, Precision, Type, Prefix);

                     if (TempInt < 0 || static_cast<size_t>(TempInt) >= TEMP_COUNT)
                     {
                         if (NULL == (TempSprintfStrPtr = (char*)InternalMalloc(++TempInt)))
                         {
                             ERROR("InternalMalloc failed\n");
                             LOGEXIT("vfwprintf returns int -1\n");
                             PERF_EXIT(vfwprintf);
                             pthrCurrent->SetLastError(ERROR_NOT_ENOUGH_MEMORY);
                             va_end(ap);
                             return -1;
                         }

                         TempSprintfStr = TempSprintfStrPtr;
                         va_copy(apcopy, ap);
                         vsnprintf(TempSprintfStr, TempInt, TempBuff, apcopy);
                         va_end(apcopy);
                         PAL_printf_arg_remover(&ap, Width, Precision, Type, Prefix);
                     }
                 }

@@ -1784,69 +1812,77 @@ int CoreVsnprintf(CPalThread *pthrCurrent, LPSTR Buffer, size_t Count, LPCSTR Fo
                 // Types that sprintf can handle
                 size_t TempCount = Count - (BufferPtr - Buffer);

 #if !HAVE_LARGE_SNPRINTF_SUPPORT
                 // Limit TempCount to 0x40000000, which is sufficient
                 // for platforms on which snprintf fails for very large
                 // sizes.
                 if (TempCount > 0x40000000)
                 {
                     TempCount = 0x40000000;
                 }
 #endif  // HAVE_LARGE_SNPRINTF_SUPPORT

                 TempInt = 0;
                 // %h (short) doesn't seem to be handled properly by local sprintf,
                 // so we do the truncation ourselves for some cases.
                 if (Type == PFF_TYPE_P && Prefix == PFF_PREFIX_SHORT)
                 {
                     // Convert from pointer -> int -> short to avoid warnings.
                     long trunc1;
                     short trunc2;

                     trunc1 = va_arg(ap, LONG);
                     trunc2 = (short) trunc1;
                     trunc1 = trunc2;

                     TempInt = snprintf(BufferPtr, TempCount, TempBuff, trunc1);
                 }
                 else if (Type == PFF_TYPE_INT && Prefix == PFF_PREFIX_SHORT)
                 {
+#if 0
                     // Convert explicitly from int to short to get
                     // correct sign extension for shorts on all systems.
                     int n;
                     short s;

                     n = va_arg(ap, int);
                     s = (short) n;

                     TempInt = snprintf(BufferPtr, TempCount, TempBuff, s);
+#else
+                    va_list apcopy;
+                    va_copy(apcopy, ap);
+                    TempInt = vsnprintf(BufferPtr, TempCount, TempBuff, apcopy);
+                    va_end(apcopy);
+                    PAL_printf_arg_remover(&ap, Width, Precision, Type, Prefix);
+#endif
                 }
                 else
                 {
                     va_list apcopy;
                     va_copy(apcopy, ap);
                     TempInt = vsnprintf(BufferPtr, TempCount, TempBuff, apcopy);
                     va_end(apcopy);
                     PAL_printf_arg_remover(&ap, Width, Precision, Type, Prefix);
                 }

                 if (TempInt < 0 || static_cast<size_t>(TempInt) >= TempCount) /* buffer not long enough */
                 {
                     BufferPtr += TempCount;
                     BufferRanOut = TRUE;
                 }
                 else
                 {
                     BufferPtr += TempInt;
                 }
             }
         }
         else
         {
             *BufferPtr++ = *Fmt++; /* copy regular chars into buffer */
         }
     }

     if (static_cast<int>(Count) > (BufferPtr - Buffer)) //Count is assumed to be in the range of int
     {
         *BufferPtr = 0; /* end the string */
@@ -2082,69 +2118,77 @@ int CoreWvsnprintf(CPalThread *pthrCurrent, LPWSTR Buffer, size_t Count, LPCWSTR
                    and place them in the buffer (BufferPtr) */
                 size_t TempCount = Count - (BufferPtr - Buffer);
                 TempInt = 0;

 #if !HAVE_LARGE_SNPRINTF_SUPPORT
                 // Limit TempCount to 0x40000000, which is sufficient
                 // for platforms on which snprintf fails for very large
                 // sizes.
                 if (TempCount > 0x40000000)
                 {
                     TempCount = 0x40000000;
                 }
 #endif  // HAVE_LARGE_SNPRINTF_SUPPORT

                 // %h (short) doesn't seem to be handled properly by local sprintf,
                 // so we do the truncation ourselves for some cases.
                 if (Type == PFF_TYPE_P && Prefix == PFF_PREFIX_SHORT)
                 {
                     // Convert from pointer -> int -> short to avoid warnings.
                     long trunc1;
                     short trunc2;

                     trunc1 = va_arg(ap, LONG);
                     trunc2 = (short)trunc1;
                     trunc1 = trunc2;

                     TempInt = snprintf((LPSTR)BufferPtr, TempCount, TempBuff, trunc1);
                 }
                 else if (Type == PFF_TYPE_INT && Prefix == PFF_PREFIX_SHORT)
                 {
+#if 0
                     // Convert explicitly from int to short to get
                     // correct sign extension for shorts on all systems.
                     int n;
                     short s;

                     n = va_arg(ap, int);
                     s = (short) n;

                     TempInt = snprintf((LPSTR)BufferPtr, TempCount, TempBuff, s);
+#else
+                    va_list apcopy;
+                    va_copy(apcopy, ap);
+                    TempInt = vsnprintf((LPSTR) BufferPtr, TempCount, TempBuff, apcopy);
+                    va_end(apcopy);
+                    PAL_printf_arg_remover(&ap, Width, Precision, Type, Prefix);
+#endif
                 }
                 else
                 {
                     va_list apcopy;
                     va_copy(apcopy, ap);
                     TempInt = vsnprintf((LPSTR) BufferPtr, TempCount, TempBuff, apcopy);
                     va_end(apcopy);
                     PAL_printf_arg_remover(&ap, Width, Precision, Type, Prefix);
                 }

                 if (TempInt == 0)
                 {
                     // The argument is "".
                     continue;
                 }
                 if (TempInt < 0 || static_cast<size_t>(TempInt) >= TempCount) /* buffer not long enough */
                 {
                     TempNumberBuffer = (LPSTR) InternalMalloc(TempCount+1);
                     if (!TempNumberBuffer)
                     {
                         ERROR("InternalMalloc failed\n");
                         pthrCurrent->SetLastError(ERROR_NOT_ENOUGH_MEMORY);
                         errno = ENOMEM;
                         va_end(ap);
                         return -1;
                     }

                     if (strncpy_s(TempNumberBuffer, TempCount+1, (LPSTR) BufferPtr, TempCount) != SAFECRT_SUCCESS)
                     {
                         ASSERT("strncpy_s failed!\n");
@@ -2463,69 +2507,77 @@ int CoreVfprintf(CPalThread *pthrCurrent, PAL_FILE *stream, const char *format,
                 if (-1 == paddingReturnValue)
                 {
                     ERROR("Internal_AddPaddingVfprintf failed\n");
                     PERF_EXIT(vfprintf);    
                     va_end(ap);
                     return -1;
                 }
                 written += paddingReturnValue;
             }
             else
             {
                 // Types that fprintf can handle.
                 TempInt = 0;

                 // %h (short) doesn't seem to be handled properly by local sprintf,
                 // so we do the truncation ourselves for some cases.
                 if (Type == PFF_TYPE_P && Prefix == PFF_PREFIX_SHORT)
                 {
                     // Convert from pointer -> int -> short to avoid warnings.
                     long trunc1;
                     short trunc2;

                     trunc1 = va_arg(ap, LONG);
                     trunc2 = (short)trunc1;
                     trunc1 = trunc2;

                     TempInt = fprintf(stream->bsdFilePtr, TempBuff, trunc1);
                 }
                 else if (Type == PFF_TYPE_INT && Prefix == PFF_PREFIX_SHORT)
                 {
+#if 0
                     // Convert explicitly from int to short to get
                     // correct sign extension for shorts on all systems.
                     int n;
                     short s;

                     n = va_arg(ap, int);
                     s = (short) n;

                     TempInt = fprintf( stream->bsdFilePtr, TempBuff, s);
+#else
+                    va_list apcopy;
+                    va_copy(apcopy, ap);
+                    TempInt = vfprintf(stream->bsdFilePtr, TempBuff, apcopy);
+                    va_end(apcopy);
+                    PAL_printf_arg_remover(&ap, Width, Precision, Type, Prefix);
+#endif
                 }
                 else
                 {
                     va_list apcopy;
                     va_copy(apcopy, ap);
                     TempInt = vfprintf(stream->bsdFilePtr, TempBuff, apcopy);
                     va_end(apcopy);
                     PAL_printf_arg_remover(&ap, Width, Precision, Type, Prefix);
                 }

                 if (-1 == TempInt)
                 {
                     ERROR("vfprintf returned an error\n");
                 }
                 else
                 {
                     written += TempInt;
                 }
             }
         }
         else
         {

 #if FILE_OPS_CHECK_FERROR_OF_PREVIOUS_CALL
             clearerr (stream->bsdFilePtr);
 #endif

             InternalFwrite(Fmt++, 1, 1, stream->bsdFilePtr, &stream->PALferrorCode); /* copy regular chars into buffer */
             if (stream->PALferrorCode == PAL_FILE_ERROR)
             {

  3. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    PAL test results with the above patch applied.

    Finished running PAL tests.

The following test(s) failed:
exception_handling/pal_sxs/test1/paltest_pal_sxs_test1. Exit code: 134
threading/CriticalSectionFunctions/test2/paltest_criticalsectionfunctions_test2. Exit code: 134
threading/CriticalSectionFunctions/test4/paltest_criticalsectionfunctions_test4. Exit code: 134
threading/DuplicateHandle/test8/paltest_duplicatehandle_test8. Exit code: 134
threading/GetCurrentThread/test1/paltest_getcurrentthread_test1. Exit code: 134
threading/GetCurrentThread/test2/paltest_getcurrentthread_test2. Exit code: 134
threading/ThreadPriority/test1/paltest_threadpriority_test1. Exit code: 134

PAL Test Results:
  Passed: 797
  Failed: 7

  4. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    Now.. I don't understand what's and why wrong. Why va_arg(3) corrupts stack only for "%hd" / "%hu", while all other (including "%h") combinations work...

    Have you got an idea @janvorli or @jkotas ?

    Thank you for enlightening me.

  5. janvorli commented on Feb 17, 2016

    @janvorli
    janvorli
    on Feb 17, 2016
    Member

    Looking at the function that converts the formatting string, the "%h" seems to be replaced incorrectly just by "%", but even %h alone is an incorrect specifier. So it is not used in the formatting and that's likely the reason why it doesn't crash while the "%hd" / "%hu" do.

    Could you please paste here the whole formatting string passed as the "format" parameter to the CoreVfwprintf function when the crash happens?

  6. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    It crashes with the following example formatting strings:

    • "foo %hu"
    • "foo %hd"

  7. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    While it works with "foo %h", "foo %d", etc. Please see the number of passed tests and their coverage.

  8. janvorli commented on Feb 17, 2016

    @janvorli
    janvorli
    on Feb 17, 2016
    Member

    @krytarowski I have stepped through the code on Linux and I have not noticed anything suspicious, except for one thing that depending on the va_arg definition in the new clang might cause a problem. The code at line 1819 is asking for int parameter, while the actual one is short. Although I believe that shorts should be passed as 32 bit values on AMD64 too (and in this case, it should be passed in a register, based on the AMD64 ABI document), who knows what the va_arg does in such case.
    Could you please disassemble the CoreVsnprintf function in the gdb using disass /m and post it somewhere so that I can compare it with the code from the clang 3.5 on Linux? The va_arg stuff is inlined, so it should be sufficient to spot potential culprit.

  9. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    Thank you @janvorli for help! Generally shorter types are promoted to longer. stdarg(3) on NetBSD says:

         If the type in question is one that gets promoted, the promoted type
     should be used as the argument to va_arg().  The following describes
     which types are promoted (and to what):
     -   short is promoted to int
     -   float is promoted to double
     -   char is promoted to int

    I'm suspecting a compiler bug.. I will try to generate assembly for comparison. I wasn't able to reproduce this issue with a small code example. Nothing interesting resides in .i files.

  10. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    I've added nop marks, just in case of optimization to close the region between two volatile fragments.

    --- src/pal/src/cruntime/printfcpp.cpp.orig     2016-01-28 19:04:13.000000000 +0000
+++ src/pal/src/cruntime/printfcpp.cpp
@@ -1811,6 +1811,7 @@ int CoreVsnprintf(CPalThread *pthrCurren
                 }
                 else if (Type == PFF_TYPE_INT && Prefix == PFF_PREFIX_SHORT)
                 {
+                    __asm__ volatile("nop;nop;nop;nop;nop;");
                     // Convert explicitly from int to short to get
                     // correct sign extension for shorts on all systems.
                     int n;
@@ -1820,6 +1821,7 @@ int CoreVsnprintf(CPalThread *pthrCurren
                     s = (short) n;

                     TempInt = snprintf(BufferPtr, TempCount, TempBuff, s);
+                    __asm__ volatile("nop;nop;nop;nop;nop;");
                 }
                 else
                 {

    The result:

    1813                    {
1814                        __asm__ volatile("nop;nop;nop;nop;nop;");
   0x0000000000412154 <+5508>:  nop
   0x0000000000412155 <+5509>:  nop
   0x0000000000412156 <+5510>:  nop
   0x0000000000412157 <+5511>:  nop
   0x0000000000412158 <+5512>:  nop
   0x0000000000412159 <+5513>:  lea    -0x430(%rbp),%rax

1815                        // Convert explicitly from int to short to get
1816                        // correct sign extension for shorts on all systems.
1817                        int n;
1818                        short s;
1819    
1820                        n = va_arg(ap, int);
   0x0000000000412160 <+5520>:  mov    -0x430(%rbp),%ecx
   0x0000000000412166 <+5526>:  cmp    $0x28,%ecx
   0x0000000000412169 <+5529>:  mov    %rax,-0x788(%rbp)
   0x0000000000412170 <+5536>:  mov    %ecx,-0x78c(%rbp)
   0x0000000000412176 <+5542>:  ja     0x4121a1 <CoreVsnprintf(CorUnix::CPalThread*, char*, unsigned long, char const*, __va_list_tag*)+5585>
   0x000000000041217c <+5548>:  mov    -0x78c(%rbp),%eax
   0x0000000000412182 <+5554>:  movslq %eax,%rcx
   0x0000000000412185 <+5557>:  mov    -0x788(%rbp),%rdx
   0x000000000041218c <+5564>:  add    0x10(%rdx),%rcx
   0x0000000000412190 <+5568>:  add    $0x8,%eax
   0x0000000000412193 <+5571>:  mov    %eax,(%rdx)
   0x0000000000412195 <+5573>:  mov    %rcx,-0x798(%rbp)
   0x000000000041219c <+5580>:  jmpq   0x4121be <CoreVsnprintf(CorUnix::CPalThread*, char*, unsigned long, char const*, __va_list_tag*)+5614>
   0x00000000004121a1 <+5585>:  mov    -0x788(%rbp),%rax
   0x00000000004121a8 <+5592>:  mov    0x8(%rax),%rcx
   0x00000000004121ac <+5596>:  mov    %rcx,%rdx
   0x00000000004121af <+5599>:  add    $0x8,%rcx
   0x00000000004121b3 <+5603>:  mov    %rcx,0x8(%rax)
   0x00000000004121b7 <+5607>:  mov    %rdx,-0x798(%rbp)
   0x00000000004121be <+5614>:  mov    -0x798(%rbp),%rax
   0x00000000004121c5 <+5621>:  lea    -0x410(%rbp),%rdx
   0x00000000004121cc <+5628>:  mov    (%rax),%ecx
   0x00000000004121ce <+5630>:  mov    %ecx,-0x530(%rbp)

1821                        s = (short) n;
   0x00000000004121d4 <+5636>:  mov    -0x530(%rbp),%ecx
   0x00000000004121da <+5642>:  mov    %cx,%si
   0x00000000004121dd <+5645>:  mov    %si,-0x532(%rbp)

1822    
1823                        TempInt = snprintf(BufferPtr, TempCount, TempBuff, s);
   0x00000000004121e4 <+5652>:  mov    -0x460(%rbp),%rdi
   0x00000000004121eb <+5659>:  mov    -0x520(%rbp),%rsi
   0x00000000004121f2 <+5666>:  movswl -0x532(%rbp),%ecx
   0x00000000004121f9 <+5673>:  mov    $0x0,%al
   0x00000000004121fb <+5675>:  callq  0x404cf0 <snprintf@plt>
   0x0000000000412200 <+5680>:  mov    %eax,-0x4f0(%rbp)

1824                        __asm__ volatile("nop;nop;nop;nop;nop;");
   0x0000000000412206 <+5686>:  nop
   0x0000000000412207 <+5687>:  nop
   0x0000000000412208 <+5688>:  nop
   0x0000000000412209 <+5689>:  nop
   0x000000000041220a <+5690>:  nop

1825                    }
   0x000000000041220b <+5691>:  jmpq   0x412287 <CoreVsnprintf(CorUnix::CPalThread*, char*, unsigned long, char const*, __va_list_tag*)+5815>
   0x0000000000412210 <+5696>:  lea    -0x450(%rbp),%rax
   0x0000000000412217 <+5703>:  lea    -0x410(%rbp),%rdx
   0x000000000041221e <+5710>:  lea    -0x430(%rbp),%rcx

1826                    else
1827                    {
1828                        va_list apcopy;
1829                        va_copy(apcopy, ap);
   0x0000000000412225 <+5717>:  mov    %rax,%rsi
   0x0000000000412228 <+5720>:  mov    0x10(%rcx),%rdi
   0x000000000041222c <+5724>:  mov    %rdi,0x10(%rsi)
   0x0000000000412230 <+5728>:  movups (%rcx),%xmm0
   0x0000000000412233 <+5731>:  movups %xmm0,(%rsi)

1830                        TempInt = vsnprintf(BufferPtr, TempCount, TempBuff, apcopy);
   0x0000000000412236 <+5734>:  mov    -0x460(%rbp),%rdi
   0x000000000041223d <+5741>:  mov    -0x520(%rbp),%rsi
   0x0000000000412244 <+5748>:  mov    %rax,%rcx
   0x0000000000412247 <+5751>:  callq  0x404c20 <vsnprintf@plt>
   0x000000000041224c <+5756>:  lea    -0x430(%rbp),%rdi
   0x0000000000412253 <+5763>:  lea    -0x450(%rbp),%rcx
   0x000000000041225a <+5770>:  mov    %eax,-0x4f0(%rbp)

1831                        va_end(apcopy);
1832                        PAL_printf_arg_remover(&ap, Width, Precision, Type, Prefix);
   0x0000000000412260 <+5776>:  mov    -0x474(%rbp),%esi
   0x0000000000412266 <+5782>:  mov    -0x478(%rbp),%edx
   0x000000000041226c <+5788>:  mov    -0x480(%rbp),%eax
   0x0000000000412272 <+5794>:  mov    -0x47c(%rbp),%r8d
   0x0000000000412279 <+5801>:  mov    %rcx,-0x7a0(%rbp)
   0x0000000000412280 <+5808>:  mov    %eax,%ecx
   0x0000000000412282 <+5810>:  callq  0x407260 <PAL_printf_arg_remover(va_list*, INT, INT, INT, INT)>
   0x0000000000412287 <+5815>:  jmpq   0x41228c <CoreVsnprintf(CorUnix::CPalThread*, char*, unsigned long, char const*, __va_list_tag*)+5820>

1833

  11. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    @janvorli if you need assembly for full function, please let me know.

  12. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    @janvorli if you need assembly for full function, please let me know.

  13. janvorli commented on Feb 17, 2016

    @janvorli
    janvorli
    on Feb 17, 2016
    Member

    @krytarowski I would really need the whole disass of the function, ideally without the added nops. I have tried to compare your piece of code with my one, it seems equivalent except for slightly different instruction ordering and registers allocations at few places and some RBP offsets. Having the whole function disass would enable me to figure out whether the offsets are wrong or they are caused just by some data structure being larger on NetBSD.

  14. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    OK, Just a sec.

  15. krytarowski commented on Feb 17, 2016

    @krytarowski
    krytarowski
    on Feb 17, 2016
    ContributorAuthor

    I've disassembled CoreVsnprintf this way:

    gdb -batch -ex 'file ./bin/obj/NetBSD.x64.Debug/src/pal/tests/palsuite/c_runtime/vsprintf/test11/paltest_vsprintf_test11' -ex 'disassemble CoreVsnprintf'

    The result is accessible here:

    ftp://ftp.netbsd.org/pub/NetBSD/misc/kamil/coreclr-disasm-1.txt

  16. 26 remaining items

  17. janvorli commented on Feb 18, 2016

    @janvorli
    janvorli
    on Feb 18, 2016
    Member

    I have thought we already do the trick for the _vsnprintf, but now I can see we don't. So that should work.

  18. krytarowski commented on Feb 18, 2016

    @krytarowski
    krytarowski
    on Feb 18, 2016
    ContributorAuthor

    Thanks! I will mimic PAL_printf.

  20. krytarowski commented on Feb 18, 2016

    @krytarowski
    krytarowski
    on Feb 18, 2016
    ContributorAuthor

    I will give it a try.. just need to fix dotnet/coreclr#3219 as it broke on NetBSD...

    Thank you.

  21. krytarowski
    added 5 commits that reference this issue on Feb 18, 2016

    Fix issue unveiled on NetBSD: Add PAL__vsnprintf shadow in PAL

    c6caad7

    Fix issue unveiled on NetBSD: Add PAL__vsnprintf shadow in PAL

    ccd8b43

    Fix issue unveiled on NetBSD: Add PAL__vsnprintf shadow in PAL

    3d06e51

    Fix issue unveiled on NetBSD: Add PAL__vsnprintf shadow in PAL

    9bf62f0

    Fix issue unveiled on NetBSD: Add PAL__vsnprintf shadow in PAL

    2f63858
  22. krytarowski
    added 2 commits that reference this issue on Apr 13, 2016

    Fix issue unveiled on NetBSD: Add PAL__vsnprintf shadow in PAL

    e910539

    Fix issue unveiled on NetBSD: Add PAL__vsnprintf shadow in PAL

    fd1f866
  23. jkotas
    closed this as completedin coreclr#3240on Apr 15, 2016
  24. msftgits
    transferred this issue fromdotnet/coreclron Jan 30, 2020
  25. msftgits
    added this to the Future milestone on Jan 30, 2020
  26. ghost locked as resolved and limited conversation to collaborators on Jan 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions