Skip to content
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

chore(ilc/mach-o): add workaround or fix for problem with dead_strip #96743

Closed
wants to merge 1 commit into from
Closed

chore(ilc/mach-o): add workaround or fix for problem with dead_strip #96743

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 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 @@ -347,6 +347,10 @@ private protected override void CreateSection(ObjectNodeSection section, string
{
".dotnet_eh_table" => S_ATTR_DEBUG,
".eh_frame" => S_ATTR_LIVE_SUPPORT | S_ATTR_STRIP_STATIC_SYMS | S_ATTR_NO_TOC,
// workaround problem with dead_strip
// see https://github.com/dotnet/runtime/issues/96663
"hydrated" => S_ATTR_NO_DEAD_STRIP,
Copy link
Member

Choose a reason for hiding this comment

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

Hydration target section is essentially BSS, or uninitalized data. The failures in the CI are from the conflicting definition of uninitialized data (ie. no actual data are in the file, just size) and S_ATTR_NO_DEAD_STRIP.

The linker cannot strip something that's not there in the first place, so this is invalid combination.

Copy link
Contributor Author

@anatawa12 anatawa12 Jan 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Umm... In my mac, without S_ATTR_NO_DEAD_STRIP on the hydrated section, another memory error occurs and it is looks hydrated data is not correct.
For example, HasComponentSize for vtable of TypeManagerHandle[] returns false, which violates this assertion.

I found several symbols in the hydrated section are removed and size of hydrated section is changed so I assumed the stripping symbols and spaces in the hydrated will cause problem with __dehydrated_data .

Copy link
Member

Choose a reason for hiding this comment

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

I'm not entirely familiar with the format of dehydrated data. Out of curiosity, did you try running the build with IlcDehydrate=false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

did you try running the build with IlcDehydrate=false?

I just tried with IlcDehydrate=false and it works.

Copy link
Member

@filipnavara filipnavara Jan 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 assume the linker doesn't know precisely where the dehydrated data start and end, and how the relocation table at the end of the data creates the dependencies that should not be stripped. This is non-trivial problem, unfortunately. Maybe we can mark the dehydrated data as non-dead-stripable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a little hard to mark dehydrated data as non-dead-stripable since it's inside const section shared with other data.
However, referencing dehydrated data from C code as a global variable to not strip dehydrated data doesn't fix problem.

(Actually, I first found dehydrated data is stripped so I tried referencing __dehydrated and __Modules symbol. This fixes that hydrated section is not initialized. however, it looks hydrated section is not restored correctly and then, I found hydrated section size is changed and symbols are stripped. that's why I added S_ATTR_NO_DEAD_STRIP flag. Finally, I added S_ATTR_NO_DEAD_STRIP to _modules as a alternative for referencing __dehydrated and __Modules symbol.)

"__modules" => S_ATTR_NO_DEAD_STRIP,
Copy link
Member

@filipnavara filipnavara Jan 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

FWIW, the attribute does make sense here (for the __modules section).

_ => section.Type switch
{
SectionType.Executable => S_ATTR_SOME_INSTRUCTIONS | S_ATTR_PURE_INSTRUCTIONS,
Expand Down
Loading