Skip to content

Commit b33cfec

Browse files
PathogenDavidPathogenDavid
committed
Reworked how function/method ABI concerns are handled to take advantage of Clang's code generation module.
In short, this change replaces ClangSharpExtensions.[Record]MustBePassedByReference with ClangSharp.Pathogen.PathogenArrangedFunction, which uses Clang's code generation module to determine how a function call should be constructed. This moves Biohazrd's handling of whether a type is returned/passed by reference from TranslatedRecord to TranslatedFunction and TranslatedParameter. This change does not include support for function pointers or vtable entries as those will require extra effort. (VTables need to be reworked to fix #147 first. Function pointers don't support most ABI concerns in general #186.) Fixes #35 (Fixed leaking return buffer semantics on global functions and static methods. Fixed as a side-effect.) Contributes to #53 (Cleaning up ABI handling.) Contributes to #108 (Linux support.) Fixes #187 (Records with constructors not being returned by reference on Microsoft x64.) Fixes #135 (Indirectly since this problematic method was obsoleted.)
1 parent 357faf9 commit b33cfec

12 files changed

+525
-60
lines changed

‎Biohazrd.CSharp/#Transformations/CSharpTranslationVerifier.cs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using Biohazrd.CSharp.Metadata;
22
using Biohazrd.Expressions;
33
using Biohazrd.Transformation;
4+
using ClangSharp.Pathogen;
45
using System.Collections.Immutable;
56
using System.Linq;
67

@@ -126,6 +127,51 @@ protected override TransformationResult TransformFunction(TransformationContext
126127
if (declaration.IsVirtual && declaration.Metadata.Has<SetLastErrorFunction>())
127128
{ declaration = declaration.WithWarning("SetLastError is not supported on virtual methods and will be ignored."); }
128129

130+
// Check for ABI corner cases we don't expect/handle
131+
// In theory some of the x86 calling conventions work, but they're untested so let's complain if they get used at the code gen level.
132+
if (declaration.FunctionAbi.CallingConvention != PathogenLlvmCallingConventionKind.C)
133+
{ declaration = declaration.WithWarning($"ABI: LLVM calling convention {declaration.FunctionAbi.CallingConvention} may not be handled correctly."); }
134+
else if (declaration.FunctionAbi.EffectiveCallingConvention != PathogenLlvmCallingConventionKind.C)
135+
{ declaration = declaration.WithWarning($"ABI: Effective LLVM calling convention {declaration.FunctionAbi.EffectiveCallingConvention} may not be handled correctly."); }
136+
137+
switch (declaration.FunctionAbi.AstCallingConvention)
138+
{
139+
case PathogenClangCallingConventionKind.C:
140+
case PathogenClangCallingConventionKind.X86StdCall:
141+
case PathogenClangCallingConventionKind.X86FastCall:
142+
case PathogenClangCallingConventionKind.X86ThisCall:
143+
case PathogenClangCallingConventionKind.Win64:
144+
break;
145+
// Add a warning for anything we don't explicitly recognize
146+
default:
147+
declaration = declaration.WithWarning($"ABI: AST calling convention {declaration.FunctionAbi.AstCallingConvention} may not be handled correctly.");
148+
break;
149+
}
150+
151+
if (declaration.FunctionAbi.Flags.HasFlag(PathogenArrangedFunctionFlags.UsesInAlloca))
152+
{ declaration = declaration.WithWarning($"ABI: Function uses inalloca, which might not be handled correctly."); }
153+
154+
if (declaration.FunctionAbi.Flags.HasFlag(PathogenArrangedFunctionFlags.HasExtendedParameterInfo))
155+
{ declaration = declaration.WithWarning($"ABI: Function has extended parameter info, which might not be handled correctly."); }
156+
157+
if (declaration.FunctionAbi.ReturnInfo.Kind is PathogenArgumentKind.Expand or PathogenArgumentKind.CoerceAndExpand)
158+
{ declaration = declaration.WithWarning($"ABI: Function return value passing kind is {declaration.FunctionAbi.ReturnInfo.Kind}, which might not be handled correctly."); }
159+
160+
for (int i = 0; i < declaration.FunctionAbi.ArgumentCount; i++)
161+
{
162+
if (declaration.FunctionAbi.Arguments[i].Kind is PathogenArgumentKind.Expand or PathogenArgumentKind.CoerceAndExpand)
163+
{
164+
string parameterDescription;
165+
166+
if (declaration.IsInstanceMethod)
167+
{ parameterDescription = i == 0 ? "this pointer parameter" : $"parameter #{i - 1}"; }
168+
else
169+
{ parameterDescription = $"parameter #{i}"; }
170+
171+
declaration = declaration.WithWarning($"ABI: Function {parameterDescription} passing kind is {declaration.FunctionAbi.Arguments[i].Kind}, which might not be handled correctly.");
172+
}
173+
}
174+
129175
return base.TransformFunction(context, declaration);
130176
}
131177

‎Biohazrd.CSharp/CSharpLibraryGenerator.Functions.cs

Lines changed: 72 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using Biohazrd.CSharp.Metadata;
2+
using ClangSharp.Pathogen;
23
using System;
34
using System.Diagnostics;
45
using System.Runtime.CompilerServices;
@@ -10,16 +11,20 @@ partial class CSharpLibraryGenerator
1011
{
1112
private ref struct EmitFunctionContext
1213
{
14+
public bool NeedsTrampoline { get; }
1315
public string DllImportName { get; }
1416
public TypeReference? ThisType { get; }
1517
public string ThisParameterName => "this";
1618
public string ReturnBufferParameterName => "__returnBuffer";
1719

1820
public EmitFunctionContext(VisitorContext context, TranslatedFunction declaration)
1921
{
20-
// When this function is an instance method, we add a suffix to the P/Invoke method to ensure they don't conflict with other methods.
22+
// We emit a trampoline for functions which are instance methods or return via reference to hide those ABI semantics
23+
NeedsTrampoline = declaration.IsInstanceMethod || declaration.ReturnByReference;
24+
25+
// When this function is uses a trampoline, we add a suffix to the P/Invoke method to ensure they don't conflict with other methods.
2126
// (For instance, when there's a SomeClass::Method() method in addition to a SomeClass::Method(SomeClass*) method.)
22-
DllImportName = declaration.IsInstanceMethod ? $"{declaration.Name}_PInvoke" : declaration.Name;
27+
DllImportName = NeedsTrampoline ? $"{declaration.Name}_PInvoke" : declaration.Name;
2328

2429
// ThisType is the type of `this` for instance methods.
2530
if (!declaration.IsInstanceMethod)
@@ -43,7 +48,7 @@ protected override void VisitFunction(VisitorContext context, TranslatedFunction
4348
{ EmitFunctionDllImport(context, emitContext, declaration); }
4449

4550
// Emit the trampoline
46-
if (declaration.IsInstanceMethod)
51+
if (emitContext.NeedsTrampoline)
4752
{ EmitFunctionTrampoline(context, emitContext, declaration); }
4853
}
4954

@@ -90,8 +95,8 @@ private void EmitFunctionDllImport(VisitorContext context, EmitFunctionContext e
9095
{ Writer.WriteLine("[return: MarshalAs(UnmanagedType.I1)]"); }
9196

9297
// Write out the function signature
93-
// Instance methods are accessed via trampoline, so we translate the DllImport as private.
94-
AccessModifier accessibility = declaration.IsInstanceMethod ? AccessModifier.Private : declaration.Accessibility;
98+
// The P/Invokes for functions accessed via trampoline are emitted as private.
99+
AccessModifier accessibility = emitContext.NeedsTrampoline ? AccessModifier.Private : declaration.Accessibility;
95100
Writer.Write($"{accessibility.ToCSharpKeyword()} static extern ");
96101

97102
// If the return value is passed by reference, the return type is the return buffer pointer
@@ -107,7 +112,7 @@ private void EmitFunctionDllImport(VisitorContext context, EmitFunctionContext e
107112

108113
private void EmitFunctionTrampoline(VisitorContext context, EmitFunctionContext emitContext, TranslatedFunction declaration)
109114
{
110-
if (emitContext.ThisType is null)
115+
if (!emitContext.NeedsTrampoline)
111116
{ throw new ArgumentException("A function trampoline is not valid in this context.", nameof(emitContext)); }
112117

113118
Writer.EnsureSeparation();
@@ -184,6 +189,8 @@ private void EmitFunctionTrampoline(VisitorContext context, EmitFunctionContext
184189

185190
// Emit the method signature
186191
Writer.Write($"{declaration.Accessibility.ToCSharpKeyword()} unsafe ");
192+
if (!declaration.IsInstanceMethod)
193+
{ Writer.Write("static "); }
187194
WriteType(context, declaration, declaration.ReturnType);
188195
Writer.Write($" {SanitizeIdentifier(declaration.Name)}(");
189196
EmitFunctionParameterList(context, emitContext, declaration, EmitParameterListMode.TrampolineParameters);
@@ -204,15 +211,27 @@ private void EmitFunctionTrampoline(VisitorContext context, EmitFunctionContext
204211
// Emit the dispatch
205212
using (Writer.Block())
206213
{
207-
Writer.Write($"fixed (");
208-
WriteType(context, declaration, emitContext.ThisType);
209-
Writer.WriteLine($" {SanitizeIdentifier(emitContext.ThisParameterName)} = &this)");
214+
bool hasThis;
215+
if (emitContext.ThisType is not null)
216+
{
217+
hasThis = true;
218+
Debug.Assert(declaration.IsInstanceMethod);
219+
220+
Writer.Write($"fixed (");
221+
WriteType(context, declaration, emitContext.ThisType!);
222+
Writer.WriteLine($" {SanitizeIdentifier(emitContext.ThisParameterName)} = &this)");
223+
}
224+
else
225+
{
226+
hasThis = false;
227+
Debug.Assert(!declaration.IsInstanceMethod);
228+
}
210229

211230
bool hasReturnValue = declaration.ReturnType is not VoidTypeReference;
212231

213232
if (hasReturnValue && declaration.ReturnByReference)
214233
{
215-
using (Writer.Block())
234+
void EmitFunctionBodyWithReturnByReference(EmitFunctionContext emitContext)
216235
{
217236
WriteType(context, declaration, declaration.ReturnType);
218237
Writer.WriteLine($" {SanitizeIdentifier(emitContext.ReturnBufferParameterName)};");
@@ -223,10 +242,21 @@ private void EmitFunctionTrampoline(VisitorContext context, EmitFunctionContext
223242

224243
Writer.WriteLine($"return {SanitizeIdentifier(emitContext.ReturnBufferParameterName)};");
225244
}
245+
246+
if (hasThis)
247+
{
248+
// If we have a fixed statement for the this pointer, wrap the return buffer logic with a block
249+
using (Writer.Block())
250+
{ EmitFunctionBodyWithReturnByReference(emitContext); }
251+
}
252+
else
253+
{ EmitFunctionBodyWithReturnByReference(emitContext); }
226254
}
227255
else
228256
{
229-
Writer.Write("{ ");
257+
// If we have a fixed statement for the this pointer, write out the curly braces for it
258+
if (hasThis)
259+
{ Writer.Write("{ "); }
230260

231261
if (hasReturnValue)
232262
{ Writer.Write("return "); }
@@ -235,7 +265,8 @@ private void EmitFunctionTrampoline(VisitorContext context, EmitFunctionContext
235265
EmitFunctionParameterList(context, emitContext, declaration, EmitParameterListMode.TrampolineArguments, thisTypeCast);
236266
Writer.Write(");");
237267

238-
Writer.WriteLine(" }");
268+
if (hasThis)
269+
{ Writer.WriteLine(" }"); }
239270
}
240271
}
241272
}
@@ -268,45 +299,55 @@ private void EmitFunctionParameterList(VisitorContext context, EmitFunctionConte
268299
// Write out the this/retbuf parameters
269300
if (writeImplicitParameters)
270301
{
271-
// Write out the this pointer
272-
if (emitContext.ThisType is not null)
302+
void WriteOutReturnBuffer(EmitFunctionContext emitContext)
273303
{
304+
if (!first)
305+
{ Writer.Write(", "); }
306+
307+
if (!declaration.IsVirtual)
308+
{ Writer.Write("out "); }
309+
else if (mode == EmitParameterListMode.TrampolineArguments)
310+
{ Writer.Write("&"); }
311+
274312
if (writeTypes)
275313
{
276-
WriteType(context, declaration, emitContext.ThisType);
314+
WriteType(context, declaration, declaration.ReturnType);
277315
Writer.Write(' ');
278316
}
279-
else if (thisCastType is not null)
280-
{
281-
Writer.Write('(');
282-
WriteType(context, declaration, thisCastType);
283-
Writer.Write(')');
284-
}
285317

286-
Writer.WriteIdentifier(emitContext.ThisParameterName);
318+
Writer.WriteIdentifier(emitContext.ReturnBufferParameterName);
287319
first = false;
288320
}
289321

290-
// Write out the return buffer parameter
291-
if (declaration.ReturnByReference)
322+
// Write out before-this return buffer
323+
if (declaration.ReturnByReference && !declaration.FunctionAbi.ReturnInfo.Flags.HasFlag(PathogenArgumentFlags.IsSRetAfterThis))
324+
{ WriteOutReturnBuffer(emitContext); }
325+
326+
// Write out the this pointer
327+
if (emitContext.ThisType is not null)
292328
{
293329
if (!first)
294330
{ Writer.Write(", "); }
295331

296-
if (!declaration.IsVirtual)
297-
{ Writer.Write("out "); }
298-
else if (mode == EmitParameterListMode.TrampolineArguments)
299-
{ Writer.Write("&"); }
300-
301332
if (writeTypes)
302333
{
303-
WriteType(context, declaration, declaration.ReturnType);
334+
WriteType(context, declaration, emitContext.ThisType);
304335
Writer.Write(' ');
305336
}
337+
else if (thisCastType is not null)
338+
{
339+
Writer.Write('(');
340+
WriteType(context, declaration, thisCastType);
341+
Writer.Write(')');
342+
}
306343

307-
Writer.WriteIdentifier(emitContext.ReturnBufferParameterName);
344+
Writer.WriteIdentifier(emitContext.ThisParameterName);
308345
first = false;
309346
}
347+
348+
// Write out after-this return buffer
349+
if (declaration.ReturnByReference && declaration.FunctionAbi.ReturnInfo.Flags.HasFlag(PathogenArgumentFlags.IsSRetAfterThis))
350+
{ WriteOutReturnBuffer(emitContext); }
310351
}
311352

312353
// Write out parameters

‎Biohazrd/#Declarations/TranslatedFunction.cs

Lines changed: 50 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@ namespace Biohazrd
1111
{
1212
public sealed record TranslatedFunction : TranslatedDeclaration
1313
{
14+
public PathogenArrangedFunction FunctionAbi { get; }
15+
1416
public CallingConvention CallingConvention { get; init; }
1517

1618
public TypeReference ReturnType { get; init; }
17-
public bool ReturnByReference { get; init; }
19+
public bool ReturnByReference => FunctionAbi.ReturnInfo.Kind == PathogenArgumentKind.Indirect;
1820
public ImmutableArray<TranslatedParameter> Parameters { get; init; }
1921

2022
public bool IsInstanceMethod { get; init; }
@@ -29,7 +31,7 @@ public sealed record TranslatedFunction : TranslatedDeclaration
2931
public string DllFileName { get; init; } = "TODO.dll";
3032
public string MangledName { get; init; }
3133

32-
internal TranslatedFunction(TranslatedFile file, FunctionDecl function)
34+
internal TranslatedFunction(TranslationUnitParser parsingContext, TranslatedFile file, FunctionDecl function)
3335
: base(file, function)
3436
{
3537
using (CXString mangling = function.Handle.Mangling)
@@ -39,22 +41,6 @@ internal TranslatedFunction(TranslatedFile file, FunctionDecl function)
3941
IsInline = function.IsInlined;
4042
SpecialFunctionKind = SpecialFunctionKind.None;
4143

42-
// Enumerate parameters
43-
ImmutableArray<TranslatedParameter>.Builder parametersBuilder = ImmutableArray.CreateBuilder<TranslatedParameter>(function.Parameters.Count);
44-
45-
foreach (ParmVarDecl parameter in function.Parameters)
46-
{ parametersBuilder.Add(new TranslatedParameter(file, parameter)); }
47-
48-
Parameters = parametersBuilder.MoveToImmutable();
49-
50-
// Get the function's calling convention
51-
string errorMessage;
52-
CXCallingConv clangCallingConvention = function.GetCallingConvention();
53-
CallingConvention = clangCallingConvention.ToDotNetCallingConvention(out errorMessage);
54-
55-
if (errorMessage is not null)
56-
{ throw new InvalidOperationException(errorMessage); }
57-
5844
// Set method-specific properties
5945
if (function is CXXMethodDecl method)
6046
{
@@ -72,8 +58,52 @@ internal TranslatedFunction(TranslatedFile file, FunctionDecl function)
7258
IsConst = false;
7359
}
7460

75-
// Determine if return value must be passed by reference
76-
ReturnByReference = function.ReturnType.MustBePassedByReference(isForInstanceMethodReturnValue: IsInstanceMethod);
61+
// Arrange the function call
62+
{
63+
PathogenCodeGenerator? codeGenerator = null;
64+
try
65+
{
66+
codeGenerator = parsingContext.CodeGeneratorPool.Rent();
67+
FunctionAbi = new PathogenArrangedFunction(codeGenerator, function);
68+
}
69+
finally
70+
{
71+
if (codeGenerator is not null)
72+
{ parsingContext.CodeGeneratorPool.Return(codeGenerator); }
73+
}
74+
75+
// Assert the arranged function matches our expectations
76+
int expectedArgumentCount = function.Parameters.Count;
77+
78+
if (IsInstanceMethod)
79+
{ expectedArgumentCount++; }
80+
81+
Debug.Assert(FunctionAbi.ArgumentCount == expectedArgumentCount);
82+
83+
Debug.Assert(IsInstanceMethod == FunctionAbi.Flags.HasFlag(PathogenArrangedFunctionFlags.IsInstanceMethod));
84+
}
85+
86+
// Enumerate parameters
87+
ImmutableArray<TranslatedParameter>.Builder parametersBuilder = ImmutableArray.CreateBuilder<TranslatedParameter>(function.Parameters.Count);
88+
{
89+
int parameterIndex = IsInstanceMethod ? 1 : 0;
90+
91+
foreach (ParmVarDecl parameter in function.Parameters)
92+
{
93+
parametersBuilder.Add(new TranslatedParameter(file, parameter, FunctionAbi.Arguments[parameterIndex]));
94+
parameterIndex++;
95+
}
96+
}
97+
98+
Parameters = parametersBuilder.MoveToImmutable();
99+
100+
// Get the function's calling convention
101+
string errorMessage;
102+
CXCallingConv clangCallingConvention = function.GetCallingConvention();
103+
CallingConvention = clangCallingConvention.ToDotNetCallingConvention(out errorMessage);
104+
105+
if (errorMessage is not null)
106+
{ throw new InvalidOperationException(errorMessage); }
77107

78108
// Handle operator overloads
79109
ref PathogenOperatorOverloadInfo operatorOverloadInfo = ref function.GetOperatorOverloadInfo();

0 commit comments

Comments
 (0)