[X86_64] Emit _GLOBAL_OFFSET_TABLE_ symbol for x86-64#1123
[X86_64] Emit _GLOBAL_OFFSET_TABLE_ symbol for x86-64#1123deepakshirkem wants to merge 1 commit intoqualcomm:mainfrom
Conversation
| void x86_64LDBackend::initTargetSymbols() { | ||
| if (config().codeGenType() == LinkerConfig::Object) | ||
| return; | ||
| auto GOTSymName = "_GLOBAL_OFFSET_TABLE_"; |
There was a problem hiding this comment.
Remove auto and qualify with appropriate type
There was a problem hiding this comment.
Used const std::string added comment.
| 0x0, // value | ||
| FragmentRef::null(), ResolveInfo::Hidden); | ||
| if (m_pGOTSymbol) | ||
| m_pGOTSymbol->setShouldIgnore(false); |
There was a problem hiding this comment.
Can you check the behavior of bfd and lld that we are doing the same ?
| @@ -0,0 +1,11 @@ | |||
| #----------GlobalOffsetTable.test--------------------- Executable ------# | |||
| #BEGIN_COMMENT | |||
There was a problem hiding this comment.
Does this test make sense to be part of Common ?
Can you add a test with linker script with GOT/GOT.PLT discarded and see the behavior ?
There was a problem hiding this comment.
Moved to Common/standalone/GlobalOffsetTable/
Added Common/standalone/GlobalOffsetTableDiscard/
|
|
||
| int *v = &_GLOBAL_OFFSET_TABLE_; | ||
|
|
||
| int a; |
4ad481e to
b8e1b80
Compare
|
Hi @quic-seaswara, Please go one more time. |
b8e1b80 to
f0726dc
Compare
|
Hi @parth-07, Please review these changes when you get a chance? Also, if possible, could you re-run the pipeline? |
There was a problem hiding this comment.
What should be the value of _GLOBAL_OFFSET_TABLE_ when there is no .got.plt section?
Example:
#!/usr/bin/env bash
cat >1.c <<\EOF
extern int _GLOBAL_OFFSET_TABLE_;
int foo() { return 1; }
int bar() { return 3; }
int *u = &_GLOBAL_OFFSET_TABLE_;
EOF
LDs=(ld.eld ld.lld ld.bfd)
SFs=(eld lld bfd)
clang -o 1.o -target x86_64-linux-gnu 1.c -c -fPIC
for i in "${!SFs[@]}"; do
${LDs[$i]} -o lib1.${SFs[$i]}.so 1.o -shared
done
For this example, eld shared library has _GLOBAL_OFFSET_TABLE_ defined to 0 whereas lld and bfd have valid GLOBAL_OFFSET_TABLE_. Can you please look into it?
Additionally, we need to document the behavior for the properties(local, default vs hidden visibility, NOTYPE vs OBJECT ...) of the _GLOBAL_OFFSET_TABLE_ symbol.
| void x86_64LDBackend::initTargetSymbols() { | ||
| if (config().codeGenType() == LinkerConfig::Object) | ||
| return; | ||
| // addSymbol() takes std::string; use explicit type over auto. |
There was a problem hiding this comment.
Can you please remove this comment. I don't think this comment is very useful.
f0726dc to
8981a64
Compare
|
Hi @parth-07, Please go one more time. |
| RUN: %link %linkopts %t1.o -o %t1.so -shared 2>&1 | ||
| RUN: %readelf -s %t1.so | %filecheck %s | ||
| #END_TEST | ||
| CHECK: {{[1-9][0-9a-f]+}}{{.*}}OBJECT{{.*}}LOCAL{{.*}}HIDDEN{{.*}}_GLOBAL_OFFSET_TABLE_ |
There was a problem hiding this comment.
it would be great if the test can be improved to point to the section as intended.
8981a64 to
d0788aa
Compare
|
Hi @quic-seaswara, test has been updated. Please take a look? |
@deepakshirkem can you add a test for this case ? |
d0788aa to
4263a92
Compare
|
Hi @quic-seaswara, Can you please re-run the pipeline. I forgot to add specific target on that test. |
There was a problem hiding this comment.
What should be the value of
_GLOBAL_OFFSET_TABLE_when there is no.got.pltsection?Example:
#!/usr/bin/env bash cat >1.c <<\EOF extern int _GLOBAL_OFFSET_TABLE_; int foo() { return 1; } int bar() { return 3; } int *u = &_GLOBAL_OFFSET_TABLE_; EOF LDs=(ld.eld ld.lld ld.bfd) SFs=(eld lld bfd) clang -o 1.o -target x86_64-linux-gnu 1.c -c -fPIC for i in "${!SFs[@]}"; do ${LDs[$i]} -o lib1.${SFs[$i]}.so 1.o -shared doneFor this example, eld shared library has
_GLOBAL_OFFSET_TABLE_defined to 0 whereas lld and bfd have validGLOBAL_OFFSET_TABLE_. Can you please look into it?Additionally, we need to document the behavior for the properties(
local,defaultvshiddenvisibility,NOTYPEvsOBJECT...) of the_GLOBAL_OFFSET_TABLE_symbol.
@deepakshirkem can you add a test for this case ?
Added here !
| # relocations. ELD was incorrectly emitting value 0x0 in this case. | ||
| #END_COMMENT | ||
| #START_TEST | ||
| REQUIRES: x86 |
There was a problem hiding this comment.
move REQUIRES to the top.
| #START_TEST | ||
| REQUIRES: x86 | ||
| RUN: %clang %clangopts -c %p/Inputs/1.c -o %t1.o -fPIC -ffunction-sections | ||
| RUN: %link %linkopts %t1.o -o %t1.so -shared 2>&1 |
There was a problem hiding this comment.
What if it is a non shared library link, can we also test for -pie ?
There is no .got.plt so what does GLOBAL_OFFSET_TABLE point to ?
There was a problem hiding this comment.
Hi @quic-seaswara, Thank you. I tested both the -pie and shared both are correctly point .got.plt. Adding new test for -pie.
quic-areg
left a comment
There was a problem hiding this comment.
The tests should live under test/X86, not test/Common
|
Hi @quic-areg, Moving all test in test/X86 |
4263a92 to
fc3f5ff
Compare
|
Hi @quic-seaswara / @quic-areg, Please take another look when you get a chance? Thank you. |
| @@ -0,0 +1,14 @@ | |||
| REQUIRES: x86 | |||
There was a problem hiding this comment.
Remove REQUIRES: x86 and move it to Common ?
There was a problem hiding this comment.
Hi @quic-seaswara, should I move only GlobalOffsetTableShared to Common, or all the tests we created? Also, if I remove REQUIRES: x86, this test will fail in CI because the fix is currently only implemented in x86_64LDBackend.cpp.
fc3f5ff to
893bd5c
Compare
ELD did not emit _GLOBAL_OFFSET_TABLE_ for x86-64, causing an undefined reference error when object files reference this symbol Add _GLOBAL_OFFSET_TABLE_ support following the same pattern used by ARM, Hexagon and RISCV backends: - initTargetSymbols(): register the symbol as AsReferred so it is only defined if actually referenced by input files. - defineGOTSymbol(): bind it to the first .got.plt fragment so it points to the GOT base address. - finalizeScanRelocations(): call defineGOTSymbol() after scan and report an error if .got.plt is discarded but the symbol is referenced, consistent with GNU ld behavior. Fixes qualcomm#570. Signed-off-by: deepakshirkem <deepakshirke509@gmail.com>
893bd5c to
367c856
Compare
|
Hi @quic-areg / @quic-seaswara , Can you go one more time. Please re-run the pipeline whenever you get chance. |
Problem
ELD does not emit the GLOBAL_OFFSET_TABLE symbol for x86-64, causing an undefined reference error when object files reference this symbol. This affects code compiled with -fPIC or using the large code model.
Fix
Add GLOBAL_OFFSET_TABLE symbol support to the x86-64 backend following the same pattern used by other backends:
initTargetSymbols():Register the symbol as AsReferred so it is only defined if actually referenced by input files.defineGOTSymbol():Bind the symbol to the first fragment of .got.plt so it points to the GOT base address.finalizeScanRelocations():Call defineGOTSymbol() after all relocations have been scanned and .got.plt is populated.Testing
Added test/X86/standalone/GlobalOffsetTable/GlobalOffsetTable.test that verifies GLOBAL_OFFSET_TABLE is emitted and points to .got.plt.
Fixes #570.
cc @quic-seaswara