PInvoke Access Violation with StringBuilder

Question

I am currently testing some PInvoke stuff and wrote a short C function to try some different things out. I successfully managed to pass Ints and return an addition, but I am having some trouble when it comes to strings.

Here is the C function:

__declspec(dllexport) int test(char *str, int slen){
    for(int i = 0; i < slen; i++){
        str[i] = 'a';
    }
    return slen;
}

And here is the C# function declaration and usage:

[DllImport("solver.dll", CharSet = CharSet.Ansi ,CallingConvention = CallingConvention.Cdecl)]
public static extern int test(StringBuilder sol, int len);

StringBuilder sol = new StringBuilder(15);
int ret = test(sol, sol.Capacity);
string str = sol.ToString();

I've been researching this for most of the day and I've seen several posts about simply passing a StringBuilder, filling it on the C end and it should be accessible when the function finishes. However I am currently getting an AccessViolation error in the C code as if the Memory hadn't been allocated, but I definitely allocate the Memory with new StringBuilder(15)

The C function definitely works if I allocate a piece of memory in the C code and pass it. Is there something I am missing?

Code is fine, apart from you not writing the null-terminator. But that wouldn't produce an AV in the C code. My conclusion is that you are not executing this code. Likely you have an out of date DLL that is being used. Start a brand new project and produce a [mcve]. That removes all the guesswork. This is the second time you have attempted to do this, and you are making heavy weather of it. Make a MCVE and all will be revealed. — David Heffernan, Oct 18 '16 at 15:10
I usually do following : sol + "\0"; to add the null zero to terminate a character array. — jdweng, Oct 18 '16 at 15:14
@jdweng No, that's just plain misleading. The onus is on the unmanaged code to properly null-terminate the string. FWIW, almost every comment I see from you in the p/invoke tag is somewhat misleading. — David Heffernan, Oct 18 '16 at 15:16
See [C# PInvoke out strings declaration](http://stackoverflow.com/questions/1994477/c-sharp-pinvoke-out-strings-declaration) — Jackdaw, Oct 18 '16 at 15:16
The missing zero terminator is most certainly a problem and easily explains the AV. The pinvoke marshaller cannot directly use the StringBuilder, the string needs to be converted from Ansi to Unicode. Golden rule applies, if a C function cannot work when called from a C program then it isn't going to get better when you pinvoke it. Use byte[] in your C# declaration or fix the C code. — Hans Passant, Oct 18 '16 at 15:31
@HansPassant Won't the buffer passed to the unmanaged code be zero initialised, given the C# code that we see? Or will there just be a null in the first character? — David Heffernan, Oct 18 '16 at 15:59
@David - it is a bit academic, no zeros could be left after he filled it to capacity. — Hans Passant, Oct 18 '16 at 16:02
@HansPassant The marshaler provides a buffer of length Capacity+1: https://msdn.microsoft.com/en-US/library/s9ts558h%28v=VS.100%29.aspx — David Heffernan, Oct 18 '16 at 16:09
Okay, but it does not break down the distinct cases of being able to pass the StringBuilder buffer as-is and having to provide a temporary buffer because of the required Ansi to Unicode conversion. How the pinvoke marshaller manages those temporary buffers is not so obvious, lots of `_alloca()` calls in fieldmarshaller.cpp. That isn't zero-initialized. — Hans Passant, Oct 18 '16 at 16:33
@HansPassant Fair enough. I suppose I was at least partially confused by the question stating that the error was in the C code. I doubt that the asker really knows where the error is, and an error in the marshaller probably looks just like an error in the C code. — David Heffernan, Oct 18 '16 at 16:35

score 1 · Answer 1 · answered Oct 18 '16 at 16:01

1

Sounds like you are missing to NUL-terminate the string buffer.

You may want to update your code like this:

for (int i = 0; i < (slen-1); i++) {
    str[i] = 'a';
}
str[slen-1] = '\0'; // Add a NUL-terminator

Note that in this case I'm assuming the buffer length passed by the caller is the total length, including room for the NUL-terminator.

(Other conventions are possible as well, for example assuming the length passed by the caller excludes the NUL-terminator; the important thing is clarity of the interface documentation.)

I'd also like to add that a usual calling convention for those exported functions is __stdcall instead of __cdecl (although that's not correlated to your problem).

answered Oct 18 '16 at 16:01

Mr.C64

41,637
14
86
162

The length passed does not include the character reserved for null terminator. So you can leave the original code as is, and just add `str[slen] = 0`. – David Heffernan Oct 18 '16 at 16:17
@DavidHeffernan: Would that be something that the OP is free to specify at the interface level? The important thing is to be clear if the length parameter includes or not room for the NUL. Or am I missing something? – Mr.C64 Oct 18 '16 at 16:20
You are right. The OP needs to define the interface clearly and stick to it. If it were me, I would define it such that I could simply pass `sb.Capacity`. – David Heffernan Oct 18 '16 at 16:27
@DavidHeffernan: The problem is that in C (or C++ _"C-ish"_) code, when there are raw buffers and their lengths, I prefer using things like `sizeof` or `countof` on the _whole_ buffer, so I tend towards my first interpretation (e.g. pass whole buffer length, including room for NUL). – Mr.C64 Oct 18 '16 at 16:29
That's a fair point, and that's how the Win32 API does it, so I take back what I said. Better to follow those patterns. – David Heffernan Oct 18 '16 at 16:34
@DavidHeffernan: No problem. – Mr.C64 Oct 18 '16 at 16:35

PInvoke Access Violation with StringBuilder

1 Answers1