[WIP] rt.sections_elf_shared: Add support for OSX (prepare for shared Phobos lib) #2322

kinke · 2018-10-05T17:43:59Z

This is almost exactly LDC's version, which uses this module for 32/64-bit OSX too, incl. support for Phobos as shared library, and doesn't use rt.sections_osx_x86[_64] at all.

This is almost exactly LDC's version, which uses this module for 32/64-bit OSX too, incl. support for Phobos as shared library, and doesn't use `rt.sections_osx_x86[_64]` at all.

dlang-bot · 2018-10-05T17:44:01Z

Thanks for your pull request and interest in making D better, @kinke! 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 annotated coverage diff directly on GitHub with CodeCov's browser extension
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

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

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

dub fetch digger
dub run digger -- build "master + druntime#2322"

kinke · 2018-10-05T17:53:37Z

For this to work on OSX, DMD needs to be adapted. After a quick glance, this block would be needed for Mach-O objects too, in addition to

a dummy TLS 'anchor' variable for the DSO,
a little helper function (void* getTlsAnchor() { return &tlsAnchor; }), returning its address for the executing thread, and
adding the pointer to that function to the compiler-generated DSO data struct.

[I'm not going to implement that; no interest in the DMD backend. ;)]

jacob-carlborg · 2018-10-06T08:52:37Z

src/rt/sections_elf_shared.d

@@ -36,6 +40,15 @@ else version (FreeBSD)
    import core.sys.freebsd.sys.elf;
    import core.sys.freebsd.sys.link_elf;
 }
+else version (OSX)


Not sure I understand when you're using SharedDarwin and when you're using OSX.

Yeah I noticed the inconsistency too. It's not my code, and as you know, I have no interest in & knowledge of the different Mac OS flavours (desktop, iOS, watchOS, whatever).

It is your code, you added SharedDarwin.

jacob-carlborg · 2018-10-06T08:57:13Z

src/rt/sections_elf_shared.d

+        void* start = null;
+        size_t size = 0;
+        _d_dyld_getTLSRange(tlsSymbol, &start, &size);
+        assert(start && size, "Could not determine TLS range.");


This check will be removed if asserts are removed, is that intentional?

Most likely not => #2324.

jacob-carlborg · 2018-10-06T08:57:59Z

src/rt/sections_elf_shared.d

@@ -352,6 +388,12 @@ else
 // Compiler to runtime interface.
 ///////////////////////////////////////////////////////////////////////////////

+version (OSX)
+    private alias ImageHeader = mach_header*;


Will this work for both 32bit and 64bit? There's a mach_header_64 type as well. I guess it will work as long as no fields are accessed.

It does work for LDC, apparently for the reason you mentioned (used for _dyld_get_image_header_containing_address(), _dyld_get_image_slide() and LDC-specific hacky (pointer-casting) getSection(); the latter is available in the sections_osx_*.d files here and would need to be moved.]

jacob-carlborg · 2018-10-06T08:59:51Z

For this to work on OSX, DMD needs to be adapted. After a quick glance, this block would be needed for Mach-O objects too

Yeah, that's the big task 😃.

kinke · 2018-10-10T19:23:15Z

that's the big task

It should be doable. My intention was to prepare the druntime side of it, so that DMD is the only blocker. I was hoping you specifically would be interested and knowledgeable enough to finally make DMD catch up with LDC in this regard ;) (and allow us to get rid of the LDC specifics in this file, which should probably be renamed to sections_posix_shared.d in the end).

Btw, the ModuleInfos section range currently required for the compiler-generated DSO data is the reason for the linking errors you got with LLD (magic linker symbols); deriving them from the image headers would be nicer indeed. That should be pretty simple for OSX via getsectbyname(); more work for ELF, although all the code to get an ELF section by name would already be in rt.backtrace.elf. I'll see if I can refactor it, so that we can use it here too (and @joakim-noah could use it for Android too - as well as user code inspecting binary images).

Maybe there's a better way to obtain the TLS range for a specific thread than LDC's variant, one based on the image headers as for ELF...

jacob-carlborg · 2018-10-11T07:39:16Z

It should be doable. My intention was to prepare the druntime side of it, so that DMD is the only blocker. I was hoping you specifically would be interested and knowledgeable enough to finally make DMD catch up with LDC in this regard ;)

