From: "H.J. Lu" <hjl.tools@gmail.com>
To: Binutils <binutils@sourceware.org>
Cc: Alan Modra <amodra@gmail.com>, Nick Clifton <nickc@redhat.com>
Subject: Re: [PATCH v2 1/2] elf: Add elf_backend_add_glibc_version_dependency
Date: Tue, 9 Jan 2024 05:45:47 -0800 [thread overview]
Message-ID: <CAMe9rOqQtoDM8Wq_rS8gccqhNmma2tYJbwaRGk_v8sdrix33mg@mail.gmail.com> (raw)
In-Reply-To: <CAMe9rOqMu+ouaxCrBz8gdqh4Dq6ndO0KOmOFWh2dzRM1dhQR+A@mail.gmail.com>
On Mon, Jan 8, 2024 at 7:22 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Jan 6, 2024 at 2:10 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > When -z mark-plt is used to add DT_X86_64_PLT, DT_X86_64_PLTSZ and
> > DT_X86_64_PLTENT, the r_addend field of the R_X86_64_JUMP_SLOT relocation
> > stores the offset of the indirect branch instruction. However, glibc
> > versions which don't have this commit in glibc 2.36:
> >
> > commit f8587a61892cbafd98ce599131bf4f103466f084
> > Author: H.J. Lu <hjl.tools@gmail.com>
> > Date: Fri May 20 19:21:48 2022 -0700
> >
> > x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
> >
> > According to x86-64 psABI, r_addend should be ignored for R_X86_64_GLOB_DAT
> > and R_X86_64_JUMP_SLOT. Since linkers always set their r_addends to 0, we
> > can ignore their r_addends.
> >
> > Reviewed-by: Fangrui Song <maskray@google.com>
> >
> > won't ignore the r_addend value in the R_X86_64_JUMP_SLOT relocation.
> > Although this commit has been backported to glibc 2.33/2.34/2.35 release
> > branches, it is safer to require glibc 2.36 for such binaries.
> >
> > Extend the glibc version dependency of GLIBC_ABI_DT_RELR for DT_RELR to
> > also add GLIBC_2.36 version dependency for -z mark-plt on the the shared C
> > library if it provides a GLIBC_2.XX symbol version.
> >
> > * elflink.c (elf_find_verdep_info): Moved to ...
> > * elf-bfd.h (elf_find_verdep_info): Here.
> > (elf_backend_data): Add elf_backend_add_glibc_version_dependency.
> > (_bfd_elf_link_add_glibc_version_dependency): New function.
> > (_bfd_elf_link_add_dt_relr_dependency): Likewise.
> > * elf64-x86-64.c (elf_x86_64_add_glibc_version_dependency):
> > Likewise.
> > (elf_backend_add_glibc_version_dependency): New.
> > * elflink.c (elf_link_add_dt_relr_dependency): Renamed to ...
> > (elf_link_add_glibc_verneed): This. Modified to support other
> > glibc dependencies.
> > (_bfd_elf_link_add_glibc_version_dependency): Likewise.
> > (_bfd_elf_link_add_dt_relr_dependency): Likewise.
> > (bfd_elf_size_dynamic_sections): Call
> > elf_backend_add_glibc_version_dependency instead of
> > elf_link_add_dt_relr_dependency.
> > * elfxx-target.h (elf_backend_add_glibc_version_dependency): New.
> > (elfNN_bed): Add elf_backend_add_glibc_version_dependency.
> >
> > ld/
> >
> > * testsuite/ld-x86-64/mark-plt-1a.rd: New file.
> > * testsuite/ld-x86-64/mark-plt-1b.rd: Likewise.
> > * testsuite/ld-x86-64/x86-64.exp: Run -z mark-plt test for
> > GLIBC_2.36 dependency.
> > ---
> > bfd/elf-bfd.h | 23 ++++
> > bfd/elf64-x86-64.c | 27 +++++
> > bfd/elflink.c | 146 +++++++++++++++-----------
> > bfd/elfxx-target.h | 5 +
> > ld/testsuite/ld-x86-64/mark-plt-1a.rd | 7 ++
> > ld/testsuite/ld-x86-64/mark-plt-1b.rd | 7 ++
> > ld/testsuite/ld-x86-64/x86-64.exp | 14 +++
> > 7 files changed, 167 insertions(+), 62 deletions(-)
> > create mode 100644 ld/testsuite/ld-x86-64/mark-plt-1a.rd
> > create mode 100644 ld/testsuite/ld-x86-64/mark-plt-1b.rd
> >
> > diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> > index a611349e3d9..3ed22fa6c52 100644
> > --- a/bfd/elf-bfd.h
> > +++ b/bfd/elf-bfd.h
> > @@ -957,6 +957,19 @@ typedef struct elf_property_list
> > struct elf_property property;
> > } elf_property_list;
> >
> > +/* This structure is used to pass information to
> > + elf_backend_add_glibc_version_dependency. */
> > +
> > +struct elf_find_verdep_info
> > +{
> > + /* General link information. */
> > + struct bfd_link_info *info;
> > + /* The number of dependencies. */
> > + unsigned int vers;
> > + /* Whether we had a failure. */
> > + bool failed;
> > +};
> > +
> > struct bfd_elf_section_reloc_data;
> >
> > struct elf_backend_data
> > @@ -1488,6 +1501,10 @@ struct elf_backend_data
> > bool (*elf_backend_write_section)
> > (bfd *, struct bfd_link_info *, asection *, bfd_byte *);
> >
> > + /* This function adds glibc version dependency. */
> > + void (*elf_backend_add_glibc_version_dependency)
> > + (struct elf_find_verdep_info *);
> > +
> > /* This function, if defined, returns TRUE if it is section symbols
> > only that are considered local for the purpose of partitioning the
> > symbol table into local and global symbols. This should be NULL
> > @@ -2583,6 +2600,12 @@ extern bool _bfd_elf_link_output_relocs
> > (bfd *, asection *, Elf_Internal_Shdr *, Elf_Internal_Rela *,
> > struct elf_link_hash_entry **);
> >
> > +extern void _bfd_elf_link_add_glibc_version_dependency
> > + (struct elf_find_verdep_info *, const char *[]);
> > +
> > +extern void _bfd_elf_link_add_dt_relr_dependency
> > + (struct elf_find_verdep_info *);
> > +
> > extern bool _bfd_elf_adjust_dynamic_copy
> > (struct bfd_link_info *, struct elf_link_hash_entry *, asection *);
> >
> > diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
> > index ec001599cc1..1a2e64c031f 100644
> > --- a/bfd/elf64-x86-64.c
> > +++ b/bfd/elf64-x86-64.c
> > @@ -5568,6 +5568,31 @@ elf_x86_64_link_setup_gnu_properties (struct bfd_link_info *info)
> > return _bfd_x86_elf_link_setup_gnu_properties (info, &init_table);
> > }
> >
> > +static void
> > +elf_x86_64_add_glibc_version_dependency
> > + (struct elf_find_verdep_info *rinfo)
> > +{
> > + unsigned int i = 0;
> > + const char *version[3] = { NULL, NULL, NULL };
> > + struct elf_x86_link_hash_table *htab;
> > +
> > + if (rinfo->info->enable_dt_relr)
> > + {
> > + version[i] = "GLIBC_ABI_DT_RELR";
> > + i++;
> > + }
> > +
> > + htab = elf_x86_hash_table (rinfo->info, X86_64_ELF_DATA);
> > + if (htab != NULL && htab->params->mark_plt)
> > + {
> > + version[i] = "GLIBC_2.36";
> > + i++;
> > + }
> > +
> > + if (i != 0)
> > + _bfd_elf_link_add_glibc_version_dependency (rinfo, version);
> > +}
> > +
> > static const struct bfd_elf_special_section
> > elf_x86_64_special_sections[]=
> > {
> > @@ -5652,6 +5677,8 @@ elf_x86_64_special_sections[]=
> > elf_x86_64_link_setup_gnu_properties
> > #define elf_backend_hide_symbol \
> > _bfd_x86_elf_hide_symbol
> > +#define elf_backend_add_glibc_version_dependency \
> > + elf_x86_64_add_glibc_version_dependency
> >
> > #undef elf64_bed
> > #define elf64_bed elf64_x86_64_bed
> > diff --git a/bfd/elflink.c b/bfd/elflink.c
> > index a577c957514..c2494b3e12e 100644
> > --- a/bfd/elflink.c
> > +++ b/bfd/elflink.c
> > @@ -46,19 +46,6 @@ struct elf_info_failed
> > bool failed;
> > };
> >
> > -/* This structure is used to pass information to
> > - _bfd_elf_link_find_version_dependencies. */
> > -
> > -struct elf_find_verdep_info
> > -{
> > - /* General link information. */
> > - struct bfd_link_info *info;
> > - /* The number of dependencies. */
> > - unsigned int vers;
> > - /* Whether we had a failure. */
> > - bool failed;
> > -};
> > -
> > static bool _bfd_elf_fix_symbol_flags
> > (struct elf_link_hash_entry *, struct elf_info_failed *);
> >
> > @@ -2217,64 +2204,64 @@ _bfd_elf_export_symbol (struct elf_link_hash_entry *h, void *data)
> > return true;
> > }
> >
> > -/* Return true if GLIBC_ABI_DT_RELR is added to the list of version
> > - dependencies successfully. GLIBC_ABI_DT_RELR will be put into the
> > - .gnu.version_r section. */
> > +/* Return the glibc version reference if VERSION_DEP is added to the
> > + list of glibc version dependencies successfully. VERSION_DEP will
> > + be put into the .gnu.version_r section. */
> >
> > -static bool
> > -elf_link_add_dt_relr_dependency (struct elf_find_verdep_info *rinfo)
> > +static Elf_Internal_Verneed *
> > +elf_link_add_glibc_verneed (struct elf_find_verdep_info *rinfo,
> > + Elf_Internal_Verneed *glibc_verref,
> > + const char *version_dep)
> > {
> > - bfd *glibc_bfd = NULL;
> > Elf_Internal_Verneed *t;
> > Elf_Internal_Vernaux *a;
> > size_t amt;
> > - const char *relr = "GLIBC_ABI_DT_RELR";
> >
> > - /* See if we already know about GLIBC_PRIVATE_DT_RELR. */
> > - for (t = elf_tdata (rinfo->info->output_bfd)->verref;
> > - t != NULL;
> > - t = t->vn_nextref)
> > + if (glibc_verref != NULL)
> > {
> > - const char *soname = bfd_elf_get_dt_soname (t->vn_bfd);
> > - /* Skip the shared library if it isn't libc.so. */
> > - if (!soname || !startswith (soname, "libc.so."))
> > - continue;
> > + t = glibc_verref;
> >
> > for (a = t->vn_auxptr; a != NULL; a = a->vna_nextptr)
> > {
> > - /* Return if GLIBC_PRIVATE_DT_RELR dependency has been
> > - added. */
> > - if (a->vna_nodename == relr
> > - || strcmp (a->vna_nodename, relr) == 0)
> > - return true;
> > -
> > - /* Check if libc.so provides GLIBC_2.XX version. */
> > - if (!glibc_bfd && startswith (a->vna_nodename, "GLIBC_2."))
> > - glibc_bfd = t->vn_bfd;
> > + /* Return if VERSION_DEP dependency has been added. */
> > + if (a->vna_nodename == version_dep
> > + || strcmp (a->vna_nodename, version_dep) == 0)
> > + return t;
> > }
> > -
> > - break;
> > }
> > + else
> > + {
> > + bool is_glibc;
> >
> > - /* Skip if it isn't linked against glibc. */
> > - if (glibc_bfd == NULL)
> > - return true;
> > + for (t = elf_tdata (rinfo->info->output_bfd)->verref;
> > + t != NULL;
> > + t = t->vn_nextref)
> > + {
> > + const char *soname = bfd_elf_get_dt_soname (t->vn_bfd);
> > + if (soname != NULL && startswith (soname, "libc.so."))
> > + break;
> > + }
> >
> > - /* This is a new version. Add it to tree we are building. */
> > - if (t == NULL)
> > - {
> > - amt = sizeof *t;
> > - t = (Elf_Internal_Verneed *) bfd_zalloc (rinfo->info->output_bfd,
> > - amt);
> > + /* Skip the shared library if it isn't libc.so. */
> > if (t == NULL)
> > + return t;
> > +
> > + is_glibc = false;
> > + for (a = t->vn_auxptr; a != NULL; a = a->vna_nextptr)
> > {
> > - rinfo->failed = true;
> > - return false;
> > + /* Return if VERSION_DEP dependency has been added. */
> > + if (a->vna_nodename == version_dep
> > + || strcmp (a->vna_nodename, version_dep) == 0)
> > + return t;
> > +
> > + /* Check if libc.so provides GLIBC_2.XX version. */
> > + if (!is_glibc && startswith (a->vna_nodename, "GLIBC_2."))
> > + is_glibc = true;
> > }
> >
> > - t->vn_bfd = glibc_bfd;
> > - t->vn_nextref = elf_tdata (rinfo->info->output_bfd)->verref;
> > - elf_tdata (rinfo->info->output_bfd)->verref = t;
> > + /* Skip if it isn't linked against glibc. */
> > + if (!is_glibc)
> > + return NULL;
> > }
> >
> > amt = sizeof *a;
> > @@ -2282,10 +2269,10 @@ elf_link_add_dt_relr_dependency (struct elf_find_verdep_info *rinfo)
> > if (a == NULL)
> > {
> > rinfo->failed = true;
> > - return false;
> > + return NULL;
> > }
> >
> > - a->vna_nodename = relr;
> > + a->vna_nodename = version_dep;
> > a->vna_flags = 0;
> > a->vna_nextptr = t->vn_auxptr;
> > a->vna_other = rinfo->vers + 1;
> > @@ -2293,7 +2280,45 @@ elf_link_add_dt_relr_dependency (struct elf_find_verdep_info *rinfo)
> >
> > t->vn_auxptr = a;
> >
> > - return true;
> > + return t;
> > +}
> > +
> > +/* Add VERSION_DEP to the list of version dependencies when linked
> > + against glibc. */
> > +
> > +void
> > +_bfd_elf_link_add_glibc_version_dependency
> > + (struct elf_find_verdep_info *rinfo,
> > + const char *version_dep[])
> > +{
> > + Elf_Internal_Verneed *t = NULL;
> > +
> > + do
> > + {
> > + t = elf_link_add_glibc_verneed (rinfo, t, *version_dep);
> > + /* Return if there is no glibc version reference. */
> > + if (t == NULL)
> > + return;
> > + version_dep++;
> > + }
> > + while (*version_dep != NULL);
> > +}
> > +
> > +/* Add GLIBC_ABI_DT_RELR to the list of version dependencies when
> > + linked against glibc. */
> > +
> > +void
> > +_bfd_elf_link_add_dt_relr_dependency (struct elf_find_verdep_info *rinfo)
> > +{
> > + if (rinfo->info->enable_dt_relr)
> > + {
> > + const char *version[] =
> > + {
> > + "GLIBC_ABI_DT_RELR",
> > + NULL
> > + };
> > + _bfd_elf_link_add_glibc_version_dependency (rinfo, version);
> > + }
> > }
> >
> > /* Look through the symbols which are defined in other shared
> > @@ -7047,12 +7072,9 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
> > if (sinfo.failed)
> > return false;
> >
> > - if (info->enable_dt_relr)
> > - {
> > - elf_link_add_dt_relr_dependency (&sinfo);
> > - if (sinfo.failed)
> > - return false;
> > - }
> > + bed->elf_backend_add_glibc_version_dependency (&sinfo);
> > + if (sinfo.failed)
> > + return false;
> >
> > if (elf_tdata (output_bfd)->verref == NULL)
> > s->flags |= SEC_EXCLUDE;
> > diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
> > index d4b14a022e3..a7f2fc6e320 100644
> > --- a/bfd/elfxx-target.h
> > +++ b/bfd/elfxx-target.h
> > @@ -667,6 +667,10 @@
> > #ifndef elf_backend_write_section
> > #define elf_backend_write_section NULL
> > #endif
> > +#ifndef elf_backend_add_glibc_version_dependency
> > +#define elf_backend_add_glibc_version_dependency \
> > + _bfd_elf_link_add_dt_relr_dependency
> > +#endif
> > #ifndef elf_backend_elfsym_local_is_section
> > #define elf_backend_elfsym_local_is_section NULL
> > #endif
> > @@ -897,6 +901,7 @@ static const struct elf_backend_data elfNN_bed =
> > elf_backend_can_make_multiple_eh_frame,
> > elf_backend_encode_eh_address,
> > elf_backend_write_section,
> > + elf_backend_add_glibc_version_dependency,
> > elf_backend_elfsym_local_is_section,
> > elf_backend_mips_irix_compat,
> > elf_backend_mips_rtype_to_howto,
> > diff --git a/ld/testsuite/ld-x86-64/mark-plt-1a.rd b/ld/testsuite/ld-x86-64/mark-plt-1a.rd
> > new file mode 100644
> > index 00000000000..1234fbe038c
> > --- /dev/null
> > +++ b/ld/testsuite/ld-x86-64/mark-plt-1a.rd
> > @@ -0,0 +1,7 @@
> > +#...
> > +Version needs section '.gnu.version_r' contains 1 entry:
> > + Addr: 0x[0-9a-f]+ +Offset: 0x[0-9a-f]+ +Link: +[0-9]+ +\(.dynstr\)
> > + +0+: Version: 1 +File: libc\.so\.6(|\.1) +Cnt: +[0-9]+
> > +#...
> > + 0x[a-f0-9]+: Name: GLIBC_2.36 Flags: none Version: [0-9]+
> > +#pass
> > diff --git a/ld/testsuite/ld-x86-64/mark-plt-1b.rd b/ld/testsuite/ld-x86-64/mark-plt-1b.rd
> > new file mode 100644
> > index 00000000000..6556a6d939e
> > --- /dev/null
> > +++ b/ld/testsuite/ld-x86-64/mark-plt-1b.rd
> > @@ -0,0 +1,7 @@
> > +#...
> > +Version needs section '.gnu.version_r' contains 1 entry:
> > + Addr: 0x[0-9a-f]+ +Offset: 0x[0-9a-f]+ +Link: +[0-9]+ +\(.dynstr\)
> > + +0+: Version: 1 +File: libc\.so\.6(|\.1) +Cnt: +[0-9]+
> > +#...
> > + 0x[a-f0-9]+: Name: GLIBC_ABI_DT_RELR Flags: none Version: [0-9]+
> > +#pass
> > diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
> > index 0af9f047600..d17bb9b7d7e 100644
> > --- a/ld/testsuite/ld-x86-64/x86-64.exp
> > +++ b/ld/testsuite/ld-x86-64/x86-64.exp
> > @@ -2257,4 +2257,18 @@ run_dump_test "mark-plt-1b-x32"
> > run_dump_test "mark-plt-1c-x32"
> > run_dump_test "mark-plt-1d-x32"
> >
> > +if { [check_compiler_available] } {
> > + run_cc_link_tests [list \
> > + [list \
> > + "Build mark-plt-1.so" \
> > + "-shared -Wl,-z,mark-plt,-z,pack-relative-relocs" \
> > + "-fPIC" \
> > + { mark-plt-1.s } \
> > + {{readelf {-W --version-info} mark-plt-1a.rd} \
> > + {readelf {-W --version-info} mark-plt-1b.rd}} \
> > + "mark-plt-1.so" \
> > + ] \
> > + ]
> > +}
> > +
> > set ASFLAGS "$saved_ASFLAGS"
> > --
> > 2.43.0
> >
>
> Nick, Alan,
>
> Any comments?
>
> Thanks.
>
> --
> H.J.
I will check it in later today.
--
H.J.
next prev parent reply other threads:[~2024-01-09 13:46 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-06 22:09 [PATCH v2 0/2] Improve -z mark-plt H.J. Lu
2024-01-06 22:10 ` [PATCH v2 1/2] elf: Add elf_backend_add_glibc_version_dependency H.J. Lu
2024-01-08 15:22 ` H.J. Lu
2024-01-09 13:45 ` H.J. Lu [this message]
2024-01-06 22:10 ` [PATCH v2 2/2] ld: Add --enable-mark-plt configure option H.J. Lu
2024-01-08 15:24 ` H.J. Lu
2024-01-09 13:46 ` H.J. Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMe9rOqQtoDM8Wq_rS8gccqhNmma2tYJbwaRGk_v8sdrix33mg@mail.gmail.com \
--to=hjl.tools@gmail.com \
--cc=amodra@gmail.com \
--cc=binutils@sourceware.org \
--cc=nickc@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).