Linker flags reorder#54
Conversation
Linker flags needed to be reorder due to issue with link order and link grouping when using static libraries. resolves: #53
There was a problem hiding this comment.
Pull request overview
Adjusts the Linux C++ toolchain default linker flags to address link-order/link-grouping problems seen with static libraries (notably libgcc resolution on AArch64), as described in issue #53.
Changes:
- Replaces
-static-libstdc++/-static-libgccwith explicit-Wl,-Bstatic+-lstdc++ -lgcc -lgcc_eh, then switches back to dynamic for system libs. - Adds an explicit
-lcto the default link flags set.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "-Wl,-Bstatic", | ||
| "-lstdc++", | ||
| "-lgcc", | ||
| "-lgcc_eh", | ||
| "-Wl,-Bdynamic", |
AlexanderLanin
left a comment
There was a problem hiding this comment.
AI-assisted review (Claude):
Critical: 0 | Important: 2 | Suggestions: 1
Summary
The PR replaces -static-libstdc++/-static-libgcc (GCC driver-level flags) with explicit -Wl,-Bstatic -lstdc++ -lgcc -lgcc_eh -Wl,-Bdynamic linker flags, and adds -lc. The goal of fixing issue #53 is legitimate, but this approach has two important correctness problems — and may not actually fix the link-order root cause.
Overall assessment
The change as written will cause duplicate C++ runtime linkage on all Linux toolchain configurations and doesn't resolve the ordering problem from issue #53. The original -static-libstdc++/-static-libgcc flags were the correct GCC mechanism for this use case.
The right fix for the outlined-atomic link-order problem (issue #53) is either:
- Wrapping the entire static library set in
--start-group/--end-group(as proposed in the issue's option 2), or - Using linkopts at the binary level to place
-lgcc -lgcc_ehafter user library deps.
|
|
||
| # Default link flags when no specific build type is selected. | ||
| DEFAULT_LINK_FLAGS = get_flag_group([ | ||
| "-Wl,-Bstatic", |
There was a problem hiding this comment.
AI-assisted review (Claude):
[Important] Duplicate C++ runtime linkage — no -nodefaultlibs in this toolchain
This toolchain uses cxx_binary (g++) as the link driver for all link actions, and nowhere in the Linux toolchain do the flags include -nodefaultlibs. Without that flag, g++ automatically appends its own default runtime libraries at the end of every link command — including -lstdc++, -lgcc, and -lgcc_eh.
The result is:
g++ -Wl,-Bstatic -lstdc++ -lgcc -lgcc_eh -Wl,-Bdynamic ... <user libs> ... -lstdc++ -lgcc [from driver, dynamic]
Static versions are linked early, dynamic versions are linked again at the end by the driver. That's dual runtime linkage: two copies of the C++ runtime with different linkage modes in the same binary, which can cause ODR violations, constructor double-invocation, or crashes.
The original -static-libstdc++ and -static-libgcc are precisely the GCC mechanism for avoiding this: they instruct the driver to substitute its own default runtime with the static versions when it appends them at the end — no duplication. They're not a workaround; they're the intended interface.
If the intent is to keep runtime libs static, the correct fix is to keep -static-libstdc++/-static-libgcc and address link order separately (see the second finding below).
| "-ldl", | ||
| "-lrt", | ||
| "-static-libstdc++", | ||
| "-static-libgcc", |
There was a problem hiding this comment.
AI-assisted review (Claude):
[Important] This doesn't actually fix the link-order problem from issue #53
Issue #53 describes symbols from libgcc not being found because the static library graph is searched left-to-right and libgcc ends up too early, before user libs that reference it. These flags move libgcc to before -lm, -ldl, -lrt, and the user's static libraries — that's the same problem in reverse.
The flags here tell the linker:
static: libstdc++ libgcc libgcc_eh → then dynamic: lm, ldl, lrt, lc → then user objects
User objects (which reference __aarch64_ldadd4_acq_rel etc.) appear after libgcc in the link command, so the linker still won't pull those symbols when processing the later objects.
The real solution is link groups. Replace this with:
DEFAULT_LINK_FLAGS = get_flag_group([
"-static-libstdc++",
"-static-libgcc",
"-Wl,--start-group",
"-lm",
"-ldl",
"-lrt",
"-lgcc",
"-lgcc_eh",
"-Wl,--end-group",
])--start-group/--end-group causes the linker to repeatedly search the grouped archives until no new undefined symbols appear, which is exactly what the issue proposes as "Option 2" and is immune to ordering problems.
| "-lrt", | ||
| "-static-libstdc++", | ||
| "-static-libgcc", | ||
| "-lc", |
There was a problem hiding this comment.
AI-assisted review (Claude):
[Suggestion] -lc is unnecessary and potentially harmful
libc is always linked dynamically by default when using g++ without -static or -nodefaultlibs. Adding -lc explicitly here will work in most cases but is redundant, and on some platforms or with some linker configurations can cause a conflict between the implicit libc pulled in by -lm/-ldl/-lrt and the explicit -lc position. It should be removed.
|
This thing requires a bit more attention. The issue here is present only when we used prebuilt libraries and trying to link with our codebase. We need to check what should be proper solution (this is more workaround rather a solution) otherwise we are risking to break shared libs, whole-archive handling, interface libraries, object files, etc. |
Linker flags needed to be reorder due to issue with link order and link grouping when using static libraries.
resolves: #53