[ELD] Fix --wrap archive extraction for __real_<sym> references#1187
[ELD] Fix --wrap archive extraction for __real_<sym> references#1187vijayvarghese wants to merge 2 commits into
Conversation
| std::string realSymbol = ("__real_" + SymName).str(); | ||
| ResolveInfo *RealSymbol = m_Module.getNamePool().findInfo(realSymbol); | ||
| if (RealSymbol && RealSymbol->isUndef()) | ||
| if ((RealSymbol && RealSymbol->isUndef()) || info->isUndef()) |
There was a problem hiding this comment.
can we update tracing for wrap symbols with --trace=wrap-symbols to show this information ?
There was a problem hiding this comment.
Added --trace=wrap-symbols tracing in ArchiveParser::shouldIncludeSymbol() to log archive member extraction triggered during wrapped symbol resolution. The trace now reports the wrapped symbol along with, from which archive it being pulled.
| std::string realSymbol = ("__real_" + SymName).str(); | ||
| ResolveInfo *RealSymbol = m_Module.getNamePool().findInfo(realSymbol); | ||
| if (RealSymbol && RealSymbol->isUndef()) | ||
| if ((RealSymbol && RealSymbol->isUndef()) || info->isUndef()) |
There was a problem hiding this comment.
Please update the docs too.
There was a problem hiding this comment.
Added additional documentation for --wrap usage covering archive symbol extraction behavior, how __real_<sym> references can trigger archive member inclusion, and how to use --trace=wrap-symbols for debugging these cases.
|
|
||
| # Compile inputs. | ||
| RUN: %clang %clangopts -c %p/Inputs/1.c -o %t1.main.o | ||
| RUN: %clang %clangopts -c %p/Inputs/2.c -o %t1.wrap.o |
There was a problem hiding this comment.
check that using LTO also makes sure the behavior is consistent.
There was a problem hiding this comment.
Added coverage for this in the latest update.
|
|
||
| #---WrapSymbolArchivePull.test------------------------ Executable ----------# | ||
| #BEGIN_COMMENT | ||
| # Regression test for GitHub issue #936: |
There was a problem hiding this comment.
Remove this line referencing github issue
| std::string realSymbol = ("__real_" + SymName).str(); | ||
| ResolveInfo *RealSymbol = m_Module.getNamePool().findInfo(realSymbol); | ||
| if (RealSymbol && RealSymbol->isUndef()) | ||
| if ((RealSymbol && RealSymbol->isUndef()) || info->isUndef()) |
There was a problem hiding this comment.
Can we make sure that _wrap is also handled accordingly ?
There was a problem hiding this comment.
Added coverage for this.
Verified __wrap_<sym> handling with an additional test where __wrap_<sym> is defined inside an archive member. In this case it is included through the existing archive inclusion flow (renameMap lookup path), and the test confirms correct resolution.
| std::string realSymbol = ("__real_" + SymName).str(); | ||
| ResolveInfo *RealSymbol = m_Module.getNamePool().findInfo(realSymbol); | ||
| if (RealSymbol && RealSymbol->isUndef()) | ||
| if ((RealSymbol && RealSymbol->isUndef()) || info->isUndef()) |
There was a problem hiding this comment.
Can we also check if __real or __wrap if not defined and what the error is ?
There was a problem hiding this comment.
Added test coverage for both cases.
Validated linker diagnostics for:
missing __wrap_<sym> → unresolved __wrap_<sym>
missing __real_<sym> → No archive pull needed, linking finished with no errors.
Also by default behavior verified if <sym> is not defined and there is a call to __real_<sym> will result in a Error: build/wrap.o: undefined reference to `<sym>'
2b9fd5a to
4c8a4bc
Compare
| # Test that --wrap archive extraction works correctly under LTO. | ||
| # When --wrap=add is active and wrap.o references __real_add, | ||
| # IRBuilder renames __real_add to add in the name pool. Later, | ||
| # when the archive is scanned, shouldIncludeSymbol() must still |
There was a problem hiding this comment.
remove comments not relevant for the test, test should contain only test specific comments.
| # Verify the archive member was pulled in. | ||
| #CHECK: {{.*}}libfoo.a{{.*}}add.o | ||
|
|
||
| #UNSUPPORTED: windows, ndk-build, riscv32, riscv64 No newline at end of file |
There was a problem hiding this comment.
Why they are not supported ?
| std::string realSymbol = ("__real_" + SymName).str(); | ||
| ResolveInfo *RealSymbol = m_Module.getNamePool().findInfo(realSymbol); | ||
| if ((RealSymbol && RealSymbol->isUndef()) || info->isUndef()) | ||
| if ((RealSymbol && RealSymbol->isUndef()) || info->isUndef()){ |
There was a problem hiding this comment.
should this be _real or _wrap ?
When --wrap=<sym> is active and an object file references __real_<sym>, the archive containing <sym> was not being extracted from the archive. Root cause: IRBuilder::addSymbol renames __real_<sym> to <sym> before archive scanning begins, so the archive check for '__real_<sym>' in the namepool always fails. The original symbol <sym> remains undefined in the namepool but was never checked. Fix: In shouldIncludeSymbol, also return Include when info->isUndef() is true for the archive symbol itself, covering the case where the __real_ reference was renamed before archive scanning. Added a test case that reproduces the exact scenario from the issue: main.o and wrap.o as direct inputs, add.o inside a static archive, linked with --wrap=add. Fixes qualcomm#936 Signed-off-by: Vijay Varghese <33589052+vijayvarghese@users.noreply.github.com>
- add --trace=wrap-symbols diagnostics for wrapped archive member extraction in ArchiveParser::shouldIncludeSymbol - add diagnostic template for archive pull trace output for wrapped symbols - add test coverage for undefined __real_<sym> and __wrap_<sym> cases - add test coverage to verify __wrap_<sym> resolution when defined inside an archive member - add LTO test to validate archive pulling behavior remains consistent during post-LTO symbol resolution - remove GitHub issue reference from test comments - update wrap documentation with additional details on archive symbol extraction and trace-based debugging Signed-off-by: Vijay Varghese <33589052+vijayvarghese@users.noreply.github.com>
4c8a4bc to
0e375ba
Compare
Problem
Fixes #936
When
--wrap=<sym>is active and an object file references__real_<sym>,the archive containing
<sym>was not being extracted, producing:Root Cause
IRBuilder::addSymbolrenames__real_<sym>to<sym>in the namepoolbefore archive scanning begins. The existing check in
shouldIncludeSymbolonly looked for
__real_<sym>in the namepool — which no longer existsunder that name — so the archive member was never pulled.
Fix
In
shouldIncludeSymbol, added|| info->isUndef()as a second condition,covering the case where the
__real_reference was renamed before archivescanning.
Test
Added
test/Common/standalone/WrapSymbolArchivePull/reproducing the exactscenario from the issue:
main.oandwrap.oas direct inputs,add.oinside a static archive, linked with
--wrap=add.