Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vcpkg | vcpkg_copy_pdbs]: Support copying .pdb when using llvm/clang-mingw #28143

Closed
wants to merge 4 commits into from

Conversation

past-due
Copy link
Contributor

@past-due past-due commented Dec 3, 2022

Describe the pull request

Clang-based mingw toolchains can support generating .PDB files (example: https://github.com/mstorsjo/llvm-mingw/blob/master/README.md#pdb-support )

However, the existing vcpkg_copy_pdbs function does not support this.

This PR makes two core changes / additions to vcpkg_copy_pdbs:

  1. Always support trying dumpbin /PDBPATH when the host is Windows (even if using a mingw toolchain)
  2. Support a backup path to find the .pdb files when cross-compiling with mingw (or if dumpbin fails or cannot be found, which may happen - even on Windows - depending on how PATHS are set up for use with the mingw toolchain).
  • Which triplets are supported/not supported? Have you updated the CI baseline?

    windows / mingw, No updates needed

  • Does your PR follow the maintainer guide?

    Yes

@Neumann-A
Copy link
Contributor

@JonLiu1993 JonLiu1993 added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Dec 5, 2022
@JonLiu1993 JonLiu1993 changed the title vcpkg_copy_pdbs: Support copying .pdb when using llvm/clang-mingw [vcpkg | vcpkg_copy_pdbs]: Support copying .pdb when using llvm/clang-mingw Dec 5, 2022
@JonLiu1993
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Mar 17, 2023
@vicroms vicroms added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Mar 20, 2023
@vicroms
Copy link
Member

vicroms commented Mar 28, 2023

Hi @past-due

We've been actively trying to remove our dependency on dumpbin by implementing the features we need directly in the vcpkg executable (see this PR to remove dumpbin calls and the changes to replace applocal.ps1).

Would you be willing to investigate whether we can get the PDB path information using a method that would not require dumpbin?

These seem like good starting points:

@vicroms vicroms added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Mar 28, 2023
@past-due
Copy link
Contributor Author

@vicroms If you are already parsing the PE header, I believe:

  • If you get the IMAGE_OPTIONAL_HEADER,
  • use that to get the pointer to the IMAGE_DEBUG_DIRECTORY (DataDirectory[IMAGE_DIRECTORY_ENTRY_DEBUG]?)
  • then if IMAGE_DEBUG_DIRECTORY.Type == IMAGE_DEBUG_TYPE_CODEVIEW, cast <base image address> + IMAGE_DEBUG_DIRECTORY.AddressOfRawData to a CodeView debugging information struct*

Which I believe takes a form like (from various snippets around the web):

struct CodeViewDebuggingInfo
{
  DWORD Signature;
  BYTE Guid[16];
  DWORD Age;
  char PdbFileName[0]; // presumably this is null-terminated? Probably a good idea to check `IMAGE_DEBUG_DIRECTORY.SizeOfData`
};

(Completely untested and unverified, but hopefully that helps point in a useful direction.)

I still think there's probably merit to keeping the fallback that this PR adds (which is to just search for a matching .pdb file in the same directory) as well.

@JonLiu1993 JonLiu1993 marked this pull request as draft June 16, 2023 08:02
@JonLiu1993
Copy link
Member

Closing this PR since it seems that no progress is being made. Please ping us to reopen if work is still being done.

@JonLiu1993 JonLiu1993 closed this May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants