Re: Prelinking on ARM with Debug Link

[Torsten Polle <Torsten dot Polle at gmx dot de>]

Hi Mark,

> Am 05.04.2016 um 15:44 schrieb Mark Wielaard <mjw@redhat.com>:
> 
> [stuff deleted]
> 
> OK. So the issue is that we use a hex representation of a negative
> number that might not fit when converted to an unsigned value.
> The simplest fix seems to be to just use a (signed) decimal
> representation, that can be converted to the unsigned value on
> assignment.

The issue remains if we use decimals. Because even the decimal 0xffffffffffffebc0 = 18446744073709546432 is too large to be represented as 32 bit value. :-(  Therefore my cross compiler on the 64bit host complains.

> But you prefer to use the hex representation instead?

The usage of hex over decimal is just a matter of taste.

> I don't completely understand what sec_load_offset represents and why it
> is an unsigned value. We use it in two places:
> 
> runtime/sym.c (_stp_linenumber_lookup):
> 
>  // if addr is a kernel address, it will need to be adjusted
>  if (!task)
>    {
>      int i;
>      unsigned long offset = 0;
>      // have to factor in the load_offset of (specifically) the .text section
>      for (i=0; i<m->num_sections; i++)
>        if (!strcmp(m->sections[i].name, ".text"))
>          {
>            offset = (m->sections[i].static_addr - m->sections[i].sec_load_offset);
>            break;
>          }
> 
>      if (addr < offset)
>        return 0;
>      addr = addr - offset;
>    }
> 
> runtime/unwind.c (adjustStartLoc):
> 
>  if (is_ehframe)
>    return startLoc + vm_addr;
>  else
>    return startLoc + vm_addr - s->sec_load_offset;

I had twiddled with the else branch in the past. I do not recall all attempts. But Iâm sure that I set sec_load_offset even to 0. The result is that the probes are set at the wrong address in libc. To be more concrete they are offset by -5184 compared to the original address. Still Iâll check your patch and let you know the result.

> The first is clearly only used for the kernel. The second is only used
> for .debug_frame addresses. And if you read the rest of the code
> (".dynamic" is user space dynamic library (ET_DYN), ".absolute" is user
> space executable (ET_EXEC), the kernel has a special name "_stext" and
> only kernel modules have section names attached because they are
> ET_REL), it looks like sec_load_offset is only used when handling kernel
> modules.
> 
> This makes sense. Except for kernel modules all other "modules" exist of
> just one executable segment/section. But kernel modules (ET_REL files)
> can have multiple executable sections with different names and offsets.
> 
> So in practice sec_load_offset is only used for kernel modules to
> calculate the section static_addr - sec_load_offset.
> 
> Given that I think we are solving the wrong problem. Calculating
> sec_load_offset for anything except kernel modules doesn't make sense.
> So maybe the simplest solution is the attached patch?
> 
> It is odd that we only seem to do this for the ".text" section. A kernel
> module can have multiple code sections with different names. This isn't
> a regression since we never seemed to handle any other section. But if
> there are problems unwinding through some kernel modules then this might
> explain it.
> 
> Cheers,
> 
> Mark
> diff --git a/translate.cxx b/translate.cxx
> index 5daf725..f10c98d 100644
> --- a/translate.cxx
> +++ b/translate.cxx
> @@ -6911,10 +6911,17 @@ dump_unwindsym_cxt (Dwfl_Module *m,
> 		    << "_debug_frame_hdr_" << secidx << ",\n";
>           c->output << ".debug_hdr_len = " << debug_frame_hdr_len << ", \n";
> 
> -	  Dwarf_Addr dwbias = 0;
> -	  dwfl_module_getdwarf (m, &dwbias);
> -	  c->output << ".sec_load_offset = 0x"
> -		    << hex << debug_frame_off - dwbias << dec << "\n";
> +	  /* Only kernel modules has section names. And the only section
> +	     name we are currently interested in (see above) is ".text".  */
> +	  if (secname == ".text")
> +	    {
> +	      Dwarf_Addr dwbias = 0;
> +	      dwfl_module_getdwarf (m, &dwbias);
> +	      c->output << ".sec_load_offset = 0x"
> +			<< hex << debug_frame_off - dwbias << dec << "\n";
> +	    }
> +	  else
> +	    c->output << ".sec_load_offset = 0\n";
> 
> 	  c->output << "#else\n";
> 	  c->output << ".debug_hdr = NULL,\nâ;

Cheers,
Torsten