access violation reading location when passing pointers to function in DLL written in c

Question

I am trying to call a function that converts sql queries between dialects. I am using an open-source project's DLL to use the functions for conversion in a C# university project. The problem i am facing is that i am getting a access violation reading location.

I've read some posts here on stack overflow which suggest that there might be a bad pointer somewhere, but i cannot find where. My pointers are not corrupt

The function for the conversion is this :

       int ConvertSql(void *parser, const char *input, int64_t size, const char 
       **output, int64_t *out_size, int64_t *lines)
        {
            if(parser == NULL)
                return -1;

            SqlParser *sql_parser = (SqlParser*)parser;

            // Run conversion
            sql_parser->Convert(input, size, output, out_size, lines);

            return 0;
        }

I am calling the function in C#

                        char *parentInput;
                        fixed(char *input = &inputStr.ToCharArray()[0])
                        {
                            parentInput = input;
                        }

                        char** output = null;
                        Int64 out_size = 0;
                        Int64 lines = 0;
                        Int64 size = inputStr.Length;
                        Console.WriteLine(new IntPtr(&out_size)+"  "+ new IntPtr(&lines)+"  "+new IntPtr(&parserObj)+"   "+new IntPtr(output));
                        int result = ConvertSql(&parserObj, intputStr, size, output, &out_size, &lines);

i get my parser object from this code which works without errors:

IntPtr parserObj = CreateParserObject();

the dllimport for the funtions are using this code:

[DllImport(dllName: "PATHTODLLFOLDER\\sqlparser.dll", EntryPoint = "CreateParserObject", CallingConvention = CallingConvention.Cdecl)]
public static extern IntPtr CreateParserObject();

[DllImport(dllName: "PATHTODLLFOLDER\\sqlparser.dll", EntryPoint = "ConvertSql", CallingConvention = CallingConvention.Cdecl)]
public unsafe static extern int ConvertSql(void *parser, String input, Int64 size, char **output, Int64 *out_size, Int64 *lines);

Answer 1

In .NET, calling an unmanaged method through P/invoke (which is what happens when you call an extern method) involves various type conversions on the parameters, which is known as "marshalling" and is done automatically by a part of the runtime known as the "marshaller".

In general, it's a terrible idea to marshal pointers. Instead, use the CLR marshaller's ability to convert certain types to pointers for you by changing the signature of your P/invoked method:

// split on multiple lines for readability
[DllImport("PATHTODLLFOLDER\\sqlparser.dll", EntryPoint = "ConvertSql", CallingConvention = CallingConvention.Cdecl)]
public static extern int ConvertSql(
    IntPtr parser, 
    [MarshalAs(UnmanagedType.LPStr)] string input,
    long size, 
    out IntPtr output, // see explanation below
    out long out_size, 
    out long lines);

A few things about the above. First, I took the liberty of using the C# type aliases (string, long), because that's more idiomatic C#, but it doesn't change the behavior. Also, since there are no pointers anymore, no need for unsafe.

First, I declared parser as an IntPtr because those get converted to void* automatically if needed and that's what's already returned by CreateParserObject() anyway.

Second, out parameters can be converted to pointers to uninitialized objects, so by marking both out_size and lines as out, you fix that other problem.

The input is a string, which has a specific format in .NET. Since your function takes a const char*, you need to tell the marshaller how to convert the characters. That's where the MarshalAs attribute comes in: it's when the default conversion doesn't work for you. UnmanagedType.LPStr means char* in this case, so the string gets converted. The runtime will manage the memory for you.

But here we hit a big snag in the road: the output. When marshalling things, there's always questions about lifetime, or, specifically: who releases the memory? The fact that output is a char** implies that the parser allocates a block of memory and then returns it through that, which means the caller has to release it. From the get go, that reeks of bad C++ design because the caller doesn't know how the memory was allocated. Was it with malloc? new[]? Platform specific APIs like LocalAlloc? Is it a pointer to a block of static memory? Usually these APIs come with documentation telling precisely what to do with the pointer once you're done with it. Good C++ APIs return smart pointers, or ask that the caller pass a block of previously allocated memory that they can then play with.

But, this is the thing you're playing with, so here's how to make it work. First, you'd think that you could declare output as [MarshalAs(UnmanagedType.LPStr)] out string: the marshaller will copy the characters into a managed string and return it... but then the native string's (on the C++ side) memory will leak because the runtime doesn't know how the string was allocated, so it prefers to not do anything about it. Also, this assumes the string is null-terminated, which might not always be the case.

So, another option would be to instead declare output as out IntPtr. Then you can use Marshal.PtrToStringAnsi to convert your pointer into a string, and then release it... but you'll need to know how it was allocated first.

Putting it all together:

var parserObj = CreateParserObject();

var output = IntPtr.Zero;
try
{
    long lines;
    long out_size;
    int result = ConvertSql(parserObj, inputStr, inputStr.Length, out output, out out_size, out lines);

    var outputStr = Marshal.PtrToStringAnsi(output, (int)out_size);
    // do what you want with outputStr here
}
finally
{
    if (output != IntPtr.Zero)
    {
        // release output here
    }
}

Oh, also, one final thought: whatever is returned by CreateParserObject() will probably have to be freed at one point. You're probably going to need another function for it, probably like:

[DllImport(/* ... */)]
public static extern void DestroyParserObject(IntPtr parserObject);

It might even already exist in your DLL.

Good luck!