Skip to content

Commit

Permalink
Fix generating debug info for static fields (dotnet#95570)
Browse files Browse the repository at this point in the history
Fixes dotnet#93868.

The existing algorithm for generating debug info about static fields was:

* If we're generating debug information about a type, generate debug information about all non-literal and non-RVA fields on it.

This is both too little, and too much.

It's too little: if the only thing we're touching on a type is reading/writing the static fields, we'd not be generating debug information for that type because the current logic only looks at methods and `MethodTable`s.

It's too much: if we didn't touch any statics on the type, the static base wasn't generated either. Debug information would refer to something that doesn't exist.

I'm updating this logic to look at actual bases instead - if a static/gcstatic/threadstatic/instance base was generated, generate debug info for the fields. Skip them otherwise because they got optimized away ("trimmed").
  • Loading branch information
MichalStrehovsky authored Dec 5, 2023
1 parent 24ee598 commit 00c9556
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1213,12 +1213,17 @@ public static void EmitObject(string objectFilePath, IReadOnlyCollection<Depende
objectWriter.EmitDebugFunctionInfo(node, nodeContents.Data.Length);
}

// Ensure any allocated MethodTables have debug info
if (node is ConstructedEETypeNode MethodTable)
{
objectWriter._userDefinedTypeDescriptor.GetTypeIndex(MethodTable.Type, needsCompleteType: true);
}
}

// Ensure all fields associated with generated static bases have debug info
foreach (MetadataType typeWithStaticBase in objectWriter._nodeFactory.MetadataManager.GetTypesWithStaticBases())
objectWriter._userDefinedTypeDescriptor.GetTypeIndex(typeWithStaticBase, needsCompleteType: true);

// Native side of the object writer is going to do more native memory allocations.
// Free up as much memory as possible so that we don't get OOM killed.
// This is potentially a waste of time. We're about to end the process and let the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ public abstract class MetadataManager : ICompilationRootProvider

private readonly List<InterfaceDispatchCellNode> _interfaceDispatchCells = new List<InterfaceDispatchCellNode>();
private readonly SortedSet<NonGCStaticsNode> _cctorContextsGenerated = new SortedSet<NonGCStaticsNode>(CompilerComparer.Instance);
private readonly SortedSet<MetadataType> _typesWithGCStaticsGenerated = new SortedSet<MetadataType>(CompilerComparer.Instance);
private readonly SortedSet<MetadataType> _typesWithNonGCStaticsGenerated = new SortedSet<MetadataType>(CompilerComparer.Instance);
private readonly SortedSet<MetadataType> _typesWithThreadStaticsGenerated = new SortedSet<MetadataType>(CompilerComparer.Instance);
private readonly SortedSet<TypeDesc> _typesWithEETypesGenerated = new SortedSet<TypeDesc>(TypeSystemComparer.Instance);
private readonly SortedSet<TypeDesc> _typesWithConstructedEETypesGenerated = new SortedSet<TypeDesc>(TypeSystemComparer.Instance);
private readonly SortedSet<MethodDesc> _methodsGenerated = new SortedSet<MethodDesc>(TypeSystemComparer.Instance);
Expand Down Expand Up @@ -245,10 +248,22 @@ protected virtual void Graph_NewMarkedNode(DependencyNodeCore<NodeFactory> obj)
_reflectableMethods.Add(reflectedMethodNode.Method);
}

var nonGcStaticSectionNode = obj as NonGCStaticsNode;
if (nonGcStaticSectionNode != null && nonGcStaticSectionNode.HasLazyStaticConstructor)
if (obj is NonGCStaticsNode nonGcStaticSectionNode)
{
_cctorContextsGenerated.Add(nonGcStaticSectionNode);
if (nonGcStaticSectionNode.HasLazyStaticConstructor)
_cctorContextsGenerated.Add(nonGcStaticSectionNode);

_typesWithNonGCStaticsGenerated.Add(nonGcStaticSectionNode.Type);
}

if (obj is GCStaticsNode gcStaticsNode)
{
_typesWithGCStaticsGenerated.Add(gcStaticsNode.Type);
}

if (obj is TypeThreadStaticIndexNode threadStaticsNode)
{
_typesWithThreadStaticsGenerated.Add(threadStaticsNode.Type);
}

var gvmEntryNode = obj as TypeGVMEntriesNode;
Expand Down Expand Up @@ -717,6 +732,20 @@ internal IEnumerable<NonGCStaticsNode> GetCctorContextMapping()
return _cctorContextsGenerated;
}

internal IEnumerable<MetadataType> GetTypesWithStaticBases()
{
var allTypes = new SortedSet<MetadataType>(CompilerComparer.Instance);
allTypes.UnionWith(_typesWithNonGCStaticsGenerated);
allTypes.UnionWith(_typesWithGCStaticsGenerated);
allTypes.UnionWith(_typesWithThreadStaticsGenerated);
return allTypes;
}

internal bool HasNonGcStaticBase(MetadataType type) => _typesWithNonGCStaticsGenerated.Contains(type);
internal bool HasGcStaticBase(MetadataType type) => _typesWithGCStaticsGenerated.Contains(type);
internal bool HasThreadStaticBase(MetadataType type) => _typesWithThreadStaticsGenerated.Contains(type);
internal bool HasConstructedEEType(TypeDesc type) => _typesWithConstructedEETypesGenerated.Contains(type);

internal IEnumerable<TypeGVMEntriesNode> GetTypeGVMEntries()
{
return _typeGVMEntries;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ private TypeDesc GetFieldDebugType(FieldDesc field)
private uint GetClassTypeIndex(TypeDesc type, bool needsCompleteType)
{
TypeDesc debugType = GetDebugType(type);
DefType defType = debugType as DefType;
MetadataType defType = debugType as MetadataType;
Debug.Assert(defType != null, "GetClassTypeIndex was called with non def type");
ClassTypeDescriptor classTypeDescriptor = new ClassTypeDescriptor
{
Expand Down Expand Up @@ -590,15 +590,40 @@ private uint GetClassTypeIndex(TypeDesc type, bool needsCompleteType)
string threadStaticDataName = NodeFactory.NameMangler.NodeMangler.ThreadStatics(type);
bool isNativeAOT = Abi == TargetAbi.NativeAot;

bool hasNonGcStatics = NodeFactory.MetadataManager.HasNonGcStaticBase(defType);
bool hasGcStatics = NodeFactory.MetadataManager.HasGcStaticBase(defType);
bool hasThreadStatics = NodeFactory.MetadataManager.HasThreadStaticBase(defType);
bool hasInstanceFields = defType.IsValueType || NodeFactory.MetadataManager.HasConstructedEEType(defType);

bool isCanonical = defType.IsCanonicalSubtype(CanonicalFormKind.Any);

foreach (var fieldDesc in defType.GetFields())
{
if (fieldDesc.HasRva || fieldDesc.IsLiteral)
continue;

if (isCanonical && fieldDesc.IsStatic)
continue;
if (fieldDesc.IsStatic)
{
if (isCanonical)
continue;
if (fieldDesc.IsThreadStatic && !hasThreadStatics)
continue;
if (fieldDesc.HasGCStaticBase)
{
if (!hasGcStatics)
continue;
}
else
{
if (!hasNonGcStatics)
continue;
}
}
else
{
if (!hasInstanceFields)
continue;
}

LayoutInt fieldOffset = fieldDesc.Offset;
int fieldOffsetEmit = fieldOffset.IsIndeterminate ? 0xBAAD : fieldOffset.AsInt;
Expand Down

0 comments on commit 00c9556

Please sign in to comment.