Fix 22942 - Check for S_TRHEAD_LOCAL_ZEROFILL alongside S_ZEROFILL #13890

MoonlightSentinel · 2022-03-25T16:36:32Z

The code generated an unexpected / invalid header for the thread_bss
section marked as S_TRHEAD_LOCAL_ZEROFILL. The generated object file
is rejected by ld included in newer XCode versions (>= 13.3).

E.g.

ld: section __DATA/__thread_bss has type zero-fill but non-zero file
    offset file '../generated/build.o' for architecture x86_64

This patch changes the existing code to treat S_TRHEAD_LOCAL_ZEROFILL
like S_ZEROFILL w.r.t. the section size.

The CirrusCI jobs need to use LDC until a working dmd release is available

dlang-bot · 2022-03-25T16:36:34Z

Thanks for your pull request and interest in making D better, @MoonlightSentinel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	22942	blocker	Invalid section type / offset for newer XCode versions

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "stable + dmd#13890"

MoonlightSentinel · 2022-03-25T18:01:39Z

Seems to be an issue introduced by newer MacOS 12?

maxhaton · 2022-03-26T15:18:03Z

src/dmd/backend/machobj.d

+    // fflush(stdout);
+
+    const flags = (I64 ? SecHdrTab64[idx].flags : SecHdrTab[idx].flags);
+    if (flags == S_ZEROFILL || flags == S_THREAD_LOCAL_ZEROFILL)


Doesn't the error message imply the check should be if(!(flags == S_ZEROFILL...))?

Don't think so. The if deals with zero-initialised sections whose size only stored in SDoffset.
FYI the section referenced in the error message is marked as S_THREAD_LOCAL_ZEROFILL

ljmf00 · 2022-03-26T20:51:32Z

.cirrus.yml

@@ -93,6 +93,8 @@ macos12_task:
    OS: osx
    # 12 CPU cores and 24 GB of memory are available
    N: 12
+    # Temporarily use LDC until a release with a fix for issue 22942 is available (probably 2.101)


I would suggest adding FIXME: to avoid potentially forgetting about this indefinitely and change the name of the tasks, otherwise, people think it's being compiled on DMD.

Added the FIXME:. Initially renamed the job but Github still waited for the old job - even though the new job was run successfully. So I would rather keep it for now to unblock the CI.

Added the FIXME:. Initially renamed the job but Github still waited for the old job - even though the new job was run successfully. So I would rather keep it for now to unblock the CI.

This is because Github has required on that CI job for that branch in their rules. You need to go to Settings, somewhere in Protected Branches and deselect that job from being required.

I don't have permissions, so I can't do it, but probably @RazvanN7 or someone else can do that.

You're probably right. Hardly worth the effort though given that these changes will need to be reverted soon (TM)

DMD releases cannot compile for XCode >= 13.3 because of issue 22942.

The code generated an unexpected / invalid header for the `thread_bss` section marked as `S_TRHEAD_LOCAL_ZEROFILL`. The generated object file is rejected by ld included in newer XCode versions (>= 13.3). E.g. ``` ld: section __DATA/__thread_bss has type zero-fill but non-zero file offset file '../generated/build.o' for architecture x86_64 ``` This patch changes the existing code to treat `S_TRHEAD_LOCAL_ZEROFILL` like `S_ZEROFILL` w.r.t. the section size.

thewilsonator · 2022-03-27T00:28:59Z

Lets get this in to unblock the pipelines.

thewilsonator · 2022-03-27T00:36:26Z

Also this will need to be cherrypicked to master as well to unblock it, too.

MoonlightSentinel · 2022-03-27T00:38:52Z

I'll open a PR to merge master into stable

dkorpel · 2022-03-28T12:35:40Z

Thank you for tracking this down! This looks really hairy.

MoonlightSentinel force-pushed the ci/mac-linker-error branch from 9a695a8 to 017497c Compare March 25, 2022 17:18

MoonlightSentinel changed the title ~~cirrus.yml: Use dmd 2.098.1 instead of 2.099.0~~ [Ignore] Debug failures for OSX on Cirrus Mar 25, 2022

MoonlightSentinel closed this Mar 25, 2022

MoonlightSentinel deleted the ci/mac-linker-error branch March 25, 2022 18:01

MoonlightSentinel restored the ci/mac-linker-error branch March 26, 2022 13:35

MoonlightSentinel reopened this Mar 26, 2022

MoonlightSentinel closed this Mar 26, 2022

MoonlightSentinel reopened this Mar 26, 2022

MoonlightSentinel force-pushed the ci/mac-linker-error branch 2 times, most recently from 2447a5f to 23f1420 Compare March 26, 2022 14:30

maxhaton reviewed Mar 26, 2022

View reviewed changes

