core.internal.elf: Improve usability by kinke · Pull Request #2983 · dlang/druntime

kinke · 2020-03-12T19:49:06Z

Paving the way for a revamped rt.sections_android coming up next (ldc-developers#178).

dlang-bot · 2020-03-12T19:49:09Z

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 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

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 run digger -- build "master + druntime#2983"

kinke · 2020-03-12T19:50:09Z

src/core/internal/elf/io.d

+            printf("not mapped into memory\n");
+        }
+    }
+}


E.g.:

section 0 not mapped into memory section 1 .interp 0x55a0f1d27270 - 0x55a0f1d2728c section 2 .note.ABI-tag 0x55a0f1d2728c - 0x55a0f1d272ac section 3 .note.gnu.build-id 0x55a0f1d272ac - 0x55a0f1d272d0 section 4 .gnu.hash 0x55a0f1d272d0 - 0x55a0f1d273b0 section 5 .dynsym 0x55a0f1d273b0 - 0x55a0f1d278d8 section 6 .dynstr 0x55a0f1d278d8 - 0x55a0f1d280da section 7 .gnu.version 0x55a0f1d280da - 0x55a0f1d28148 section 8 .gnu.version_r 0x55a0f1d28148 - 0x55a0f1d281a8 section 9 .rela.dyn 0x55a0f1d281a8 - 0x55a0f1d28358 section 10 .rela.plt 0x55a0f1d28358 - 0x55a0f1d284f0 section 11 .init 0x55a0f1d284f0 - 0x55a0f1d28507 section 12 .plt 0x55a0f1d28510 - 0x55a0f1d28630 section 13 .plt.got 0x55a0f1d28630 - 0x55a0f1d28638 section 14 .text 0x55a0f1d28640 - 0x55a0f1d28d72 section 15 .fini 0x55a0f1d28d74 - 0x55a0f1d28d7d section 16 .rodata 0x55a0f1d28d80 - 0x55a0f1d28df0 section 17 .eh_frame_hdr 0x55a0f1d28df0 - 0x55a0f1d28e9c section 18 .eh_frame 0x55a0f1d28ea0 - 0x55a0f1d291d8 section 19 .gcc_except_table 0x55a0f1d291d8 - 0x55a0f1d291f8 section 20 .tbss 0x55a0f1f29cd8 - 0x55a0f1f29ce8 section 21 .init_array 0x55a0f1f29cd8 - 0x55a0f1f29ce8 section 22 .fini_array 0x55a0f1f29ce8 - 0x55a0f1f29cf8 section 23 .dynamic 0x55a0f1f29cf8 - 0x55a0f1f29f28 section 24 .got 0x55a0f1f29f28 - 0x55a0f1f2a000 section 25 .data 0x55a0f1f2a000 - 0x55a0f1f2a058 section 26 __minfo 0x55a0f1f2a058 - 0x55a0f1f2a060 section 27 .bss 0x55a0f1f2a060 - 0x55a0f1f2a070 section 28 .comment not mapped into memory section 29 .symtab not mapped into memory section 30 .strtab not mapped into memory section 31 .shstrtab not mapped into memory

kinke · 2020-03-12T19:50:55Z

src/core/internal/elf/dl.d

+        printf("    path: %s\n", path ? path.ptr : "");
+        printf("    base: %p\n", object.baseAddress);
+    }
+}


E.g.:

DSO name: runtime/druntime-test-runner-shared path: /home/martin/build-ldc/runtime/druntime-test-runner-shared base: 0x5589831e6000 DSO name: linux-vdso.so.1 path: [vdso] base: 0x7ffdeb9f7000 DSO name: /home/martin/build-ldc/lib/libdruntime-ldc-unittest-shared.so.91 path: /home/martin/build-ldc/lib/libdruntime-ldc-unittest-shared.so.2.0.91 base: 0x7ff21166b000 DSO name: /lib/x86_64-linux-gnu/libgcc_s.so.1 path: /lib/x86_64-linux-gnu/libgcc_s.so.1 base: 0x7ff211453000 DSO name: /lib/x86_64-linux-gnu/libc.so.6 path: /lib/x86_64-linux-gnu/libc-2.27.so base: 0x7ff211062000 DSO name: /lib64/ld-linux-x86-64.so.2 path: /lib/x86_64-linux-gnu/ld-2.27.so base: 0x7ff211b53000 DSO name: /lib/x86_64-linux-gnu/libm.so.6 path: /lib/x86_64-linux-gnu/libm-2.27.so base: 0x7ff210cc4000 DSO name: /lib/x86_64-linux-gnu/libpthread.so.0 path: /lib/x86_64-linux-gnu/libpthread-2.27.so base: 0x7ff210aa5000 DSO name: /lib/x86_64-linux-gnu/librt.so.1 path: /lib/x86_64-linux-gnu/librt-2.27.so base: 0x7ff21089d000 DSO name: /lib/x86_64-linux-gnu/libdl.so.2 path: /lib/x86_64-linux-gnu/libdl-2.27.so base: 0x7ff210699000

kinke · 2020-03-12T19:57:52Z