I'm definitely interested. If I'm knowledgeable enough that's a different question 😃. I would think I could handle the druntime side of it, i.e. basically what you have in this PR. But the compiler side, I'm not so sure. If the assembly is the same as for ELF, I could probably get the data into the correct sections for Mach-O but if the assembly is different then it would be much more difficult for me. Also there's the standard issue with the lack of time.

Btw, the ModuleInfos section range currently required for the compiler-generated DSO data is the reason for the linking errors you got with LLD (magic linker symbols); deriving them from the image headers would be nicer indeed. That should be pretty simple for OSX via getsectbyname(); more work for ELF, although all the code to get an ELF section by name would already be in rt.backtrace.elf. I'll see if I can refactor it, so that we can use it here too (and @joakim-noah could use it for Android too - as well as user code inspecting binary images).

Cool. BTW, I know that DMD had problems in the past with bracketed sections and the linker messing them up. That's why DMD stopped using them when someone showed how easy it was to get access to a section. But I guess LDC doesn't have those problems since it's using LLVM.

jacob-carlborg · 2018-10-20T19:02:15Z

FYI, DMD only uses the native TLS implementation on macOS 64bit. On 32bit it still uses the old emulated TLS implementation.

Geod24 · 2020-04-02T17:21:18Z

So @kinke , what's the status with this ? Still blocked on the DMD backend I suppose ?

kinke · 2020-04-02T19:45:18Z

Yes. It should be feasible and simpler now without 32-bit x86 macOS support though (i.e., with native TLS).

ibuclaw · 2020-04-03T05:47:45Z

Incase it hasn't been said already, this module is in desperate need of a rename if you add Darwin support to it.

ibuclaw · 2020-12-07T01:14:36Z

src/rt/sections_elf_shared.d

