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
Incorrect wrapping of go strings #850
Comments
Anybody taking on this bug? It's almost been two months since I reported it. And it's not a low-priority bug that can be put off indefinitely. It's pretty serious. And there's a solution as I described. It shouldn't take more than 10 minutes to add the code to Lib/go/std_string.i and test it. |
If you think you have the fix, submit a PR and it'll get automatically tested. |
I believe this was fixed in 9cd3e28 |
This means that the 3.0.9 release fixed this. |
@hzarnani Please can you confirm whether this is fixed in 3.0.9 or newer? |
I will check that ASAP (by tomorrow if not today). Thank you. |
@hzarnani ping |
Looking at the code now generated for the example above, we now make a copy of the string using |
Synopsis
cgo exports Go strings as a C struct that has a pointer to the buffer and the size of that string:
There are cases where swig wraps Go strings by just passing on the
p
field and ignoring the size fieldn
. I reckon that's based on the (false) assumption that Go strings are null-terminated. This causes problems at least in two cases -- with string literals (constant strings that get emitted in the text section of a binary) and with calling string processors such as thestrings.Split
function. In both cases, the C code tries to interpret the buffer passed in much beyond the end of the actual Go string. If lucky, there's a'\0'
character somewhere and the string interpretation ends without a crash. However, I have seen crashes (non-deterministically based on how Go's code generation decides to arrange string literals, or where the Go runtime environment places a Go string).Reproducing the Bug
The following demonstrates a non-crashing example of the above-mentioned problem.
We begin by showing the version of swig/go and the platform on which it was run.
Consider a simple C++ library exported in header bam.hpp that defines a function
bam
which takes as input a C string and prints it to standard output. We define a swig interface main_simple.i that includes the entire header and generates wrappers. (We also define main_alloc_str.i which is used later to show that it solves the problem.) Finally, a Go program is defined that tokenizes the first argument to the program (with comma as delimiter) and passes the first token tobam
to print it. (If no argument is specified, a default string"foo,bar,bam"
is used.) Below is the source code. Note that the only difference between the two *.i files is that the longer one extends the simpler one by overriding howchar *
arguments are dealt with.bam.hpp contains:
main_simple.i contains:
call_bam.go contains:
The following is expected to print only
1st
on the screen but prints the whole argument.That's because in the Go program,
csv
'sp
points to a buffer"1st,2nd,3rd"
with ann
value of 11, and the output ofstrings.Split
points to the same buffer (has the samep
as that ofcsv
) and only differs in itsn
-- 3.The problem is much worse if a string literal in the program is passed to
bam
.As can be seen, there was a very large, multi-line string constant in which
"foo,bar,bam"
was contained, and there's a lot of stuff that follows it.The problem is due to swig ignoring the length of the go string and passing the
p
pointer directly down tobam
.Possible Fix
If we actually take into account the size of the string, we get the expected results both in interpreting the string literal as well as the argument passed in from the command line.
That's because the wrappers used
strncpy
:A Better Solution
... probably involves inserting a call to cgo's
C.CString
and calling stdlib'sfree
viaC.free
in the generated Go code instead of allocating and deallocating the string in the C wrapper.The text was updated successfully, but these errors were encountered: