Skip to content

Commit 507f50e

Browse files
authored
Fix issue #114504 by not emitting empty gcrefmaps in R2R (#118594)
* Add important comment so it's possible to find the source of AuxiliaryDataRva * Skip emitting an RVA for the GCRefMap if it contains no methods
1 parent 2f54ba3 commit 507f50e

File tree

3 files changed

+12
-14
lines changed

3 files changed

+12
-14
lines changed

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapNode.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ public GCRefMapNode(ImportSectionNode importSection)
3434

3535
public int Offset => 0;
3636

37+
public bool IsEmpty => _methods.Count == 0;
38+
3739
public void AddImport(Import import)
3840
{
3941
lock (_methods)
@@ -53,9 +55,9 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
5355
if (_methods.Count == 0 || relocsOnly)
5456
{
5557
return new ObjectData(
56-
data: Array.Empty<byte>(),
57-
relocs: Array.Empty<Relocation>(),
58-
alignment: 1,
58+
data: Array.Empty<byte>(),
59+
relocs: Array.Empty<Relocation>(),
60+
alignment: 1,
5961
definedSymbols: new ISymbolDefinitionNode[] { this });
6062
}
6163

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ImportSectionNode.cs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ protected override int GetAlignmentRequirement(NodeFactory factory)
4141
private readonly byte _entrySize;
4242
private readonly string _name;
4343
private readonly bool _emitPrecode;
44-
private readonly bool _emitGCRefMap;
4544

4645
private bool _materializedSignature;
4746

@@ -52,12 +51,11 @@ public ImportSectionNode(string name, ReadyToRunImportSectionType importType, Re
5251
_flags = flags;
5352
_entrySize = entrySize;
5453
_emitPrecode = emitPrecode;
55-
_emitGCRefMap = emitGCRefMap;
5654

5755
_imports = new ImportTable(_name + "_ImportBegin", entrySize);
5856
_signatures = new ArrayOfEmbeddedPointersNode<Signature>(_name + "_SigBegin", new EmbeddedObjectNodeComparer(CompilerComparer.Instance));
5957
_signatureList = new List<Signature>();
60-
_gcRefMap = _emitGCRefMap ? new GCRefMapNode(this) : null;
58+
_gcRefMap = emitGCRefMap ? new GCRefMapNode(this) : null;
6159
}
6260

6361
public void MaterializeSignature(NodeFactory r2rFactory)
@@ -89,10 +87,7 @@ public void AddImport(NodeFactory factory, Import import)
8987
_signatureList.Add(import.ImportSignature.Target);
9088
}
9189

92-
if (_emitGCRefMap)
93-
{
94-
_gcRefMap.AddImport(import);
95-
}
90+
_gcRefMap?.AddImport(import);
9691
}
9792

9893
public string Name => _name;
@@ -135,8 +130,10 @@ public override void EncodeData(ref ObjectDataBuilder dataBuilder, NodeFactory f
135130
dataBuilder.EmitUInt(0);
136131
}
137132

138-
if (_emitGCRefMap)
133+
// If we emit an Rva for an empty gcrefmap, it will have no size header and be impossible for r2rdump to read correctly
134+
if (_gcRefMap?.IsEmpty == false)
139135
{
136+
// This indirectly generates the AuxiliaryDataRva by emitting a placeholder 0 which will be replaced later
140137
dataBuilder.EmitReloc(_gcRefMap, RelocType.IMAGE_REL_BASED_ADDR32NB, 0);
141138
}
142139
else
@@ -149,7 +146,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
149146
{
150147
yield return new DependencyListEntry(_imports, "Import section fixup data");
151148
yield return new DependencyListEntry(_signatures, "Import section signatures");
152-
if (_emitGCRefMap)
149+
if (_gcRefMap != null)
153150
{
154151
yield return new DependencyListEntry(_gcRefMap, "GC ref map");
155152
}

src/tests/Common/CLRTest.CrossGen.targets

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,7 @@ if [ ! -z ${RunCrossGen2+x} ]%3B then
110110
done
111111
112112
echo -o:"$__OutputFile" >> "$__ResponseFile"
113-
## Enable once #114504 is fixed
114-
## echo -O >> "$__ResponseFile"
113+
echo -O >> "$__ResponseFile"
115114
echo --targetarch:$(TargetArchitecture) >> "$__ResponseFile"
116115
echo --targetos:$(TargetOS) >> "$__ResponseFile"
117116
echo --verify-type-and-field-layout >> "$__ResponseFile"

0 commit comments

Comments
 (0)