Skip to content

Comments

fix issue 4014 - add option to add debug info for all referenced types#6908

Merged
WalterBright merged 2 commits intodlang:masterfrom
rainers:cv_symdebref
Jun 23, 2017
Merged

fix issue 4014 - add option to add debug info for all referenced types#6908
WalterBright merged 2 commits intodlang:masterfrom
rainers:cv_symdebref

Conversation

@rainers
Copy link
Member

@rainers rainers commented Jun 15, 2017

This reboots #398 and replaces #4051. If option -gf is passed on the command line, debug info for all referenced types will be added to the object file.

I've come to the conclusion that adding a link to debug information generated elsewhere does not work, especially with shared libraries: even if a DLL has debug info built-in the debugger will not look at it while debugging another DLL/executable.

@dlang-bot
Copy link
Contributor

dlang-bot commented Jun 15, 2017

Thanks for your pull request, @rainers!

Bugzilla references

Fix Bugzilla Description
4014 CodeView debug type info not linked in from library

@rainers
Copy link
Member Author

rainers commented Jun 15, 2017

This exposes a problem in the tool chain

dmd -> OMF object file -> optlink -> cv2pdb -> PDB file

that results in a PDB file that causes the VS debugger to crash when displaying an instance of Object. As far as I can tell this seems to be caused by optlink resorting the type debug info for class methods, which seems to confuse the PDB writer (mspdb*.dll) due to unexpected forward references. This happens without this PR on other classes, too, but doesn't appear as consistent.

As a workaround I've patched cv2pdb to remove any member function infos, these are not used by the debugger anyway.

@rainers
Copy link
Member Author

rainers commented Jun 15, 2017

No idea why, but building the test suite with REQUIRED_ARGS=-gf is faster than building with -g on my laptop (win64):

-g: 3 runs between 4min11s and 4min16s
-gf: 3 runs between 4min05s and 4min10s
none: 1 run takes 17min46s !!! This seems caused by test17338.d from #6718

@rainers
Copy link
Member Author

rainers commented Jun 15, 2017

-g: 3 runs between 4min11s and 4min16s
-gf: 3 runs between 4min05s and 4min10s
none: 1 run takes 17min46s !!! This seems caused by test17338.d from #6718

With #6909 a build without debug info generation took 3min33.

Testing win32 with -g and -gf showed no noticable difference at all, but the build without debug generation failed on my system (stackoverflow in test15779).

src/ddmd/mars.d Outdated
}
else if (strcmp(p + 1, "gf") == 0)
{
global.params.symdebug = global.params.symdebug ? global.params.symdebug : 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very strange construct, it looks like a bug. Better would be:

if (!global.params.symdebug) global.params.symdebug = 1;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why is this code never run by any tests?

@WalterBright
Copy link
Member

Need a test case that throws -gf.

@rainers
Copy link
Member Author

rainers commented Jun 17, 2017

This is a very strange construct,

Ok, amended.

Need a test case that throws -gf.

I've added a test case checking the information in the PDB file (it fails with -g only). Could be the start of more extensive tests for sensible debug information...

@WalterBright
Copy link
Member

Even a test that just throws -gf and does not actually check the result would be better. At least we'd know it doesn't crash executing the gf code.

@rainers
Copy link
Member Author

rainers commented Jun 18, 2017

Even a test that just throws -gf and does not actually check the result would be better.

Better than a test that does check the result?

What's been wrong with the last version was that the test code was compiled only for Win64, so it didn't show up in the code coverage. I've also changed it so that the checks are skipped if there is no installation of msdia*.dll on the test system.

@WalterBright
Copy link
Member

Better than a test that does check the result?

Better than nothing.

@WalterBright
Copy link
Member

Jenkins failing again.

@rainers
Copy link
Member Author

rainers commented Jun 18, 2017

Jenkins failing again.

It's pretty tough to find errors in the jenkins/dub test output. These seems to be the relevant messages:

[INFO] Running /var/lib/jenkins/dlang_projects@3/dlang/dub/test/feat663-search.sh...
No matches found.
Script failure.
readlink -f ${BASH_SOURCE[0]}
[INFO] Running /var/lib/jenkins/dlang_projects@3/dlang/dub/test/interactive-remove.sh...
No package dub was found matching the dependency 0.9.20
No package dub was found matching the dependency 0.9.21
Version strings must not be empty.
Script failure.

Doesn't seem related to this PR. @MartinNowak, any idea ?

@CyberShadow
Copy link
Member

There was a code.dlang.org outage recently, and I got similar error messages when dub tried to contact the website. Perhaps that's what happened there as well.

@rainers
Copy link
Member Author

rainers commented Jun 18, 2017

There was a code.dlang.org outage recently,

Thanks for the info. I've repushed to trigger the builds again.

@WalterBright WalterBright merged commit 8aeca3c into dlang:master Jun 23, 2017
@ibuclaw
Copy link
Member

ibuclaw commented Aug 19, 2017

Why do you need a new switch for this? Surely the default for the debug flag is to do more rather than less?

@rainers
Copy link
Member Author

rainers commented Aug 19, 2017

Why do you need a new switch for this? Surely the default for the debug flag is to do more rather than less?

To get Walters approval. He's afraid that this might cause a slowdown in compilation speed. I think it is hardly measurable. See #398 for some discussion.

@ibuclaw
Copy link
Member

ibuclaw commented Aug 20, 2017

You're building with debug codegen turned on, speed is irrelevant, and accuracy takes precedence. I already have to apologise when people hit internal error: 'this' is not an aggregate in gdb, even though it's caused by dmd making shortcuts. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants