Fix incorrect format string arguments

[GrabYourPitchforks]

Handles issues like wide-char strings being passed as %s, narrow-char strings being passed as %S, 64-bit integers being passed as %d, etc. It's likely we'll need to make a second pass at these in a few weeks so that we can cover other configurations, but this is an initial list of matches from my dev box.

Fixes violation of CodeQL rule cpp/wrong-type-format-argument (https://lgtm.com/rules/2160310550/).

[ghost]

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Handles issues like wide-char strings being passed as %s, narrow-char strings being passed as %S, 64-bit integers being passed as %d, etc. It's likely we'll need to make a second pass at these in a few weeks so that we can cover other configurations, but this is an initial list of matches from my dev box.

Fixes violation of CodeQL rule cpp/wrong-type-format-argument (https://lgtm.com/rules/2160310550/).

Author:	GrabYourPitchforks
Assignees:	-
Labels:	area-Host
Milestone:	-

[GrabYourPitchforks]

Clearly that worked better on my dev box than in CI.

[GrabYourPitchforks]

This is blocked by #67470. I can work around it for now by creating a custom header definition, but this isn't a workable long-term solution since I'll eventually need to import other C++ headers like <type_traits> while working on other CodeQL rules.

[GrabYourPitchforks]

@dotnet/macios Do you have thoughts on why the OSX build might be failing? When importing <cstdint>, this is the dump I see (full log here).

/Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk/usr/include/c++/v1/cstdint:162:8: error: no member named 'int_least8_t' in the global namespace
using::int_least8_t;
     ~~^
/Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk/usr/include/c++/v1/cstdint:163:8: error: no member named 'int_least16_t' in the global namespace
using::int_least16_t;
     ~~^
/Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk/usr/include/c++/v1/cstdint:164:8: error: no member named 'int_least32_t' in the global namespace
using::int_least32_t;
     ~~^
/Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk/usr/include/c++/v1/cstdint:165:8: error: no member named 'int_least64_t' in the global namespace
using::int_least64_t;
     ~~^
...

As far as I'm aware there's nothing special going on here; all I'm doing is referencing a well-known C++ header file.

The Linux builds are failing for a similar reason.

[jkotas]

Do you have thoughts on why the OSX build might be failing?

You cannot use standard C++ headers under src\coreclr today. Most of the code under src/corclr can only use CoreCLR PAL (ie headers under src/coreclr/pal/inc).

This is what needs to happen to remove that restriction: #37310 (comment)

[jkotas]

cc @AaronRobinsonMSFT

[jkotas]

The tests are failing with:

      Assert failure(PID 18953 [0x00004a09], Thread: 18968 [0x4a18]): Consistency check failed: _vsnwprintf_s failed. Potential globalization bug.FAILED: 0
          File: /__w/1/s/src/coreclr/utilcode/sstring.cpp Line: 2114
          Image: /datadisks/disk1/work/AA1009BA/p/corerun

[jkotas]

Suggested change

	result.Printf("%s (message resource %S)",
	result.Printf(W("%S (message resource %s)"),

This may fix the test failure.

[GrabYourPitchforks]

Thanks! Was using this as a good excuse to learn how to use runfo. 😊

[GrabYourPitchforks]

The stack trace shows that the failure is this line:

- strArgument.Printf(W("0x%x"), pInstruction->uArg);
+ strArgument.Printf(W("0x%zx"), pInstruction->uArg);

But since ILInstruction.uArg is typed as UINT_PTR, it seems like the new line should be correct?

I'm pretty sure that in context, this value always fits into 32 bits anyway, so maybe the answer is simply to cast back to uint32 and leave the original format string as-is.

[jkotas]

The assert says that _vsnwprintf_s failed with unexpected error. Notice that all failures with this error are on non-Windows. I think it means that _vsnwprintf_s implementation in the Unix PAL is missing handling of zx format string.

[GrabYourPitchforks]

Would it be ok for me to tweak https://github.com/dotnet/runtime/blob/main/src/coreclr/pal/src/safecrt/output.inl to recognize this pattern as part of this PR? I could also just use I since that's recognized, but it might make the call site easier to understand if I support using the standards-compliant z.

Edit: Looks like we'd need some more generalized cleanup throughout this if we wanted to support standard syntaxes. I'll just use I for now since it doesn't add much debt to what's already there.

[GrabYourPitchforks]

@jkotas @AaronRobinsonMSFT Any final thoughts? The tip that the coreclr subdir uses a special copy of the CRT functions really helped unblock me on this. 👍

[jkotas]

LGTM