Fix Issue 18068 - No file names and line numbers in stack trace #2172

wilzbach · 2018-05-03T02:49:36Z

Turns out all that changed was PIE and the addresses with ASLR.

Example:

import std.exception;
void main() {
    enforce(0);
}

So we finally get our line number back in the stack trace on Linux:

> dmddev -g hello.d && ./hello
/home/seb/dlang/dmd/generated/linux/release/64/../../../../../phobos/std/exception.d:515 pure @safe void std.exception.bailOut!(Exception).bailOut(immutable(char)[], ulong, const(char[])) [0x555869fe]
/home/seb/dlang/dmd/generated/linux/release/64/../../../../../phobos/std/exception.d:436 pure @safe int std.exception.enforce!().enforce!(int).enforce(int, lazy const(char)[], immutable(char)[], ulong) [0x55586979]
hello.d:3 _Dmain [0x555868f7]

CC @JinShil @ZombineDev

This is a PoC because I'm not sure on the best way to get the base offset of the binary.

dlang-bot · 2018-05-03T02:49:38Z

Thanks for your pull request, @wilzbach!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	18068	regression	No file names and line numbers in stack trace

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

Regression or critical bug fixes should always target the stable branch. Learn more about rebasing to stable or the D release process.

To target stable perform these two steps:

Rebase your branch to upstream/stable:

git rebase --onto upstream/stable upstream/master

Change the base branch of your PR to stable

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#2172"

JinShil · 2018-05-03T04:13:13Z

Nice work! I think you can use https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/baselib-dladdr-3.html

It's already bound in the druntime at

druntime/src/core/sys/linux/dlfcn.d

Line 293 in 05944a2

int dladdr(in void* __address, Dl_info* __info);

wilzbach · 2018-05-03T08:55:13Z

The behavior of dladdr() is only specified in dynamically linked programs.

However, using the first result from dl_iterate_phdr seems to work too.

JinShil · 2018-05-03T12:01:56Z

Tested locally. DMD test17559 finally passes. In fact for the first time, the entire DMD test suite passes when run locally on my PC. Great!

JinShil · 2018-05-03T12:36:16Z

Did a little more testing locally. This is working great.

kinke · 2018-05-03T19:17:43Z

I had a go at this too as one of the fixes in #2151. I was unsure about how to obtain the base address too and came up with cumbersome parsing of /proc/self/maps. Other differences:

As only the executable's DWARF lineinfo data is used, I check the ELF header whether it's a DSO, and only then use its base address.
I add the base address to the relative DWARF address, making sure the virtual address is unique, which may not be the case in this version here, where the base offset is subtracted from each frame address (incl. frames in other DSOs). ~~If unlucky, you'll get wrong file/line infos for such frames.~~ Erm nope that's not possible, there won't be any lineinfos for addresses outside the executable's memory region.

ibuclaw · 2018-05-04T08:21:55Z

src/rt/backtrace/dwarf.d

+                }
+                ElfObj elfObj;
+                dl_iterate_phdr(&dl_iterate_phdr_cb_ngc_tracehandler, &elfObj);
+                ElfW!"Addr" offset = elfObj.l_addr;


ibuclaw · 2018-05-04T08:23:33Z

src/rt/backtrace/dwarf.d

