[Bug libc/28857] FAIL: elf/tst-audit24a

["adhemerval.zanella at linaro dot org" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=28857

--- Comment #3 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to dave.anglin from comment #2)
> Is it DL_FIXUP_ADDR_VALUE that's problematic?  Maybe it should be defined to
> _dl_lookup_address()?

I think you mean DL_FIXUP_BINDNOW_ADDR_VALUE, but I don't this is the issue
since it is just used for pointer equality to see the symbind does change the
binding.

> 
> On 32-bit hppa, a pointer to a function descriptor has the plabel bit set
> (bit 30).  This must be cleared to
> get the actual address of the descriptor ((unsigned int)addr & ~2).  If the
> descriptor has been bound,
> the first word of the descriptor is the physical address of the function. 
> If it hasn't been bound, the first
> word of the descriptor points to a trampoline in the PLT.  It will fixup the
> descriptor on the first call to
> the function.
> 
> _dl_lookup_address() in dl-fptr.c resolves the descriptor and returns the
> actual function address.
> 
> If that's not required, maybe DL_FIXUP_ADDR_VALUE(addr) should be:
> #define DL_FIXUP_ADDR_VALUE(addr) (*(struct fdesc *) ((unsigned int)addr &
> ~2))
> DL_FIXUP_BINDNOW_RELOC looks questionable as well.

The _dl_lookup_address seems to be indeed required and I also think we should
only updated the binding if la_symbind actually acts uppon it.  The following
patch fixes the tests on hppa:

diff --git a/elf/Makefile b/elf/Makefile
index 5bdf0a383d..7372cb191c 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -2210,7 +2210,7 @@ $(objpfx)tst-audit24c.out: $(objpfx)tst-auditmod24c.so
 $(objpfx)tst-audit24c: $(objpfx)tst-audit24amod1.so \
                       $(objpfx)tst-audit24amod2.so
 tst-audit24c-ENV = LD_BIND_NOW=1 LD_AUDIT=$(objpfx)tst-auditmod24c.so
-LDFLAGS-tst-audit24b = -Wl,-z,lazy
+LDFLAGS-tst-audit24c = -Wl,-z,lazy

 $(objpfx)tst-audit24d.out: $(objpfx)tst-auditmod24d.so
 $(objpfx)tst-audit24d: $(objpfx)tst-audit24dmod1.so \
diff --git a/elf/dl-audit.c b/elf/dl-audit.c
index 794bfd45cd..efc0492474 100644
--- a/elf/dl-audit.c
+++ b/elf/dl-audit.c
@@ -257,7 +257,8 @@ _dl_audit_symbind (struct link_map *l, struct reloc_result
*reloc_result,
       reloc_result->flags = flags;
     }

-  DL_FIXUP_BINDNOW_RELOC (value, new_value, sym.st_value);
+  if (flags & LA_SYMB_ALTVALUE)
+    DL_FIXUP_BINDNOW_RELOC (value, new_value, sym.st_value);
 }

 void
diff --git a/elf/tst-auditmod24a.c b/elf/tst-auditmod24a.c
index d8e88f3984..3075dfae2f 100644
--- a/elf/tst-auditmod24a.c
+++ b/elf/tst-auditmod24a.c
@@ -110,5 +110,7 @@ la_symbind32 (Elf32_Sym *sym, unsigned int ndx,
       return sym->st_value;
     }

-  abort ();
+  if (symname[0] != '\0')
+    abort ();
+  return sym->st_value;
 }
diff --git a/elf/tst-auditmod24d.c b/elf/tst-auditmod24d.c
index 8c803ecc0a..badc6be451 100644
--- a/elf/tst-auditmod24d.c
+++ b/elf/tst-auditmod24d.c
@@ -116,5 +116,7 @@ la_symbind32 (Elf32_Sym *sym, unsigned int ndx,
        }
     }

-  abort ();
+  if (symname[0] != '\0')
+    abort ();
+  return sym->st_value;
 }
diff --git a/elf/tst-auditmod25.c b/elf/tst-auditmod25.c
index 526f5c54bc..20640a8daf 100644
--- a/elf/tst-auditmod25.c
+++ b/elf/tst-auditmod25.c
@@ -72,7 +72,7 @@ la_symbind32 (Elf32_Sym *sym, unsigned int ndx,
              unsigned int *flags, const char *symname)
 #endif
 {
-  if (*refcook != -1 && *defcook != -1)
+  if (*refcook != -1 && *defcook != -1 && symname[0] != '\0')
     fprintf (stderr, "la_symbind: %s %u\n", symname,
             *flags & (LA_SYMB_NOPLTENTER | LA_SYMB_NOPLTEXIT) ? 1 : 0);
   return sym->st_value;
diff --git a/sysdeps/hppa/dl-fptr.c b/sysdeps/hppa/dl-fptr.c
index 2584557c4f..4cc2cb21b1 100644
--- a/sysdeps/hppa/dl-fptr.c
+++ b/sysdeps/hppa/dl-fptr.c
@@ -407,3 +407,4 @@ _dl_lookup_address (const void *address)

   return (ElfW(Addr)) desc[0];
 }
+rtld_hidden_def (_dl_lookup_address)
diff --git a/sysdeps/hppa/dl-lookupcfg.h b/sysdeps/hppa/dl-lookupcfg.h
index 8da2412fea..3929fc84ae 100644
--- a/sysdeps/hppa/dl-lookupcfg.h
+++ b/sysdeps/hppa/dl-lookupcfg.h
@@ -30,6 +30,7 @@ rtld_hidden_proto (_dl_symbol_address)
 #define DL_SYMBOL_ADDRESS(map, ref) _dl_symbol_address(map, ref)

 Elf32_Addr _dl_lookup_address (const void *address);
+rtld_hidden_proto (_dl_lookup_address)

 #define DL_LOOKUP_ADDRESS(addr) _dl_lookup_address ((const void *) addr)

@@ -81,5 +82,8 @@ void attribute_hidden _dl_unmap (struct link_map *map);
 #define DL_FIXUP_VALUE_ADDR(value) ((uintptr_t) &(value))
 #define DL_FIXUP_ADDR_VALUE(addr) (*(struct fdesc *) (addr))
 #define DL_FIXUP_BINDNOW_ADDR_VALUE(addr) (addr)
-#define DL_FIXUP_BINDNOW_RELOC(value, new_value, st_value) \
-  (*value) = *(struct fdesc *) (st_value)
+#define DL_FIXUP_BINDNOW_RELOC(value, new_value, st_value)     \
+  ({                                                           \
+     value->ip = _dl_lookup_address ((void *) new_value);      \
+     value->gp = ((struct fdesc *) (new_value))->gp;           \
+  })

I had to add a workaround on tests because on hppa (and it seems to be the only
ABI I have see it), some shared library adds a dynamic PLT relocation that I am
not sure why it is required:

$ readelf -r elf/tst-audit25mod1.so
[...]
Relocation section '.rela.plt' at offset 0x464 contains 6 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00002008  00000081 R_PARISC_IPLT                508

It breaks some assumptions on the test, where a symbol with an empty name ("")
is passed on la_symbind.

-- 
You are receiving this mail because:
You are on the CC list for the bug.