Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FormatMessage lpBuffer not marked as ref/out #537

Closed
eksime opened this issue May 1, 2022 · 4 comments
Closed

FormatMessage lpBuffer not marked as ref/out #537

eksime opened this issue May 1, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@eksime
Copy link

eksime commented May 1, 2022

Actual behavior

When generated, the lpBuffer parameter of FormatMessage is incorrectly generated as just a plain LPTSTR, this prevents the use of FORMAT_MESSAGE_ALLOCATE_BUFFER as the allocated buffer should be written to the lpBuffer param.

Expected behavior

When the FORMAT_MESSAGE_ALLOCATE_BUFFER flag is specified in dwFlags, the lpBuffer param should receive the address of the allocated buffer - as it is not passed by ref / as an out, the function instead returns error 87 parameter incorrect.

Repro steps

  1. NativeMethods.txt content:
NTSTATUS
LocalFree
LoadLibrary
FormatMessage
  1. NativeMethods.json content (if present):
    No content.

  2. Any of your own code that should be shared?

The following code will reproduce the issue, originally found when trying to get an error from a return value from NtQueryInformationProcess - the status code below has been copied from that & the expected string result from the GetErrorMessage function should be "The specified information record length does not match the length required for the specified information class."

static unsafe string GetErrorMessage(NTSTATUS status)
{
    PWSTR lpBuffer = default;
    using (FreeLibrarySafeHandle hModule = PInvoke.LoadLibrary("ntdll.dll"))
    {
        try
        {
            PInvoke.SetLastError(WIN32_ERROR.NO_ERROR);
            uint result = PInvoke.FormatMessage(
                FORMAT_MESSAGE_OPTIONS.FORMAT_MESSAGE_ALLOCATE_BUFFER |
                FORMAT_MESSAGE_OPTIONS.FORMAT_MESSAGE_FROM_SYSTEM |
                FORMAT_MESSAGE_OPTIONS.FORMAT_MESSAGE_FROM_HMODULE | 
                FORMAT_MESSAGE_OPTIONS.FORMAT_MESSAGE_IGNORE_INSERTS,
                hModule.DangerousGetHandle().ToPointer(),
                status,
                0,
                lpBuffer, // this should be a ref
                0,
                null);
            
            if (result == 0)
            {
                throw new Win32Exception();
            }
            
            return new string(lpBuffer.Value, 0, (int)result);
        } 
        finally
        {
            PInvoke.LocalFree((IntPtr)lpBuffer.Value);
        }
    }
}
// ...
NTSTATUS status = new NTSTATUS(-1073741820);
string err = GetErrorMessage(status);

Context

  • CsWin32 version: 0.1.647-beta
  • Win32Metadata version: not explicitly set
  • Target Framework: net472
  • LangVersion: 8
@eksime eksime added the bug Something isn't working label May 1, 2022
@raffaeler
Copy link

Formally, I believe there should be an overload taking a void* for that parameter.

@AArnott
Copy link
Member

AArnott commented Oct 5, 2022

I don't think the emitted code is inaccurate, since it matches the headers for this function. And it's functional as is, as demonstrated by this (working) code:

using System.ComponentModel;
using Windows.Win32;
using Windows.Win32.Foundation;
using Windows.Win32.System.Diagnostics.Debug;

unsafe
{
    PWSTR formattedMessage;
    fixed (char* lpSource = "Hello, World!")
    {
        uint length = PInvoke.FormatMessage(
             FORMAT_MESSAGE_OPTIONS.FORMAT_MESSAGE_ALLOCATE_BUFFER |
             FORMAT_MESSAGE_OPTIONS.FORMAT_MESSAGE_FROM_STRING,
             lpSource,
             0,
             0,
             (PWSTR)(void*)&formattedMessage,
             0,
             null);
        if (length == 0)
        {
            throw new Win32Exception();
        }

        try
        {
            Console.WriteLine($"Result: {formattedMessage}");
        }
        finally
        {
            PInvoke.LocalFree((nint)formattedMessage.Value);
        }
    }
}

The cast on lpBuffer is pretty close to the cast that is documented that even C++ folks have to do, and our goal is mostly to match the C++ experience, adding C# idioms in generalizable ways or in a few high value places.

Now I'd like to show a sample of how to do something more interesting with FormatMessage, where we provide formatting args, for example. If I can figure out how to do that, I'll let you know.

@AArnott
Copy link
Member

AArnott commented Oct 5, 2022

I think #665 will be required to provide formatting args.

@AArnott AArnott closed this as not planned Won't fix, can't repro, duplicate, stale Oct 5, 2022
@AArnott
Copy link
Member

AArnott commented Oct 5, 2022

It turns out that #665 is irrelevant, as that deals with varargs and FormatMessage uses va_list*, which is a C++ specific trick. To make this work in C#, you have to specify the FORMAT_MESSAGE_ARGUMENT_ARRAY flag, and follow the documented rules for doing so. I got a "Hello, World!" message to be printed including a formatting arg with the following code:

unsafe
{
    PWSTR formattedMessage;
    Span<IntPtr> formattingArgs = stackalloc IntPtr[1];
    fixed (char* lpSource = "Hello, %1%!")
    fixed (char* lpWorld = "World")
    fixed (IntPtr* pFormattingArgs = formattingArgs)
    {
        formattingArgs[0] = (IntPtr)lpWorld;
        uint length = PInvoke.FormatMessage(
             FORMAT_MESSAGE_OPTIONS.FORMAT_MESSAGE_ALLOCATE_BUFFER |
             FORMAT_MESSAGE_OPTIONS.FORMAT_MESSAGE_FROM_STRING | 
             FORMAT_MESSAGE_OPTIONS.FORMAT_MESSAGE_ARGUMENT_ARRAY,
             lpSource,
             0,
             0,
             (PWSTR)(void*)&formattedMessage,
             0,
             (sbyte**)pFormattingArgs);
        if (length == 0)
        {
            throw new Win32Exception();
        }

        try
        {
            Console.WriteLine($"Result: {formattedMessage}");
        }
        finally
        {
            PInvoke.LocalFree((nint)formattedMessage.Value);
        }
    }
}

Presumably this should scale to multiple formatting args, but the documented spec for the %1 use is quite confusing, so I'll call it good here. This is crazy complex, but then, I guess I can't expect much better from such a 'dynamic' native method being called from a very different language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants