This is an archive of the discontinued LLVM Phabricator instance.

Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO
ClosedPublic

Authored by yln on Mar 15 2022, 1:41 PM.

Details

Summary

For MachO, lower @llvm.global_dtors into @llvm_global_ctors with
__cxa_atexit calls to avoid emitting the deprecated __mod_term_func.

Reuse the existing WebAssemblyLowerGlobalDtors.cpp to accomplish this.

Enable fallback to the old behavior via Clang driver flag
(-fregister-global-dtors-with-atexit) or llc / code generation flag
(-lower-global-dtors-via-cxa-atexit). This escape hatch will be
removed in the future.

Diff Detail

Event Timeline

yln created this revision.Mar 15 2022, 1:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2022, 1:41 PM
yln requested review of this revision.Mar 15 2022, 1:41 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 15 2022, 1:41 PM
yln added a comment.Mar 15 2022, 1:54 PM

Hi @Orlando and @RKSimon, would you be able to confirm that this version avoids the previous opt: Unknown command line argument '-lower-global-dtors' error?

Lit invocation:
env LIT_FILTER='dtor' ninja check-llvm

yln added a comment.EditedMar 16 2022, 6:43 PM

FYI - you could have continued to use this phab for review, it makes it easier to compare the changes you've made

I will definitely do this next time!

Diff to D121327:
I've added 2 additional calls to initializeLowerGlobalDtorsLegacyPassPass(). The one in the constructor of the legacy pass should ensure that the pass is always registered.

yln added a comment.Mar 16 2022, 6:50 PM

Plan to land tomorrow morning. Test passes on the Debian bot:

PASS: LLVM :: Transforms/LowerGlobalDestructors/lower-global-dtors.ll (72299 of 93291)
RKSimon accepted this revision.Mar 17 2022, 7:09 AM

LGTM - but please hang around after you commit

This revision is now accepted and ready to land.Mar 17 2022, 7:09 AM
This revision was landed with ongoing or failed builds.Mar 17 2022, 10:47 AM
This revision was automatically updated to reflect the committed changes.
yln added a comment.EditedMar 17 2022, 4:13 PM

LGTM - but please hang around after you commit

Thanks! Looks like this time it worked out. Let me know if there are any other problems.

Hi, this causes crash on Mac building bot.
Here is the reduced repro:

opt -lower-global-dtors /tmp/reduced.ll -o /dev/null

$ cat /tmp/reduced.ll
%struct.mach_header = type { i32, i32, i32, i32, i32, i32, i32 }

@__dso_handle = external global %struct.mach_header
@llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 1, void ()* undef, i8* null }]
yln added a comment.Mar 23 2022, 6:42 PM

Thank you @zequanwu, for providing the minimized reproducer! It really made fixing this much easier! :)

I've re-landed the change and added a test specifically for this issue: lower-global-dtors-existing-dos_handle.ll