[krytarowski]

NetBSD's libc internally uses a function named _vsnprintf. We need to mask
PAL's version of this function to remove clash with the system. With it
snprintf(3) calls were jumping out of libc and landing back to PAL.

It was unveiled with many PAL tests.

Detailed documentation about _symbols in the NetBSD libc is documented
in the NetBSD sources in a file: src/lib/libc/README

Thanks Jan Vorli (Microsoft) for help with debugging.

Fix #3199

[krytarowski]

@janvorli @jkotas PTAL

[jkotas]

Maybe also delete _vsnprintf implementation in printf.cpp because of it is unused now? (And it can only cause problems by colliding with the system-provided one.)

[krytarowski]

Done!

[janvorli]

By removing the _vsnprintf and exposing the PAL__vsnprintf externally, we have lost the logging (PERFENTRY, ENTRY, ...). It seems it would make sense to put it into the PAL__vsnprintf. It would then probably also make sense to replace all usages of PAL__vsnprintf in PAL (there are 6 of them) by the CoreVsnprintf.
@jkotas what do you think?

[jkotas]

Make sense. Also, there is build break on OSX

[krytarowski]

@janvorli

By removing the _vsnprintf and exposing the PAL__vsnprintf externally, we have lost the logging (PERFENTRY, ENTRY, ...). It seems it would make sense to put it into the PAL__vsnprintf. It would then probably also make sense to replace all usages of PAL__vsnprintf in PAL (there are 6 of them) by the CoreVsnprintf.

To be sure. Please make more clear this part: "It seems it would make sense to put it into the PAL__vsnprintf."

Thank you!

[janvorli]

Look at the deleted _vsnprintf:

int         
 __cdecl        
 _vsnprintf(char *buffer,       
            size_t count,       
            const char *format,         
            va_list argptr)     
 {      
     LONG Length;       

     PERF_ENTRY(_vsnprintf);        
     ENTRY("_vsnprintf (buffer=%p, count=%d, format=%p (%s), argptr=%p)\n",         
           buffer, count, format, format, argptr);      

     Length = PAL__vsnprintf(buffer, count, format, argptr);        

     LOGEXIT("_vsnprintf returns int %d\n", Length);        
     PERF_EXIT(_vsnprintf);     

     return Length;     
 }

What I meant is to add the PERF_ENTRY, ENTRY, LOGEXIT and PERF_EXIT to the PAL__vsnprintf. Just copy them from here and modify the name from _vsnprintf to PAL__vsnprintf.

[krytarowski]

CoreVsnprintf accepts additional parameter of CPalThread *pthrCurrent. Is it a problem to replace PAL__vsnprintf() with CoreVsnprintf()?

[krytarowski]

I can use InternalGetCurrentThread(), is it safe?

[krytarowski]

int
PALAPIV
wsprintfA(
      OUT LPSTR buffer,
      IN LPCSTR format,
      ...)
{
    LONG Length;
    va_list ap;

    PERF_ENTRY(wsprintfA);
    ENTRY("wsprintfA (buffer=%p, format=%p (%s))\n", buffer, format, format);

    va_start(ap, format);
    Length = CoreVsnprintf(InternalGetCurrentThread(), buffer, 1024, format, ap);
    va_end(ap);

For example, is LONG needed here while CoreVsnprintf() returns int?

[janvorli]

Yes, it is safe, since when it was calling PAL__vsnprintf, it was doing the same thing in the PAL__vsnprintf.

[janvorli]

LONG is defined as int, since LONG on windows is 32 bit.

[krytarowski]

OK, I will leave the LONG usage. I'm testing my patch and will upload soon.

[krytarowski]

Done! It might be cleaned up.. but it should be good enough.

[krytarowski]

It failed with tests... before I will be back to .Net, what's the suggested solution? It looks like tests are trying to call _vsnprintf().

[janvorli]

I would try to generate pre-processed version of one of the failing tests to see what's going on. It should be ok for the tests to call _vsnprintf, as it should be defined as PAL__vsnprintf by the pal header, right?

[janvorli]

Actually I can see now that we have InternalVsnprintf exposed in the printfcpp.hpp and it is just a wrapper for the CoreVsnprintf to make it public, so it would be cleaner to use that instead of adding the function prototype.

[krytarowski]

I will do it.

[krytarowski]

It looks like pal.h change has been lost. I will restore it and see what happens.

[krytarowski]

Should be fine now. Let see what buildslaves will report.

[ghost]

OS X build failed: http://dotnet-ci.cloudapp.net/job/dotnet_coreclr/job/checked_osx_prtest/663/console

Undefined symbols for architecture x86_64:
  "__vsnprintf", referenced from:
      sos::ObjectIterator::BuildError(char*, unsigned long, char const*, ...) const in gcroot.cpp.o
      void sos::Throw<sos::HeapCorruption>(char const*, ...) in gcroot.cpp.o
      void sos::Throw<sos::Exception>(char const*, ...) in gcroot.cpp.o
      TableOutput::WriteColumnFormat(int, char const*, ...) in strike.cpp.o
      void sos::Throw<sos::Exception>(char const*, ...) in strike.cpp.o
      void sos::Throw<sos::Exception>(char const*, ...) in sos.cpp.o
      void sos::Throw<sos::DataRead>(char const*, ...) in sos.cpp.o
      ...
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [src/ToolBox/SOS/Strike/libsos.dylib] Error 1
make[1]: *** [src/ToolBox/SOS/Strike/CMakeFiles/sos.dir/all] Error 2
make: *** [all] Error 2
Failed to build coreclr components.

[krytarowski]

@kangaroo would you like to help with MacOSX investigation? Thanks

[jkotas]

@dotnet-bot test this please

[krytarowski]

Thank you, I got exhausted during investigation of this issue and I needed to take rest from it for few days.

[krytarowski]

I don't know MacOSX.. I will leave this for the last pull-request for myself. Unless someone will spot the error.

Thanks!

[krytarowski]

@dotnet-bot test this please

[krytarowski]

@jkotas @janvorli PTAL

MacOSX should be now fixed.

[krytarowski]

ping - this is pretty large commit to maintain locally

[jkotas]

LGTM

[krytarowski]

Thank you!