+            else
+            {
+                // non-ASLR fallback
+                size_t offset;


wilzbach · 2018-05-04T16:29:40Z

I was unsure about how to obtain the base address too

FYI @MartinNowak and I looked at the implementation in FreeBSD and Linux and dl_iterate_phdr will always iterate over the main binary for the first iteration.

FreeBSD:

https://github.com/freebsd/freebsd/blob/d9ac9c210e73d5141195a5497d3e96dfc755d1dc/libexec/rtld-elf/rtld.c#L572
https://github.com/freebsd/freebsd/blob/d9ac9c210e73d5141195a5497d3e96dfc755d1dc/libexec/rtld-elf/rtld.c#L637
https://github.com/freebsd/freebsd/blob/d9ac9c210e73d5141195a5497d3e96dfc755d1dc/libexec/rtld-elf/rtld.c#L3723

Linux:

https://github.com/bminor/glibc/blob/15c19147a8361b7281519b9784b3b823c447cf7a/elf/dl-iteratephdr.c#L32
https://github.com/bminor/glibc/blob/master/elf/rtld.c#L1114

(LM_ID_BASE is always 0 - see https://github.com/bminor/glibc/blob/09533208febe923479261a27b7691abef297d604/dlfcn/dlfcn.h#L47)

I had a go at this too as one of the fixes in #2151

That's a shame to have this work duplicated :/

As only the executable's DWARF lineinfo data is used, I check the ELF header whether it's a DSO, and only then use its base address.

@MartinNowak recommended to only to use the executable code segment (and not the entire DSO), but apparently that currently fails on the CIs. I'm not sure that's actually necessary. Can the binary segment order really be dynamically changed by the loader?

kinke · 2018-05-04T17:57:11Z

FYI MartinNowak and I looked at the implementation in FreeBSD and Linux and dl_iterate_phdr will always iterate over the main binary for the first iteration.

Good, my main point was that the fix shouldn't be Linux-only.

That's a shame to have this work duplicated

I expected the bot to add something to the dlang issue. - Glad the maps-file parsing can be avoided, so it did pay off. :)

@MartinNowak recommended to only to use the executable code segment (and not the entire DSO), but apparently that currently fails on the CIs. I'm not sure that's actually necessary. Can the binary segment order really be dynamically changed by the loader?

No idea, but I'd have considered it unnecessary as well. My concern was that offsetting the addresses for non-relocatable executables may likely be wrong, but I see that you added the check now. - CI fails because of missing .sys in BSD imports.

I'd suggest applying the offset to the DWARF addresses in resolveAddresses() as I do. You don't have to touch the frame addresses this way, which are later used in the stack trace string, so you don't end up with invalid relative addresses for frames outside the executable & keep all frame addresses absolute.
You'll also have to implement (dummy) baseAddress() for new rt.backtrace.macho. And I recommend returning a void* or size_t instead of the platform-dependent ElfBaseAddress struct.

wilzbach · 2018-05-07T19:34:47Z

No idea, but I'd have considered it unnecessary as well. My concern was that offsetting the addresses for non-relocatable executables may likely be wrong, but I see that you added the check now.

I actually removed the check for now as it didn't seem to make a difference.

BTW I also added a test for shared libraries and it seems like debug symbols from shared libraries don't work at all (and have never worked). I still added the test, s.t. it's easier to fix this in a follow-up.

JinShil · 2018-05-11T02:11:33Z

src/rt/backtrace/elf.d

@@ -58,6 +58,62 @@ struct Image

        return null;
    }
+
+    @property size_t baseAddress()


This is an awful lot of logic for a property, and would be run needlessly if called multiple times by a caller that isn't aware of the implementation. The baseAddress shouldn't change during the lifetime of an Image, so wouldn't it be better to put this in the constructor, and store the result as a field?

kinke · 2018-05-16T19:15:14Z

BTW I also added a test for shared libraries and it seems like debug symbols from shared libraries don't work at all (and have never worked).

I see 2 remaining problems with the current backtrace code (in general, unrelated to this PR):

As mentioned above, only the executable's DWARF lineinfo section is scanned, so no file/line infos for frames in other DSOs.
The function names are derived from the dynamic symbols table (via backtrace_symbols()), requiring the ld flag -export-dynamic (added implicitly by DMD, not by LDC), which may significantly increase the binary file size. I guess DWARF debuginfos could be used here as well (if available via -g, falling back to backtrace_symbols() for release binaries).

jacob-carlborg · 2018-05-18T11:39:54Z

What's the status of this?

wilzbach · 2018-05-18T11:53:48Z

Sorry. I haven't had much time for this lately, but I will get to this once I am back from my hiking trip (~end of next week).
IIRC not much was missing here, shared libraries have never been supported for getting the debug info.

jacob-carlborg · 2018-05-18T12:57:41Z

Sorry. I haven't had much time for this lately, but I will get to this once I am back from my hiking trip (~end of next week).

Why are you replying here during your hike 😃.

wilzbach · 2018-05-18T13:47:39Z

