[PATCH v2 0/4] Don't use patchelf

[Rafael Ávila de Espíndola]

Also available at https://github.com/espindola/scylla/ espindola/no-patch-elf-v2

This series reverts from using patchelf to using scripts that
explicitly run ld.so. It then avoids gdb's confusion by dropping
"exec -a" from those scripts.

Changes since v1:
* Go back to "ld.so ./binary" instead of using a CWD relative
PT_INTERP.

Rafael Ávila de Espíndola (4):
Revert "relocatable: switch from run-time relocation to install-time
relocation"
relocatable: Don't use "exec -a"
build: Split the build and host linker flags
relocatable: build with a dummy interpreter

configure.py | 7 +++-
dist/debian/rules.mustache | 4 +-
install.sh | 34 ++++-------------
reloc/build_reloc.sh | 5 ++-
scripts/create-relocatable-package.py | 54 ++++++++++++++++++++++++++-
5 files changed, 70 insertions(+), 34 deletions(-)

--
2.23.0

[Rafael Ávila de Espíndola]

This reverts commit 698b72b5018868df6a839d08fd24c642db97ffcd.

Using patchelf can cause build-ids to be missing from core files
(issue #4983).

This is as clean a revert as possible. Followup patches will introduce
an alternative fix for #4673.

Fixes #4983

Signed-off-by: Rafael Ávila de Espíndola <espi...@scylladb.com>
---
dist/debian/rules.mustache | 4 +--
install.sh | 34 ++++--------------
scripts/create-relocatable-package.py | 50 +++++++++++++++++++++++++--
3 files changed, 56 insertions(+), 32 deletions(-)

diff --git a/dist/debian/rules.mustache b/dist/debian/rules.mustache
index 6a1b41f06..0a8934192 100755
--- a/dist/debian/rules.mustache
+++ b/dist/debian/rules.mustache
@@ -34,9 +34,7 @@ override_dh_installinit:
dh_installinit --no-start --name node-exporter

override_dh_strip:
- # The binaries (ethtool...patchelf) don't pass dh_strip after going through patchelf. Since they are
- # already stripped, nothing is lost if we exclude them, so that's what we do.
- dh_strip -Xlibprotobuf.so.15 -Xld.so -Xethtool -Xgawk -Xgzip -Xhwloc-calc -Xhwloc-distrib -Xifconfig -Xlscpu -Xnetstat -Xpatchelf --dbg-package={{product}}-server-dbg
+ dh_strip -Xlibprotobuf.so.15 -Xld.so --dbg-package={{product}}-server-dbg

override_dh_makeshlibs:

diff --git a/install.sh b/install.sh
index 3ea1a073d..bb2516852 100755
--- a/install.sh
+++ b/install.sh
@@ -80,29 +80,6 @@ while [ $# -gt 0 ]; do
esac
done

-patchelf() {
- # patchelf comes from the build system, so it needs the build system's ld.so and
- # shared libraries. We can't use patchelf on patchelf itself, so invoke it via
- # ld.so.
- LD_LIBRARY_PATH="$PWD/libreloc" libreloc/ld.so libexec/patchelf "$@"
-}
-
-adjust_bin() {
- local bin="$1"
- # We could add --set-rpath too, but then debugedit (called by rpmbuild) barfs
- # on the result. So use LD_LIBRARY_PATH in the thunk, below.
- patchelf \
- --set-interpreter "$prefix/libreloc/ld.so" \
- "$root/$prefix/libexec/$bin"
- cat > "$root/$prefix/bin/$bin" <<EOF
-#!/bin/bash -e
-export GNUTLS_SYSTEM_PRIORITY_FILE="\${GNUTLS_SYSTEM_PRIORITY_FILE-$prefix/libreloc/gnutls.config}"
-export LD_LIBRARY_PATH="$prefix/libreloc"
-exec -a "\$0" "$prefix/libexec/$bin" "\$@"
-EOF
- chmod +x "$root/$prefix/bin/$bin"
-}
-
if [ -z "$prefix" ]; then
if $nonroot; then
prefix=~/scylladb
@@ -158,13 +135,16 @@ install -m644 dist/common/systemd/*.slice -Dt "$rsystemd"
install -m644 dist/common/systemd/*.timer -Dt "$rsystemd"
install -m755 seastar/scripts/seastar-cpu-map.sh -Dt "$rprefix"/scripts
install -m755 seastar/dpdk/usertools/dpdk-devbind.py -Dt "$rprefix"/scripts
-install -m755 libreloc/* -Dt "$rprefix/libreloc"
+install -m755 bin/* -Dt "$rprefix/bin"
# some files in libexec are symlinks, which "install" dereferences
# use cp -P for the symlinks instead.
-install -m755 libexec/* -Dt "$rprefix/libexec"
-for bin in libexec/*; do
- adjust_bin "${bin#libexec/}"
+install -m755 libexec/*.bin -Dt "$rprefix/libexec"
+for f in libexec/*; do
+ if [[ "$f" != *.bin ]]; then
+ cp -P "$f" "$rprefix/libexec"
+ fi
done
+install -m755 libreloc/* -Dt "$rprefix/libreloc"

install -d -m755 "$rdoc"/scylla
install -m644 README.md -Dt "$rdoc"/scylla/
diff --git a/scripts/create-relocatable-package.py b/scripts/create-relocatable-package.py
index 0bdd95b95..179d56541 100755
--- a/scripts/create-relocatable-package.py
+++ b/scripts/create-relocatable-package.py
@@ -61,7 +61,6 @@ args = ap.parse_args()

executables = ['build/{}/scylla'.format(args.mode),
'build/{}/iotune'.format(args.mode),
- '/usr/bin/patchelf',
'/usr/bin/lscpu',
'/usr/bin/gawk',
'/usr/bin/gzip',
@@ -97,9 +96,56 @@ ar = tarfile.open(fileobj=gzip_process.stdin, mode='w|')
pathlib.Path('build/SCYLLA-RELOCATABLE-FILE').touch()
ar.add('build/SCYLLA-RELOCATABLE-FILE', arcname='SCYLLA-RELOCATABLE-FILE')

+# This thunk is a shell script that arranges for the executable to be invoked,
+# under the following conditions:
+#
+# - the same argument vector is passed to the executable, including argv[0]
+# - the executable name (/proc/pid/comm, shown in top(1)) is the same
+# - the dynamic linker is taken from this package rather than the executable's
+# default (which is hardcoded to point to /lib64/ld-linux-x86_64.so or similar)
+# - LD_LIBRARY_PATH points to the lib/ directory so shared library dependencies
+# are satisified from there rather than the system default (e.g. /lib64)
+
+# To do that, the dynamic linker is invoked using a symbolic link named after the
+# executable, not its standard name. We use "bash -a" to set argv[0].
+
+# The full tangled web looks like:
+#
+# foobar/bin/scylla a shell script invoking everything
+# foobar/libexec/scylla.bin the real binary
+# foobar/libexec/scylla a symlink to ../lib/ld.so
+# foobar/libreloc/ld.so the dynamic linker
+# foobar/libreloc/lib... all the other libraries
+
+# the transformations (done by the thunk and symlinks) are:
+#
+# bin/scylla args -> libexec/scylla libexec/scylla.bin args -> lib/ld.so libexec/scylla.bin args
+
+thunk = b'''\
+#!/bin/bash
+
+x="$(readlink -f "$0")"
+b="$(basename "$x")"
+d="$(dirname "$x")/.."
+ldso="$d/libexec/$b"
+realexe="$d/libexec/$b.bin"
+export GNUTLS_SYSTEM_PRIORITY_FILE="${GNUTLS_SYSTEM_PRIORITY_FILE-$d/libreloc/gnutls.config}"
+LD_LIBRARY_PATH="$d/libreloc" exec -a "$0" "$ldso" "$realexe" "$@"
+'''
+
for exe in executables:
basename = os.path.basename(exe)
- ar.add(exe, arcname='libexec/' + basename)
+ ar.add(exe, arcname='libexec/' + basename + '.bin')
+ ti = tarfile.TarInfo(name='bin/' + basename)
+ ti.size = len(thunk)
+ ti.mode = 0o755
+ ti.mtime = os.stat(exe).st_mtime
+ ar.addfile(ti, fileobj=io.BytesIO(thunk))
+ ti = tarfile.TarInfo(name='libexec/' + basename)
+ ti.type = tarfile.SYMTYPE
+ ti.linkname = '../libreloc/ld.so'
+ ti.mtime = os.stat(exe).st_mtime
+ ar.addfile(ti)
for lib, libfile in libs.items():
ar.add(libfile, arcname='libreloc/' + lib)
if have_gnutls:
--
2.23.0

[Rafael Ávila de Espíndola]

When using "ld.so ./foo.bin", ld.so already adjusts argv so that foo
sees "./foo.bin" in argv[0].

We also invoke ld.so via symbolic link (./foo ./foo.bin), so that top
shows "foo" instead of "ld.so".

Given those facts, using "exec -a" is redundant. It is also
undesirable because it confuses gdb which then fails to display
thread_local variables.

Fixes #4673

Signed-off-by: Rafael Ávila de Espíndola <espi...@scylladb.com>
---

scripts/create-relocatable-package.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/create-relocatable-package.py b/scripts/create-relocatable-package.py
index 179d56541..4ae616154 100755
--- a/scripts/create-relocatable-package.py
+++ b/scripts/create-relocatable-package.py
@@ -107,7 +107,7 @@ ar.add('build/SCYLLA-RELOCATABLE-FILE', arcname='SCYLLA-RELOCATABLE-FILE')

# are satisified from there rather than the system default (e.g. /lib64)

# To do that, the dynamic linker is invoked using a symbolic link named after the

-# executable, not its standard name. We use "bash -a" to set argv[0].

+# executable, not its standard name.

# The full tangled web looks like:

#
@@ -130,7 +130,7 @@ d="$(dirname "$x")/.."
ldso="$d/libexec/$b"
realexe="$d/libexec/$b.bin"
export GNUTLS_SYSTEM_PRIORITY_FILE="${GNUTLS_SYSTEM_PRIORITY_FILE-$d/libreloc/gnutls.config}"
-LD_LIBRARY_PATH="$d/libreloc" exec -a "$0" "$ldso" "$realexe" "$@"
+LD_LIBRARY_PATH="$d/libreloc" exec "$ldso" "$realexe" "$@"
'''

for exe in executables:
--
2.23.0

[Rafael Ávila de Espíndola]

A general build system knows about 3 machines:

* build: where the building is running
* host: where the built software will run
* target: the machine the software will produce code for

The target machine is only relevant for compilers, so we can ignore
it.

Until now we could ignore the build and host distinction too. This
patch adds the first difference: don't use host ld_flags when linking
build tools (gen_crc_combine_table).

The reason for this change is to make it possible to build with
-Wl,--dynamic-linker pointing to a path that will exist on the host
machine, but may not exist on the build machine.

Signed-off-by: Rafael Ávila de Espíndola <espi...@scylladb.com>
---

configure.py | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/configure.py b/configure.py
index 050f789cd..b5e37448c 100755
--- a/configure.py
+++ b/configure.py
@@ -1289,6 +1289,7 @@ with open(buildfile_tmp, 'w') as f:
cxx = {cxx}
cxxflags = {user_cflags} {warnings} {defines}
ldflags = {gold_linker_flag} {user_ldflags}
+ ldflags_build = {gold_linker_flag}
libs = {libs}
pool link_pool
depth = {link_pool_depth}
@@ -1342,6 +1343,10 @@ with open(buildfile_tmp, 'w') as f:
command = $cxx $ld_flags_{mode} -s $ldflags -o $out $in $libs $libs_{mode}
description = LINK (stripped) $out
pool = link_pool
+ rule link_build.{mode}
+ command = $cxx $ld_flags_{mode} $ldflags_build -o $out $in $libs $libs_{mode}
+ description = LINK (build) $out
+ pool = link_pool
rule ar.{mode}
command = rm -f $out; ar cr $out $in; ranlib $out
description = AR $out
@@ -1448,7 +1453,7 @@ with open(buildfile_tmp, 'w') as f:
compiles['$builddir/' + mode + '/utils/gz/gen_crc_combine_table.o'] = 'utils/gz/gen_crc_combine_table.cc'
f.write('build {}: run {}\n'.format('$builddir/' + mode + '/gen/utils/gz/crc_combine_table.cc',
'$builddir/' + mode + '/utils/gz/gen_crc_combine_table'))
- f.write('build {}: {}.{} {}\n'.format('$builddir/' + mode + '/utils/gz/gen_crc_combine_table', regular_link_rule, mode,
+ f.write('build {}: link_build.{} {}\n'.format('$builddir/' + mode + '/utils/gz/gen_crc_combine_table', mode,
'$builddir/' + mode + '/utils/gz/gen_crc_combine_table.o'))
f.write(' libs = $seastar_libs_{}\n'.format(mode))
f.write(
--
2.23.0

[Rafael Ávila de Espíndola]

This is a safeguard that the binary is run via our script and not with
the system dynamic loader and libraries.

Signed-off-by: Rafael Ávila de Espíndola <espi...@scylladb.com>
---

reloc/build_reloc.sh | 5 ++++-
scripts/create-relocatable-package.py | 4 ++++
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/reloc/build_reloc.sh b/reloc/build_reloc.sh
index 67b5b691c..a0e4bc622 100755
--- a/reloc/build_reloc.sh
+++ b/reloc/build_reloc.sh
@@ -8,7 +8,10 @@
# force using sha1 for now. That has no impact in gold and bfd since
# that is their default. We set it in here instead of configure.py to
# not slow down regular builds.
-DEFAULT_FLAGS="--enable-dpdk --cflags=-ffile-prefix-map=$PWD=. --ldflags=-Wl,--build-id=sha1"
+# The relocatable package includes its own dynamic linker, which is
+# used via a script. To avoid accidentally using the system one, set
+# the interpreter to a non-existing file.
+DEFAULT_FLAGS="--enable-dpdk --cflags=-ffile-prefix-map=$PWD=. --ldflags=-Wl,--build-id=sha1,--dynamic-linker=/no/such/file"

DEFAULT_MODE="release"

diff --git a/scripts/create-relocatable-package.py b/scripts/create-relocatable-package.py
index 4ae616154..2831d00c4 100755
--- a/scripts/create-relocatable-package.py
+++ b/scripts/create-relocatable-package.py
@@ -46,6 +46,10 @@ def ldd(executable):
# provided by kernel
continue
libraries['ld.so'] = os.path.realpath(elements[0])
+ elif '/' in elements[0]:
+ # The only case where lld prints / in the first element is
+ # for /no/such/file.
+ pass
else:
libraries[elements[0]] = os.path.realpath(elements[2])
return libraries
--
2.23.0

[Avi Kivity]

This is already committed, 761b19cee580e8c47597904b1edbc7414a86dcba.
Please rebase over it.

[Avi Kivity]

On 10/12/2019 01.26, Rafael Ávila de Espíndola wrote:

> This is a safeguard that the binary is run via our script and not with
> the system dynamic loader and libraries.
>

I understand the motivation, but this can cause pain for someone
building with build_reloc and then running the resulting binary on the
development machine.


Since libreloc/ is not on PATH, I don't think there's much danger of it
getting executed, and even if it does, it will fail quickly.

[Avi Kivity]

On 10/12/2019 01.26, Rafael Ávila de Espíndola wrote:

> Also available at https://github.com/espindola/scylla/ espindola/no-patch-elf-v2
>
> This series reverts from using patchelf to using scripts that
> explicitly run ld.so. It then avoids gdb's confusion by dropping
> "exec -a" from those scripts.
>
> Changes since v1:
> * Go back to "ld.so ./binary" instead of using a CWD relative
> PT_INTERP.

What does "pgrep -a scylla" look like with this? And "top -c"?


If I run "/lib64/ld-linux-x86-64.so.2 /bin/top -c" I see
"/lib64/ld-linux-x86-64.so.2 /bin/top -c", whereas if I run "top -c" I
get "top -c". So with the symlink we'd get "scylla
/path/to/libreloc/scylla", no?

[Nadav Har'El]

On Tue, Dec 10, 2019 at 11:07 AM Avi Kivity <a...@scylladb.com> wrote:

On 10/12/2019 01.26, Rafael Ávila de Espíndola wrote:
> This is a safeguard that the binary is run via our script and not with
> the system dynamic loader and libraries.
>

I understand the motivation, but this can cause pain for someone
building with build_reloc and then running the resulting binary on the
development machine.

Like Cato the Elder, I would like to reiterate my plea that the build_reloc

script not become a mandatory method of compiling the code, and that it

should remain (as it was in the past) a script for packaging the result of

the normal compilation. In my development work (not when building official

releases) I'd like exactly the same executable to be usable on my development

machine and eventually - in a tar.gz to be sent off to some remote test machine.

Since libreloc/ is not on PATH, I don't think there's much danger of it
getting executed, and even if it does, it will fail quickly.

I agree that this protection doesn't really protect against any real problem.

--
You received this message because you are subscribed to the Google Groups "ScyllaDB development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scylladb-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/scylladb-dev/56d614e7-f304-97e6-74f9-dc61b02f781e%40scylladb.com.

[Rafael Avila de Espindola]

Avi Kivity <a...@scylladb.com> writes:

> On 10/12/2019 01.26, Rafael Ávila de Espíndola wrote:
>> This is a safeguard that the binary is run via our script and not with
>> the system dynamic loader and libraries.
>>
>
> I understand the motivation, but this can cause pain for someone
> building with build_reloc and then running the resulting binary on the
> development machine.

Is that a common use case?

> Since libreloc/ is not on PATH, I don't think there's much danger of it
> getting executed, and even if it does, it will fail quickly.

True, but the crash can still be non obvious a year from now when we get
a crash report from someone trying to run the ELF directly.

In any case, I left this for the end so it is easy to omit if not
wanted.

Cheers,
Rafael

[Rafael Avila de Espindola]

Avi Kivity <a...@scylladb.com> writes:

> On 10/12/2019 01.26, Rafael Ávila de Espíndola wrote:
>> Also available at https://github.com/espindola/scylla/ espindola/no-patch-elf-v2
>>
>> This series reverts from using patchelf to using scripts that
>> explicitly run ld.so. It then avoids gdb's confusion by dropping
>> "exec -a" from those scripts.
>>
>> Changes since v1:
>> * Go back to "ld.so ./binary" instead of using a CWD relative
>> PT_INTERP.
>
>
> What does "pgrep -a scylla" look like with this?

136142 /home/espindola/scylla/scylla/t/bin/../libexec/scylla /home/espindola/scylla/scylla/t/bin/../libexec/scylla.bin --developer-mode yes --experimental=on -m 2G

> And "top -c"?

136142 espindo+ 20 0 16.0t 912756 44596 S 18.8 0.7 0:38.02 /home/espindola/scylla/scylla/t/bin/../libexec/scylla /home/espindola/scylla/scylla/t/bin/../libexec/scylla.bin --developer-mode yes --experimental=on -m 2G

IHMO that is the lesser evil. My second choice would be to try to change
gdb and require anyone looking at cores to have a recent (built from
source in practice) gdb.

Cheers,
Rafael

[Rafael Ávila de Espíndola]

Also available at https://github.com/espindola/scylla/ espindola/no-patch-elf-v3

This series reverts from using patchelf to using scripts that
explicitly run ld.so. It then avoids gdb's confusion by dropping
"exec -a" from those scripts.

Changes since v2:
* Rebased on current master.

Rafael Ávila de Espíndola (3):

Revert "relocatable: switch from run-time relocation to install-time
relocation"
relocatable: Don't use "exec -a"

relocatable: build with a dummy interpreter

dist/debian/rules.mustache | 4 +-
install.sh | 34 ++++-------------
reloc/build_reloc.sh | 5 ++-
scripts/create-relocatable-package.py | 54 ++++++++++++++++++++++++++-

4 files changed, 64 insertions(+), 33 deletions(-)

--
2.23.0

[Rafael Ávila de Espíndola]

This reverts commit 698b72b5018868df6a839d08fd24c642db97ffcd.

Using patchelf can cause build-ids to be missing from core files
(issue #4983).

This is as clean a revert as possible. Followup patches will introduce
an alternative fix for #4673.

Fixes #4983

Signed-off-by: Rafael Ávila de Espíndola <espi...@scylladb.com>
---

diff --git a/scripts/create-relocatable-package.py b/scripts/create-relocatable-package.py
index 0bdd95b95..179d56541 100755
--- a/scripts/create-relocatable-package.py
+++ b/scripts/create-relocatable-package.py

@@ -61,7 +61,6 @@ args = ap.parse_args()

executables = ['build/{}/scylla'.format(args.mode),
'build/{}/iotune'.format(args.mode),
- '/usr/bin/patchelf',
'/usr/bin/lscpu',
'/usr/bin/gawk',
'/usr/bin/gzip',
@@ -97,9 +96,56 @@ ar = tarfile.open(fileobj=gzip_process.stdin, mode='w|')
pathlib.Path('build/SCYLLA-RELOCATABLE-FILE').touch()

ar.add('build/SCYLLA-RELOCATABLE-FILE', arcname='SCYLLA-RELOCATABLE-FILE')

+# This thunk is a shell script that arranges for the executable to be invoked,
+# under the following conditions:
+#
+# - the same argument vector is passed to the executable, including argv[0]
+# - the executable name (/proc/pid/comm, shown in top(1)) is the same
+# - the dynamic linker is taken from this package rather than the executable's
+# default (which is hardcoded to point to /lib64/ld-linux-x86_64.so or similar)
+# - LD_LIBRARY_PATH points to the lib/ directory so shared library dependencies

+# are satisified from there rather than the system default (e.g. /lib64)
+
+# To do that, the dynamic linker is invoked using a symbolic link named after the
+# executable, not its standard name. We use "bash -a" to set argv[0].
+

[Rafael Ávila de Espíndola]

When using "ld.so ./foo.bin", ld.so already adjusts argv so that foo
sees "./foo.bin" in argv[0].

We also invoke ld.so via symbolic link (./foo ./foo.bin), so that top
shows "foo" instead of "ld.so".

Given those facts, using "exec -a" is redundant. It is also
undesirable because it confuses gdb which then fails to display
thread_local variables.

Fixes #4673

Signed-off-by: Rafael Ávila de Espíndola <espi...@scylladb.com>
---

scripts/create-relocatable-package.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/create-relocatable-package.py b/scripts/create-relocatable-package.py
index 179d56541..4ae616154 100755
--- a/scripts/create-relocatable-package.py
+++ b/scripts/create-relocatable-package.py
@@ -107,7 +107,7 @@ ar.add('build/SCYLLA-RELOCATABLE-FILE', arcname='SCYLLA-RELOCATABLE-FILE')

# are satisified from there rather than the system default (e.g. /lib64)

# To do that, the dynamic linker is invoked using a symbolic link named after the

-# executable, not its standard name. We use "bash -a" to set argv[0].

+# executable, not its standard name.

# The full tangled web looks like:

[Rafael Ávila de Espíndola]

This is a safeguard that the binary is run via our script and not with
the system dynamic loader and libraries.

Signed-off-by: Rafael Ávila de Espíndola <espi...@scylladb.com>
---

reloc/build_reloc.sh | 5 ++++-
scripts/create-relocatable-package.py | 4 ++++
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/reloc/build_reloc.sh b/reloc/build_reloc.sh
index 67b5b691c..a0e4bc622 100755
--- a/reloc/build_reloc.sh
+++ b/reloc/build_reloc.sh
@@ -8,7 +8,10 @@
# force using sha1 for now. That has no impact in gold and bfd since
# that is their default. We set it in here instead of configure.py to
# not slow down regular builds.
-DEFAULT_FLAGS="--enable-dpdk --cflags=-ffile-prefix-map=$PWD=. --ldflags=-Wl,--build-id=sha1"
+# The relocatable package includes its own dynamic linker, which is
+# used via a script. To avoid accidentally using the system one, set
+# the interpreter to a non-existing file.
+DEFAULT_FLAGS="--enable-dpdk --cflags=-ffile-prefix-map=$PWD=. --ldflags=-Wl,--build-id=sha1,--dynamic-linker=/no/such/file"

DEFAULT_MODE="release"

diff --git a/scripts/create-relocatable-package.py b/scripts/create-relocatable-package.py
index 4ae616154..2831d00c4 100755
--- a/scripts/create-relocatable-package.py
+++ b/scripts/create-relocatable-package.py

@@ -46,6 +46,10 @@ def ldd(executable):
# provided by kernel
continue
libraries['ld.so'] = os.path.realpath(elements[0])
+ elif '/' in elements[0]:
+ # The only case where lld prints / in the first element is
+ # for /no/such/file.
+ pass
else:
libraries[elements[0]] = os.path.realpath(elements[2])
return libraries
--

2.23.0

[Avi Kivity]

What about hacking the build to use
///////////////////...[path_max-something].../////////////////lib64/ld-linux-whatever.so?
That works without any side effects, no?

[Rafael Avila de Espindola]

It should. It would be my 3rd option. The reason being that we should
still check that the program headers were not modified (run readelf in
install.sh) and my general uneasiness with anything that writes ELFs but
doesn't get a lot of testing.

Cheers,
Rafael

[Avi Kivity]

I think it's part of Nix (where it's likely used for the same thing). So
it does get usage. Of course, nobody cared enough to fix the build-id
thing, but maybe nix uses something else.

[Rafael Avila de Espindola]

Rafael Avila de Espindola <espi...@scylladb.com> writes:

> Avi Kivity <a...@scylladb.com> writes:
>
>> On 10/12/2019 01.26, Rafael Ávila de Espíndola wrote:
>>> Also available at https://github.com/espindola/scylla/ espindola/no-patch-elf-v2
>>>
>>> This series reverts from using patchelf to using scripts that
>>> explicitly run ld.so. It then avoids gdb's confusion by dropping
>>> "exec -a" from those scripts.
>>>
>>> Changes since v1:
>>> * Go back to "ld.so ./binary" instead of using a CWD relative
>>> PT_INTERP.
>>
>>
>> What does "pgrep -a scylla" look like with this?
>
>
> 136142 /home/espindola/scylla/scylla/t/bin/../libexec/scylla /home/espindola/scylla/scylla/t/bin/../libexec/scylla.bin --developer-mode yes --experimental=on -m 2G
>
>> And "top -c"?
>
> 136142 espindo+ 20 0 16.0t 912756 44596 S 18.8 0.7 0:38.02 /home/espindola/scylla/scylla/t/bin/../libexec/scylla /home/espindola/scylla/scylla/t/bin/../libexec/scylla.bin --developer-mode yes --experimental=on -m 2G

BTW, even with the "exec -a" I get

147540 scylla /home/espindola/scylla/scylla/t/bin/../libexec/scylla.bin --developer-mode yes --experimental=on -m 2G

and

47540 espindo+ 20 0 16.0t 918944 43724 S 11.8 0.7 0:32.87 scylla /home/espindola/scylla/scylla/t/bin/../libexec/scylla.bin --developer-mode yes --experimental=on -m 2G

so the "exec -a" part doesn't seem to add much value.

I reported the gdb issue at https://sourceware.org/bugzilla/show_bug.cgi?id=25269

Cheers,
Rafael

[Rafael Avila de Espindola]

Compare that with the testing a linker gets.

Cheers,
Rafael

[Rafael Avila de Espindola]

Rafael Avila de Espindola <espi...@scylladb.com> writes:

Another disadvantage of that is that we get a solution that works for
our binaries (scylla and iotune) but fails for other binaries we bundle
in the relocatable package.

Cheers,
Rafael

[Avi Kivity]

Sure, but patchelf does almost nothing, and on data that always looks
the same. If it breaks, we'll get an executable that doesn't start (or
doesn't generate build ids on core dumps), not subtle bugs.

[Avi Kivity]

I think we apply patchelf to everything. We won't have the build-ids for
core dumps for them, but we know the exact version from the frozen
toolchain anyway.