On 10/4/18 9:12 AM, Kyle Russell wrote: > Hey Mark, > > Do you think this approach is reasonable? If so, I have another patch I'd > like > to propose that would enable us to better catch error scenarios (like the last > two patches address) that we might encounter during do_image_prelink. We just > happened to detect these last two issues even though image_prelink didn't fail > the build, so I'd like to provide an option to enable a little more assertive > error path, if desired.
I'm getting caught back up on my prelink 'TODO' set. The approach seems reasonable to me. However, I appear to have lost the reference to the original patch email. I'm going to try to reapply from this email (or the list archive), but I may need you to resend it. I'll let you [and others know] once I get these things merged. Thanks! --Mark > Thanks, > Kyle > > On Fri, Sep 28, 2018 at 10:57 AM Kyle Russell <bkyleruss...@gmail.com > <mailto:bkyleruss...@gmail.com>> wrote: > > binutils-2.28 (17026142ef35b62ac88bfe517b4160614902cb28) adds support > for copying read-only dynamic symbols into .data.rel.ro > <http://data.rel.ro> > instead of .bss > since .bss is technically writable. This causes prelink to error out on > any binary containing COPY relocations in .data.rel.ro > <http://data.rel.ro>. > > Read-only variables defined in shared libraries should be copied directly > into the space allocated for them in .data.rel.ro <http://data.rel.ro> by > the linker. > > To achieve this, we determine whether either of the two sections > containing copy relocations is .data.rel.ro <http://data.rel.ro>. If so, > we > relocate the > symbol memory directly into the existing section instead of constructing > a new .(s)dynbss section once prelink_build_conflicts() returns. > > Fixes cxx1.sh, cxx2.sh, and cxx3.sh on Fedora 28 (which uses > binutils-2.29). > --- > src/conflict.c | 51 +++++++++++++++++++++++++++++++++++++------------- > src/undo.c | 9 +++++++++ > 2 files changed, 47 insertions(+), 13 deletions(-) > > diff --git a/src/conflict.c b/src/conflict.c > index 9ae2ddb..5613ace 100644 > --- a/src/conflict.c > +++ b/src/conflict.c > @@ -450,7 +450,7 @@ get_relocated_mem (struct prelink_info *info, DSO > *dso, > GElf_Addr addr, > int > prelink_build_conflicts (struct prelink_info *info) > { > - int i, ndeps = info->ent->ndepends + 1; > + int i, reset_dynbss = 0, reset_sdynbss = 0, ndeps = > info->ent->ndepends + 1; > struct prelink_entry *ent; > int ret = 0; > DSO *dso; > @@ -675,6 +675,11 @@ prelink_build_conflicts (struct prelink_info *info) > dso->filename); > goto error_out; > } > + > + name = strptr (dso, dso->ehdr.e_shstrndx, > dso->shdr[bss1].sh_name); > + if (strcmp(name, ".data.rel.ro <http://data.rel.ro>") == 0) > + reset_sdynbss = 1; > + > firstbss2 = i; > info->sdynbss_size = cr.rela[i - 1].r_offset - > cr.rela[0].r_offset; > info->sdynbss_size += cr.rela[i - 1].r_addend; > @@ -702,6 +707,10 @@ prelink_build_conflicts (struct prelink_info *info) > } > } > > + name = strptr (dso, dso->ehdr.e_shstrndx, dso->shdr[bss2].sh_name); > + if (strcmp(name, ".data.rel.ro <http://data.rel.ro>") == 0) > + reset_dynbss = 1; > + > info->dynbss_size = cr.rela[cr.count - 1].r_offset > - cr.rela[firstbss2].r_offset; > info->dynbss_size += cr.rela[cr.count - 1].r_addend; > @@ -719,9 +728,9 @@ prelink_build_conflicts (struct prelink_info *info) > && strcmp (name = strptr (dso, dso->ehdr.e_shstrndx, > dso->shdr[bss1].sh_name), > ".dynbss") != 0 > - && strcmp (name, ".sdynbss") != 0) > + && strcmp (name, ".sdynbss") != 0 && strcmp (name, ".data.rel.ro > <http://data.rel.ro>") != 0) > { > - error (0, 0, "%s: COPY relocations don't point into .bss or > .sbss > section", > + error (0, 0, "%s: COPY relocations don't point into .bss, .sbss, > or .data.rel.ro <http://data.rel.ro> sections", > dso->filename); > goto error_out; > } > @@ -730,9 +739,9 @@ prelink_build_conflicts (struct prelink_info *info) > && strcmp (name = strptr (dso, dso->ehdr.e_shstrndx, > dso->shdr[bss2].sh_name), > ".dynbss") != 0 > - && strcmp (name, ".sdynbss") != 0) > + && strcmp (name, ".sdynbss") != 0 && strcmp (name, ".data.rel.ro > <http://data.rel.ro>") != 0) > { > - error (0, 0, "%s: COPY relocations don't point into .bss or > .sbss > section", > + error (0, 0, "%s: COPY relocations don't point into .bss, .sbss, > or .data.rel.ro <http://data.rel.ro> section", > dso->filename); > goto error_out; > } > @@ -768,16 +777,21 @@ prelink_build_conflicts (struct prelink_info *info) > } > > assert (j < ndeps); > + GElf_Addr symaddr = s->u.ent->base + s->value; > + char *buf; > + > if (i < firstbss2) > - j = get_relocated_mem (info, ndso, s->u.ent->base + s->value, > - info->sdynbss + cr.rela[i].r_offset > - - info->sdynbss_base, > cr.rela[i].r_addend, > - cr.rela[i].r_offset); > + if (reset_sdynbss) > + buf = get_data(dso, cr.rela[i].r_offset, NULL, NULL); > + else > + buf = info->sdynbss + cr.rela[i].r_offset - > info->sdynbss_base; > else > - j = get_relocated_mem (info, ndso, s->u.ent->base + s->value, > - info->dynbss + cr.rela[i].r_offset > - - info->dynbss_base, > cr.rela[i].r_addend, > - cr.rela[i].r_offset); > + if (reset_dynbss) > + buf = get_data(dso, cr.rela[i].r_offset, NULL, NULL); > + else > + buf = info->dynbss + cr.rela[i].r_offset - > info->dynbss_base; > + > + j = get_relocated_mem (info, ndso, symaddr, buf, > cr.rela[i].r_addend, cr.rela[i].r_offset); > > switch (j) > { > @@ -815,6 +829,17 @@ prelink_build_conflicts (struct prelink_info *info) > if (info->dsos[i]) > close_dso (info->dsos[i]); > > + if (reset_sdynbss) > + { > + free (info->sdynbss); > + info->sdynbss = NULL; > + } > + > + if (reset_dynbss) > + { > + free (info->dynbss); > + info->dynbss = NULL; > + } > info->dsos = NULL; > free (cr.rela); > return ret; > diff --git a/src/undo.c b/src/undo.c > index 8a55bf2..4c38dab 100644 > --- a/src/undo.c > +++ b/src/undo.c > @@ -633,6 +633,15 @@ prelink_undo (DSO *dso) > d->d_buf = NULL; > dso->shdr[i].sh_type = SHT_NOBITS; > } > + else if (dso->shdr[i].sh_type == SHT_PROGBITS > + && strcmp (name, ".data.rel.ro <http://data.rel.ro>") > == 0) > + { > + scn = dso->scn[i]; > + d = elf_getdata (scn, NULL); > + assert (d != NULL && elf_getdata (scn, d) == NULL); > + assert (d->d_size == dso->shdr[i].sh_size); > + assert (memset(d->d_buf, 0, d->d_size) == d->d_buf); > + } > else if (dso->shdr[i].sh_type == SHT_RELA > && shdr[i].sh_type == SHT_REL) > { > -- > 2.17.1 > -- _______________________________________________ yocto mailing list yocto@yoctoproject.org https://lists.yoctoproject.org/listinfo/yocto
