Skip to content

Use .alt_entry/.private_extern on Apple platforms #106224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
filipnavara wants to merge 12 commits into from
Closed

Use .alt_entry/.private_extern on Apple platforms #106224

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
13e6013
Use .alt_entry on Apple platforms
filipnavara Aug 10, 2024
5fe3a06
ILC: Emit N_ALT_ENTRY for symbols in the middle of the node and alter…
filipnavara Aug 10, 2024
ea6ab79
Make sure the whole hydration target section is treated as an atom by
filipnavara Aug 10, 2024
4ecbb30
Fix reported symbol count for the .hydrated section
filipnavara Aug 10, 2024
0ede301
Bring back N_NO_DEAD_STRIP even for N_ALT_ENTRY
filipnavara Aug 10, 2024
9ed567b
Add code to synthesize the initial node symbol if not present
filipnavara Aug 12, 2024
0e123f7
Use one-atom-per-section for all data sections, use asserts to guard …
filipnavara Aug 12, 2024
d6198d4
Mark funclet frames as N_ALT_ENTRY, make sure the new linker doesn't …
filipnavara Aug 12, 2024
7a7bf6e
Fix typo
filipnavara Aug 12, 2024
4f50681
Add helper property to ObjectNodeSection
filipnavara Aug 14, 2024
94e8839
Merge branch 'main' into alt_entry
MichalStrehovsky Aug 14, 2024
c0c546f
Update ObjectWriter.cs
MichalStrehovsky Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/coreclr/nativeaot/Runtime/unix/unixasmmacrosamd64.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ C_FUNC(\Name):
.endm

.macro ALTERNATE_ENTRY Name
#if defined(__APPLE__)
.alt_entry C_FUNC(\Name)
.private_extern C_FUNC(\Name)
#else
.global C_FUNC(\Name)
#endif
C_FUNC(\Name):
.endm

Expand Down
5 changes: 4 additions & 1 deletion src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@ C_FUNC(\Name):
.endm

.macro ALTERNATE_ENTRY Name
#if defined(__APPLE__)
.alt_entry C_FUNC(\Name)
.private_extern C_FUNC(\Name)
#else
.global C_FUNC(\Name)
#if !defined(__APPLE__)
.hidden C_FUNC(\Name)
#endif
C_FUNC(\Name):
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/pal/inc/unixasmmacrosarm64.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
.endm

.macro PATCH_LABEL Name
#if defined(__APPLE__)
.alt_entry C_FUNC(\Name)
.private_extern C_FUNC(\Name)
#else
.global C_FUNC(\Name)
#endif
C_FUNC(\Name):
.endm

Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/MachNative.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ internal static class MachNative
public const byte N_INDR = 0xA;
public const byte N_SECT = 0xE;
public const byte N_PBUD = 0xC;
public const byte N_PEXT = 0x10;

// Symbol descriptor flags
public const ushort REFERENCE_FLAG_UNDEFINED_NON_LAZY = 0;
Expand All @@ -106,6 +107,7 @@ internal static class MachNative
public const ushort REFERENCE_FLAG_PRIVATE_UNDEFINED_LAZY = 5;
public const ushort REFERENCED_DYNAMICALLY = 0x10;
public const ushort N_NO_DEAD_STRIP = 0x20;
public const ushort N_ALT_ENTRY = 0x200;
public const ushort N_WEAK_REF = 0x40;
public const ushort N_WEAK_DEF = 0x80;

