This is an archive of the discontinued LLVM Phabricator instance.

Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain
ClosedPublic

Authored by mhjacobson on Jul 27 2021, 2:10 AM.

Details

Summary

The way the GNU linker deals with static archives is that it only loads objects
from the archive that are needed by *previously seen objects*. That is, in this
case, if libm or libc depend on symbols from libgcc, they might be out of luck.

As it happens, it's common for avr-libc to depend on symbols (specifically, the
deduplicated function prologue/epilogue) from libgcc. The linker, as invoked by
clang currently, fails to link the program in this case because of the argument
ordering.

In general, libraries should be supplied to the linker in a dependents-then-
dependencies order.

However, to more perfectly match GCC's behavior here--and since we don't control
the libraries in question--simply use --start-group and --end-group. These
flags tell the linker to repeatedly search all the intervening libraries until
no new undefined symbol references are created.

Diff Detail

Event Timeline

mhjacobson created this revision.Jul 27 2021, 2:10 AM
mhjacobson requested review of this revision.Jul 27 2021, 2:10 AM

Modified regression test Driver/avr-ld.c to account for the new arguments.

benshi001 added inline comments.Jul 27 2021, 7:56 PM
clang/lib/Driver/ToolChains/AVR.cpp
430

You are appreciated to use 'git show -U999999' or 'git diff -U999999' for better context display.

clang/test/Driver/avr-ld.c
23 ↗(On Diff #362185)

could you add explict options (which you added) to these tests ?

Updated the diff to add more context.

mhjacobson marked an inline comment as done.Jul 27 2021, 9:26 PM
mhjacobson added inline comments.
clang/lib/Driver/ToolChains/AVR.cpp
430

Sorry! Fixed, I think.

mhjacobson marked an inline comment as done.

Explicitly added --start-group and --end-group to the expectation in the test.

mhjacobson marked an inline comment as done.Jul 27 2021, 9:40 PM
benshi001 accepted this revision.Jul 29 2021, 6:33 AM

Do you need I land your patch for you?

What name and email do you expected to see in the commit message ?

This revision is now accepted and ready to land.Jul 29 2021, 6:33 AM
benshi001 added a comment.EditedJul 29 2021, 6:42 AM

Do you mind if I change the title to "[AVR][clang] Pass --start-group and --end-group options to avr-ld" ?

Thanks, Ben. Yes, would you mind to land the patch for me? Name and e-mail are

Matt Jacobson <mhjacobson@me.com>

And yes, please do change the title as appropriate. I wasn't sure what the proper naming convention was.

Thanks again!