Skip to content

Commit b3a7cd0

Browse files
authored
Much improved support for marshaling const char* to string
* Changed support for marshaling "const char *"s as strings so that: 1) CppSharp.Types.Std.CSharpMarshalToNative generates setters that allocate unmanaged memory for the native bytes rather than an unpinned pointer into managed memory. Also set a null termination char. 2) Add tracking for when unmanaged memory is allocated for "const char *" strings. Free it when reassigned or dispsoed. 3) Added explicit support for Encoding.Default for ANSI support. 4) Allow setting a string value to null. * Added test to prove that the unpinned ptr to managed memory approach wasn't working, and that the new approach appears to work. * Change CSharpSources.GenerateDisposeMethods to free unmanaged memory held by IntPtr's corresponding to "const char *" strings. * Changed copy constructor to deep-copy owned string refs to avoid ref counting. * Update CSharpSources.GenerateFieldGetter to treat Char16 and Char32 the same as WideChar to avoid compilation errors on the generated sources if char32_t or char16_t are used. * Added tests. * Workaround for mac C++ compilation issue.
1 parent 03bf194 commit b3a7cd0

File tree

5 files changed

+329
-29
lines changed

5 files changed

+329
-29
lines changed

src/Generator/Generators/CSharp/CSharpSources.cs

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,12 @@ public override bool VisitClassDecl(Class @class)
455455
var dict = $@"global::System.Collections.Concurrent.ConcurrentDictionary<IntPtr, {
456456
printedClass}>";
457457
WriteLine("internal static readonly {0} NativeToManagedMap = new {0}();", dict);
458+
459+
// Add booleans to track who owns unmanaged memory for string fields
460+
foreach (var field in @class.Layout.Fields.Where(f => f.QualifiedType.Type.IsConstCharString()))
461+
{
462+
WriteLine($"private bool __{field.Name}_OwnsNativeMemory = false;");
463+
}
458464
}
459465
PopBlock(NewLineKind.BeforeNextBlock);
460466
}
@@ -871,7 +877,7 @@ private void GenerateClassField(Field field, bool @public = false)
871877
PopBlock(NewLineKind.BeforeNextBlock);
872878
}
873879

874-
#endregion
880+
#endregion
875881

876882
private void GeneratePropertySetter<T>(T decl,
877883
Class @class, bool isAbstract = false, Property property = null)
@@ -1411,13 +1417,17 @@ private void GenerateFieldGetter(Field field, Class @class, QualifiedType return
14111417
if (templateSubstitution != null && returnType.Type.IsDependent)
14121418
Write($"({templateSubstitution.ReplacedParameter.Parameter.Name}) (object) ");
14131419
if ((final.IsPrimitiveType() && !final.IsPrimitiveType(PrimitiveType.Void) &&
1414-
(!final.IsPrimitiveType(PrimitiveType.Char) &&
1415-
!final.IsPrimitiveType(PrimitiveType.WideChar) ||
1420+
((!final.IsPrimitiveType(PrimitiveType.Char) &&
1421+
!final.IsPrimitiveType(PrimitiveType.WideChar) &&
1422+
!final.IsPrimitiveType(PrimitiveType.Char16) &&
1423+
!final.IsPrimitiveType(PrimitiveType.Char32)) ||
14161424
(!Context.Options.MarshalCharAsManagedChar &&
14171425
!((PointerType) field.Type).QualifiedPointee.Qualifiers.IsConst)) &&
14181426
templateSubstitution == null) ||
14191427
(!((PointerType) field.Type).QualifiedPointee.Qualifiers.IsConst &&
1420-
final.IsPrimitiveType(PrimitiveType.WideChar)))
1428+
(final.IsPrimitiveType(PrimitiveType.WideChar) ||
1429+
final.IsPrimitiveType(PrimitiveType.Char16) ||
1430+
final.IsPrimitiveType(PrimitiveType.Char32))))
14211431
Write($"({field.Type.GetPointee().Desugar()}*) ");
14221432
}
14231433
WriteLine($"{@return};");
@@ -1626,7 +1636,7 @@ private void GenerateVariable(Class @class, Variable variable)
16261636
PopBlock(NewLineKind.BeforeNextBlock);
16271637
}
16281638

1629-
#region Virtual Tables
1639+
#region Virtual Tables
16301640

16311641
public List<VTableComponent> GetUniqueVTableMethodEntries(Class @class)
16321642
{
@@ -2033,9 +2043,9 @@ public bool HasVirtualTables(Class @class)
20332043
return @class.IsGenerated && @class.IsDynamic && GetUniqueVTableMethodEntries(@class).Count > 0;
20342044
}
20352045

2036-
#endregion
2046+
#endregion
20372047

2038-
#region Events
2048+
#region Events
20392049

20402050
public override bool VisitEvent(Event @event)
20412051
{
@@ -2143,9 +2153,9 @@ private void GenerateEventRaiseWrapper(Event @event, string delegateInstance)
21432153
UnindentAndWriteCloseBrace();
21442154
}
21452155

2146-
#endregion
2156+
#endregion
21472157

2148-
#region Constructors
2158+
#region Constructors
21492159

