Skip to content

Commit 9509d91

Browse files
zacharycmontoyaandrewlock
zacharycmontoya
and
andrewlock
authored
[Tracing] Instrument static methods defined in non-generic value types (#7920)
## Summary of changes Adds the capability to instrument static methods defined in non-generic value types ## Reason for change We will need to instrument OpenTelemetry Baggage, which utilizes static methods in a Baggage struct ## Implementation details - When instrumenting a method for CallTarget instrumentation, optionally update the LocalVarSig with a local variable of the owning type if the following conditions are met: 1. The method is static 2. The owning type of the method is a value type - Update `TracerMethodRewriter::Rewrite` such that when it is loading the object instance to put on the stack for a static method, for non-generic value types it does the following: - `ldloca.s [localIndex]` (Loads the variable addresses) - `initobj [valueType]` (Initializes, or re-initializes, an empty struct) - `ldloc.s [localIndex]` (Loads the struct onto the stack) ## Test coverage The `CallTargetNativeTest` is updated with the following cases to demonstrate the new functionality, especially as it compares to reference types: - New `ArgumentsGenericStatic` types demonstrate the successful instrumentation of static methods in _generic_ reference types - New `ArgumentsStaticStruct` types demonstrate the successful instrumentation of static methods in value types - New `ArgumentsGenericStaticStruct` types demonstrate the unsuccessful instrumentation of static methods in _generic_ value types ## Other details Note: This does not implement the functionality for the live debugger IL rewriting, though it does update the method call so that everything compiles. --------- Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
1 parent 13ff155 commit 9509d91

16 files changed

Lines changed: 3114 additions & 50 deletions

File tree

‎tracer/src/Datadog.Tracer.Native/calltarget_tokens.cpp‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -358,9 +358,9 @@ mdMethodSpec CallTargetTokens::GetCallTargetDefaultValueMethodSpec(const TypeSig
358358
}
359359

360360
HRESULT CallTargetTokens::ModifyLocalSig(ILRewriter* reWriter, TypeSignature* methodReturnValue,
361-
std::vector<TypeSignature>* methodTypeArguments,
361+
std::vector<TypeSignature>* methodTypeArguments, FunctionInfo* caller,
362362
ULONG* callTargetStateIndex, ULONG* exceptionIndex,
363-
ULONG* callTargetReturnIndex, ULONG* returnValueIndex,
363+
ULONG* callTargetReturnIndex, ULONG* staticValueTypeIndex, ULONG* returnValueIndex,
364364
mdToken* callTargetStateToken, mdToken* exceptionToken,
365365
mdToken* callTargetReturnToken, std::vector<ULONG>& additionalLocalIndices, bool isAsyncMethod)
366366
{
@@ -399,7 +399,17 @@ HRESULT CallTargetTokens::ModifyLocalSig(ILRewriter* reWriter, TypeSignature* me
399399
}
400400
constexpr int variableNumber = 3;
401401
const auto additionalLocalsCount = GetAdditionalLocalsCount(*methodTypeArguments);
402-
ULONG newLocalsCount = variableNumber + additionalLocalsCount;
402+
403+
// For instrumenting static methods in value types, see if we can get the TypeRef for the type.
404+
// If so, include add a local variable with the value type in the signature.
405+
bool isValueType = caller->type.valueType;
406+
bool isStaticValueType = isValueType && !(caller->method_signature.CallingConvention() & IMAGE_CEE_CS_CALLCONV_HASTHIS) && GetCurrentTypeRef(&caller->type, isValueType) != mdTokenNil;
407+
408+
ULONG newLocalsCount = variableNumber + additionalLocalsCount + (isStaticValueType ? 1 : 0);
409+
410+
// Gets the static value type buffer and size
411+
unsigned staticValueTypeTypeRefBuffer;
412+
auto staticValueTypeTypeRefSize = CorSigCompressToken(caller->type.id, &staticValueTypeTypeRefBuffer);
403413

404414
// Gets the calltarget state type buffer and size
405415
unsigned callTargetStateTypeRefBuffer;
@@ -496,6 +506,13 @@ HRESULT CallTargetTokens::ModifyLocalSig(ILRewriter* reWriter, TypeSignature* me
496506
newSignature.Append(ELEMENT_TYPE_VALUETYPE);
497507
newSignature.Append(&callTargetReturnBuffer, callTargetReturnSize);
498508
}
509+
510+
// Static method in a ValueType
511+
if (isStaticValueType)
512+
{
513+
newSignature.Append(ELEMENT_TYPE_VALUETYPE);
514+
newSignature.Append(&staticValueTypeTypeRefBuffer, staticValueTypeTypeRefSize);
515+
}
499516

500517
// Add custom locals
501518
AddAdditionalLocals(methodReturnValue, methodTypeArguments, newSignature, isAsyncMethod);
@@ -519,7 +536,7 @@ HRESULT CallTargetTokens::ModifyLocalSig(ILRewriter* reWriter, TypeSignature* me
519536
*callTargetReturnToken = callTargetReturn;
520537

521538
const auto sizeOfOtherIndexes = additionalLocalIndices.size();
522-
auto indexStart = variableNumber + additionalLocalsCount;
539+
auto indexStart = variableNumber + additionalLocalsCount + (isStaticValueType ? 1 : 0);
523540

524541
if (returnSignatureType != nullptr)
525542
{
@@ -533,6 +550,15 @@ HRESULT CallTargetTokens::ModifyLocalSig(ILRewriter* reWriter, TypeSignature* me
533550
*exceptionIndex = newLocalsCount - indexStart--;
534551
*callTargetReturnIndex = newLocalsCount - indexStart--;
535552

553+
if (isStaticValueType)
554+
{
555+
*staticValueTypeIndex = newLocalsCount - indexStart--;
556+
}
557+
else
558+
{
559+
*staticValueTypeIndex = static_cast<ULONG>(ULONG_MAX);
560+
}
561+
536562
for (unsigned int i = 0; i < sizeOfOtherIndexes; i++)
537563
{
538564
additionalLocalIndices[i] = newLocalsCount - indexStart--;
@@ -870,9 +896,9 @@ mdAssemblyRef CallTargetTokens::GetCorLibAssemblyRef()
870896
}
871897

872898
HRESULT CallTargetTokens::ModifyLocalSigAndInitialize(void* rewriterWrapperPtr, TypeSignature* methodReturnType,
873-
std::vector<TypeSignature>* methodTypeArguments,
899+
std::vector<TypeSignature>* methodTypeArguments, FunctionInfo* caller,
874900
ULONG* callTargetStateIndex, ULONG* exceptionIndex,
875-
ULONG* callTargetReturnIndex, ULONG* returnValueIndex,
901+
ULONG* callTargetReturnIndex, ULONG* staticValueTypeIndex, ULONG* returnValueIndex,
876902
mdToken* callTargetStateToken, mdToken* exceptionToken,
877903
mdToken* callTargetReturnToken, ILInstr** firstInstruction,
878904
std::vector<ULONG>& additionalLocalIndices, bool isAsyncMethod)
@@ -881,8 +907,8 @@ HRESULT CallTargetTokens::ModifyLocalSigAndInitialize(void* rewriterWrapperPtr,
881907

882908
// Modify the Local Var Signature of the method
883909

884-
auto hr = ModifyLocalSig(rewriterWrapper->GetILRewriter(), methodReturnType, methodTypeArguments,
885-
callTargetStateIndex, exceptionIndex, callTargetReturnIndex,
910+
auto hr = ModifyLocalSig(rewriterWrapper->GetILRewriter(), methodReturnType, methodTypeArguments, caller,
911+
callTargetStateIndex, exceptionIndex, callTargetReturnIndex, staticValueTypeIndex,
886912
returnValueIndex, callTargetStateToken, exceptionToken, callTargetReturnToken,
887913
additionalLocalIndices, isAsyncMethod);
888914

‎tracer/src/Datadog.Tracer.Native/calltarget_tokens.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ class CallTargetTokens
4646
mdMemberRef GetCallTargetReturnValueDefaultMemberRef(mdTypeSpec callTargetReturnTypeSpec);
4747
mdMethodSpec GetCallTargetDefaultValueMethodSpec(const TypeSignature* methodArgument);
4848

49-
HRESULT ModifyLocalSig(ILRewriter* reWriter, TypeSignature* methodReturnValue, std::vector<TypeSignature>* methodTypeArguments,
50-
ULONG* callTargetStateIndex, ULONG* exceptionIndex, ULONG* callTargetReturnIndex, ULONG* returnValueIndex,
49+
HRESULT ModifyLocalSig(ILRewriter* reWriter, TypeSignature* methodReturnValue, std::vector<TypeSignature>* methodTypeArguments, FunctionInfo* caller,
50+
ULONG* callTargetStateIndex, ULONG* exceptionIndex, ULONG* callTargetReturnIndex, ULONG* staticValueTypeIndex, ULONG* returnValueIndex,
5151
mdToken* callTargetStateToken, mdToken* exceptionToken, mdToken* callTargetReturnToken,
5252
std::vector<ULONG>& additionalLocalIndices, bool isAsyncMethod = false);
5353

@@ -97,8 +97,8 @@ class CallTargetTokens
9797

9898
mdMethodDef GetCallTargetStateSkipMethodBodyMemberRef();
9999

100-
HRESULT ModifyLocalSigAndInitialize(void* rewriterWrapperPtr, TypeSignature* methodReturnType, std::vector<TypeSignature>* methodTypeArguments,
101-
ULONG* callTargetStateIndex, ULONG* exceptionIndex, ULONG* callTargetReturnIndex, ULONG* returnValueIndex,
100+
HRESULT ModifyLocalSigAndInitialize(void* rewriterWrapperPtr, TypeSignature* methodReturnType, std::vector<TypeSignature>* methodTypeArguments, FunctionInfo* caller,
101+
ULONG* callTargetStateIndex, ULONG* exceptionIndex, ULONG* callTargetReturnIndex, ULONG* staticValueTypeIndex, ULONG* returnValueIndex,
102102
mdToken* callTargetStateToken, mdToken* exceptionToken,
103103
mdToken* callTargetReturnToken, ILInstr** firstInstruction, std::vector<ULONG>& additionalLocalIndices, bool
104104
isAsyncMethod = false);

‎tracer/src/Datadog.Tracer.Native/debugger_method_rewriter.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2239,6 +2239,7 @@ HRESULT DebuggerMethodRewriter::Rewrite(RejitHandlerModule* moduleHandler,
22392239
ULONG callTargetStateIndex = static_cast<ULONG>(ULONG_MAX);
22402240
ULONG exceptionIndex = static_cast<ULONG>(ULONG_MAX);
22412241
ULONG callTargetReturnIndex = static_cast<ULONG>(ULONG_MAX);
2242+
ULONG staticValueTypeIndex = static_cast<ULONG>(ULONG_MAX);
22422243
ULONG returnValueIndex = static_cast<ULONG>(ULONG_MAX);
22432244
mdToken callTargetStateToken = mdTokenNil;
22442245
mdToken exceptionToken = mdTokenNil;
@@ -2275,8 +2276,8 @@ HRESULT DebuggerMethodRewriter::Rewrite(RejitHandlerModule* moduleHandler,
22752276
methodReturnType = caller->method_signature.GetReturnValue();
22762277
}
22772278
auto debuggerLocals = std::vector<ULONG>(debuggerTokens->GetAdditionalLocalsCount(methodArguments));
2278-
hr = debuggerTokens->ModifyLocalSigAndInitialize(&rewriterWrapper, &methodReturnType, &methodArguments, &callTargetStateIndex, &exceptionIndex,
2279-
&callTargetReturnIndex, &returnValueIndex, &callTargetStateToken,
2279+
hr = debuggerTokens->ModifyLocalSigAndInitialize(&rewriterWrapper, &methodReturnType, &methodArguments, caller, &callTargetStateIndex, &exceptionIndex,
2280+
&callTargetReturnIndex, &staticValueTypeIndex, &returnValueIndex, &callTargetStateToken,
22802281
&exceptionToken, &callTargetReturnToken, &firstInstruction, debuggerLocals, isAsyncMethod);
22812282

22822283
ULONG lineProbeCallTargetStateIndex = debuggerLocals[0];

‎tracer/src/Datadog.Tracer.Native/tracer_method_rewriter.cpp‎

Lines changed: 68 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,12 @@ HRESULT TracerMethodRewriter::Rewrite(RejitHandlerModule* moduleHandler, RejitHa
7171
Current CallTarget Limitations:
7272
===============================
7373
74-
1. Static methods in a ValueType (struct) cannot be instrumented.
75-
2. Generic ValueTypes (struct) cannot be instrumented.
76-
3. Nested ValueTypes (struct) inside a Generic parent type will not expose the type instance (the instance will
74+
1. Generic ValueTypes (struct) cannot be instrumented.
75+
2. Nested ValueTypes (struct) inside a Generic parent type will not expose the type instance (the instance will
7776
be always null).
78-
4. Nested types (reference types) inside a Generic parent type will not expose the type instance (the instance
77+
3. Nested types (reference types) inside a Generic parent type will not expose the type instance (the instance
7978
will be casted as an object type).
80-
5. Methods in a Generic type will not expose the Generic type instance (the instance will be casted as a non
79+
4. Methods in a Generic type will not expose the Generic type instance (the instance will be casted as a non
8180
generic base type or object type).
8281
*/
8382

@@ -183,6 +182,7 @@ HRESULT TracerMethodRewriter::Rewrite(RejitHandlerModule* moduleHandler, RejitHa
183182
ULONG callTargetStateIndex = static_cast<ULONG>(ULONG_MAX);
184183
ULONG exceptionIndex = static_cast<ULONG>(ULONG_MAX);
185184
ULONG callTargetReturnIndex = static_cast<ULONG>(ULONG_MAX);
185+
ULONG staticValueTypeIndex = static_cast<ULONG>(ULONG_MAX);
186186
ULONG returnValueIndex = static_cast<ULONG>(ULONG_MAX);
187187

188188
std::vector<ULONG> indexes(tracerTokens->GetAdditionalLocalsCount(methodArguments));
@@ -193,7 +193,7 @@ HRESULT TracerMethodRewriter::Rewrite(RejitHandlerModule* moduleHandler, RejitHa
193193
auto returnType = caller->method_signature.GetReturnValue();
194194

195195
tracerTokens->ModifyLocalSigAndInitialize(
196-
&reWriterWrapper, &returnType, &methodArguments, &callTargetStateIndex, &exceptionIndex, &callTargetReturnIndex,
196+
&reWriterWrapper, &returnType, &methodArguments, caller, &callTargetStateIndex, &exceptionIndex, &callTargetReturnIndex, &staticValueTypeIndex,
197197
&returnValueIndex, &callTargetStateToken, &exceptionToken, &callTargetReturnToken, &firstInstruction, indexes);
198198

199199
ULONG exceptionValueIndex = indexes[0];
@@ -207,19 +207,39 @@ HRESULT TracerMethodRewriter::Rewrite(RejitHandlerModule* moduleHandler, RejitHa
207207
// *** Load instance into the stack (if not static)
208208
if (isStatic)
209209
{
210-
if (caller->type.valueType)
210+
bool callerTypeIsValueType = caller->type.valueType;
211+
mdToken callerTypeToken = tracerTokens->GetCurrentTypeRef(&caller->type, callerTypeIsValueType);
212+
if (callerTypeIsValueType && callerTypeToken != mdTokenNil)
213+
{
214+
reWriterWrapper.LoadLocalAddress(staticValueTypeIndex);
215+
if (caller->type.type_spec != mdTypeSpecNil)
216+
{
217+
reWriterWrapper.InitObj(caller->type.type_spec);
218+
}
219+
else if (!caller->type.isGeneric)
220+
{
221+
reWriterWrapper.InitObj(caller->type.id);
222+
}
223+
else
224+
{
225+
// Generic struct instrumentation is not supported
226+
// IMetaDataImport::GetMemberProps and IMetaDataImport::GetMemberRefProps returns
227+
// The parent token as mdTypeDef and not as a mdTypeSpec
228+
// that's because the method definition is stored in the mdTypeDef
229+
// The problem is that we don't have the exact Spec of that generic
230+
// We can't emit LoadObj or Box because that would result in an invalid IL.
231+
// This problem doesn't occur on a class type because we can always relay in the
232+
// object type.
233+
Logger::Warn("*** CallTarget_RewriterCallback(): Generic struct (struct TypeName<T>) instrumentation is not supported.");
234+
return S_FALSE;
235+
}
236+
237+
reWriterWrapper.LoadLocal(staticValueTypeIndex);
238+
}
239+
else
211240
{
212-
// Static methods in a ValueType can't be instrumented.
213-
// In the future this can be supported by adding a local for the valuetype and initialize it to the default
214-
// value. After the signature modification we need to emit the following IL to initialize and load into the
215-
// stack.
216-
// ldloca.s [localIndex]
217-
// initobj [valueType]
218-
// ldloc.s [localIndex]
219-
Logger::Warn("*** CallTarget_RewriterCallback(): Static methods in a ValueType cannot be instrumented. ");
220-
return S_FALSE;
241+
reWriterWrapper.LoadNull();
221242
}
222-
reWriterWrapper.LoadNull();
223243
}
224244
else
225245
{
@@ -505,21 +525,39 @@ HRESULT TracerMethodRewriter::Rewrite(RejitHandlerModule* moduleHandler, RejitHa
505525
// *** Load instance into the stack (if not static)
506526
if (isStatic)
507527
{
508-
if (caller->type.valueType)
528+
bool callerTypeIsValueType = caller->type.valueType;
529+
mdToken callerTypeToken = tracerTokens->GetCurrentTypeRef(&caller->type, callerTypeIsValueType);
530+
if (caller->type.valueType && callerTypeToken != mdTokenNil)
531+
{
532+
endMethodTryStartInstr = reWriterWrapper.LoadLocalAddress(staticValueTypeIndex);
533+
if (caller->type.type_spec != mdTypeSpecNil)
534+
{
535+
reWriterWrapper.InitObj(caller->type.type_spec);
536+
}
537+
else if (!caller->type.isGeneric)
538+
{
539+
reWriterWrapper.InitObj(caller->type.id);
540+
}
541+
else
542+
{
543+
// Generic struct instrumentation is not supported
544+
// IMetaDataImport::GetMemberProps and IMetaDataImport::GetMemberRefProps returns
545+
// The parent token as mdTypeDef and not as a mdTypeSpec
546+
// that's because the method definition is stored in the mdTypeDef
547+
// The problem is that we don't have the exact Spec of that generic
548+
// We can't emit LoadObj or Box because that would result in an invalid IL.
549+
// This problem doesn't occur on a class type because we can always relay in the
550+
// object type.
551+
Logger::Warn("*** CallTarget_RewriterCallback(): Generic struct (struct TypeName<T>) instrumentation is not supported.");
552+
return S_FALSE;
553+
}
554+
555+
reWriterWrapper.LoadLocal(staticValueTypeIndex);
556+
}
557+
else
509558
{
510-
// Static methods in a ValueType can't be instrumented.
511-
// In the future this can be supported by adding a local for the valuetype
512-
// and initialize it to the default value. After the signature
513-
// modification we need to emit the following IL to initialize and load
514-
// into the stack.
515-
// ldloca.s [localIndex]
516-
// initobj [valueType]
517-
// ldloc.s [localIndex]
518-
Logger::Warn("CallTarget_RewriterCallback: Static methods in a ValueType cannot "
519-
"be instrumented. ");
520-
return S_FALSE;
559+
endMethodTryStartInstr = reWriterWrapper.LoadNull();
521560
}
522-
endMethodTryStartInstr = reWriterWrapper.LoadNull();
523561
}
524562
else
525563
{

‎tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CallTargetNativeTests.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,20 +65,20 @@ public async Task MethodArgumentsInstrumentation(int numberOfArguments, bool fas
6565
{
6666
// On number of arguments = 0 the throw exception on integrations async continuation runs.
6767
// So we have 1 more case with an exception being reported from the integration.
68-
Assert.Equal(180, beginMethodCount);
69-
Assert.Equal(180, endMethodCount);
68+
Assert.Equal(240, beginMethodCount);
69+
Assert.Equal(240, endMethodCount);
7070
Assert.Equal(44, exceptionCount);
7171
}
7272
else if (numberOfArguments == 1)
7373
{
74-
Assert.Equal(175, beginMethodCount);
75-
Assert.Equal(175, endMethodCount);
74+
Assert.Equal(235, beginMethodCount);
75+
Assert.Equal(235, endMethodCount);
7676
Assert.Equal(40, exceptionCount);
7777
}
7878
else
7979
{
80-
Assert.Equal(168, beginMethodCount);
81-
Assert.Equal(168, endMethodCount);
80+
Assert.Equal(228, beginMethodCount);
81+
Assert.Equal(228, endMethodCount);
8282
Assert.Equal(40, exceptionCount);
8383
}
8484

0 commit comments

Comments
 (0)