src/core/internal/elf/io.d

-         * Tries to find the index of the section with the specified name.
-         * Returns: -1 if not found, otherwise 0-based section index.
-         */
-        size_t findSectionIndexByName(const(char)[] sectionName) const


This function has been removed. These modules have just been added with v2.091 (#2330); at the time I originally worked on this (~1.5 years ago), there was no move in druntime (only in Phobos), so I've resorted to this little helper for findSectionHeaderByName().

kinke · 2020-03-12T20:01:12Z

src/core/internal/elf/dl.d

+    {
+        const name = object.name();
+        assert(name.length);
+        const path = object.getPath(buffer);


I've originally asserted that path isn't null (no error), but that failed for some Linux CI systems for LDC. I haven't bothered looking into what the issue is.

Geod24

Nice change. Since the unittests output something, can they be made integration tests ?

Geod24 · 2020-03-13T00:55:00Z

src/core/internal/elf/dl.d

 private @nogc nothrow:

-version (linux)
+version (CRuntime_Bionic)


So __USE_GNU is true unconditionally, which means program_invocation_name from core.sys.linux.errno is also available when compiling for Android. Should one of those be guarded by a version ?

Heh, I was a bit lazy and took this workaround from the guy who tested this stuff. I'll try to get ahold of the Android NDK headers and check,

By the looks of it, __USE_GNU etc. is defined for most Linux C libs in https://github.com/dlang/druntime/blob/master/src/core/sys/posix/config.d (notable exception: version(CRuntime_Musl)), incl. being false for Android/Bionic. The uncoditional enums in core.sys.linux.config (publicly importing core.sys.posix.config but 'overriding' with unconditional enums, apparently from glibc) seem wrong and obsolete. I guess the proper fix would be to make sure all expected enums are already set up in the Posix module, for all Linux C libs.

That looks like a dangerous change though (I've just seen that __USE_GNU is false for glibc in ...posix.config!), so I don't want to add it in this PR. I'll add a TODO comment here.

kubo39 · 2020-03-13T10:28:54Z

src/core/internal/elf/dl.d

+
+        char[N + 128] lineBuffer = void;
+
+        snprintf(lineBuffer.ptr, lineBuffer.length, "/proc/%d/maps", getpid());


In FreeBSD, using kinfo_getvmmap seems more suitable, this works even if /proc is not mounted.
https://www.freebsd.org/cgi/man.cgi?query=kinfo_getvmmap

I'm not sure that's worth it (incl. the libutil dependency); I've just tried to write this in a portable way (e.g., no /proc/self/maps etc.).

kinke · 2020-03-13T13:35:04Z

Since the unittests output something, can they be made integration tests ?

You mean a test with Makefile etc. in the test dir? My primary motivation was to add examples of how to use these modules (and test that the code is not totally broken), without digging into platform specifics for thorough segment/section checks. That's why I'd prefer to keep the tests directly in these modules, so that people find them.

As alternative to get ahold of the ELF file path in case name() doesn't return a full path.

…able()

…med sections E.g., this allows to conveniently scan for multiple named sections in a single pass.

kinke requested review from MartinNowak, andralex, schveiguy and wilzbach as code owners March 12, 2020 19:49

kinke commented Mar 12, 2020

View reviewed changes

thewilsonator approved these changes Mar 13, 2020

View reviewed changes

Geod24 reviewed Mar 13, 2020

View reviewed changes

kubo39 reviewed Mar 13, 2020

View reviewed changes

kinke added 5 commits March 14, 2020 15:11

core.internal.elf.dl: Account for Android/Bionic special case

f943104

core.internal.elf.dl: Add method SharedObject.getPath()

4d7bcd1

As alternative to get ahold of the ELF file path in case name() doesn't return a full path.

core.internal.elf.dl: Add convenience factory SharedObject.thisExecut…

e3c8f38

…able()

core.internal.elf.io: Enable convenience method for iterating over na…

c3f8b11

…med sections E.g., this allows to conveniently scan for multiple named sections in a single pass.

Add some core.internal.elf unittests

cd814db

kinke force-pushed the elf branch from ffb4898 to cd814db Compare March 14, 2020 14:11

kinke added 2 commits March 14, 2020 16:10

Android: Fix __WORDSIZE for 64-bit targets

e3bc484

Android: Fix Elf_Symndx for 64-bit targets

bfca174

kinke requested review from Burgos and CyberShadow as code owners March 14, 2020 15:11

Geod24 added the auto-merge label Mar 15, 2020

dlang-bot merged commit fc96727 into dlang:master Mar 15, 2020

kinke deleted the elf branch March 16, 2020 01:15


		char[N + 128] lineBuffer = void;

		snprintf(lineBuffer.ptr, lineBuffer.length, "/proc/%d/maps", getpid());

Uh oh!

Conversation

kinke commented Mar 12, 2020

Uh oh!

dlang-bot commented Mar 12, 2020

Bugzilla references

Testing this PR locally

Uh oh!

kinke Mar 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Geod24 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kinke commented Mar 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kinke Mar 12, 2020 •

edited

Loading