Create xplat and Windows native symbol packages by dagood · Pull Request #7756 · dotnet/corefx

dagood · 2016-04-14T19:50:19Z

On Windows, in Debug configuration, creates symbol packages with the .pdb from the native build.

On Linux/OSX, in Release configuration, creates symbol packages with symbols that have been stripped from the release library. On Linux the symbol files are .so.dbg, on OSX .dylib.dwarf.

The symbol stripping code is from https://github.com/dotnet/coreclr/blob/afdce3a592e/CMakeLists.txt#L279-L340 as well as some simplified platform detection logic from earlier in that file. I also looked at the PRs for stripping coreclr symbols (dotnet/coreclr#3872) and creating coreclr symbol packages (dotnet/coreclr#4218) a lot while making these changes.

I've tested on Win10, OSX 10.11, and Ubuntu 14.04.4. I've uploaded the resulting native symbol packages to https://www.myget.org/gallery/dagood-test-native-symbols-corefx.

I had to rig a few things to test this out (I'll be looking into them further):

On OSX, I had to force my RuntimeOS to osx.10.10 in build-packages.sh. It's being used to filter which packages are created, and my osx.10.11 resulted in no runtime packages being made.
On all platforms I passed /p:ConfigurationGroup=Release (or Debug) to build-packages. The command should probably have a switch built in.

/cc @ericstj @weshaggard @brianrob

ellismg · 2016-04-14T20:43:51Z

src/Native/CMakeLists.txt

+    endif(CLR_CMAKE_PLATFORM_UNIX)
+endfunction()
+
+function(install_clr targetName)


NIT: I believe I understand why this function is named like it is (since these targets initially came from CoreCLR), but maybe we should instead call it install_corefx or install_library_and_symbols ?

Yep, that's the reason, but I agree. I like install_library_and_symbols. System.Native still uses normal install for the static lib and using a more descriptive name for the dynamic lib will make the difference more obvious than install vs. install_corefx. (And if we need to change install_... to do more stuff than just libraries and symbols I think we could just refactor at that point.) Renaming.

ellismg · 2016-04-15T01:00:56Z

You can check out the symbol packages by clicking the "more" links or directly downloading by URL convention. I have not tried to debug with these symbols--I don't know how to set up a reasonable scenario.

On windows, you could consider using symchk (which comes with the debugging tools for windows) to ensure that the symbols in the package are private symbols and line up with the binaries we are producing.

@mikem8361 might know if there is a similar tool on OSX/Linux to ensure that the a symbol file matches up to a .so

ellismg · 2016-04-15T01:03:15Z

src/Native/pkg/dir.targets

+    </File>
+
+    <!-- Include placeholder pdb on non-Windows platforms to ensure package is treated like a symbol package. -->
+    <File Include="$(MSBuildThisFileDirectory)\_.pdb" Condition="'$(PackageTargetRuntime)'!='' AND '$(OSGroup)'!='Windows_NT'">


Have we talked with MyGet/NuGet to understand if this sort of thing is reasonable to be doing? I worry a little bit it's going to cause problems for downstream consumers.

No, we're trying to work out what the way forward here should be in general, too, so we still need to talk to MyGet. I added you to the thread about this. --But we should definitely talk to them before publishing these to our real feed, even just to make sure they're aware of this.

I don't see how these would cause any problems, though--these packages upload straight to a MyGet-specific feature (skipping the dotnet-core feed), so it seems like it would require quite some effort to notice this, let alone get broken by it. Maybe I don't understand the kind of problems you're thinking of.

(Edit) I've been in contact with MyGet, VSTS, NuGet, and there haven't been objections.

dagood · 2016-04-15T16:31:46Z

Good idea. I tried out dwarfdump --uuid on my OSX box and it returned the same values for corresponding files, so that's a start. That doesn't seem like a rigorous check, though.

symchk passes the Windows/Debug dll+pdb pair, but for Windows/Release it says SYMCHK: clrcompression.dll FAILED - No CodeView information found. (Not that I had a pdb to test against.)

ellismg · 2016-04-15T18:50:33Z

but for Windows/Release it says SYMCHK: clrcompression.dll FAILED - No CodeView information found. (Not that I had a pdb to test against.)

We'll need to get a symbol file produced before we start using this clrcompression.dll for real We should get a bug on it.

dagood · 2016-04-15T20:17:48Z

I added a linker flag to create Windows/Release clrcompression.pdb and started making a symbol package for it. I also found people suggesting the REF and ICF linker optimizations (https://msdn.microsoft.com/en-us/library/bxwfs976.aspx) so I threw those in for now--let me know if they're not appropriate.

mikem8361 · 2016-04-15T21:32:12Z

@mikem8361 https://github.com/mikem8361 might know if there is a similar tool on OSX/Linux to ensure that the a symbol file matches up to a .so
I don’t know of any tools yet. I’m still learning (even after a year or so) about Linux and OS X.

dagood · 2016-04-19T15:32:08Z

#7840 does the Windows native pdb packaging, so once that goes in I'll remove the Windows changes I have here to get rid of the merge conflict that'll show up. 😄 (For testing I resolved the merge locally and it works after that.)

ianhays · 2016-04-19T19:25:14Z

@dagood done! PDBs will be produced with clrcompression now.

dagood · 2016-04-19T20:34:41Z

@ianhays Thanks! Rebased on master.

dagood · 2016-05-18T22:35:15Z

Rebased this on latest and cleaned up. Previously I'd thought we were going a different direction with the symbol packages, but it turns out this PR is the way we're going forward. At some point we won't need the placeholder _.pdb and src/_._ and I'll remove them then. In general, MyGet is on board and there aren't objections or better solutions from anyone else. (Unless someone here can think of one now. 😄)

mmitche · 2016-05-19T17:59:29Z

@dotnet-bot test Innerloop OSX Release Build and Test (rebooted machine by accident)

dagood · 2016-05-20T16:06:37Z

@ellismg Now that the symbols plan is more solid, could you look again? @ericstj Maybe you could take a look too? Thanks!

ericstj · 2016-05-20T17:02:28Z

Packaging changes look reasonable to me. Make sure to diff the nuspecs before/after your change to make sure you only see changes you expect. I commonly use msbuild packages.builds /p:BuildPackageLibraryReferences=false /m /p move the bin/packages/debug/specs directory, apply my changes, rerun that command and diff the old and new specs directory.

ellismg · 2016-05-20T17:08:03Z

src/Native/pkg/dir.props

+
+  <!-- Add path globs specific to native binaries to exclude unnecessary files in packages. -->
+  <ItemGroup>
+    <AdditionalLibPackageExcludes Include="%2A%2A\%2A.dbg" Condition="'$(OSGroupPath)'=='Linux'" />


Can we just use $(SymbolFileExtension) here instead (and have the glob be the same?)

Good idea, and I think I'll do similar for the other globs. (I'll also move the condition to the ItemGroup to continue not adding these on Windows.)

ellismg · 2016-05-20T17:09:50Z

@dagood, you will need another rebase to pick up the Open Suse 13.2 package authoring stuff, so you can make changes to that pkgproj as well, I think.

Simple replacement. This allows the symbol files to be added later with the correct File items.

dagood · 2016-05-20T17:36:01Z

@ellismg Thanks, rebased again and changed those pkgprojs.

@ericstj Good idea. It indeed added pdbs to Windows Debug and Release nuspecs, and only added the xplat native symbols on the Release build (where they would be stripped into the symbol file). Only other change was that it ordered the .so files to be after the .a files in the packages that had both, due to NativeFile vs File.

ellismg · 2016-05-20T18:44:15Z

Other than my small comments, LGTM. Thanks for doing this, Davis.

Rename install_clr function to install_library_and_symbols.

dagood · 2016-05-20T21:15:19Z

@dotnet-bot test Innerloop OSX Debug Build and Test
@dotnet-bot test Innerloop OSX Release Build and Test
(Jenkins reboot.)

Create xplat and Windows native symbol packages Commit migrated from dotnet/corefx@9823256

dnfclas added the cla-already-signed label Apr 14, 2016

dotnet-bot added the 2 - In Progress label Apr 14, 2016

ellismg reviewed Apr 14, 2016
View reviewed changes

ellismg reviewed Apr 15, 2016
View reviewed changes

ianhays mentioned this pull request Apr 18, 2016

Fix clrcompression windows build #7840

Merged

dagood force-pushed the native-symbol-packages branch from 80723d6 to 207c31a Compare April 19, 2016 20:23

stephentoub assigned dagood May 15, 2016

dagood force-pushed the native-symbol-packages branch 2 times, most recently from f907ce9 to 50dc286 Compare May 18, 2016 22:18

dagood force-pushed the native-symbol-packages branch from 50dc286 to f5c0bb4 Compare May 19, 2016 16:49

ellismg reviewed May 20, 2016
View reviewed changes

Change File to NativeFile in native pkgprojs.

8f52a65

Simple replacement. This allows the symbol files to be added later with the correct File items.

dagood force-pushed the native-symbol-packages branch from f5c0bb4 to bb49b7a Compare May 20, 2016 18:48

Package native symbols. Port xplat native binary stripping from coreclr.

834d657

Rename install_clr function to install_library_and_symbols.

dagood force-pushed the native-symbol-packages branch from bb49b7a to 834d657 Compare May 20, 2016 18:50

dagood merged commit 9823256 into dotnet:master May 20, 2016

dotnet-bot removed the 2 - In Progress label May 20, 2016

dagood deleted the native-symbol-packages branch May 20, 2016 22:10

karelz modified the milestone: 1.0.0-rtm Dec 3, 2016

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

Merge pull request dotnet/corefx#7756 from dagood/native-symbol-packages

ecc2062

Create xplat and Windows native symbol packages Commit migrated from dotnet/corefx@9823256

Conversation

dagood commented Apr 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ellismg Apr 14, 2016

Choose a reason for hiding this comment

Uh oh!

dagood Apr 14, 2016

Choose a reason for hiding this comment

Uh oh!

ellismg commented Apr 15, 2016

Uh oh!

ellismg Apr 15, 2016

Choose a reason for hiding this comment

Uh oh!

dagood Apr 15, 2016 • 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!

dagood commented Apr 15, 2016

Uh oh!

ellismg commented Apr 15, 2016

Uh oh!

dagood commented Apr 15, 2016

Uh oh!

mikem8361 commented Apr 15, 2016

Uh oh!

dagood commented Apr 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ianhays commented Apr 19, 2016

Uh oh!

dagood commented Apr 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dagood commented May 18, 2016

Uh oh!

mmitche commented May 19, 2016

Uh oh!

dagood commented May 20, 2016

Uh oh!

ericstj commented May 20, 2016

Uh oh!

ellismg May 20, 2016

Choose a reason for hiding this comment

Uh oh!

dagood May 20, 2016

Choose a reason for hiding this comment

Uh oh!

ellismg commented May 20, 2016

Uh oh!

dagood commented May 20, 2016

Uh oh!

ellismg commented May 20, 2016

Uh oh!

dagood commented May 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

dagood commented Apr 14, 2016 •

edited

Loading

dagood Apr 15, 2016 •

edited

Loading

dagood commented Apr 19, 2016 •

edited

Loading

dagood commented Apr 19, 2016 •

edited

Loading