21502160
public void GenerateClassConstructors(Class @class)
21512161
{
@@ -2266,6 +2276,21 @@ c is ClassTemplateSpecialization ?
22662276
}
22672277
}
22682278

2279+
// If we have any fields holding references to unmanaged memory allocated here, free the
2280+
// referenced memory. Don't rely on testing if the field's IntPtr is IntPtr.Zero since
2281+
// unmanaged memory isn't always initialized and/or a reference may be owned by the
2282+
// native side.
2283+
//
2284+
// TODO: We should delegate to the dispose methods of references we hold to other
2285+
// generated type instances since those instances could also hold references to
2286+
// unmanaged memory.
2287+
foreach (var field in @class.Layout.Fields.Where(f => f.QualifiedType.Type.IsConstCharString()))
2288+
{
2289+
var ptr = $"(({Helpers.InternalStruct}*){Helpers.InstanceIdentifier})->{field.Name}";
2290+
WriteLine($"if (__{field.Name}_OwnsNativeMemory)");
2291+
WriteLineIndent($"Marshal.FreeHGlobal({ptr});");
2292+
}
2293+
22692294
WriteLine("if ({0})", Helpers.OwnsNativeInstanceIdentifier);
22702295
WriteLineIndent("Marshal.FreeHGlobal({0});", Helpers.InstanceIdentifier);
22712296

@@ -2482,9 +2507,9 @@ private void GenerateClassConstructorBase(Class @class, Method method)
24822507
WriteLineIndent(": this()");
24832508
}
24842509

2485-
#endregion
2510+
#endregion
24862511

2487-
#region Methods / Functions
2512+
#region Methods / Functions
24882513

24892514
public void GenerateFunction(Function function, string parentName)
24902515
{
@@ -2898,6 +2923,21 @@ private void GenerateClassConstructor(Method method, Class @class)
28982923
var classInternal = TypePrinter.PrintNative(@class);
28992924
WriteLine($@"*(({classInternal}*) {Helpers.InstanceIdentifier}) = *(({
29002925
classInternal}*) {method.Parameters[0].Name}.{Helpers.InstanceIdentifier});");
2926+
2927+
// Copy any string references owned by the source to the new instance so we
2928+
// don't have to ref count them.
2929+
foreach (var field in @class.Fields.Where(f => f.QualifiedType.Type.IsConstCharString()))
2930+
{
2931+
var prop = @class.Properties.Where(p => p.Field == field).FirstOrDefault();
2932+
// If there is no property or no setter then this instance can never own the native
2933+
// memory. Worry about the case where there's only a setter (write-only) when we
2934+
// understand the use case and how it can occur.
2935+
if (prop != null && prop.HasGetter && prop.HasSetter)
2936+
{
2937+
WriteLine($"if ({method.Parameters[0].Name}.__{field.OriginalName}_OwnsNativeMemory)");
2938+
WriteLineIndent($@"this.{prop.Name} = {method.Parameters[0].Name}.{prop.Name};");
2939+
}
2940+
}
29012941
}
29022942
}
29032943
else
@@ -3230,7 +3270,7 @@ private string FormatMethodParameters(IEnumerable<Parameter> @params)
32303270
return TypePrinter.VisitParameters(@params, true).Type;
32313271
}
32323272

3233-
#endregion
3273+
#endregion
32343274

32353275
public override bool VisitTypedefNameDecl(TypedefNameDecl typedef)
32363276
{

src/Generator/Types/Std/Stdlib.CSharp.cs

Lines changed: 69 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.Collections.Generic;
22
using System.Linq;
3+
using System.Runtime.InteropServices;
34
using System.Text;
45
using CppSharp.AST;
56
using CppSharp.AST.Extensions;
@@ -96,15 +97,18 @@ public override Type CSharpSignatureType(TypePrinterContext ctx)
9697
return new CustomType(typePrinter.IntPtrType);
9798
}
9899

99-
var (enconding, _) = GetEncoding();
100+
var (encoding, _) = GetEncoding();
100101

101-
if (enconding == Encoding.ASCII)
102-
return new CustomType("[MarshalAs(UnmanagedType.LPStr)] string");
103-
else if (enconding == Encoding.UTF8)
102+
if (encoding == Encoding.ASCII || encoding == Encoding.Default)
103+
// This is not really right. ASCII is 7-bit only - the 8th bit is stripped; ANSI has
104+
// multi-byte support via a code page. MarshalAs(UnmanagedType.LPStr) marshals as ANSI.
105+
// Perhaps we need a CppSharp.Runtime.ASCIIMarshaller?
106+
return new CustomType("[MarshalAs(UnmanagedType.LPStr)] string");
107+
else if (encoding == Encoding.UTF8)
104108
return new CustomType("[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(CppSharp.Runtime.UTF8Marshaller))] string");
105-
else if (enconding == Encoding.Unicode || enconding == Encoding.BigEndianUnicode)
109+
else if (encoding == Encoding.Unicode || encoding == Encoding.BigEndianUnicode)
106110
return new CustomType("[MarshalAs(UnmanagedType.LPWStr)] string");
107-
else if (enconding == Encoding.UTF32)
111+
else if (encoding == Encoding.UTF32)
108112
return new CustomType("[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(CppSharp.Runtime.UTF32Marshaller))] string");
109113

110114
throw new System.NotSupportedException(
@@ -129,14 +133,63 @@ public override void CSharpMarshalToNative(CSharpMarshalContext ctx)
129133
if (substitution != null)
130134
param = $"({substitution.Replacement}) (object) {param}";
131135

132-
string bytes = $"__bytes{ctx.ParameterIndex}";
133-
string bytePtr = $"__bytePtr{ctx.ParameterIndex}";
134-
ctx.Before.WriteLine($@"byte[] {bytes} = global::System.Text.Encoding.{
135-
GetEncoding().Name}.GetBytes({param});");
136-
ctx.Before.WriteLine($"fixed (byte* {bytePtr} = {bytes})");
137-
ctx.HasCodeBlock = true;
138-
ctx.Before.WriteOpenBraceAndIndent();
139-
ctx.Return.Write($"new global::System.IntPtr({bytePtr})");
136+
// Allow setting native field to null via setter property.
137+
if (ctx.MarshalKind == MarshalKind.NativeField)
138+
{
139+
// Free memory if we're holding a pointer to unmanaged memory that we (think we)
140+
// allocated. We can't simply compare with IntPtr.Zero since the reference could be
141+
// owned by the native side.
142+
143+
// TODO: Surely, we can do better than stripping out the name of the field using
144+
// string manipulation on the ReturnVarName, but I don't see it yet. Seems like it
145+
// would be really helpful to have ctx hold a Decl property representing the
146+
// "appropriate" Decl when we get here. When MarshalKind == NativeField, Decl would
147+
// be set to the Field we're operating on.
148+
var fieldName = ctx.ReturnVarName.Substring(ctx.ReturnVarName.LastIndexOf("->") + 2);
149+
150+
ctx.Before.WriteLine($"if (__{fieldName}_OwnsNativeMemory)");
151+
ctx.Before.WriteLineIndent($"Marshal.FreeHGlobal({ctx.ReturnVarName});");
152+
ctx.Before.WriteLine($"__{fieldName}_OwnsNativeMemory = true;");
153+
ctx.Before.WriteLine($"if ({param} == null)");
154+
ctx.Before.WriteOpenBraceAndIndent();
155+
ctx.Before.WriteLine($"{ctx.ReturnVarName} = global::System.IntPtr.Zero;");
156+
ctx.Before.WriteLine("return;");
157+
ctx.Before.UnindentAndWriteCloseBrace();
158+
}
159+
160+
var bytes = $"__bytes{ctx.ParameterIndex}";
161+
var bytePtr = $"__bytePtr{ctx.ParameterIndex}";
162+
var encodingName = GetEncoding().Name;
163+
164+
switch (encodingName)
165+
{
166+
case nameof(Encoding.Unicode):
167+
ctx.Before.WriteLine($@"var {bytePtr} = Marshal.StringToHGlobalUni({param});");
168+
break;
169+
case nameof(Encoding.Default):
170+
ctx.Before.WriteLine($@"var {bytePtr} = Marshal.StringToHGlobalAnsi({param});");
171+
break;
172+
default:
173+
{
174+
var encodingBytesPerChar = GetCharWidth() / 8;
175+
var writeNulMethod = encodingBytesPerChar switch
176+
{
177+
1 => nameof(Marshal.WriteByte),
178+
2 => nameof(Marshal.WriteInt16),
179+
4 => nameof(Marshal.WriteInt32),
180+
_ => throw new System.NotImplementedException(
181+
$"Encoding bytes per char: {encodingBytesPerChar} is not implemented.")
182+
};
183+
184+
ctx.Before.WriteLine($@"var {bytes} = global::System.Text.Encoding.{encodingName}.GetBytes({param});");
185+
ctx.Before.WriteLine($@"var {bytePtr} = Marshal.AllocHGlobal({bytes}.Length + {encodingBytesPerChar});");
186+
ctx.Before.WriteLine($"Marshal.Copy({bytes}, 0, {bytePtr}, {bytes}.Length);");
187+
ctx.Before.WriteLine($"Marshal.{writeNulMethod}({bytePtr} + {bytes}.Length, 0);");
188+
}
189+
break;
190+
}
191+
192+
ctx.Return.Write($"{bytePtr}");
140193
}
141194

142195
public override void CSharpMarshalToManaged(CSharpMarshalContext ctx)
@@ -168,6 +221,8 @@ public override void CSharpMarshalToManaged(CSharpMarshalContext ctx)
168221
switch (GetCharWidth())
169222
{
170223
case 8:
224+
if (Context.Options.Encoding == Encoding.Default) // aka ANSI with system default code page
225+
return (Context.Options.Encoding, nameof(Encoding.Default));
171226
if (Context.Options.Encoding == Encoding.ASCII)
172227
return (Context.Options.Encoding, nameof(Encoding.ASCII));
173228
if (Context.Options.Encoding == Encoding.UTF8)

0 commit comments

Comments
 (0)