Skip to content

Conversation

uweigand
Copy link
Contributor

  • Fix byte order of debug directory fields

  • Fix byte order of PDB stream data (PdbHeap::SetData)

* Fix byte order of debug directory fields

* Fix byte order of PDB stream data (PdbHeap::SetData)
@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-ILTools-coreclr and removed community-contribution Indicates that the PR has been added by a community member labels Sep 21, 2021
@ghost
Copy link

ghost commented Sep 21, 2021

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Fix byte order of debug directory fields

  • Fix byte order of PDB stream data (PdbHeap::SetData)

Author: uweigand
Assignees: -
Labels:

area-ILTools-coreclr

Milestone: -

@uweigand
Copy link
Contributor Author

CC @BruceForstall

Yet more endian issues in ilasm, this time when creating debug info / PDB files. The first problem is that the members of the DebugDirectory in the main .DLL need to be properly byte-swapped. The second problem is that when defining the PDB stream data, fields need to be swapped as well (in PdbHeap::SetData in md). The rest of the PDB contents seem fine, they're emitted via the MiniMd mechanism that already handles byte order properly.

This bug currently causes source-build to fail on s390x with the latest .NET 6 code, because a PDB file for an assembly created via ilasm contains invalid data, and therefore illink.dll fails to generate an updated PDB when trimming the assembly, and because of that missing final PDB the packaging fails.

Therefore, it would be great if this fix could still make it into .NET 6, ideally RC2.

@JulieLeeMSFT
Copy link
Member

CC @dotnet/jit-contrib PTAL. Community request to backport to .NET 6 RC2.

@BruceForstall
Copy link
Contributor

/azp run runtime-coreclr ilasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

LGTM

@BruceForstall
Copy link
Contributor

The failures are #58481

@BruceForstall BruceForstall merged commit d32d871 into dotnet:main Sep 22, 2021
@BruceForstall
Copy link
Contributor

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1260093631

@BruceForstall
Copy link
Contributor

/backport to release/6.0-rc2

@github-actions
Copy link
Contributor

Started backporting to release/6.0-rc2: https://github.com/dotnet/runtime/actions/runs/1262715856

@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants