Re: Prelinking on ARM with Debug Link

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

Hi Mark,

> Torsten Polle writes
>
> Hi Mark,
>
> Mark Wielaard writes:
>> On Fri, Nov 27, 2015 at 04:06:14PM +0100, Mark Wielaard wrote:
>>> On Fri, Nov 27, 2015 at 01:57:31PM +0100, Torsten Polle wrote:
>>> > In file included from /tmp/stap/taptrek_src.c:6010:0:
>>> > /tmp/stap/stap-symbols.h:95902:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
>>> 
>>> But the stap-symbols.h file has only 8107 lines.
>
>> If you happen to know which library it is that causes the issue, then
>> it would be beneficial to also have the output of eu-readelf -Sl for
>> both the main and debug file.
>
>> And maybe the stap -vv output of the script to know which files and
>> probes are used.
>
>> Thanks,
>> Mark
>
> thanks for the support.
>
> I'll provide you with a reproduction scenario that is much smaller.
>
> Kind Regards,
> Torsten

please find a less complex example attached.

The output of readelf -Sl for the affected library.
libc-2.18.so.txt

The library is using a debug link to link to a file with the debug information.
libc-2.18.so.debug.txt

The command to run the script "taptrek_run_ZIX2.stp" is stored in "cmd.sh".

The output of executing "cmd.sh" is in "cmd-output.txt".

The folder "stap" contains the result of the stap run. I only removed
the already compiled object files to keep the size small.

Thanks,
Torsten

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

[-- Attachment #1: Type: text/plain, Size: 1436 bytes --]

Hi Mark,
 
Now with attachment.

> Torsten Polle writes
>
> Hi Mark,
>
> Mark Wielaard writes:
>> On Fri, Nov 27, 2015 at 04:06:14PM +0100, Mark Wielaard wrote:
>>> On Fri, Nov 27, 2015 at 01:57:31PM +0100, Torsten Polle wrote:
>>> > In file included from /tmp/stap/taptrek_src.c:6010:0:
>>> > /tmp/stap/stap-symbols.h:95902:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
>>>
>>> But the stap-symbols.h file has only 8107 lines.
>
>> If you happen to know which library it is that causes the issue, then
>> it would be beneficial to also have the output of eu-readelf -Sl for
>> both the main and debug file.
>
>> And maybe the stap -vv output of the script to know which files and
>> probes are used.
>
>> Thanks,
>> Mark
>
> thanks for the support.
>
> I'll provide you with a reproduction scenario that is much smaller.
>
> Kind Regards,
> Torsten

Please find a less complex example attached.

The output of readelf -Sl for the affected library.
libc-2.18.so.txt

The library is using a debug link to link to a file with the debug information.
libc-2.18.so.debug.txt

The command to run the script "taptrek_run_ZIX2.stp" is stored in "cmd.sh".

The output of executing "cmd.sh" is in "cmd-output.txt".

The folder "stap" contains the result of the stap run. I only removed
the already compiled object files to keep the size small.

Thanks,
Torsten

[-- Attachment #2: prelinking.tar.bz2 --]
[-- Type: application/x-bzip2, Size: 63461 bytes --]

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

Hi Mark,

I’ve not received any answer on my posting. So I wonder whether the mail might have slipped your attention or there is any other information you might need.

> Am 01.12.2015 um 20:29 schrieb Torsten Polle <Torsten.Polle@gmx.de>:
> 
> Hi Mark,
> 
> Now with attachment.
> 
>> Torsten Polle writes
>> 
>> Hi Mark,
>> 
>> Mark Wielaard writes:
>>> On Fri, Nov 27, 2015 at 04:06:14PM +0100, Mark Wielaard wrote:
>>>> On Fri, Nov 27, 2015 at 01:57:31PM +0100, Torsten Polle wrote:
>>>>> In file included from /tmp/stap/taptrek_src.c:6010:0:
>>>>> /tmp/stap/stap-symbols.h:95902:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
>>>> 
>>>> But the stap-symbols.h file has only 8107 lines.
>> 
>>> If you happen to know which library it is that causes the issue, then
>>> it would be beneficial to also have the output of eu-readelf -Sl for
>>> both the main and debug file.
>> 
>>> And maybe the stap -vv output of the script to know which files and
>>> probes are used.
>> 
>>> Thanks,
>>> Mark
>> 
>> thanks for the support.
>> 
>> I'll provide you with a reproduction scenario that is much smaller.
>> 
>> Kind Regards,
>> Torsten
> 
> Please find a less complex example attached.
> 
> The output of readelf -Sl for the affected library.
> libc-2.18.so.txt
> 
> The library is using a debug link to link to a file with the debug information.
> libc-2.18.so.debug.txt
> 
> The command to run the script "taptrek_run_ZIX2.stp" is stored in "cmd.sh".
> 
> The output of executing "cmd.sh" is in "cmd-output.txt".
> 
> The folder "stap" contains the result of the stap run. I only removed
> the already compiled object files to keep the size small.
> 
> Thanks,
> Torsten
> <prelinking.tar.bz2>

Thanks,
Torsten

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

Hi Torsten,

On Tue, 2016-02-09 at 21:55 +0100, Torsten Polle wrote:
> I’ve not received any answer on my posting. So I wonder whether the
> mail might have slipped your attention or there is any other
> information you might need.

Sorry, I forgot about this issue.

It has been some time since I hacked on this code, so I don't
immediately know what is going on. It would be nice to have a somewhat
simpler reproducer. You use a large stap script using guru mode mixing
user and kernel probes. Is all that really necessary to replicate the
issue? Is the issue only triggered by the cross compiling?

It seems the issue is this generated code from stap-symbols.h
for /lib/libc-2.18.so:

static struct _stp_section _stp_module_1_sections[] = {
{
.name = ".dynamic",
.size = 0x13b9e8,
.symbols = _stp_module_1_symbols_0,
.num_symbols = 0,
#if defined(STP_USE_DWARF_UNWINDER) && defined(STP_NEED_UNWIND_DATA)
.debug_hdr = _stp_module_1_debug_frame_hdr_0,
.debug_hdr_len = 22604, 
.sec_load_offset = 0xffffffffffffebc0
#else
.debug_hdr = NULL,
.debug_hdr_len = 0,
.sec_load_offset = 0
#endif /* STP_USE_DWARF_UNWINDER && STP_NEED_UNWIND_DATA */
},
};

The .sec_load_offset went negative and produced a large hex output that
doesn't fit. We'll have to find out if the issue is that the value
became negative. Or whether the issue is that the generated value is too
big for the (cross-compiled) field type. Or maybe both.

Cheers,

mark

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

Hi Mark,

Thanks for the answer.

> Am 10.02.2016 um 17:17 schrieb Mark Wielaard <mjw@redhat.com>:
> 
> Hi Torsten,
> 
> On Tue, 2016-02-09 at 21:55 +0100, Torsten Polle wrote:
>> I’ve not received any answer on my posting. So I wonder whether the
>> mail might have slipped your attention or there is any other
>> information you might need.
> 
> Sorry, I forgot about this issue.
> 
> It has been some time since I hacked on this code, so I don't
> immediately know what is going on. It would be nice to have a somewhat
> simpler reproducer. You use a large stap script using guru mode mixing
> user and kernel probes. Is all that really necessary to replicate the
> issue? Is the issue only triggered by the cross compiling?

Do you mean the example in my original mail or do you refer to the much simpler example from [1]?

> It seems the issue is this generated code from stap-symbols.h
> for /lib/libc-2.18.so:
> 
> static struct _stp_section _stp_module_1_sections[] = {
> {
> .name = ".dynamic",
> .size = 0x13b9e8,
> .symbols = _stp_module_1_symbols_0,
> .num_symbols = 0,
> #if defined(STP_USE_DWARF_UNWINDER) && defined(STP_NEED_UNWIND_DATA)
> .debug_hdr = _stp_module_1_debug_frame_hdr_0,
> .debug_hdr_len = 22604, 
> .sec_load_offset = 0xffffffffffffebc0
> #else
> .debug_hdr = NULL,
> .debug_hdr_len = 0,
> .sec_load_offset = 0
> #endif /* STP_USE_DWARF_UNWINDER && STP_NEED_UNWIND_DATA */
> },
> };
> 
> The .sec_load_offset went negative and produced a large hex output that
> doesn't fit. We'll have to find out if the issue is that the value
> became negative. Or whether the issue is that the generated value is too
> big for the (cross-compiled) field type. Or maybe both.

I made some feeble attempts to fix this issue, i.e. I tried to figure out whether the negative offset made any sense at all. But all attempts on my part failed so far. So any help is much appreciated.

> Cheers,
> 
> mark

Thanks,
Torsten

[1] https://sourceware.org/ml/systemtap/2015-q4/msg00219.html

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

On Wed, 2016-02-10 at 21:12 +0100, Torsten Polle wrote:
> > Am 10.02.2016 um 17:17 schrieb Mark Wielaard <mjw@redhat.com>:
> > It has been some time since I hacked on this code, so I don't
> > immediately know what is going on. It would be nice to have a somewhat
> > simpler reproducer. You use a large stap script using guru mode mixing
> > user and kernel probes. Is all that really necessary to replicate the
> > issue? Is the issue only triggered by the cross compiling?
> 
> Do you mean the example in my original mail or do you refer to the much simpler example from [1]?

Maybe I am not reading the simpler example correctly.
But it looked like it was still mixing user and kernel probes, used guru
mode and cross compiling. Are all of those factors needed to trigger the
bug?

If at all possible just one simple probe against libc.so to show what is
going wrong would be ideal.

Thanks,

Mark

Aw: Re: Prelinking on ARM with Debug Link

[Torsten Polle]

[-- Attachment #1: Type: text/plain, Size: 1815 bytes --]

> Gesendet: Mittwoch, 10. Februar 2016 um 21:35 Uhr
> Von: "Mark Wielaard" <mjw@redhat.com>
> An: "Torsten Polle" <Torsten.Polle@gmx.de>
> Cc: systemtap@sourceware.org
> Betreff: Re: Prelinking on ARM with Debug Link
> On Wed, 2016-02-10 at 21:12 +0100, Torsten Polle wrote:
> > > Am 10.02.2016 um 17:17 schrieb Mark Wielaard <mjw@redhat.com>:
> > > It has been some time since I hacked on this code, so I don't
> > > immediately know what is going on. It would be nice to have a somewhat
> > > simpler reproducer. You use a large stap script using guru mode mixing
> > > user and kernel probes. Is all that really necessary to replicate the
> > > issue? Is the issue only triggered by the cross compiling?
> >
> > Do you mean the example in my original mail or do you refer to the much simpler example from [1]?
> 
> Maybe I am not reading the simpler example correctly.
> But it looked like it was still mixing user and kernel probes, used guru
> mode and cross compiling. Are all of those factors needed to trigger the
> bug?
> 
> If at all possible just one simple probe against libc.so to show what is
> going wrong would be ideal.

Dear Mark,
 
Please find a simplified example attached. It's not the probe itself but rather
the fact that I include unwind information for libc-2.18.so.

I also found another combination that triggers the error. If the script looks as follows

probe never
{
	print_ubacktrace();
}

probe process("/lib/libc-2.18.so").function("__libc_malloc@/opt/codesourcery/arm-none-linux-gnueabi/src/glibc/malloc/malloc.c").call
{
    printf("call\n");
}
probe process("/lib/libc-2.18.so").function("__libc_malloc@/opt/codesourcery/arm-none-linux-gnueabi/src/glibc/malloc/malloc.c").return
{
    printf("return\n");
}

I don't need the -d option.

> Thanks,
> 
> Mark

Kind Regards,
Torsten

[-- Attachment #2: prelinking.tar.bz2 --]
[-- Type: application/octet-stream, Size: 223744 bytes --]

Re: Aw: Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

On Thu, 2016-02-11 at 11:49 +0100, Torsten Polle wrote:
> > On Wed, 2016-02-10 at 21:12 +0100, Torsten Polle wrote:
> > > > Am 10.02.2016 um 17:17 schrieb Mark Wielaard <mjw@redhat.com>:
> > Maybe I am not reading the simpler example correctly.
> > But it looked like it was still mixing user and kernel probes, used guru
> > mode and cross compiling. Are all of those factors needed to trigger the
> > bug?
> > 
> > If at all possible just one simple probe against libc.so to show what is
> > going wrong would be ideal.
>  
> Please find a simplified example attached. It's not the probe itself but rather
> the fact that I include unwind information for libc-2.18.so.
> 
> I also found another combination that triggers the error. If the script looks as follows
> 
> probe never
> {
> 	print_ubacktrace();
> }
> 
> probe process("/lib/libc-2.18.so").function("__libc_malloc@/opt/codesourcery/arm-none-linux-gnueabi/src/glibc/malloc/malloc.c").call
> {
>     printf("call\n");
> }
> probe process("/lib/libc-2.18.so").function("__libc_malloc@/opt/codesourcery/arm-none-linux-gnueabi/src/glibc/malloc/malloc.c").return
> {
>     printf("return\n");
> }
> 
> I don't need the -d option.

OK. Which other options are necessary?
In particular in your cmd.sh you seem to be using -a arm -B
CROSS_COMPILE=arm-none-linux-gnueabi- and -B CONFIG_DEBUG_INFO=y
Is there a reason to use those options to reproduce the issue? I am
particularly trying to figure out if you need or are using a cross build
and if so between which architectures.

Thanks,

Mark

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

> Am 16.02.2016 um 11:08 schrieb Mark Wielaard <mjw@redhat.com>:
> 
> OK. Which other options are necessary?
> In particular in your cmd.sh you seem to be using -a arm -B
> CROSS_COMPILE=arm-none-linux-gnueabi- and -B CONFIG_DEBUG_INFO=y
> Is there a reason to use those options to reproduce the issue? I am
> particularly trying to figure out if you need or are using a cross build
> and if so between which architectures.
> 
> Thanks,
> 
> Mark

Hi Mark,

I’ve a cross compile environment.

-a arm is necessary to choose the right architecture.
-B CROSS_COMPILE=arm-none-linux-gnueabi- sets the compiler prefix for the Linux kernel compilation.

-B CONFIG_DEBUG_INFO=y is not necessary.

The target architecture is 32bit ARM. The host architecture is 64bit X86. That’s the reason why field .sec_load_offset is initialised with a 64bit wide hexadecimal number. But as the field .sec_load_offset is defined only as „unsigned long“, the compiler complains. As a work around I tried to output decimal number instead for the initialisation of .sec_load_offset. I can compile alright. But the resulting backtrace calculations shown in my previous example produce strange results.

Kind Regards,
Torsten

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

On Tue, 2016-02-16 at 21:46 +0100, Torsten Polle wrote:
> The target architecture is 32bit ARM. The host architecture is 64bit
> X86. That’s the reason why field .sec_load_offset is initialised with
> a 64bit wide hexadecimal number. But as the field .sec_load_offset is
> defined only as „unsigned long“, the compiler complains. As a work
> around I tried to output decimal number instead for the initialisation
> of .sec_load_offset. I can compile alright. But the resulting
> backtrace calculations shown in my previous example produce strange
> results.

OK, I don't have such a setup, but maybe all that is needed, if we
assume negative values are ok, is to not write sec_load_offset out in
hex, but simply in dec. Could you try the attached patch?

Thanks,

Mark

[-- Attachment #2: sec_load_offset.patch --]
[-- Type: text/x-patch, Size: 1247 bytes --]

diff --git a/translate.cxx b/translate.cxx
index f792343..1874163 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -6845,8 +6845,10 @@ dump_unwindsym_cxt (Dwfl_Module *m,
 
 	  Dwarf_Addr dwbias = 0;
 	  dwfl_module_getdwarf (m, &dwbias);
-	  c->output << ".sec_load_offset = 0x"
-		    << hex << debug_frame_off - dwbias << dec << "\n";
+	  /* Note we use dec, not hex, in case host width > target width
+	     and offset is negative.  */
+	  c->output << ".sec_load_offset = "
+		    << debug_frame_off - dwbias << "\n";
 
 	  c->output << "#else\n";
 	  c->output << ".debug_hdr = NULL,\n";
@@ -6865,8 +6867,10 @@ dump_unwindsym_cxt (Dwfl_Module *m,
               c->output << "#if defined(STP_NEED_LINE_DATA)\n";
               Dwarf_Addr dwbias = 0;
               dwfl_module_getdwarf (m, &dwbias);
-              c->output << ".sec_load_offset = 0x"
-                        << hex << debug_frame_off - dwbias << dec << "\n";
+	      /* Note we use dec, not hex, in case host width > target width
+		 and offset is negative.  */
+              c->output << ".sec_load_offset = "
+                        << debug_frame_off - dwbias << "\n";
               c->output << "#else\n";
             }
 	  c->output << ".sec_load_offset = 0\n";

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]

Am 18.02.2016 um 17:21 schrieb Mark Wielaard <mjw@redhat.com>:
> 
> On Tue, 2016-02-16 at 21:46 +0100, Torsten Polle wrote:
>> The target architecture is 32bit ARM. The host architecture is 64bit
>> X86. That’s the reason why field .sec_load_offset is initialised with
>> a 64bit wide hexadecimal number. But as the field .sec_load_offset is
>> defined only as „unsigned long“, the compiler complains. As a work
>> around I tried to output decimal number instead for the initialisation
>> of .sec_load_offset. I can compile alright. But the resulting
>> backtrace calculations shown in my previous example produce strange
>> results.
> 
> OK, I don't have such a setup, but maybe all that is needed, if we
> assume negative values are ok, is to not write sec_load_offset out in
> hex, but simply in dec. Could you try the attached patch?
> 
> Thanks,
> 
> Mark

Hi Mark,

the principle idea of your patch works. But I had to rewrite it to really work in my environment. Could you please have a look at my proposal? Would something like that be acceptable?

I would like to double check in my environment if we could fall back to the hex notation again.

Thanks,
Torsten

[-- Attachment #2: 0001-Fix-Compilation-fails-for-prelinked-libraries.patch --]
[-- Type: application/octet-stream, Size: 3013 bytes --]

From c2854c6c8400ae3658e6037d2a0bc656ee550a33 Mon Sep 17 00:00:00 2001
Message-Id: <c2854c6c8400ae3658e6037d2a0bc656ee550a33.1456177234.git.Torsten.Polle@gmx.de>
From: Torsten Polle <Torsten.Polle@gmx.de>
Date: Mon, 22 Feb 2016 22:39:59 +0100
Subject: [PATCH] Fix: Compilation fails for prelinked libraries.

The offset to the section ".text" in a prelinked library might increase due to
changed relocations that need more space. If the prelinked library contains a
debug link and the linked file is not prelinked, the offset to the ".text"
section is different. If the difference between the offset of the ".text"
section in the prelinked library and non-prelinked library (with e.g. debug
information) becomes negative, the representation of this negative number has
to fit into an "unsigned int". If the width of the host is larger than the
width of the target, the compilation for the target fails.

Signed-off-by: Torsten Polle <Torsten.Polle@gmx.de>
---
 translate.cxx |   34 ++++++++++++++++++++++++++++++----
 1 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/translate.cxx b/translate.cxx
index f792343..f0364e9 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -6844,9 +6844,22 @@ dump_unwindsym_cxt (Dwfl_Module *m,
           c->output << ".debug_hdr_len = " << debug_frame_hdr_len << ", \n";
 
 	  Dwarf_Addr dwbias = 0;
+	  Dwarf_Addr elf_bias;
+	  GElf_Ehdr *ehdr, ehdr_mem;
+	  Elf *elf;
+	  elf = dwfl_module_getelf(m, &elf_bias);
+	  ehdr = gelf_getehdr(elf, &ehdr_mem);
 	  dwfl_module_getdwarf (m, &dwbias);
-	  c->output << ".sec_load_offset = 0x"
-		    << hex << debug_frame_off - dwbias << dec << "\n";
+	  if (ehdr->e_ident[EI_CLASS] == ELFCLASS32)
+	    {
+	      c->output << ".sec_load_offset = "
+			<< (uint32_t)debug_frame_off - (uint32_t)dwbias << "u\n";
+	    }
+	  else
+	    {
+	      c->output << ".sec_load_offset = 0x"
+			<< hex << debug_frame_off - dwbias << dec << "\n";
+	    }
 
 	  c->output << "#else\n";
 	  c->output << ".debug_hdr = NULL,\n";
@@ -6864,9 +6877,22 @@ dump_unwindsym_cxt (Dwfl_Module *m,
             {
               c->output << "#if defined(STP_NEED_LINE_DATA)\n";
               Dwarf_Addr dwbias = 0;
+	      Dwarf_Addr elf_bias;
+	      GElf_Ehdr *ehdr, ehdr_mem;
+	      Elf *elf;
+	      elf = dwfl_module_getelf(m, &elf_bias);
+	      ehdr = gelf_getehdr(elf, &ehdr_mem);
               dwfl_module_getdwarf (m, &dwbias);
-              c->output << ".sec_load_offset = 0x"
-                        << hex << debug_frame_off - dwbias << dec << "\n";
+	      if (ehdr->e_ident[EI_CLASS] == ELFCLASS32)
+		{
+		  c->output << ".sec_load_offset = "
+			    << (uint32_t)debug_frame_off - (uint32_t)dwbias << "u\n";
+		}
+	      else
+		{
+		  c->output << ".sec_load_offset = 0x"
+			    << hex << debug_frame_off - dwbias << dec << "\n";
+		}
               c->output << "#else\n";
             }
 	  c->output << ".sec_load_offset = 0\n";
-- 
1.7.4.1

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

On Mon, 2016-02-22 at 22:45 +0100, Torsten Polle wrote:
> the principle idea of your patch works. But I had to rewrite it to
> really work in my environment. Could you please have a look at my
> proposal? Would something like that be acceptable?

Could you give an example of what didn't work?
You cast both debug_frame_off and dwbias to uint32_t before
substracting. Is that really necessary/correct? Both are Dwarf_Addrs
which are always uint64_t. And we care about the difference here.

> I would like to double check in my environment if we could fall back to the hex notation again.

Isn't it simpler and consistent to just always use decimal in this case?
There should at least be a comment why we use a different notation for
elf32 vs elf64 targets.

Thanks,

Mark

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

> Am 23.02.2016 um 17:46 schrieb Mark Wielaard <mjw@redhat.com>:
> 
> On Mon, 2016-02-22 at 22:45 +0100, Torsten Polle wrote:
>> the principle idea of your patch works. But I had to rewrite it to
>> really work in my environment. Could you please have a look at my
>> proposal? Would something like that be acceptable?
> 
> Could you give an example of what didn't work?
> You cast both debug_frame_off and dwbias to uint32_t before
> substracting. Is that really necessary/correct? Both are Dwarf_Addrs
> which are always uint64_t. And we care about the difference here.

In my case debug_frame_off is larger than dwbias. If for instance debug_frame_off is 0 and dwbias is -1, the result is -1. As the arithmethics on the 64bit host is unsigned, the result is 0xffffffffffffffff in hex notation and 18446744073709551615 in decimal notation. As stap-symbols.h is used for the target, the values are too large for the 32bit wide unsigned int, which is the data type for the field sec_load_offset. Therefore the compiler complains. The type cast to uint32_t forces the host compiler to use only a 32bit value. Therefore the result is 0xffffffff (4294967295). At least that is what happens on my host. I need the „u“ because even 4294967295 is considered to be too large.

>> I would like to double check in my environment if we could fall back to the hex notation again.
> 
> Isn't it simpler and consistent to just always use decimal in this case?
> There should at least be a comment why we use a different notation for
> elf32 vs elf64 targets.

I would go for a consistent notation. Personally, I prefer the hex notation for the addresses. I hope to be able to check whether the hex notation with casts works tomorrow and get back to you.

Regards,
Torsten

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

[-- Attachment #1: Type: text/plain, Size: 2049 bytes --]


> Am 23.02.2016 um 23:16 schrieb Torsten Polle <Torsten.Polle@gmx.de>:
> 
> 
>> Am 23.02.2016 um 17:46 schrieb Mark Wielaard <mjw@redhat.com>:
>> 
>> On Mon, 2016-02-22 at 22:45 +0100, Torsten Polle wrote:
>>> the principle idea of your patch works. But I had to rewrite it to
>>> really work in my environment. Could you please have a look at my
>>> proposal? Would something like that be acceptable?
>> 
>> Could you give an example of what didn't work?
>> You cast both debug_frame_off and dwbias to uint32_t before
>> substracting. Is that really necessary/correct? Both are Dwarf_Addrs
>> which are always uint64_t. And we care about the difference here.
> 
> In my case debug_frame_off is larger than dwbias. If for instance debug_frame_off is 0 and dwbias is -1, the result is -1. As the arithmethics on the 64bit host is unsigned, the result is 0xffffffffffffffff in hex notation and 18446744073709551615 in decimal notation. As stap-symbols.h is used for the target, the values are too large for the 32bit wide unsigned int, which is the data type for the field sec_load_offset. Therefore the compiler complains. The type cast to uint32_t forces the host compiler to use only a 32bit value. Therefore the result is 0xffffffff (4294967295). At least that is what happens on my host. I need the „u“ because even 4294967295 is considered to be too large.
> 
>>> I would like to double check in my environment if we could fall back to the hex notation again.
>> 
>> Isn't it simpler and consistent to just always use decimal in this case?
>> There should at least be a comment why we use a different notation for
>> elf32 vs elf64 targets.
> 
> I would go for a consistent notation. Personally, I prefer the hex notation for the addresses. I hope to be able to check whether the hex notation with casts works tomorrow and get back to you.
> 
> Regards,
> Torsten

Mark,

Please find a new version of the patch attached. I changed the patch to use hexadecimal numbers consistently.

Kind Regards,
Torsten

[-- Attachment #2: 0001-PATCH-v2-Fix-Compilation-fails-for-prelinked-librari.patch --]
[-- Type: application/octet-stream, Size: 3100 bytes --]

From 215d886c44bc4e2411082cb7636d12284e334897 Mon Sep 17 00:00:00 2001
Message-Id: <215d886c44bc4e2411082cb7636d12284e334897.1456692346.git.Torsten.Polle@gmx.de>
From: Torsten Polle <Torsten.Polle@gmx.de>
Date: Sun, 28 Feb 2016 21:45:33 +0100
Subject: [PATCH] [PATCH v2] Fix: Compilation fails for prelinked libraries.

The offset to the section ".text" in a prelinked library might increase due to
changed relocations that need more space. If the prelinked library contains a
debug link and the linked file is not prelinked, the offset to the ".text"
section is different. If the difference between the offset of the ".text"
section in the prelinked library and non-prelinked library (with e.g. debug
information) becomes negative, the representation of this negative number has
to fit into an uint32_t. If the width of the host is larger than the
width of the target, the (cross-) compilation for the target fails.

Changes since v1:
* Use hex values for the 32bit case.

Signed-off-by: Torsten Polle <Torsten.Polle@gmx.de>
---
 translate.cxx |   34 ++++++++++++++++++++++++++++++----
 1 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/translate.cxx b/translate.cxx
index f792343..56d23e8 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -6844,9 +6844,22 @@ dump_unwindsym_cxt (Dwfl_Module *m,
           c->output << ".debug_hdr_len = " << debug_frame_hdr_len << ", \n";
 
 	  Dwarf_Addr dwbias = 0;
+	  Dwarf_Addr elf_bias;
+	  GElf_Ehdr *ehdr, ehdr_mem;
+	  Elf *elf;
+	  elf = dwfl_module_getelf(m, &elf_bias);
+	  ehdr = gelf_getehdr(elf, &ehdr_mem);
 	  dwfl_module_getdwarf (m, &dwbias);
-	  c->output << ".sec_load_offset = 0x"
-		    << hex << debug_frame_off - dwbias << dec << "\n";
+	  if (ehdr->e_ident[EI_CLASS] == ELFCLASS32)
+	    {
+	      c->output << ".sec_load_offset = 0x"
+			<< hex << (uint32_t)(debug_frame_off - dwbias) << dec << "\n";
+	    }
+	  else
+	    {
+	      c->output << ".sec_load_offset = 0x"
+			<< hex << debug_frame_off - dwbias << dec << "\n";
+	    }
 
 	  c->output << "#else\n";
 	  c->output << ".debug_hdr = NULL,\n";
@@ -6864,9 +6877,22 @@ dump_unwindsym_cxt (Dwfl_Module *m,
             {
               c->output << "#if defined(STP_NEED_LINE_DATA)\n";
               Dwarf_Addr dwbias = 0;
+	      Dwarf_Addr elf_bias;
+	      GElf_Ehdr *ehdr, ehdr_mem;
+	      Elf *elf;
+	      elf = dwfl_module_getelf(m, &elf_bias);
+	      ehdr = gelf_getehdr(elf, &ehdr_mem);
               dwfl_module_getdwarf (m, &dwbias);
-              c->output << ".sec_load_offset = 0x"
-                        << hex << debug_frame_off - dwbias << dec << "\n";
+	      if (ehdr->e_ident[EI_CLASS] == ELFCLASS32)
+		{
+		  c->output << ".sec_load_offset = 0x"
+			    << hex << (uint32_t)(debug_frame_off - dwbias) << dec << "\n";
+		}
+	      else
+		{
+		  c->output << ".sec_load_offset = 0x"
+			    << hex << debug_frame_off - dwbias << dec << "\n";
+		}
               c->output << "#else\n";
             }
 	  c->output << ".sec_load_offset = 0\n";
-- 
1.7.4.1

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

> Am 28.02.2016 um 21:51 schrieb Torsten Polle <Torsten.Polle@gmx.de>:
> 
> 
>> Am 23.02.2016 um 23:16 schrieb Torsten Polle <Torsten.Polle@gmx.de>:
>> 
>> 
>>> Am 23.02.2016 um 17:46 schrieb Mark Wielaard <mjw@redhat.com>:
>>> 
>>> On Mon, 2016-02-22 at 22:45 +0100, Torsten Polle wrote:
>>>> the principle idea of your patch works. But I had to rewrite it to
>>>> really work in my environment. Could you please have a look at my
>>>> proposal? Would something like that be acceptable?
>>> 
>>> Could you give an example of what didn't work?
>>> You cast both debug_frame_off and dwbias to uint32_t before
>>> substracting. Is that really necessary/correct? Both are Dwarf_Addrs
>>> which are always uint64_t. And we care about the difference here.
>> 
>> In my case debug_frame_off is larger than dwbias. If for instance debug_frame_off is 0 and dwbias is -1, the result is -1. As the arithmethics on the 64bit host is unsigned, the result is 0xffffffffffffffff in hex notation and 18446744073709551615 in decimal notation. As stap-symbols.h is used for the target, the values are too large for the 32bit wide unsigned int, which is the data type for the field sec_load_offset. Therefore the compiler complains. The type cast to uint32_t forces the host compiler to use only a 32bit value. Therefore the result is 0xffffffff (4294967295). At least that is what happens on my host. I need the „u“ because even 4294967295 is considered to be too large.
>> 
>>>> I would like to double check in my environment if we could fall back to the hex notation again.
>>> 
>>> Isn't it simpler and consistent to just always use decimal in this case?
>>> There should at least be a comment why we use a different notation for
>>> elf32 vs elf64 targets.
>> 
>> I would go for a consistent notation. Personally, I prefer the hex notation for the addresses. I hope to be able to check whether the hex notation with casts works tomorrow and get back to you.
>> 
>> Regards,
>> Torsten
> 
> Mark,
> 
> Please find a new version of the patch attached. I changed the patch to use hexadecimal numbers consistently.
> 
> Kind Regards,
> Torsten
> <0001-PATCH-v2-Fix-Compilation-fails-for-prelinked-librari.patch>

Mark,

any comments or thoughts on my proposal?

Kind Regards,
Torsten

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

Hi Torsten,

On Wed, 2016-03-30 at 22:05 +0200, Torsten Polle wrote:
> > Am 28.02.2016 um 21:51 schrieb Torsten Polle <Torsten.Polle@gmx.de>: 
> >> Am 23.02.2016 um 23:16 schrieb Torsten Polle <Torsten.Polle@gmx.de>:
> >>> Am 23.02.2016 um 17:46 schrieb Mark Wielaard <mjw@redhat.com>:
> >>> 
> >>> On Mon, 2016-02-22 at 22:45 +0100, Torsten Polle wrote:
> >>>> the principle idea of your patch works. But I had to rewrite it to
> >>>> really work in my environment. Could you please have a look at my
> >>>> proposal? Would something like that be acceptable?
> >>> 
> >>> Could you give an example of what didn't work?
> >>> You cast both debug_frame_off and dwbias to uint32_t before
> >>> substracting. Is that really necessary/correct? Both are Dwarf_Addrs
> >>> which are always uint64_t. And we care about the difference here.
> >> 
> >> In my case debug_frame_off is larger than dwbias. If for instance debug_frame_off is 0 and dwbias is -1, the result is -1. As the arithmethics on the 64bit host is unsigned, the result is 0xffffffffffffffff in hex notation and 18446744073709551615 in decimal notation. As stap-symbols.h is used for the target, the values are too large for the 32bit wide unsigned int, which is the data type for the field sec_load_offset. Therefore the compiler complains. The type cast to uint32_t forces the host compiler to use only a 32bit value. Therefore the result is 0xffffffff (4294967295). At least that is what happens on my host. I need the „u“ because even 4294967295 is considered to be too large.
> >> 
> >>>> I would like to double check in my environment if we could fall back to the hex notation again.
> >>> 
> >>> Isn't it simpler and consistent to just always use decimal in this case?
> >>> There should at least be a comment why we use a different notation for
> >>> elf32 vs elf64 targets.
> >> 
> >> I would go for a consistent notation. Personally, I prefer the hex notation for the addresses. I hope to be able to check whether the hex notation with casts works tomorrow and get back to you.
>
> > Please find a new version of the patch attached. I changed the patch to use hexadecimal numbers consistently.
> > 
> > Kind Regards,
> > Torsten
> > <0001-PATCH-v2-Fix-Compilation-fails-for-prelinked-librari.patch>
>
> any comments or thoughts on my proposal?

Sorry I had forgotten about this issue.
In your example you have a debug_frame_off with zero and a dwbias of -1.
That looks somewhat suspicious. Are we sure that isn't a special case?

A debug_frame_off of zero seems to indicate that there isn't a
debug_frame in the first place. dwbias being -1 might indicate that
dwfl_module_getdwarf () failed. We don't seem to check whether the call
succeeds.

Could you try and see if any of the above is the case in your situation?

Thanks,

Mark

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

> Am 01.04.2016 um 15:07 schrieb Mark Wielaard <mjw@redhat.com>:
> 
> Hi Torsten,
> 
> On Wed, 2016-03-30 at 22:05 +0200, Torsten Polle wrote:
>>> Am 28.02.2016 um 21:51 schrieb Torsten Polle <Torsten.Polle@gmx.de>: 
>>>> Am 23.02.2016 um 23:16 schrieb Torsten Polle <Torsten.Polle@gmx.de>:
>>>>> Am 23.02.2016 um 17:46 schrieb Mark Wielaard <mjw@redhat.com>:
>>>>> 
>>>>> On Mon, 2016-02-22 at 22:45 +0100, Torsten Polle wrote:
>>>>>> the principle idea of your patch works. But I had to rewrite it to
>>>>>> really work in my environment. Could you please have a look at my
>>>>>> proposal? Would something like that be acceptable?
>>>>> 
>>>>> Could you give an example of what didn't work?
>>>>> You cast both debug_frame_off and dwbias to uint32_t before
>>>>> substracting. Is that really necessary/correct? Both are Dwarf_Addrs
>>>>> which are always uint64_t. And we care about the difference here.
>>>> 
>>>> In my case debug_frame_off is larger than dwbias. If for instance debug_frame_off is 0 and dwbias is -1, the result is -1. As the arithmethics on the 64bit host is unsigned, the result is 0xffffffffffffffff in hex notation and 18446744073709551615 in decimal notation. As stap-symbols.h is used for the target, the values are too large for the 32bit wide unsigned int, which is the data type for the field sec_load_offset. Therefore the compiler complains. The type cast to uint32_t forces the host compiler to use only a 32bit value. Therefore the result is 0xffffffff (4294967295). At least that is what happens on my host. I need the „u“ because even 4294967295 is considered to be too large.
>>>> 
>>>>>> I would like to double check in my environment if we could fall back to the hex notation again.
>>>>> 
>>>>> Isn't it simpler and consistent to just always use decimal in this case?
>>>>> There should at least be a comment why we use a different notation for
>>>>> elf32 vs elf64 targets.
>>>> 
>>>> I would go for a consistent notation. Personally, I prefer the hex notation for the addresses. I hope to be able to check whether the hex notation with casts works tomorrow and get back to you.
>> 
>>> Please find a new version of the patch attached. I changed the patch to use hexadecimal numbers consistently.
>>> 
>>> Kind Regards,
>>> Torsten
>>> <0001-PATCH-v2-Fix-Compilation-fails-for-prelinked-librari.patch>
>> 
>> any comments or thoughts on my proposal?
> 
> Sorry I had forgotten about this issue.
> In your example you have a debug_frame_off with zero and a dwbias of -1.
> That looks somewhat suspicious. Are we sure that isn't a special case?

No, this is not a special case. It’s just a simplified example to illustrate the calculations. ;-)

In my initial mail [1], you can see the original values and calculations.

> The offending line in stap-symbols.h looks as follows:
> .sec_load_offset = 0xffffffffffffebc0 /* 4dec8000 4dec9440 */

The values in the comment are the result of the following instrumentation.

>                  Dwarf_Addr dwbias = 0;
>                  dwfl_module_getdwarf (m, &dwbias);
>                  c->output << ".sec_load_offset = 0x"
>                                    << hex << debug_frame_off - dwbias << dec
>                                    << "/* "  << hex << debug_frame_off << " " << dwbias << dec << "*/"
>                                    << "\n";

This means the actual values are 
debug_frame_off = 0x4dec8000
dwbias          = 0x4dec9440

> The offset of the .text section in the non-prelinked library is 0x15c40. 
> The offset of the .text section in the prelinked library is 0x4decf080 - 0xdeb8000 = 0x17080.
> 
> This means that .sec_load_offset is exactly the difference 0x15c40 - 0x17080 ~ 0xffffffffffffebc0.

You can find the results from a very simple reproduction in [2].

Thanks,
Torsten

[1] https://sourceware.org/ml/systemtap/2015-q4/msg00171.html
[2] https://sourceware.org/ml/systemtap/2015-q4/msg00219.html

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 6679 bytes --]

Hi Torsten,

Sorry this is taking so long. I don't believe I fully understand the
issue. Because I don't immediately know what the right answer is I put
it away for a bit. Then it takes time again to get the full context
(which I believe I still don't fully have).

On Fri, 2016-04-01 at 23:18 +0200, Torsten Polle wrote:
> > Am 01.04.2016 um 15:07 schrieb Mark Wielaard <mjw@redhat.com>:
> > On Wed, 2016-03-30 at 22:05 +0200, Torsten Polle wrote:
> >>> Am 28.02.2016 um 21:51 schrieb Torsten Polle <Torsten.Polle@gmx.de>: 
> >>>> Am 23.02.2016 um 23:16 schrieb Torsten Polle <Torsten.Polle@gmx.de>:
> >>>>> Am 23.02.2016 um 17:46 schrieb Mark Wielaard <mjw@redhat.com>:
> >>>>> 
> >>>>> On Mon, 2016-02-22 at 22:45 +0100, Torsten Polle wrote:
> >>>>>> the principle idea of your patch works. But I had to rewrite it to
> >>>>>> really work in my environment. Could you please have a look at my
> >>>>>> proposal? Would something like that be acceptable?
> >>>>> 
> >>>>> Could you give an example of what didn't work?
> >>>>> You cast both debug_frame_off and dwbias to uint32_t before
> >>>>> substracting. Is that really necessary/correct? Both are Dwarf_Addrs
> >>>>> which are always uint64_t. And we care about the difference here.
> >>>> 
> >>>> In my case debug_frame_off is larger than dwbias. If for instance debug_frame_off is 0 and dwbias is -1, the result is -1. As the arithmethics on the 64bit host is unsigned, the result is 0xffffffffffffffff in hex notation and 18446744073709551615 in decimal notation. As stap-symbols.h is used for the target, the values are too large for the 32bit wide unsigned int, which is the data type for the field sec_load_offset. Therefore the compiler complains. The type cast to uint32_t forces the host compiler to use only a 32bit value. Therefore the result is 0xffffffff (4294967295). At least that is what happens on my host. I need the „u“ because even 4294967295 is considered to be too large.
> >>>> 
> >>>>>> I would like to double check in my environment if we could fall back to the hex notation again.
> >>>>> 
> >>>>> Isn't it simpler and consistent to just always use decimal in this case?
> >>>>> There should at least be a comment why we use a different notation for
> >>>>> elf32 vs elf64 targets.
> >>>> 
> >>>> I would go for a consistent notation. Personally, I prefer the hex notation for the addresses. I hope to be able to check whether the hex notation with casts works tomorrow and get back to you.
> >> 
> >>> Please find a new version of the patch attached. I changed the patch to use hexadecimal numbers consistently.
> >>> 
> >>> Kind Regards,
> >>> Torsten
> >>> <0001-PATCH-v2-Fix-Compilation-fails-for-prelinked-librari.patch>
> >> 
> >> any comments or thoughts on my proposal?
> > 
> > Sorry I had forgotten about this issue.
> > In your example you have a debug_frame_off with zero and a dwbias of -1.
> > That looks somewhat suspicious. Are we sure that isn't a special case?
> 
> No, this is not a special case. It’s just a simplified example to illustrate the calculations. ;-)
> 
> In my initial mail [1], you can see the original values and calculations.
> 
> > The offending line in stap-symbols.h looks as follows:
> > .sec_load_offset = 0xffffffffffffebc0 /* 4dec8000 4dec9440 */
> 
> The values in the comment are the result of the following instrumentation.
> 
> >                  Dwarf_Addr dwbias = 0;
> >                  dwfl_module_getdwarf (m, &dwbias);
> >                  c->output << ".sec_load_offset = 0x"
> >                                    << hex << debug_frame_off - dwbias << dec
> >                                    << "/* "  << hex << debug_frame_off << " " << dwbias << dec << "*/"
> >                                    << "\n";
> 
> This means the actual values are 
> debug_frame_off = 0x4dec8000
> dwbias          = 0x4dec9440
> 
> > The offset of the .text section in the non-prelinked library is 0x15c40. 
> > The offset of the .text section in the prelinked library is 0x4decf080 - 0xdeb8000 = 0x17080.
> > 
> > This means that .sec_load_offset is exactly the difference 0x15c40 - 0x17080 ~ 0xffffffffffffebc0.

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. But you prefer to use the hex representation instead?

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;

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

[-- Attachment #2: kernel_module_sec_load_offset.patch --]
[-- Type: text/x-patch, Size: 949 bytes --]

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";

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

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

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

On Wed, 2016-04-06 at 22:44 +0200, Torsten Polle wrote:
> > Am 05.04.2016 um 15:44 schrieb Mark Wielaard <mjw@redhat.com>:
> > 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.

This code is certainly confusing. And arm32 might have different
defaults from all other architectures (which normally have .eh_frames
instead of just .debug_frames, on arm32 unwinding is often done through
EXIDX - exception index tables - instead, but stap doesn't support
those). So it might indeed be that in your case the non-ehframe path is
taken. But I believe that even in that sec_load_offset should be zero.
So it might impact unwinding through user space shared libraries. But it
should not impact setting probe points in shared libraries.

On my own setup (x86_64 native) the patch showed no regressions with
make installcheck. I would be interesting to hear of any test results
you get with my patch.

Thanks,

Mark

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

Hi Mark,

> Am 06.04.2016 um 23:56 schrieb Mark Wielaard <mjw@redhat.com>:
> 
> On Wed, 2016-04-06 at 22:44 +0200, Torsten Polle wrote:
>>> Am 05.04.2016 um 15:44 schrieb Mark Wielaard <mjw@redhat.com>:
>>> 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.
> 
> This code is certainly confusing. And arm32 might have different
> defaults from all other architectures (which normally have .eh_frames
> instead of just .debug_frames, on arm32 unwinding is often done through
> EXIDX - exception index tables - instead, but stap doesn't support
> those). So it might indeed be that in your case the non-ehframe path is
> taken. But I believe that even in that sec_load_offset should be zero.

I’ve checked your patch. As a result the backtrace calculations break in my environment.

I’ve therefore instrumented adjustStartLoc() as follows:

  if (is_ehframe) {
    printk(KERN_ERR "eh: s=%lu, v=%lu, l=%lu\n", startLoc, vm_addr, s->sec_load_offset);
    return startLoc + vm_addr;
  }
  else {
    printk(KERN_ERR "no eh: s=%lu, v=%lu, l=%lu\n", startLoc, vm_addr, s->sec_load_offset);
    return startLoc + vm_addr - s->sec_load_offset;
  }

As a result I get the following output:

[ 1215.876537] no eh: s=297684, v=1307279360, l=4294962112
[ 1215.876542] no eh: s=297532, v=1307279360, l=4294962112
[ 1215.876546] no eh: s=297568, v=1307279360, l=4294962112
[ 1215.876551] no eh: s=297612, v=1307279360, l=4294962112
[ 1215.876555] no eh: s=297612, v=1307279360, l=4294962112
[ 1215.876560] no eh: s=297612, v=1307279360, l=4294962112

This means that the non-ehframe path is taken and that sec_load_offset is non-zero.

> So it might impact unwinding through user space shared libraries. But it
> should not impact setting probe points in shared libraries.
> 
> On my own setup (x86_64 native) the patch showed no regressions with
> make installcheck. I would be interesting to hear of any test results
> you get with my patch.
> 
> Thanks,
> 
> Mark

Regards,
Torsten

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

On Mon, Apr 11, 2016 at 08:47:00PM +0200, Torsten Polle wrote:
> I’ve checked your patch. As a result the backtrace calculations
> break in my environment.

I assume this is an in-kernel backtrace, does it involve kernel modules?
Or is it a user backtrace, executable only? shared libraries?
Could you show a probe script and example backtrace?

Please do include some verbose output. If you can provide the output
of stap -DDEBUG_UNWIND=1 that might be helpful.

> I’ve therefore instrumented adjustStartLoc() as follows:
> 
>   if (is_ehframe) {
>     printk(KERN_ERR "eh: s=%lu, v=%lu, l=%lu\n", startLoc, vm_addr, s->sec_load_offset);
>     return startLoc + vm_addr;
>   }
>   else {
>     printk(KERN_ERR "no eh: s=%lu, v=%lu, l=%lu\n", startLoc, vm_addr, s->sec_load_offset);
>     return startLoc + vm_addr - s->sec_load_offset;
>   }
> 
> As a result I get the following output:
> 
> [ 1215.876537] no eh: s=297684, v=1307279360, l=4294962112
> [ 1215.876542] no eh: s=297532, v=1307279360, l=4294962112
> [ 1215.876546] no eh: s=297568, v=1307279360, l=4294962112
> [ 1215.876551] no eh: s=297612, v=1307279360, l=4294962112
> [ 1215.876555] no eh: s=297612, v=1307279360, l=4294962112
> [ 1215.876560] no eh: s=297612, v=1307279360, l=4294962112
> 
> This means that the non-ehframe path is taken and that sec_load_offset
> is non-zero.

And that calculation wraps around because it is all unsigned.
I admit I don't understand what this calculation represents.

Cheers,

Mark

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

Hi Mark,

> Am 11.04.2016 um 23:01 schrieb Mark Wielaard <mjw@redhat.com>:
> 
> On Mon, Apr 11, 2016 at 08:47:00PM +0200, Torsten Polle wrote:
>> I’ve checked your patch. As a result the backtrace calculations
>> break in my environment.
> 
> I assume this is an in-kernel backtrace, does it involve kernel modules?
> Or is it a user backtrace, executable only? shared libraries?
> Could you show a probe script and example backtrace?

in principle, I’m talking about the example in [1]. In short, I’m talking about getting a proper backtrace in the prelinked library libc-2.18.so. I’m saying „in principle“ because the example does not set a real probe, but only makes SystemTap include the necessary unwinding information for libc-2.18.so.

The real (actually already stripped down) script can be found in [2].

In the script taptrek_run_izA4.stp in line 240, I’m using the following statement
	uaddr = __ustack_raw(depth);
to get backtrace information. Please don’t get distracted by the usage of the function __ustack_raw(). The function print_ubacktrace() would provide a similar result.

> Please do include some verbose output. If you can provide the output
> of stap -DDEBUG_UNWIND=1 that might be helpful.

I’ll try my best and come back to you. I would provide the information with my patch applied, because otherwise the result is simply garbage.

> I’ve therefore instrumented adjustStartLoc() as follows:
>> 
>>  if (is_ehframe) {
>>    printk(KERN_ERR "eh: s=%lu, v=%lu, l=%lu\n", startLoc, vm_addr, s->sec_load_offset);
>>    return startLoc + vm_addr;
>>  }
>>  else {
>>    printk(KERN_ERR "no eh: s=%lu, v=%lu, l=%lu\n", startLoc, vm_addr, s->sec_load_offset);
>>    return startLoc + vm_addr - s->sec_load_offset;
>>  }
>> 
>> As a result I get the following output:
>> 
>> [ 1215.876537] no eh: s=297684, v=1307279360, l=4294962112
>> [ 1215.876542] no eh: s=297532, v=1307279360, l=4294962112
>> [ 1215.876546] no eh: s=297568, v=1307279360, l=4294962112
>> [ 1215.876551] no eh: s=297612, v=1307279360, l=4294962112
>> [ 1215.876555] no eh: s=297612, v=1307279360, l=4294962112
>> [ 1215.876560] no eh: s=297612, v=1307279360, l=4294962112
>> 
>> This means that the non-ehframe path is taken and that sec_load_offset
>> is non-zero.
> 
> And that calculation wraps around because it is all unsigned.
> I admit I don't understand what this calculation represents.

s=297612     =    0x48A8C is a (relative) address in the .text section of libc-2.18.so
v=1307279360 = 0x4DEB8000 is the start address of the loaded libc-2.18.so
l=4294962112 = 0xFFFFEBC0 is the „negative“ sec_load_offset, which accommodates for the offset between the .text section in the prelinked library and the .text section in the non-prelinked file with the debug information.

As a result the location 0x4DF00A8C = 0x4DEB8000 + 0x48A8C in the prelinked file is converted into a location 0x4DEFF64C = 0x4DEB8000 + 0x48A8C  - 0x1440 in the file with debug information, which can be used .

prelinked:                  debug information: 
+----------------------+    +----------------------+
|         ...          |    |         ...          |
+----------------------+    +----------------------+
| .rel.dyn size=0x3c18 |    | .rel.dyn size=0x2810 |
+----------------------+    +----------------------+
|         ...          |    |         ...          |
+----------------------+    +----------------------+
| .text                |    | .text                |
| 0x4DF00A8C           | -> | 0x4DEFF64C           |
+----------------------+    +----------------------+

> Cheers,
> Mark

Regards,
Torsten

[1] https://sourceware.org/ml/systemtap/2015-q4/msg00219.html
[2] https://sourceware.org/ml/systemtap/2015-q4/msg00184.html

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

Hi Torsten,

On Tue, 2016-04-12 at 22:26 +0200, Torsten Polle wrote:
> > Am 11.04.2016 um 23:01 schrieb Mark Wielaard <mjw@redhat.com>:
> > 
> > On Mon, Apr 11, 2016 at 08:47:00PM +0200, Torsten Polle wrote:
> >> I’ve checked your patch. As a result the backtrace calculations
> >> break in my environment.
> > 
> > I assume this is an in-kernel backtrace, does it involve kernel
> modules?
> > Or is it a user backtrace, executable only? shared libraries?
> > Could you show a probe script and example backtrace?
> 
> in principle, I’m talking about the example in [1]. In short, I’m
> talking about getting a proper backtrace in the prelinked library
> libc-2.18.so. I’m saying „in principle“ because the example does not
> set a real probe, but only makes SystemTap include the necessary
> unwinding information for libc-2.18.so.
> 
> The real (actually already stripped down) script can be found in [2].
> 
> In the script taptrek_run_izA4.stp in line 240, I’m using the
> following statement
> 	uaddr = __ustack_raw(depth);
> to get backtrace information. Please don’t get distracted by the usage
> of the function __ustack_raw(). The function print_ubacktrace() would
> provide a similar result.

I am not sure why the example needs to be so complicated.
The usage of mixed user/kernel space probes/backtraces makes it really
hard to understand what is going on. Can't you show what you get with a
simple probe inside libc.so with a print_ubacktrace?

> > Please do include some verbose output. If you can provide the output
> > of stap -DDEBUG_UNWIND=1 that might be helpful.
> 
> I’ll try my best and come back to you. I would provide the information with my patch applied, because otherwise the result is simply garbage.

Please do include the exact patch you are using.

> > I’ve therefore instrumented adjustStartLoc() as follows:
> >> 
> >>  if (is_ehframe) {
> >>    printk(KERN_ERR "eh: s=%lu, v=%lu, l=%lu\n", startLoc, vm_addr, s->sec_load_offset);
> >>    return startLoc + vm_addr;
> >>  }
> >>  else {
> >>    printk(KERN_ERR "no eh: s=%lu, v=%lu, l=%lu\n", startLoc, vm_addr, s->sec_load_offset);
> >>    return startLoc + vm_addr - s->sec_load_offset;
> >>  }
> >> 
> >> As a result I get the following output:
> >> 
> >> [ 1215.876537] no eh: s=297684, v=1307279360, l=4294962112
> >> [ 1215.876542] no eh: s=297532, v=1307279360, l=4294962112
> >> [ 1215.876546] no eh: s=297568, v=1307279360, l=4294962112
> >> [ 1215.876551] no eh: s=297612, v=1307279360, l=4294962112
> >> [ 1215.876555] no eh: s=297612, v=1307279360, l=4294962112
> >> [ 1215.876560] no eh: s=297612, v=1307279360, l=4294962112
> >> 
> >> This means that the non-ehframe path is taken and that sec_load_offset
> >> is non-zero.
> > 
> > And that calculation wraps around because it is all unsigned.
> > I admit I don't understand what this calculation represents.
> 
> s=297612     =    0x48A8C is a (relative) address in the .text section of libc-2.18.so
> v=1307279360 = 0x4DEB8000 is the start address of the loaded libc-2.18.so
> l=4294962112 = 0xFFFFEBC0 is the „negative“ sec_load_offset, which accommodates for the offset between the .text section in the prelinked library and the .text section in the non-prelinked file with the debug information.
> 
> As a result the location 0x4DF00A8C = 0x4DEB8000 + 0x48A8C in the prelinked file is converted into a location 0x4DEFF64C = 0x4DEB8000 + 0x48A8C  - 0x1440 in the file with debug information, which can be used .
> 
> prelinked:                  debug information: 
> +----------------------+    +----------------------+
> |         ...          |    |         ...          |
> +----------------------+    +----------------------+
> | .rel.dyn size=0x3c18 |    | .rel.dyn size=0x2810 |
> +----------------------+    +----------------------+
> |         ...          |    |         ...          |
> +----------------------+    +----------------------+
> | .text                |    | .text                |
> | 0x4DF00A8C           | -> | 0x4DEFF64C           |
> +----------------------+    +----------------------+

Thanks, that is interesting.
But it is also wrong that we use section offsets in user space modules.
The sections really shouldn't play a role, except for kernel modules.
User space modules are mapped into memory according to their segments
(phdrs). Could you show the segments  plus section to segment mapping of
the libc-2.18.so and libc-2.18.so.debug files used with eu-readelf -Sl

Thanks,

Mark

Prelinking on ARM with Debug Link

[Torsten Polle]

Hi Mark,

I reported an issue with prelinking before. While I was able to solve my issue with your help before, I got stuck with my current issue. As you might remember, I’m using a cross compile environment, i.e. my host is a X86 64bit machine, while my target is a 32bit ARM machine (Freescale i.MX6).

Here I have a problem to compile my probes when I want to include unwind information.

The error message is as follow:

In file included from /tmp/stap/taptrek_src.c:6010:0:
/tmp/stap/stap-symbol.h:95831:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
 },
 ^

The offending line in stap-symbols.h looks as follows:
.sec_load_offset = 0xffffffffffffebc0 /* 4dec8000 4dec9440*/

The line includes debug output, because I checked the code in translate.cxx that generates this output and wanted to understand what is going wrong.

                  Dwarf_Addr dwbias = 0;
                  dwfl_module_getdwarf (m, &dwbias);
                  c->output << ".sec_load_offset = 0x"
                                    << hex << debug_frame_off - dwbias << dec
                                    << "/* "  << hex << debug_frame_off << " " << dwbias << dec << "*/"
                                    << "\n";

As I use a 64bit host, the hex value is given as 64bit long hex value. Additional the value is very large, because the value is actually the difference between debug_frame_off=0x4dec8000 and dwbias=0x4dec9440. As dwbias is larger than debug_frame_off, the value becomes negative. But the field .sec_load_offset as unsigned long and therefore does not take negative values.

I use a stripped and prelinked library on the target. The same library is available on the host as well. The debug link in the library points then to a non-prelinked file with the debug information. This works fine for most cases. E.g. I can set user space probe points. Only when I include the unwind information, this phenomenon occurs.

The offset of the .text section in the non-prelinked library is 0x15c40. 
The offset of the .text section in the prelinked library is 0x4decf080 - 0xdeb8000 = 0x17080.

This means that .sec_load_offset is exactly the difference 0x15c40 - 0x17080 ~ 0xffffffffffffebc0.

I wonder how I can overcome this situation.

Kind Regards,
Torsten

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

Hi Torsten,

On Wed, 2015-11-25 at 21:52 +0100, Torsten Polle wrote:
> Here I have a problem to compile my probes when I want to include unwind information.
> 
> The error message is as follow:
> 
> In file included from /tmp/stap/taptrek_src.c:6010:0:
> /tmp/stap/stap-symbol.h:95831:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
>  },
>  ^
> 
> The offending line in stap-symbols.h looks as follows:
> .sec_load_offset = 0xffffffffffffebc0 /* 4dec8000 4dec9440*/
> 
> The line includes debug output, because I checked the code in translate.cxx that generates this output and wanted to understand what is going wrong.
> 
>                   Dwarf_Addr dwbias = 0;
>                   dwfl_module_getdwarf (m, &dwbias);
>                   c->output << ".sec_load_offset = 0x"
>                                     << hex << debug_frame_off - dwbias << dec
>                                     << "/* "  << hex << debug_frame_off << " " << dwbias << dec << "*/"
>                                     << "\n";
> 
> As I use a 64bit host, the hex value is given as 64bit long hex value. Additional the value is very large, because the value is actually the difference between debug_frame_off=0x4dec8000 and dwbias=0x4dec9440. As dwbias is larger than debug_frame_off, the value becomes negative. But the field .sec_load_offset as unsigned long and therefore does not take negative values.

Yeah, this goes wrong since you are cross compiling for 32bit, so
unsigned long is "suddenly" smaller than expected.

I don't immediately know what the correct solution is. the
sec_load_offset is defined for _stp_section with the following comment:

        /* Synthesized index for .debug_frame table, keep section
           offset to adjust addresses relative to load address. */
        void *debug_hdr;
        uint32_t debug_hdr_len;
        unsigned long sec_load_offset;

I added this a couple of years back:

commit 4d83bd9b6f5ccc4abd212ca5d6a6477cb52f78cc
Author: Mark Wielaard <mjw@redhat.com>
Date:   Mon Jul 5 21:14:42 2010 +0200

  Put generated debug_hdr in _stp_section, add sec_load_offset for adjustment.
    
  Make sure to adjust .debug_frame addresses to section load address.
  Which means keeping track of the (synthetic) .debug_frame_hdr index
  per section. For now keep track of "magic sections". Will need to
  be extended to track all loadable code sections as we do for symbol
  tables. See http://sourceware.org/ml/systemtap/2010-q3/msg00012.html

So the problem we are facing is that kernel modules aren't ET_DYN, but
ET_REL files and the separate sections themselves are relocatable. So we
want to keep the load offset per (code) section.

We need that offset since we are creating a binary search table for
the .debug_frame (.eh_frame always comes with .eh_frame_hdr,
but .debug_frame would normally require a linear search which we want to
avoid). See adjustStartLoc in unwind.c.

Later this got extended to also be used by _stp_linenumber_lookup for
kernel modules, apparently for the same reason.

commit 598a50ae1cbc0f84914a4eee56a287273bb420b7
Author: Abegail Jakop <ajakop@redhat.com>
Date:   Mon Dec 22 15:04:34 2014 -0500

  PR12276: fix addr adjustment for symfileline()
    
  runtime/sym*: adjust the given address to account for the load offset
  runtime/transport/symbols.c: include the body of _stp_module_update_self()
  if STP_NEED_LINE_DATA so that the sec_load_offset can be updated
  tapset/linux/context-symbols.stp: remove the pragma myproc-unpriveleged
  since it's spelled wrong, and it's not needed anyways
  translate.cxx: add funtion find_debug_frame_offset() to set the
  sec_load_offset for the ".text" section

I assume it is the first usage (under STP_NEED_UNWIND_DATA) but the
other code looks similar (under STP_NEED_LINE_DATA).

It looks like the debug_frame_off is setup differently for the two
cases. So it might be good to investigate which one is used in the case
above to help our investigation.

Thanks,

Mark

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

Hi Mark,

Mark Wielaard writes:
 > Hi Torsten,

 > On Wed, 2015-11-25 at 21:52 +0100, Torsten Polle wrote:
 >> Here I have a problem to compile my probes when I want to include unwind information.
 >> 
 >> The error message is as follow:
 >> 
 >> In file included from /tmp/stap/taptrek_src.c:6010:0:
 >> /tmp/stap/stap-symbol.h:95831:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
 >> },
 >> ^
 >> 
 >> The offending line in stap-symbols.h looks as follows:
 >> .sec_load_offset = 0xffffffffffffebc0 /* 4dec8000 4dec9440*/
 >> 
 >> The line includes debug output, because I checked the code in
 >> translate.cxx that generates this output and wanted to understand
 >> what is going wrong.
 >> 
 >> Dwarf_Addr dwbias = 0;
 >> dwfl_module_getdwarf (m, &dwbias);
 c-> output << ".sec_load_offset = 0x"
 >> << hex << debug_frame_off - dwbias << dec
 >> << "/* "  << hex << debug_frame_off << " " << dwbias << dec << "*/"
 >> << "\n";
 >> 
 >> As I use a 64bit host, the hex value is given as 64bit long hex
 >> value. Additional the value is very large, because the value is
 >> actually the difference between debug_frame_off=0x4dec8000 and
 >> dwbias=0x4dec9440. As dwbias is larger than debug_frame_off, the
 >> value becomes negative. But the field .sec_load_offset as unsigned
 >> long and therefore does not take negative values.

 > Yeah, this goes wrong since you are cross compiling for 32bit, so
 > unsigned long is "suddenly" smaller than expected.

 > I don't immediately know what the correct solution is. the
 > sec_load_offset is defined for _stp_section with the following comment:

 >         /* Synthesized index for .debug_frame table, keep section
 >            offset to adjust addresses relative to load address. */
 >         void *debug_hdr;
 >         uint32_t debug_hdr_len;
 >         unsigned long sec_load_offset;

 > I added this a couple of years back:

 > commit 4d83bd9b6f5ccc4abd212ca5d6a6477cb52f78cc
 > Author: Mark Wielaard <mjw@redhat.com>
 > Date:   Mon Jul 5 21:14:42 2010 +0200

 >   Put generated debug_hdr in _stp_section, add sec_load_offset for adjustment.
    
 >   Make sure to adjust .debug_frame addresses to section load address.
 >   Which means keeping track of the (synthetic) .debug_frame_hdr index
 >   per section. For now keep track of "magic sections". Will need to
 >   be extended to track all loadable code sections as we do for symbol
 >   tables. See http://sourceware.org/ml/systemtap/2010-q3/msg00012.html

 > So the problem we are facing is that kernel modules aren't ET_DYN, but
 > ET_REL files and the separate sections themselves are relocatable. So we
 > want to keep the load offset per (code) section.

 > We need that offset since we are creating a binary search table for
 > the .debug_frame (.eh_frame always comes with .eh_frame_hdr,
 > but .debug_frame would normally require a linear search which we want to
 > avoid). See adjustStartLoc in unwind.c.

 > Later this got extended to also be used by _stp_linenumber_lookup for
 > kernel modules, apparently for the same reason.

 > commit 598a50ae1cbc0f84914a4eee56a287273bb420b7
 > Author: Abegail Jakop <ajakop@redhat.com>
 > Date:   Mon Dec 22 15:04:34 2014 -0500

 >   PR12276: fix addr adjustment for symfileline()
    
 >   runtime/sym*: adjust the given address to account for the load offset
 >   runtime/transport/symbols.c: include the body of _stp_module_update_self()
 >   if STP_NEED_LINE_DATA so that the sec_load_offset can be updated
 >   tapset/linux/context-symbols.stp: remove the pragma myproc-unpriveleged
 >   since it's spelled wrong, and it's not needed anyways
 >   translate.cxx: add funtion find_debug_frame_offset() to set the
 >   sec_load_offset for the ".text" section

 > I assume it is the first usage (under STP_NEED_UNWIND_DATA) but the
 > other code looks similar (under STP_NEED_LINE_DATA).

 > It looks like the debug_frame_off is setup differently for the two
 > cases. So it might be good to investigate which one is used in the case
 > above to help our investigation.

I had some problems with my mailer. I apology if you got this mail twice.

Your assumption is correct. It is the first usage (under STP_NEED_UNWIND_DATA).

 > Thanks,
 > Mark

Kind Regards,
Torsten

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

On Thu, 2015-11-26 at 23:04 +0100, Torsten Polle wrote:
> Mark Wielaard writes:\
>  > On Wed, 2015-11-25 at 21:52 +0100, Torsten Polle wrote:
>  >> Here I have a problem to compile my probes when I want to include unwind information.
>  >> 
>  >> The error message is as follow:
>  >> 
>  >> In file included from /tmp/stap/taptrek_src.c:6010:0:
>  >> /tmp/stap/stap-symbol.h:95831:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
>  >> },
>  >> ^
>  >> 
>  >> The offending line in stap-symbols.h looks as follows:
>  >> .sec_load_offset = 0xffffffffffffebc0 /* 4dec8000 4dec9440*/
>  >> 
>  >> The line includes debug output, because I checked the code in
>  >> translate.cxx that generates this output and wanted to understand
>  >> what is going wrong.

What is the stap command line and script you are using?
Could you post (more of the) taptrek_src.c and stap-symbols.h for
context?

Thanks,

Mark

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

> Gesendet: Freitag, 27. November 2015 um 09:04 Uhr
> Von: "Mark Wielaard" <mjw@redhat.com>
> An: "Torsten Polle" <Torsten.Polle@gmx.de>
> Cc: systemtap@sourceware.org
> Betreff: Re: Prelinking on ARM with Debug Link
> On Thu, 2015-11-26 at 23:04 +0100, Torsten Polle wrote:
> > Mark Wielaard writes:\
> > > On Wed, 2015-11-25 at 21:52 +0100, Torsten Polle wrote:
> > >> Here I have a problem to compile my probes when I want to include unwind information.
> > >>
> > >> The error message is as follow:
> > >>
> > >> In file included from /tmp/stap/taptrek_src.c:6010:0:
> > >> /tmp/stap/stap-symbol.h:95831:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
> > >> },
> > >> ^
> > >>
> > >> The offending line in stap-symbols.h looks as follows:
> > >> .sec_load_offset = 0xffffffffffffebc0 /* 4dec8000 4dec9440*/
> > >>
> > >> The line includes debug output, because I checked the code in
> > >> translate.cxx that generates this output and wanted to understand
> > >> what is going wrong.
> 
> What is the stap command line and script you are using?
> Could you post (more of the) taptrek_src.c and stap-symbols.h for
> context?
> 
> Thanks,
> Mark

Hi Mark,

Please find attached the requested information.

I've stripped down the output of SystemTap in folder stap.

commandline.txt contains the full command line.

taptrek_run_izA4.stp contains the script used.

Kind Regards,
Torsten

[-- Attachment #2: prelinking.tar.bz2 --]
[-- Type: application/octet-stream, Size: 74446 bytes --]

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

Hi Torsten,

On Fri, Nov 27, 2015 at 01:57:31PM +0100, Torsten Polle wrote:
> I've stripped down the output of SystemTap in folder stap.
> 
> commandline.txt contains the full command line.
> 
> taptrek_run_izA4.stp contains the script used.

That is a giant script using both kernel and user space probes
and guru mode functions. Are you able to reproduce it with something
a bit smaller?

> WARNING: tapset "/opt/tooling/adit/systemtap/share/systemtap/tapset/linux/arm/syscalls.stp" has errors, and will be skipped

You might want to look into why that is.

> In file included from /tmp/stap/taptrek_src.c:6010:0:
> /tmp/stap/stap-symbols.h:95902:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]

But the stap-symbols.h file has only 8107 lines.

Cheers,

Mark 

Re: Prelinking on ARM with Debug Link

[Mark Wielaard]

On Fri, Nov 27, 2015 at 04:06:14PM +0100, Mark Wielaard wrote:
> On Fri, Nov 27, 2015 at 01:57:31PM +0100, Torsten Polle wrote:
> > In file included from /tmp/stap/taptrek_src.c:6010:0:
> > /tmp/stap/stap-symbols.h:95902:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
> 
> But the stap-symbols.h file has only 8107 lines.

If you happen to know which library it is that causes the issue,
then it would be beneficial to also have the output of eu-readelf -Sl
for both the main and debug file.

And maybe the stap -vv output of the script to know which files and
probes are used.

Thanks,

Mark

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

Hi Mark,

Mark Wielaard writes:
 > On Fri, Nov 27, 2015 at 04:06:14PM +0100, Mark Wielaard wrote:
 >> On Fri, Nov 27, 2015 at 01:57:31PM +0100, Torsten Polle wrote:
 >> > In file included from /tmp/stap/taptrek_src.c:6010:0:
 >> > /tmp/stap/stap-symbols.h:95902:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
 >> 
 >> But the stap-symbols.h file has only 8107 lines.

 > If you happen to know which library it is that causes the issue, then
 > it would be beneficial to also have the output of eu-readelf -Sl for
 > both the main and debug file.

 > And maybe the stap -vv output of the script to know which files and
 > probes are used.

 > Thanks,
 > Mark

thanks for the support.

I'll provide you with a reproduction scenario that is much smaller.

Kind Regards,
Torsten

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

Hi Mark,

> Mark Wielaard writes:
>  > Hi Torsten,
> 
>  > On Fri, Nov 27, 2015 at 01:57:31PM +0100, Torsten Polle wrote:
>  >> I've stripped down the output of SystemTap in folder stap.
>  >> 
>  >> commandline.txt contains the full command line.
>  >> 
>  >> taptrek_run_izA4.stp contains the script used.
> 
>  > That is a giant script using both kernel and user space probes and
>  > guru mode functions. Are you able to reproduce it with something a
>  > bit smaller?
> 
> The script is actually a lot longer. I only sent a stripped down version of the original script.
> 
>  >> WARNING: tapset "/opt/tooling/adit/systemtap/share/systemtap/tapset/linux/arm/syscalls.stp" has errors, and will be skipped
> 
>  > You might want to look into why that is.
> 
> Yes, I know. I had re-added support of a system call (execve), which
> does not work since some time in SystemTap. But since the support
> depends on the kernel version, I get a warning about an empty file when
> the kernel version is greater than 3.7.
> 
>  >> In file included from /tmp/stap/taptrek_src.c:6010:0:
>  >> /tmp/stap/stap-symbols.h:95902:1: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
> 
>  > But the stap-symbols.h file has only 8107 lines.
> 
> The file is huge. Even compressed, it was about 500kB. Therefore I only
> sent the part with the sec_module_load parts in it.
> 
>  > Cheers,
>  > Mark 

Thanks,
Torsten

Re: Prelinking on ARM with Debug Link

[David Smith]

On 11/27/2015 02:01 PM, Torsten Polle wrote:
> Hi Mark,
> 
>> Mark Wielaard writes:
>>  >> WARNING: tapset "/opt/tooling/adit/systemtap/share/systemtap/tapset/linux/arm/syscalls.stp" has errors, and will be skipped
>>
>>  > You might want to look into why that is.
>>
>> Yes, I know. I had re-added support of a system call (execve), which
>> does not work since some time in SystemTap. But since the support
>> depends on the kernel version, I get a warning about an empty file when
>> the kernel version is greater than 3.7.

In kernel versions less than 3.7, the execve syscall was in
arch-specific code. So, each arch has their own implementation of
[nd_]syscall.execve in tapsets/linux/ARCH/[nd_]syscalls.stp.

In kernel versions >= 3.7, the execve syscall was moved to generic
kernel code, and support for it is located in
tapset/linux/[nd_]syscalls.stp.

I don't believe we've ever supported an arm/arm64 kernel less than 3.7,
so there isn't arch-specific execve support for arm/arm64. But, if you
have got working execve support for arm/arm64 kernels less than 3.7, I
don't see a real problem with checking it in (assuming it looks reasonable).

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]

Hi David,

David Smith writes:
 > On 11/27/2015 02:01 PM, Torsten Polle wrote:
 >> Hi Mark,
 >> 
 >>> Mark Wielaard writes:
 >>> >> WARNING: tapset "/opt/tooling/adit/systemtap/share/systemtap/tapset/linux/arm/syscalls.stp" has errors, and will be skipped
 >>> 
 >>> > You might want to look into why that is.
 >>> 
 >>> Yes, I know. I had re-added support of a system call (execve), which
 >>> does not work since some time in SystemTap. But since the support
 >>> depends on the kernel version, I get a warning about an empty file when
 >>> the kernel version is greater than 3.7.

 > In kernel versions less than 3.7, the execve syscall was in
 > arch-specific code. So, each arch has their own implementation of
 > [nd_]syscall.execve in tapsets/linux/ARCH/[nd_]syscalls.stp.

 > In kernel versions >= 3.7, the execve syscall was moved to generic
 > kernel code, and support for it is located in
 > tapset/linux/[nd_]syscalls.stp.

 > I don't believe we've ever supported an arm/arm64 kernel less than 3.7,

I'm using SystemTap successfully with a 2.6.34 kernel on ARM since 4
years now. So far it worked without much problems.

 > so there isn't arch-specific execve support for arm/arm64. But, if you
 > have got working execve support for arm/arm64 kernels less than 3.7, I
 > don't see a real problem with checking it in (assuming it looks reasonable).

I've support that is working in my environment. At least the defined
variables give some decent result. Please judge the quality for yourself.

The error message reported comes from the fact that the file
syscalls.stp is regarded as empty by SystemTap if the kernel version is
greater then 3.7. I could not find a nice solution so far.

 > -- 
 > David Smith
 > dsmith@redhat.com
 > Red Hat
 > http://www.redhat.com
 > 256.217.0141 (direct)
 > 256.837.0057 (fax)

Kind Regards,
Torsten

[-- Attachment #2: 0001-Fix-Kernel-2.6.34-no-longer-supported.patch --]
[-- Type: application/octet-stream, Size: 4180 bytes --]

From 5b9dddd3ac920fd7f1f4d326e02b78fad56bfd9e Mon Sep 17 00:00:00 2001
Message-Id: <5b9dddd3ac920fd7f1f4d326e02b78fad56bfd9e.1448999281.git.Torsten.Polle@gmx.de>
From: Torsten Polle <Torsten.Polle@gmx.de>
Date: Fri, 29 May 2015 21:45:51 +0200
Subject: [PATCH] Fix: Kernel 2.6.34 no longer supported.


Signed-off-by: Torsten Polle <Torsten.Polle@gmx.de>
---
 tapset/linux/arm/aux_syscalls.stp |    6 +++++-
 tapset/linux/arm/syscalls.stp     |   27 +++++++++++++++++++++++++++
 tapset/linux/aux_syscalls.stp     |    4 ++++
 3 files changed, 36 insertions(+), 1 deletions(-)
 create mode 100644 tapset/linux/arm/syscalls.stp

diff --git a/tapset/linux/arm/aux_syscalls.stp b/tapset/linux/arm/aux_syscalls.stp
index ebbab4f..9e1e722 100644
--- a/tapset/linux/arm/aux_syscalls.stp
+++ b/tapset/linux/arm/aux_syscalls.stp
@@ -4,7 +4,7 @@
 %{
 // Get _stp_val_array and _stp_lookup_* definitions.
 #include "linux/syscalls-common.h"
-
+#include <asm/ptrace.h>
 static const _stp_val_array const _stp_arch_ptrace_request_list[] = {
 	V(PTRACE_GETREGS),
 	V(PTRACE_SETREGS),
@@ -19,8 +19,10 @@ static const _stp_val_array const _stp_arch_ptrace_request_list[] = {
 	V(PTRACE_SETCRUNCHREGS),
 	V(PTRACE_GETVFPREGS),
 	V(PTRACE_SETVFPREGS),
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,7,0)
 	V(PTRACE_GETHBPREGS),
 	V(PTRACE_SETHBPREGS),
+#endif
 	{0, NULL}
 };
 %}
@@ -45,8 +47,10 @@ function _arch_ptrace_argstr(request, pid, addr, data)
 	    || request == %{ /* pure */ PTRACE_SETCRUNCHREGS %}
 	    || request == %{ /* pure */ PTRACE_GETVFPREGS %}
 	    || request == %{ /* pure */ PTRACE_SETVFPREGS %}
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,7,0)
 	    || request == %{ /* pure */ PTRACE_GETHBPREGS %}
 	    || request == %{ /* pure */ PTRACE_SETHBPREGS %})
+#endif
 		return sprintf("%s, %d, %#x, %p",
 			       __arch_ptrace_request_str(request), pid,
 			       addr, data)
diff --git a/tapset/linux/arm/syscalls.stp b/tapset/linux/arm/syscalls.stp
new file mode 100644
index 0000000..fd7d980
--- /dev/null
+++ b/tapset/linux/arm/syscalls.stp
@@ -0,0 +1,27 @@
+# ARM system calls
+
+%( kernel_v < "3.7" %?
+# execve _____________________________________________________
+#
+# In kernels < 3.7, sys_execve() was in arch-specific code (and had
+# varying arguments). It was just a wrapper around generic
+# do_execve(), but the wrapper could error out before calling
+# do_execve(). So, we'll have to handle it in arch-specific tapset
+# code to catch all calls.
+#
+#  int sys_execve(char __user *filenamei, char __user * __user *argv,
+#		  char __user * __user *envp, struct pt_regs *regs)
+probe syscall.execve = kernel.function("sys_execve").call
+{
+	name = "execve"
+	filename = user_string_quoted($filenamei)
+	args = __get_argv($argv, 0)
+	env_str = __count_envp($envp)
+	argstr = sprintf("%s, %s, %s", filename, args, env_str)
+}
+probe syscall.execve.return = kernel.function("sys_execve").return
+{
+	name = "execve"
+	retstr = return_str(1, $return)
+}
+%)
diff --git a/tapset/linux/aux_syscalls.stp b/tapset/linux/aux_syscalls.stp
index 6ddb08b..929396d 100644
--- a/tapset/linux/aux_syscalls.stp
+++ b/tapset/linux/aux_syscalls.stp
@@ -4332,6 +4332,7 @@ function _stp_keyctl_default_keyrings_str:string(id:long)
 #include <linux/key.h>
 
 static const _stp_val_array const _stp_keyctl_perm_list[] = {
+#if defined(CONFIG_KEYS)
 	V(KEY_POS_VIEW),
 	V(KEY_POS_READ),
 	V(KEY_POS_WRITE),
@@ -4360,6 +4361,7 @@ static const _stp_val_array const _stp_keyctl_perm_list[] = {
 	V(KEY_OTH_LINK),
 	V(KEY_OTH_SETATTR),
 	V(KEY_OTH_ALL),
+#endif /* CONFIG_KEYS */
 	{0, NULL}
 };
 %}
@@ -5603,6 +5605,7 @@ function _struct_sysinfo_u:string(uaddr:long)
 %{
 #include <linux/kexec.h>
 static const _stp_val_array const _stp_kexec_flags_list[] = {
+#if defined(CONFIG_KEXEC)
 	V(KEXEC_ON_CRASH),
 #ifdef KEXEC_PRESERVE_CONTEXT
 	V(KEXEC_PRESERVE_CONTEXT),
@@ -5624,6 +5627,7 @@ static const _stp_val_array const _stp_kexec_flags_list[] = {
 #ifdef KEXEC_ARCH_MIPS
 	V(KEXEC_ARCH_MIPS),
 #endif
+#endif /* CONFIG_KEXEC */
 	{0, NULL}
 };
 
-- 
1.7.4.1

Re: Prelinking on ARM with Debug Link

[David Smith]

On 12/01/2015 02:10 PM, Torsten Polle wrote:
> Hi David,
> 
> David Smith writes:
>  > I don't believe we've ever supported an arm/arm64 kernel less than 3.7,
> 
> I'm using SystemTap successfully with a 2.6.34 kernel on ARM since 4
> years now. So far it worked without much problems.
> 
>  > so there isn't arch-specific execve support for arm/arm64. But, if you
>  > have got working execve support for arm/arm64 kernels less than 3.7, I
>  > don't see a real problem with checking it in (assuming it looks reasonable).
> 
> I've support that is working in my environment. At least the defined
> variables give some decent result. Please judge the quality for yourself.
> 
> The error message reported comes from the fact that the file
> syscalls.stp is regarded as empty by SystemTap if the kernel version is
> greater then 3.7. I could not find a nice solution so far.

Torsten,

I've checked in your changes, they all looked reasonable. I added
support for nd_syscall.execve. I worked around the empty tapset warning
by adding a 'never' probe the the new tapset files.

Thanks for the patch.

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: Prelinking on ARM with Debug Link

[Torsten Polle]

David Smith writes:
 > On 12/01/2015 02:10 PM, Torsten Polle wrote:
 >> Hi David,
 >> 
 >> David Smith writes:
 >> > I don't believe we've ever supported an arm/arm64 kernel less than 3.7,
 >> 
 >> I'm using SystemTap successfully with a 2.6.34 kernel on ARM since 4
 >> years now. So far it worked without much problems.
 >> 
 >> > so there isn't arch-specific execve support for arm/arm64. But, if you
 >> > have got working execve support for arm/arm64 kernels less than 3.7, I
 >> > don't see a real problem with checking it in (assuming it looks reasonable).
 >> 
 >> I've support that is working in my environment. At least the defined
 >> variables give some decent result. Please judge the quality for yourself.
 >> 
 >> The error message reported comes from the fact that the file
 >> syscalls.stp is regarded as empty by SystemTap if the kernel version is
 >> greater then 3.7. I could not find a nice solution so far.

 > Torsten,

 > I've checked in your changes, they all looked reasonable. I added
 > support for nd_syscall.execve. I worked around the empty tapset warning
 > by adding a 'never' probe the the new tapset files.

David,

Thanks,
Torsten