[DTrace-devel] [PATCH v2 07/20] dt_link: finish backing out varint, fixing dt_link-generated object files

[Kris Van Hees]

Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

On Wed, Sep 07, 2022 at 01:59:55PM +0100, Nick Alcock via DTrace-devel wrote:
> For a while now, linking dtrace -G-generated object files has been
> saying
> 
> /usr/bin/ld: .../build/test-triggers--usdt-tst-special.o: error adding symbols: no error
> 
> This is more than slightly confusing.  For starters, collect2 is
> corrupting the error message: the actual error is
> 
> /usr/bin/ld: .../build/test-triggers--usdt-tst-special.o: error adding symbols: file format not recognized
> 
> This is not much more helpful.  The actual cause is in
> elf_link_add_object_symbols:
> 
>       name = bfd_elf_string_from_elf_section (abfd, hdr->sh_link,
> 					      isym->st_name);
> 
> where the symbol name comes out as NULL.  This is because we are hitting
> a sanity check in bfd_elf_string_from_elf_section:
> 
>       /* PR 24273: The string section's contents may have already
> 	 been loaded elsewhere, eg because a corrupt file has the
> 	 string section index in the ELF header pointing at a group
> 	 section.  So be paranoid, and test that the last byte of
> 	 the section is zero.  */
>       if (hdr->sh_size == 0 || hdr->contents[hdr->sh_size - 1] != 0)
> 	return NULL;
> 
> This paranoia has won out! We are generating string tables with
> hdr->sh_size one too small, pointing at the last byte of the last
> string.
> 
> The cause is somewhat gross.  dt_link.c:process_obj computes the length
> of the new strtab it's constructing by adding all the symbol names to a
> dt_strtab, asking it for its size, subtracting one for the \0 at the
> first byte, and then throwing the strtab away and directly editing the
> strtab in the object file, using the strtab it generated earlier only
> for its size.  Doing things in two stages like this is a bit risky: it's
> riskier yet because it's using a dt_strtab, which is not a model of an
> ELF strtab at all but rather of a DTrace string table.  No harm, they're
> the same: except that when varints landed the format of a string
> naturally changed to gain an extra length byte at the start. The size
> computation was changed to subtract two bytes rather than one (itself
> buggy: *every string in the strtab* had an extra byte on the front of
> it, so the wrong number of bytes was being subtracted and the resulting
> sh_size was too large).  But when the varint code was backed out (in
> 29e3f422 "Remove the string length prefix"), this size computation was
> not changed back, leading to a sh_size that is one byte too low,
> triggering this failure.
> 
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
>  libdtrace/dt_link.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libdtrace/dt_link.c b/libdtrace/dt_link.c
> index 21ef56c0a3cb..5daaf2b02212 100644
> --- a/libdtrace/dt_link.c
> +++ b/libdtrace/dt_link.c
> @@ -1337,10 +1337,10 @@ process_obj(dtrace_hdl_t *dtp, const char *obj, int *eprobesp)
>  		 */
>  		if (nsym > 0) {
>  			/*
> -			 * The first two bytes of the string table are reserved
> +			 * The first byte of the string table is reserved
>  			 * for the \0 entry.
>  			 */
> -			len = dt_strtab_size(strtab) - 2;
> +			len = dt_strtab_size(strtab) - 1;
>  
>  			assert(len > 0);
>  			assert(dt_strtab_index(strtab, "") == 0);
> -- 
> 2.37.1.265.g363c192786.dirty
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel