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

Scan ABIv2 tables pointer-by-pointer to skip intra-section padding #89

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DHowett-MSFT
Copy link
Contributor

On Windows with incremental linking enabled, structures laid out in the
.objcrt sections may be padded on either side with alignment-sized sets
of null bytes. Since Clang sets the alignment for each ABI section to a
pointer's width, we can skip over those padding bytes by moving
uintptr_twise.

On Windows with incremental linking enabled, structures laid out in the
.objcrt sections may be padded on either side with alignment-sized sets
of null bytes. Since Clang sets the alignment for each ABI section to a
pointer's width, we can skip over those padding bytes by moving
uintptr_twise.
@DHowett-MSFT
Copy link
Contributor Author

I've created this as a draft so that we can discuss it. I agree that this is best solved with a linker directive or by using another linker.

My main contention, which is primary centered around WinObjC (and therefore, we are willing to carry this patch on our own) is twofold:

  • The release of a MSVC linker that supports this is months away, and will go out long after the first update of Visual Studio 2019.
  • WinObjC isn't otherwise shipping a linker, and would not like to take on the burden of shipping the llvm linker in-package for WinObjC consumers to use.
    • (I don't have engineering time to dedicate to llvm-lld compatibility throughout the VS toolstack, unfortunately.)

This patch is fragile, but it tries to only rely on known invariants: that the first field of every runtime structure will never intentionally be NULL.

@davidchisnall
Copy link
Member

I think I'd rather keep this in the WinObjC version, where the ABI guarantees are weaker. I'm not sure that using lld-link.exe instead of link.exe is that much of a problem (the LLVM toolchain files for Visual Studio will use lld-link and clang), but if it is then having this in WinObjC for a release or two and then removing it seems like a better option than upstreaming it. I presume the directive will be silently ignored by linkers that don't support it, so we should be able to make clang emit it soon and then turn this into dead code as soon as the new linker is released.

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

Successfully merging this pull request may close these issues.

None yet

3 participants