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