Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Print source file directory in stack traces #2945

Merged
merged 1 commit into from Feb 22, 2020

Conversation

yazd
Copy link
Contributor

@yazd yazd commented Feb 21, 2020

DMD's implementation of dwarf debug data puts the directory in the filename directly. A more correct implementation, like LDC's, uses the include_directories for that.
The current implementation only uses the filenames when printing the stacktrace. This means that stacktraces for programs compiled using LDC only contained the filename (not including the filepath).

package.d:2577 [0x55605bed2022]
stdio.d:3002 [0x55605bed8b53]
primitives.d:276 [0x55605bed8b00]
primitives.d:379 [0x55605bed88db]
format.d:3284 [0x55605bed8320]
format.d:4230 [0x55605bed81c9]
format.d:1854 [0x55605bed3058]
format.d:575 [0x55605bed2863]
stdio.d:1596 [0x55605bed22c6]
stdio.d:3927 [0x55605bed1652]

vs

/home/yazan/repos/d-devel/ldc/runtime/phobos/std/range/primitives.d:2458 [0x56518645c434]
/home/yazan/repos/d-devel/ldc/runtime/phobos/std/range/package.d:2619 [0x56518645cd32]
/home/yazan/repos/d-devel/ldc/runtime/phobos/std/stdio.d:3041 [0x565186464690]
/home/yazan/repos/d-devel/ldc/runtime/phobos/std/range/primitives.d:276 [0x565186464640]
/home/yazan/repos/d-devel/ldc/runtime/phobos/std/range/primitives.d:379 [0x56518646442b]
/home/yazan/repos/d-devel/ldc/runtime/phobos/std/format.d:3455 [0x565186463e8a]
/home/yazan/repos/d-devel/ldc/runtime/phobos/std/format.d:4429 [0x565186463d39]
/home/yazan/repos/d-devel/ldc/runtime/phobos/std/format.d:1875 [0x56518645dcb8]
/home/yazan/repos/d-devel/ldc/runtime/phobos/std/format.d:576 [0x56518645d459]
/home/yazan/repos/d-devel/ldc/runtime/phobos/std/stdio.d:1609 [0x56518645cee4]
/home/yazan/repos/d-devel/ldc/runtime/phobos/std/stdio.d:3967 [0x56518645c372]

This commit fixes the stack trace printing to use directory name as well if available so that it works for DMD and LDC.

@dlang-bot
Copy link
Contributor

dlang-bot commented Feb 21, 2020

Thanks for your pull request and interest in making D better, @yazd! 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
20591 normal ldc doesn't print files' directories when printing stack trace

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

@thewilsonator
Copy link
Contributor

Please also add a test and either a changelog entry or reference a bugzilla issue

@yazd
Copy link
Contributor Author

yazd commented Feb 21, 2020

There is already a test in druntime for this (line_trace under https://github.com/dlang/druntime/blob/master/test/exceptions/).
There is no change for dmd and I can confirm that the test passes for dmd, before and after this commit.
I am not sure if ldc is running this test, because it shouldn't pass without this commit. And I am stuck trying to find out how to run runtime tests for ldc. Can anyone help?

@thewilsonator
Copy link
Contributor

Don't worry about LDC, it has its own version of druntime that is updated periodically.

@dlang-bot dlang-bot added the Bug Fix Include reference to corresponding bugzilla issue label Feb 21, 2020
@yazd
Copy link
Contributor Author

yazd commented Feb 21, 2020

Is the test failure random?

If there is anything left for me to do, let me know.

Edit: oh I see, it's due to a conflict. I'll resolve that now.

@thewilsonator thewilsonator merged commit b4fba44 into dlang:master Feb 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Fix Include reference to corresponding bugzilla issue
Projects
None yet
3 participants