From bcbc99a77326803efd1f501c98c750f3b5d2aa37 Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Mon, 9 Dec 2024 14:38:10 -0500 Subject: [PATCH 1/6] remove NotImplementedException when method is dynamic --- .../managed/cdacreader/src/Legacy/SOSDacImpl.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index e9c9644121eb1..d434b84fc30ca 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -360,10 +360,10 @@ int ISOSDacInterface.GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDes } #endif - if (data->bIsDynamic != 0) - { - throw new NotImplementedException(); // TODO[cdac]: get the dynamic method managed object - } + // Since the cDAC does not currently support fetching CorLib bound managed fields, + // this API does not populate the data->managedDynamicMethodObject field as in the + // original implementation. While this field appears to be unused, it must remain + // in the return type for compatibility. hr = HResults.S_OK; } @@ -405,7 +405,8 @@ int ISOSDacInterface.GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDes Debug.Assert(data->MDToken == dataLocal.MDToken); Debug.Assert(data->GCInfo == dataLocal.GCInfo); Debug.Assert(data->GCStressCodeCopy == dataLocal.GCStressCodeCopy); - Debug.Assert(data->managedDynamicMethodObject == dataLocal.managedDynamicMethodObject); + // managedDynamicMethodObject is not currently populated by the cDAC API. + // Debug.Assert(data->managedDynamicMethodObject == dataLocal.managedDynamicMethodObject); Debug.Assert(data->requestedIP == dataLocal.requestedIP); Debug.Assert(data->cJittedRejitVersions == dataLocal.cJittedRejitVersions); From c5a3de3d8b6c5af856d7059869716fad60d3d9c7 Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Mon, 9 Dec 2024 14:44:32 -0500 Subject: [PATCH 2/6] clean up comment --- src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index d434b84fc30ca..4bd402bbf4257 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -362,8 +362,8 @@ int ISOSDacInterface.GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDes // Since the cDAC does not currently support fetching CorLib bound managed fields, // this API does not populate the data->managedDynamicMethodObject field as in the - // original implementation. While this field appears to be unused, it must remain - // in the return type for compatibility. + // original implementation. While the field is unused, it must remain in the + // return type for compatibility. hr = HResults.S_OK; } @@ -405,7 +405,7 @@ int ISOSDacInterface.GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDes Debug.Assert(data->MDToken == dataLocal.MDToken); Debug.Assert(data->GCInfo == dataLocal.GCInfo); Debug.Assert(data->GCStressCodeCopy == dataLocal.GCStressCodeCopy); - // managedDynamicMethodObject is not currently populated by the cDAC API. + // managedDynamicMethodObject is not currently populated by the cDAC API and may differ from legacyImpl. // Debug.Assert(data->managedDynamicMethodObject == dataLocal.managedDynamicMethodObject); Debug.Assert(data->requestedIP == dataLocal.requestedIP); Debug.Assert(data->cJittedRejitVersions == dataLocal.cJittedRejitVersions); From 2b6a641a9c0e2b316dbe36ed0e57521b066c135f Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Mon, 9 Dec 2024 14:46:33 -0500 Subject: [PATCH 3/6] comment cleanup --- src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index 4bd402bbf4257..1cc7f4c22973b 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -362,8 +362,8 @@ int ISOSDacInterface.GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDes // Since the cDAC does not currently support fetching CorLib bound managed fields, // this API does not populate the data->managedDynamicMethodObject field as in the - // original implementation. While the field is unused, it must remain in the - // return type for compatibility. + // legacy implementation. While the field is unused, it must remain in the return + // type for compatibility. hr = HResults.S_OK; } From 382080e5177f1572eade2f13e046ff321aa5dddd Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Mon, 9 Dec 2024 15:01:25 -0500 Subject: [PATCH 4/6] rework comment --- src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index 1cc7f4c22973b..4f65eae36a69e 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -360,10 +360,10 @@ int ISOSDacInterface.GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDes } #endif - // Since the cDAC does not currently support fetching CorLib bound managed fields, - // this API does not populate the data->managedDynamicMethodObject field as in the - // legacy implementation. While the field is unused, it must remain in the return - // type for compatibility. + // Unlike the legacy implementation, the cDAC does not currently populate + // data->managedDynamicMethodObject. This field is unused in both SOS and CLRMD + // and would require accessing CorLib bound managed fields which the cDAC does not + // currently support. However, it must remain in the return type for compatibility. hr = HResults.S_OK; } From 839bfde26f1ff75d3ea6377bd955fbbde362a52c Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Mon, 9 Dec 2024 16:52:26 -0500 Subject: [PATCH 5/6] zero out field --- src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index 4f65eae36a69e..d43f77a803716 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -364,6 +364,7 @@ int ISOSDacInterface.GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDes // data->managedDynamicMethodObject. This field is unused in both SOS and CLRMD // and would require accessing CorLib bound managed fields which the cDAC does not // currently support. However, it must remain in the return type for compatibility. + data->managedDynamicMethodObject = 0; hr = HResults.S_OK; } From 7536b547e7a30a7f813c82478f493c9342b22549 Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Mon, 9 Dec 2024 17:00:00 -0500 Subject: [PATCH 6/6] assert zero --- src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index d43f77a803716..fbd747b3df172 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -407,7 +407,7 @@ int ISOSDacInterface.GetMethodDescData(ulong methodDesc, ulong ip, DacpMethodDes Debug.Assert(data->GCInfo == dataLocal.GCInfo); Debug.Assert(data->GCStressCodeCopy == dataLocal.GCStressCodeCopy); // managedDynamicMethodObject is not currently populated by the cDAC API and may differ from legacyImpl. - // Debug.Assert(data->managedDynamicMethodObject == dataLocal.managedDynamicMethodObject); + Debug.Assert(data->managedDynamicMethodObject == 0); Debug.Assert(data->requestedIP == dataLocal.requestedIP); Debug.Assert(data->cJittedRejitVersions == dataLocal.cJittedRejitVersions);