Expand Down
27 changes: 25 additions & 2 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/MachObjectWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ private sealed record CompactUnwindCode(string PcStartSymbolName, uint PcLength,
private readonly uint _cpuType;
private readonly uint _cpuSubType;
private readonly List<MachSection> _sections = new();
private int? _hydrationTargetSectionIndex;

// Exception handling sections
private MachSection _compactUnwindSection;
Expand Down Expand Up @@ -360,6 +361,11 @@ private protected override void CreateSection(ObjectNodeSection section, string
int sectionIndex = _sections.Count;
_sections.Add(machSection);

if (section == ObjectNodeSection.HydrationTargetSection)
{
_hydrationTargetSectionIndex = sectionIndex;
}

base.CreateSection(section, comdatName, symbolName ?? $"lsection{sectionIndex}", sectionStream);
}

Expand Down Expand Up @@ -472,6 +478,21 @@ private protected override void EmitSymbolTable(

// Sort and insert all defined symbols
var sortedDefinedSymbols = new List<MachSymbol>(definedSymbols.Count);
// Emit a private external symbol for the beginning of the hydration target section
// to ensure the layout is kept intact by the linker. This works in tandem with setting
// the N_ALT_ENTRY flag for all other symbols in the same section.
if (_hydrationTargetSectionIndex.HasValue)
{
MachSection hydrationTargetSection = _sections[_hydrationTargetSectionIndex.Value];
sortedDefinedSymbols.Add(new MachSymbol
{
Name = ".hydrated",
Section = hydrationTargetSection,
Value = hydrationTargetSection.VirtualAddress,
Descriptor = N_NO_DEAD_STRIP,
Type = N_SECT | N_EXT | N_PEXT,
});
}
foreach ((string name, SymbolDefinition definition) in definedSymbols)
{
MachSection section = _sections[definition.SectionIndex];
Expand All @@ -483,8 +504,10 @@ private protected override void EmitSymbolTable(
Name = name,
Section = section,
Value = section.VirtualAddress + (ulong)definition.Value,
Descriptor = N_NO_DEAD_STRIP,
Type = N_SECT | N_EXT,
Descriptor =
definition.AltEntry || definition.SectionIndex == _hydrationTargetSectionIndex ?
N_ALT_ENTRY : N_NO_DEAD_STRIP,
Copy link
Member Author

@filipnavara filipnavara Aug 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking the N_NO_DEAD_STRIP should no longer be required with the changes in this PR. I left it in out of abundance of caution and to minimize the chance of something going wrong.

Copy link
Member Author

@filipnavara filipnavara Aug 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out, caution was warranted. New Xcode linker still messes up the .hydrated section even if it's completely composed of N_ALT_ENTRY symbols. I changed it to N_ALT_ENTRY | N_NO_DEAD_STRIP and I will probably follow up with a bug report to Apple if I can make a small repro.

Copy link
Member Author

@filipnavara filipnavara Aug 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the pattern, will fix ILC to avoid producing it:

test_data.S:

.subsections_via_symbols
.section __DATA,hydrated
.global .hydrated // <-- symbol we generate at beginning of .hydrated section
.hydrated:
lsection6: // <-- local symbol for beginning of section
.alt_entry _foo_data
.private_extern _foo_data
_foo_data:
  .dword 0
.alt_entry _bar_data
.private_extern _bar_data
_bar_data:
  .dword 1

test_code.S:

extern char *foo_data;
int main() { return (int)(long)&foo_data; }

Build with clang test_data.S test_code.c -o ./test -Wl,-dead_strip [-Wl,-ld_classic]. Check output with nm -nm ./test

For new linker we get:

0000000100000000 (__TEXT,__text) [referenced dynamically] external __mh_execute_header
0000000100003f8c (__TEXT,__text) external _main
0000000100004000 (__DATA,hydrated) external .hydrated
0000000100004000 (__DATA,hydrated) non-external (was a private external) _foo_data

For old linker we get:

0000000100000000 (__TEXT,__text) [referenced dynamically] external __mh_execute_header
0000000100003f9c (__TEXT,__text) external _main
0000000100004000 (__DATA,hydrated) external .hydrated
0000000100004000 (__DATA,hydrated) non-external (was a private external) _foo_data
0000000100004008 (__DATA,hydrated) non-external (was a private external) _bar_data

This only happens if we produce the local section symbol.

Copy link
Member Author

@filipnavara filipnavara Aug 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed as Apple bug FB14743667. The workaround in ILC is orthogonal to the changes in this PR, so I will likely submit it separately after I do sufficient testing. Keeping N_NO_DEAD_STRIP is sufficient workaround for now.

Copy link
Member Author

@filipnavara filipnavara Aug 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funnily enough, if I rename lsectionXX to ltmpXX (ie. the same symbol prefix as emitted for LLVM temporary variables) it works. That seems to be cheap enough workaround.

Turns out, even in the old ld64 there are hacks (1, 2) for l-prefixed and ltmp-prefixed labels getting special treatment.

Type = (byte)(N_SECT | N_EXT | (definition.Global ? 0 : N_PEXT)),
});
}
sortedDefinedSymbols.Sort((symA, symB) => string.CompareOrdinal(symA.Name, symB.Name));
Expand Down
13 changes: 8 additions & 5 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace ILCompiler.ObjectWriter
{
public abstract class ObjectWriter
{
private protected sealed record SymbolDefinition(int SectionIndex, long Value, int Size = 0, bool Global = false);
private protected sealed record SymbolDefinition(int SectionIndex, long Value, int Size = 0, bool Global = false, bool AltEntry = false);
private protected sealed record SymbolicRelocation(long Offset, RelocType Type, string SymbolName, long Addend = 0);
private protected sealed record BlockToRelocate(int SectionIndex, long Offset, byte[] Data, Relocation[] Relocations);

Expand Down Expand Up @@ -246,11 +246,12 @@ protected internal void EmitSymbolDefinition(
string symbolName,
long offset = 0,
int size = 0,
bool global = false)
bool global = false,
bool altEntry = false)
{
_definedSymbols.Add(
symbolName,
new SymbolDefinition(sectionIndex, offset, size, global));
new SymbolDefinition(sectionIndex, offset, size, global, altEntry));
}

/// <summary>
Expand Down Expand Up @@ -421,14 +422,16 @@ private void EmitObject(string objectFilePath, IReadOnlyCollection<DependencyNod
sectionWriter.EmitSymbolDefinition(
n == node ? currentSymbolName : GetMangledName(n),
n.Offset + thumbBit,
n.Offset == 0 && isMethod ? nodeContents.Data.Length : 0);
n.Offset == 0 && isMethod ? nodeContents.Data.Length : 0,
altEntry: n.Offset != 0);
if (_nodeFactory.GetSymbolAlternateName(n) is string alternateName)
{
sectionWriter.EmitSymbolDefinition(
ExternCName(alternateName),
n.Offset + thumbBit,
n.Offset == 0 && isMethod ? nodeContents.Data.Length : 0,
global: true);
global: true,
altEntry: true);
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/SectionWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,16 @@ public readonly void EmitSymbolDefinition(
string symbolName,
long relativeOffset = 0,
int size = 0,
bool global = false)
bool global = false,
bool altEntry = false)
{
_objectWriter.EmitSymbolDefinition(
SectionIndex,
symbolName,
Position + relativeOffset,
size,
global);
global,
altEntry);
}

public readonly void EmitSymbolReference(
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/vm/arm64/asmhelpers.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,7 @@ NESTED_END ThePreStub, _TEXT

LEAF_ENTRY ThePreStubPatch, _TEXT
nop
.globl C_FUNC(ThePreStubPatchLabel)
C_FUNC(ThePreStubPatchLabel):
PATCH_LABEL ThePreStubPatchLabel
ret lr
LEAF_END ThePreStubPatch, _TEXT

Expand Down