1385783 - Elfhack error when building with lld

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

===
=== If you get failures below, please file a bug describing the error
=== and your environment (compiler and linker versions), and use
=== --disable-elf-hack until this is fixed.
===
# Fail if the library doesn't have INIT_ARRAY .dynamic info
===
=== If you get failures below, please file a bug describing the error
readelf -d test-array.so | grep '(INIT_ARRAY)'
=== and your environment (compiler and linker versions), and use
=== --disable-elf-hack until this is fixed.
 0x0000000000000019 (INIT_ARRAY)         0xb000
===
# Fail if the library doesn't have INIT .dynamic info
/data/jenkins/workspace/firefox-clang-lld-last/obj-x86_64-pc-linux-gnu/build/unix/elfhack/elfhack -b -f test-array.so
readelf -d test-ctors.so | grep '(INIT)'
test-array.so: Reduced by 8456 bytes
 0x000000000000000c (INIT)               0x8017
# Fail if the backup file doesn't exist
[ -f 'test-array.so.bak' ]
/data/jenkins/workspace/firefox-clang-lld-last/obj-x86_64-pc-linux-gnu/build/unix/elfhack/elfhack -b -f test-ctors.so
# Fail if the new library doesn't contain less relocations
[ $(objdump -R test-array.so.bak | wc -l) -gt $(objdump -R test-array.so | wc -l) ]
test-ctors.so: Reduced by 8416 bytes
# Fail if the backup file doesn't exist
[ -f 'test-ctors.so.bak' ]
# Fail if the new library doesn't contain less relocations
[ $(objdump -R test-ctors.so.bak | wc -l) -gt $(objdump -R test-ctors.so | wc -l) ]
# Will either crash or return exit code 1 if elfhack is broken
LD_PRELOAD=/data/jenkins/workspace/firefox-clang-lld-last/obj-x86_64-pc-linux-gnu/build/unix/elfhack/test-array.so /data/jenkins/workspace/firefox-clang-lld-last/obj-x86_64-pc-linux-gnu/build/unix/elfhack/dummy
Segmentation fault
Makefile:50: recipe for target 'libs' failed

Compiler: clang 6.0 trunk (Debian Jessie)
Linker  : LLD 6.0 trunk

When using --disable-elf-hack everything is OK

Reproducible: Always

Comment 1

[Nathan Froyd [:froydnj]]

Can you attach the pre-elfhacked library?

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Attachment: test-array.so

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Attachment: test-ctors.so

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Not 100% they are the "pre-elfhacked library"

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Attachment: test-array.so

Probably the one you want

Generated with:
jenkins@irill4:~/workspace/firefox-clang-lld-last/obj-x86_64-pc-linux-gnu/build/unix/elfhack$  /usr/bin/clang++-6.0 -std=gnu++11 -Qunused-arguments   -Qunused-arguments -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis -Wc++11-compat-pedantic -Wc++14-compat -Wc++14-compat-pedantic -Wc++1z-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wno-gnu-zero-variadic-macro-arguments -Wformat-security -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-strict-aliasing -fno-rtti -fno-math-errno -pthread -pipe -fexceptions  -g -Os -fno-omit-frame-pointer  -Werror -fPIC -shared -Wl,-z,defs -Wl,-h,test-array.so -o test-array.so -lpthread  -Wl,-z,noexecstack -Wl,-z,text -Wl,-z,relro -Wl,--build-id   -fuse-ld=lld  -Wl,-rpath-link,/data/jenkins/workspace/firefox-clang-lld-last/obj-x86_64-pc-linux-gnu/dist/bin -Wl,-rpath-link,/usr/local/lib  -fcolor-diagnostics test-array.o -nostartfiles

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Attachment: test-ctors.so

jenkins@irill4:~/workspace/firefox-clang-lld-last/obj-x86_64-pc-linux-gnu/build/unix/elfhack$  /usr/bin/clang++-6.0 -std=gnu++11 -Qunused-arguments   -Qunused-arguments -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis -Wc++11-compat-pedantic -Wc++14-compat -Wc++14-compat-pedantic -Wc++1z-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wno-gnu-zero-variadic-macro-arguments -Wformat-security -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-strict-aliasing -fno-rtti -fno-math-errno -pthread -pipe -fexceptions  -g -Os -fno-omit-frame-pointer  -Werror -fPIC -shared -Wl,-z,defs -Wl,-h,test-ctors.so -o test-ctors.so -lpthread  -Wl,-z,noexecstack -Wl,-z,text -Wl,-z,relro -Wl,--build-id   -fuse-ld=lld  -Wl,-rpath-link,/data/jenkins/workspace/firefox-clang-lld-last/obj-x86_64-pc-linux-gnu/dist/bin -Wl,-rpath-link,/usr/local/lib  -fcolor-diagnostics test-ctors.o -nostartfiles

Comment 7

[Mike Hommey [:glandium]]

Attachment: Bug 1385783 - Don't assume both elfhack sections are next to each other.

Review commit: https://reviewboard.mozilla.org/r/163744/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163744/

Comment 8

[Mike Hommey [:glandium]]

Attachment: Bug 1385783 - Insert the elfhack code before the first executable section.

The lld linker creates separate segments for purely executable sections
(such as .text) and sections preceding those (such as .rel.dyn). Neither
gold nor bfd ld do that, and just put all those sections in the same
executable segment.

Since elfhack is putting its executable code between the two relocation
sections, it ends up in a non-executable segment, leading to a crash
when it's time to run that code.

We thus insert the elfhack code before the first executable section
instead of between the two relocation sections (which is where the
elfhack data lies, and stays).

Review commit: https://reviewboard.mozilla.org/r/163746/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163746/

Comment 9

[Nathan Froyd [:froydnj]]

Comment on attachment 8892769 [details]
Bug 1385783 - Don't assume both elfhack sections are next to each other.

https://reviewboard.mozilla.org/r/163744/#review169332

This patch looks pretty non-controversial, even if the try push looks pretty busted.

Comment 10

[Nathan Froyd [:froydnj]]

Comment on attachment 8892770 [details]
Bug 1385783 - Insert the elfhack code before the first executable section.

https://reviewboard.mozilla.org/r/163746/#review169334

r- for the try push red.

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8892769 [details]
Bug 1385783 - Don't assume both elfhack sections are next to each other.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163744/diff/1-2/

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 8892770 [details]
Bug 1385783 - Insert the elfhack code before the first executable section.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163746/diff/1-2/

Comment 13

[Nathan Froyd [:froydnj]]

Comment on attachment 8892770 [details]
Bug 1385783 - Insert the elfhack code before the first executable section.

https://reviewboard.mozilla.org/r/163746/#review169872

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd0ea10b4001
Don't assume both elfhack sections are next to each other. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/46a4358a2266
Insert the elfhack code before the first executable section. r=froydnj

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/bd0ea10b4001
https://hg.mozilla.org/mozilla-central/rev/46a4358a2266