(Because it takes about 12h to get there and luckily there's free roaming in Europe nowadays :) )

JinShil · 2018-06-08T22:31:49Z

Ping! Is this ready to go? It'd be awesome to get this in the upcoming release.

wilzbach · 2018-06-26T05:19:16Z

Superseded by #2230

wilzbach requested a review from andralex as a code owner May 3, 2018 02:49

dlang-bot added the Bug Fix Include reference to corresponding bugzilla issue label May 3, 2018

wilzbach force-pushed the debug_line-aslr branch from 6390282 to 40438ac Compare May 3, 2018 08:54

wilzbach changed the title ~~[PoC] Fix Issue 18068 - No file names and line numbers in stack trace~~ Fix Issue 18068 - No file names and line numbers in stack trace May 3, 2018

JinShil approved these changes May 4, 2018

View reviewed changes

ibuclaw reviewed May 4, 2018

View reviewed changes

wilzbach force-pushed the debug_line-aslr branch from 40438ac to 5f36fa4 Compare May 4, 2018 14:52

wilzbach mentioned this pull request May 4, 2018

Stop eagerly generating error message for mismatch arguments dlang/dmd#8216

Closed

wilzbach force-pushed the debug_line-aslr branch from 855c2d2 to 8be4a1c Compare May 4, 2018 16:25

wilzbach force-pushed the debug_line-aslr branch from 8be4a1c to c68901c Compare May 4, 2018 16:30

wilzbach force-pushed the debug_line-aslr branch 2 times, most recently from cee17a2 to 65cbc66 Compare May 7, 2018 14:47

Fix Issue 18068 - No file names and line numbers in stack trace

7ccfb95

wilzbach force-pushed the debug_line-aslr branch 2 times, most recently from 309ddda to a0abca3 Compare May 7, 2018 18:50

wilzbach added 2 commits May 7, 2018 21:55

Add shared library exception line_trace test

5142b4d

Add DwarfDebugMachine debug logging

e263004

wilzbach force-pushed the debug_line-aslr branch from cf8ba37 to e263004 Compare May 7, 2018 19:55

JinShil reviewed May 11, 2018

View reviewed changes

JinShil mentioned this pull request Jun 14, 2018

Add base address offset for PIE executables #2215

Closed

yshui mentioned this pull request Jun 15, 2018

fix issue 18981: SIGSEGV during backtrace when debug info is compressed #2214

Merged

JinShil mentioned this pull request Jun 22, 2018

Fix Issue 18068 - No file names and line numbers in stack trace #2230

Merged

wilzbach closed this Jun 26, 2018

wilzbach deleted the debug_line-aslr branch August 20, 2018 12:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Issue 18068 - No file names and line numbers in stack trace #2172

Fix Issue 18068 - No file names and line numbers in stack trace #2172

wilzbach commented May 3, 2018

dlang-bot commented May 3, 2018 •

edited

JinShil commented May 3, 2018

wilzbach commented May 3, 2018

JinShil commented May 3, 2018

JinShil commented May 3, 2018

kinke commented May 3, 2018 •

edited

ibuclaw May 4, 2018

ibuclaw May 4, 2018 •

edited

wilzbach commented May 4, 2018

kinke commented May 4, 2018 •

edited

wilzbach commented May 7, 2018

JinShil May 11, 2018

kinke commented May 16, 2018 •

edited

jacob-carlborg commented May 18, 2018

wilzbach commented May 18, 2018

jacob-carlborg commented May 18, 2018

wilzbach commented May 18, 2018

JinShil commented Jun 8, 2018

wilzbach commented Jun 26, 2018

Fix Issue 18068 - No file names and line numbers in stack trace #2172

Fix Issue 18068 - No file names and line numbers in stack trace #2172

Conversation

wilzbach commented May 3, 2018

dlang-bot commented May 3, 2018 • edited

Bugzilla references

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

Testing this PR locally

JinShil commented May 3, 2018

wilzbach commented May 3, 2018

JinShil commented May 3, 2018

JinShil commented May 3, 2018

kinke commented May 3, 2018 • edited

ibuclaw May 4, 2018

Choose a reason for hiding this comment

ibuclaw May 4, 2018 • edited

Choose a reason for hiding this comment

wilzbach commented May 4, 2018

kinke commented May 4, 2018 • edited

wilzbach commented May 7, 2018

JinShil May 11, 2018

Choose a reason for hiding this comment

kinke commented May 16, 2018 • edited

jacob-carlborg commented May 18, 2018

wilzbach commented May 18, 2018

jacob-carlborg commented May 18, 2018

wilzbach commented May 18, 2018

JinShil commented Jun 8, 2018

wilzbach commented Jun 26, 2018

dlang-bot commented May 3, 2018 •

edited

kinke commented May 3, 2018 •

edited

ibuclaw May 4, 2018 •

edited

kinke commented May 4, 2018 •

edited

kinke commented May 16, 2018 •

edited