+    {
+        auto header = _dyld_get_image_header_containing_address(addr);
+        if (result) *result = header;
+        return !!header;


Without knowing this existed, I independently came up with this implementation for findDSOInfoForAddr:

foreach (i; 0 .. _dyld_image_count()) { const header = _dyld_get_image_header(i); const slide = _dyld_get_image_vmaddr_slide(i); const section = getSection(header, slide, SEG_DATA, SECT_DATA); // less than base address of section means quick reject if (!section.length || addr < section.ptr) continue; if (addr < section.ptr + section.length) { result.header = header; result.slide = slide; return true; } } return false;

Probably could align ourselves up, mixing what you have here with _dyld_get_image_vmaddr_slide for getting the slide value. So you don't have to rely on a private function later.

I think:

const header = _dyld_get_image_header_containing_address(addr); if (header is null) return false; foreach (i; 0 .. _dyld_image_count()) { if (header == _dyld_get_image_header(i)) { result.header = header; result.slide = _dyld_get_image_vmaddr_slide(i); return true; } } return false;

On darwin9, and older versions, will have to use dladdr() instead (though realistically even supporting darwin8 (OSX 10.4) is a bit of a stretch).

This PR is more than 2 years old - this now looks like this: https://github.com/ldc-developers/druntime/blob/a261a84e4c6b208c65ae39da0ae844ae3dd06744/src/rt/sections_elf_shared.d#L903-L907

The slide can be calculated like this [1]. Also note this [2]:

"The following functions allow you to iterate through all loaded images. This is not a thread safe operation. Another thread can add or remove an image during the iteration.

Many uses of these routines can be replace by a call to dladdr() which will return the mach_header and name of an image, given an address in the image. dladdr() is thread safe."

[1] https://github.com/ldc-developers/druntime/blob/a261a84e4c6b208c65ae39da0ae844ae3dd06744/src/rt/sections_darwin_64.d#L134

[2] https://opensource.apple.com/source/dyld/dyld-750.6/include/mach-o/dyld.h.auto.html

@jacob-carlborg good to know, though you're still using _dyld_image_count and get_image_header

Yeah, IIRC I couldn't get that to work with dladdr.

Geod24 · 2022-07-09T22:52:20Z

Druntime have been merged into DMD. Please re-submit your PR to dlang/dmd repository.

ibuclaw · 2022-07-10T06:20:06Z

FYI I ended up naming these files after their object format (sections.macho, sections.elf, sections.pecoff), rather than trying to shoehorn it all into one place. All inevitable duplication then was moved over to a sections.common module.

rt.sections_elf_shared: Add support for OSX

52d71d2

This is almost exactly LDC's version, which uses this module for 32/64-bit OSX too, incl. support for Phobos as shared library, and doesn't use `rt.sections_osx_x86[_64]` at all.

kinke requested a review from Burgos as a code owner October 5, 2018 17:43

dlang-bot added the WIP Work In Progress - not ready for review or pulling label Oct 5, 2018

kinke changed the title ~~[WIP] rt.sections_elf_shared: Add support for OSX~~ [WIP] rt.sections_elf_shared: Add support for OSX (prepare for shared Phobos lib) Oct 5, 2018

kinke mentioned this pull request Oct 5, 2018

druntime: Streamline rt.sections_elf_shared with upstream ldc-developers/ldc#2868

Merged

jacob-carlborg reviewed Oct 6, 2018

View reviewed changes

kinke mentioned this pull request Dec 2, 2020

darwin: Add __xxx_OS_VERSION_MIN constants for Darwin platforms #3299

Closed

ibuclaw reviewed Dec 7, 2020

View reviewed changes

dlang-bot added Needs Rebase needs a `git rebase` performed stalled labels May 27, 2021

Geod24 closed this Jul 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] rt.sections_elf_shared: Add support for OSX (prepare for shared Phobos lib) #2322

[WIP] rt.sections_elf_shared: Add support for OSX (prepare for shared Phobos lib) #2322

kinke commented Oct 5, 2018

dlang-bot commented Oct 5, 2018

kinke commented Oct 5, 2018 •

edited

Loading

jacob-carlborg Oct 6, 2018

kinke Oct 10, 2018

jacob-carlborg Oct 10, 2018

jacob-carlborg Oct 6, 2018

kinke Oct 10, 2018

jacob-carlborg Oct 6, 2018 •

edited

Loading

kinke Oct 10, 2018 •

edited

Loading

jacob-carlborg commented Oct 6, 2018

kinke commented Oct 10, 2018 •

edited

Loading

jacob-carlborg commented Oct 11, 2018 •

edited

Loading

jacob-carlborg commented Oct 20, 2018

Geod24 commented Apr 2, 2020

kinke commented Apr 2, 2020 •

edited

Loading

ibuclaw commented Apr 3, 2020

ibuclaw Dec 7, 2020 •

edited

Loading

ibuclaw Dec 7, 2020 •

edited

Loading

kinke Dec 7, 2020

jacob-carlborg Dec 7, 2020

ibuclaw Dec 7, 2020

jacob-carlborg Dec 9, 2020

Geod24 commented Jul 9, 2022

ibuclaw commented Jul 10, 2022

[WIP] rt.sections_elf_shared: Add support for OSX (prepare for shared Phobos lib) #2322

[WIP] rt.sections_elf_shared: Add support for OSX (prepare for shared Phobos lib) #2322

Conversation

kinke commented Oct 5, 2018

dlang-bot commented Oct 5, 2018

Bugzilla references

Testing this PR locally

kinke commented Oct 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacob-carlborg Oct 6, 2018 • edited Loading

Choose a reason for hiding this comment

kinke Oct 10, 2018 • edited Loading

Choose a reason for hiding this comment

jacob-carlborg commented Oct 6, 2018

kinke commented Oct 10, 2018 • edited Loading

jacob-carlborg commented Oct 11, 2018 • edited Loading

jacob-carlborg commented Oct 20, 2018

Geod24 commented Apr 2, 2020

kinke commented Apr 2, 2020 • edited Loading

ibuclaw commented Apr 3, 2020

ibuclaw Dec 7, 2020 • edited Loading

Choose a reason for hiding this comment

ibuclaw Dec 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Geod24 commented Jul 9, 2022

ibuclaw commented Jul 10, 2022

kinke commented Oct 5, 2018 •

edited

Loading

jacob-carlborg Oct 6, 2018 •

edited

Loading

kinke Oct 10, 2018 •

edited

Loading

kinke commented Oct 10, 2018 •

edited

Loading

jacob-carlborg commented Oct 11, 2018 •

edited

Loading

kinke commented Apr 2, 2020 •

edited

Loading

ibuclaw Dec 7, 2020 •

edited

Loading

ibuclaw Dec 7, 2020 •

edited

Loading