[PATCH] D86909: [lld-macho] Initial support for common symbols
Jez Ng via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 4 15:35:27 PDT 2020
int3 added inline comments.
================
Comment at: lld/MachO/SymbolTable.cpp:85
+ if (auto *common = dyn_cast<CommonSymbol>(s)) {
+ if (size >= common->size) {
+ common->file = file;
----------------
MaskRay wrote:
> int3 wrote:
> > int3 wrote:
> > > int3 wrote:
> > > > MaskRay wrote:
> > > > > This is strange. Is it `>`?
> > > > This is intentional. The `same-size.s` test verifies this behavior, and can be used to check that ld64 does the same thing.
> > > actually, I may have messed up the test...
> > okay fixed. ld64 actually takes the smaller alignment if the sizes are equal. still different from lld-ELF's behavior though...
> This rule is hard to believe...
>
> A larger alignment imposes a larger requirement. Shouldn't the larger alignment be used?
>
> In ELF, size and align are separately considered.
I dunno... ld64 really does seem to behave like this. I haven't found the corresponding part of its code that handles common symbol resolution though
ld64 does take a `-max_default_common_align` option, and I double-checked to make sure that the max was high enough that it wouldn't interfere with our test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86909/new/
https://reviews.llvm.org/D86909
More information about the llvm-commits
mailing list