MoonlightSentinel force-pushed the ci/mac-linker-error branch from a252167 to 6579640 Compare March 26, 2022 15:20

MoonlightSentinel changed the base branch from master to stable March 26, 2022 16:41

dlang-bot added Severity:Bug Fix Severity:Enhancement labels Mar 26, 2022

MoonlightSentinel force-pushed the ci/mac-linker-error branch from d54af66 to 002b749 Compare March 26, 2022 16:41

MoonlightSentinel changed the title ~~[Ignore] Debug failures for OSX on Cirrus~~ Fix 22942 - Check for S_TRHEAD_LOCAL_ZEROFILL alongside S_ZEROFILL Mar 26, 2022

MoonlightSentinel closed this Mar 26, 2022

MoonlightSentinel reopened this Mar 26, 2022

MoonlightSentinel added Review:Blocking Other Work review and pulling should be a priority Compiler:Backend glue code, optimizer, code generation and removed Severity:Enhancement Severity:Bug Fix labels Mar 26, 2022

MoonlightSentinel marked this pull request as ready for review March 26, 2022 18:22

dlang-bot added the Severity:Bug Fix label Mar 26, 2022

MoonlightSentinel force-pushed the ci/mac-linker-error branch 2 times, most recently from 1740564 to b505033 Compare March 26, 2022 19:10

ljmf00 reviewed Mar 26, 2022

View reviewed changes

MoonlightSentinel force-pushed the ci/mac-linker-error branch from b505033 to 08ff060 Compare March 26, 2022 22:47

MoonlightSentinel added 2 commits March 27, 2022 00:47

CirrusCI: Temporarily use ldc instead of dmd for MacOs >= 12

dc63f52

DMD releases cannot compile for XCode >= 13.3 because of issue 22942.

thewilsonator approved these changes Mar 27, 2022

View reviewed changes

thewilsonator added the Merge:auto-merge label Mar 27, 2022

MoonlightSentinel added Infrastructure:Continuous Integration Merge:auto-merge and removed Infrastructure:Continuous Integration Merge:auto-merge labels Mar 27, 2022

MoonlightSentinel merged commit d06857e into dlang:stable Mar 27, 2022

MoonlightSentinel deleted the ci/mac-linker-error branch March 27, 2022 00:47

ibuclaw mentioned this pull request Feb 20, 2024

[stable] GHA: Switch to old Xcode as workaround for macOS-13 jobs #16194

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix 22942 - Check for S_TRHEAD_LOCAL_ZEROFILL alongside S_ZEROFILL #13890

Fix 22942 - Check for S_TRHEAD_LOCAL_ZEROFILL alongside S_ZEROFILL #13890

MoonlightSentinel commented Mar 25, 2022 •

edited

Loading

dlang-bot commented Mar 25, 2022 •

edited

Loading

MoonlightSentinel commented Mar 25, 2022

maxhaton Mar 26, 2022

MoonlightSentinel Mar 26, 2022 •

edited

Loading

ljmf00 Mar 26, 2022

MoonlightSentinel Mar 26, 2022

ljmf00 Mar 26, 2022

ljmf00 Mar 26, 2022

MoonlightSentinel Mar 27, 2022

thewilsonator commented Mar 27, 2022

thewilsonator commented Mar 27, 2022

MoonlightSentinel commented Mar 27, 2022

dkorpel commented Mar 28, 2022

Fix 22942 - Check for S_TRHEAD_LOCAL_ZEROFILL alongside S_ZEROFILL #13890

Fix 22942 - Check for S_TRHEAD_LOCAL_ZEROFILL alongside S_ZEROFILL #13890

Conversation

MoonlightSentinel commented Mar 25, 2022 • edited Loading

dlang-bot commented Mar 25, 2022 • edited Loading

Bugzilla references

Testing this PR locally

MoonlightSentinel commented Mar 25, 2022

maxhaton Mar 26, 2022

Choose a reason for hiding this comment

MoonlightSentinel Mar 26, 2022 • edited Loading

Choose a reason for hiding this comment

ljmf00 Mar 26, 2022

Choose a reason for hiding this comment

MoonlightSentinel Mar 26, 2022

Choose a reason for hiding this comment

ljmf00 Mar 26, 2022

Choose a reason for hiding this comment

ljmf00 Mar 26, 2022

Choose a reason for hiding this comment

MoonlightSentinel Mar 27, 2022

Choose a reason for hiding this comment

thewilsonator commented Mar 27, 2022

thewilsonator commented Mar 27, 2022

MoonlightSentinel commented Mar 27, 2022

dkorpel commented Mar 28, 2022

MoonlightSentinel commented Mar 25, 2022 •

edited

Loading

dlang-bot commented Mar 25, 2022 •

edited

Loading

MoonlightSentinel Mar 26, 2022 •

edited

Loading