[llvm-dlltool] Implement the --identify option #127465
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mstorsjo
commented
Feb 17, 2025
| bool Contains; | ||
| if (!objContainsSymbol( | ||
| Obj, ObjName, "__NULL_IMPORT_DESCRIPTOR", Contains)) | ||
| return false; |
There was a problem hiding this comment.
A lot of the error handling here feels a bit clumsy, feel free to suggest ways to make it more idiomatic or concise. (I've strived to always return things back to the caller, rather than calling exit() within the functions on error, to make the code more suitable for moving into a library if relevant.)
There was a problem hiding this comment.
Maybe we could use the archive symbol map to look up __NULL_IMPORT_DESCRIPTOR instead. That would replace the entire forEachCoff with a simple iteration over Archive.symbols(), eliminating the need for error handling.
There was a problem hiding this comment.
Nice, thanks! That does indeed make things a bit less unwieldy!
|
Also; for MSVC/LLVM style import libraries, one could simply just look at the member file names as well, for the individual import files (not the header/trailer - as import libraries can contain other object files as well). But as we need logic like this for GNU import libraries, we can use the same logic for both kinds (as MSVC/LLVM style import libraries have regular object files for headers/trailers). |
cjacek
reviewed
Feb 17, 2025
| bool Contains; | ||
| if (!objContainsSymbol( | ||
| Obj, ObjName, "__NULL_IMPORT_DESCRIPTOR", Contains)) | ||
| return false; |
There was a problem hiding this comment.
Maybe we could use the archive symbol map to look up __NULL_IMPORT_DESCRIPTOR instead. That would replace the entire forEachCoff with a simple iteration over Archive.symbols(), eliminating the need for error handling.
This option prints the name of the DLL that gets imported, when linking against an import library. This is implemented using the same strategy as GNU dlltool does; looking for the contents of .idata$6 or .idata$7 chunks. The right section name to check for is chosen by identifying whether the library is GNU or LLVM style. In the case of GNU import libraries, the DLL name is in an .idata$7 chunk. However there are also other chunks with that section name (for entries for the IAT or ILT); identify these by looking for whether a chunk contains relocations. Alternatively, one could also just look for .idata$2 chunks, look for relocations at the right offset, and locate data at the symbol that the relocation points at (which may be in the same or in another object file).
mstorsjo
force-pushed
the
dlltool-identify
branch
from
213c839 to
473e10c
Compare
Oops, sorry about that - fixed now! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/204/builds/1273 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/17050 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/205/builds/1253 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/130/builds/10273 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/203/builds/2463 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/13319 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/13316 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/80/builds/10351 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/13373 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/81/builds/4957 Here is the relevant piece of the build log for the reference |
|
It seems safe enough to me. As you said, it's self-contained and behind an option, so the risk of regression is low. |
|
Failed to cherry-pick: and https://github.com/llvm/llvm-project/actions/runs/14002157716 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
|
/pull-request #132483 |
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Mar 25, 2025
This option prints the name of the DLL that gets imported, when linking against an import library. This is implemented using the same strategy as GNU dlltool does; looking for the contents of .idata$6 or .idata$7 chunks. The right section name to check for is chosen by identifying whether the library is GNU or LLVM style. In the case of GNU import libraries, the DLL name is in an .idata$7 chunk. However there are also other chunks with that section name (for entries for the IAT or ILT); identify these by looking for whether a chunk contains relocations. Alternatively, one could also just look for .idata$2 chunks, look for relocations at the right offset, and locate data at the symbol that the relocation points at (which may be in the same or in another object file). (cherry picked from commit dcc08a1)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This option prints the name of the DLL that gets imported, when linking against an import library.
This is implemented using the same strategy as GNU dlltool does; looking for the contents of .idata$6 or .idata$7 chunks. The right section name to check for is chosen by identifying whether the library is GNU or LLVM style. In the case of GNU import libraries, the DLL name is in an .idata$7 chunk. However there are also other chunks with that section name (for entries for the IAT or ILT); identify these by looking for whether a chunk contains relocations.
Alternatively, one could also just look for .idata$2 chunks, look for relocations at the right offset, and locate data at the symbol that the relocation points at (which may be in the same or in another object file).