tests/backtrace-dwarf.c failure due to -freorder-blocks-and-partition

tests/backtrace-dwarf.c failure due to -freorder-blocks-and-partition

[Martin Liška]

Hello.

As slightly discussed with Mark, there are tests that expect 'main'
will be present in backtrace. That's not always true on x86_64 because
-freorder-blocks-and-partition option is on by default. Then one can see:

[   88s] FAIL: run-backtrace-dwarf.sh
[   88s] ============================
[   88s] 
[   88s] 0x7f1fd49800cb	raise
[   88s] 0x7f1fd49694e9	abort
[   88s] 0x5627fddd0188	callme
[   88s] 0x5627fddd0192	doit
[   88s] 0x5627fddd01a3	main.cold.1
[   88s] 0x7f1fd496afeb	__libc_start_main
[   88s] 0x5627fddd04aa	_start
[   88s] /home/abuild/rpmbuild/BUILD/elfutils-0.173/tests/backtrace-dwarf: dwfl_thread_getframes: no error
[   88s] 0x5627fddd01a3	main.cold.1

Thus I'm suggesting to disable the option for tests?
Thoughts?
Martin

Re: tests/backtrace-dwarf.c failure due to -freorder-blocks-and-partition

[Mark Wielaard]

Hi Martin,

On Fri, 2018-08-03 at 09:41 +0200, Martin Liška wrote:
> As slightly discussed with Mark, there are tests that expect 'main'
> will be present in backtrace. That's not always true on x86_64
> because
> -freorder-blocks-and-partition option is on by default. Then one can
> see:
> 
> [   88s] FAIL: run-backtrace-dwarf.sh
> [   88s] ============================
> [   88s] 
> [   88s] 0x7f1fd49800cb	raise
> [   88s] 0x7f1fd49694e9	abort
> [   88s] 0x5627fddd0188	callme
> [   88s] 0x5627fddd0192	doit
> [   88s] 0x5627fddd01a3	main.cold.1
> [   88s] 0x7f1fd496afeb	__libc_start_main
> [   88s] 0x5627fddd04aa	_start
> [   88s] /home/abuild/rpmbuild/BUILD/elfutils-0.173/tests/backtrace-
> dwarf: dwfl_thread_getframes: no error
> [   88s] 0x5627fddd01a3	main.cold.1
> 
> Thus I'm suggesting to disable the option for tests?
> Thoughts?

So the problem is that some tests look for a 'main' symbol.
This is imho for C based programs a natural way to see if we can unwind
to the start of the program (everything before 'main' is infrastructure
that isn't really relevant to the user). But in some cases the 'main'
symbol is munged into something else. 'main.cold.1' in this case.

The first question is, does the program also contain a 'main' symbol?
If so, what does it cover?
Could you eu-readelf -s tests/backtrace-dwarf | grep main

Now if it does, the question is why didn't we see it?
Is main.cold.1 an alias? Then we probably should look harder/smarter.
Or does it now cover any of the backtrace addresses?

If there isn't, or it isn't actually called, then the question is, is
that actually legal? It seems, at least for C and C++ based programs
that they should start in 'main'. If not they are not, is that because
gcc did an illegal transformation? Or does it only look that way
because we cannot unwind correctly (did it do some tail call)?

We could just use -freorder-blocks-and-partition. But I would like to
first really understand why it is necessary.

If you could maybe post the binary somewhere for inspection that would
be great.

Thanks,

Mark

Re: tests/backtrace-dwarf.c failure due to -freorder-blocks-and-partition

[Martin Liška]

On 08/03/2018 11:46 AM, Mark Wielaard wrote:
> Hi Martin,
> 
> On Fri, 2018-08-03 at 09:41 +0200, Martin Liška wrote:
>> As slightly discussed with Mark, there are tests that expect 'main'
>> will be present in backtrace. That's not always true on x86_64
>> because
>> -freorder-blocks-and-partition option is on by default. Then one can
>> see:
>>
>> [   88s] FAIL: run-backtrace-dwarf.sh
>> [   88s] ============================
>> [   88s] 
>> [   88s] 0x7f1fd49800cb	raise
>> [   88s] 0x7f1fd49694e9	abort
>> [   88s] 0x5627fddd0188	callme
>> [   88s] 0x5627fddd0192	doit
>> [   88s] 0x5627fddd01a3	main.cold.1
>> [   88s] 0x7f1fd496afeb	__libc_start_main
>> [   88s] 0x5627fddd04aa	_start
>> [   88s] /home/abuild/rpmbuild/BUILD/elfutils-0.173/tests/backtrace-
>> dwarf: dwfl_thread_getframes: no error
>> [   88s] 0x5627fddd01a3	main.cold.1
>>
>> Thus I'm suggesting to disable the option for tests?
>> Thoughts?
> 
> So the problem is that some tests look for a 'main' symbol.
> This is imho for C based programs a natural way to see if we can unwind
> to the start of the program (everything before 'main' is infrastructure
> that isn't really relevant to the user). But in some cases the 'main'
> symbol is munged into something else. 'main.cold.1' in this case.
> 
> The first question is, does the program also contain a 'main' symbol?
> If so, what does it cover?
> Could you eu-readelf -s tests/backtrace-dwarf | grep main

Yes it does, can be shown with gcc 8.* on x86_64:

$ cat cold.c
int main(int argc, char **argv)
{
  if (argc != 111)
    __builtin_abort ();

  return 0;
}

$ gcc cold.c -O2 
$ readelf -s a.out | grep main
     2: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND __libc_start_main@GLIBC_2.2.5 (2)
    37: 0000000000400430     5 FUNC    LOCAL  DEFAULT   14 main.cold.0
    60: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND __libc_start_main@@GLIBC_
    69: 0000000000400440    20 FUNC    GLOBAL DEFAULT   14 main

$ gdb ./a.out 
r
Starting program: /tmp/a.out 

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50	  return ret;
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff7a384e9 in __GI_abort () at abort.c:79
#2  0x0000000000400435 in main.cold ()
#3  0x00007ffff7a39feb in __libc_start_main (main=0x400440 <main>, argc=1, argv=0x7fffffffdc88, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdc78) at ../csu/libc-start.c:308
#4  0x000000000040048a in _start () at ../sysdeps/x86_64/start.S:120

If using debug info (-g), then it's fine:

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50	  return ret;
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff7a384e9 in __GI_abort () at abort.c:79
#2  0x0000000000400435 in main (argc=<optimized out>, argv=<optimized out>) at cold.c:4
#3  0x00007ffff7a39feb in __libc_start_main (main=0x400440 <main>, argc=1, argv=0x7fffffffdc88, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdc78) at ../csu/libc-start.c:308
#4  0x000000000040048a in _start () at ../sysdeps/x86_64/start.S:120

> 
> Now if it does, the question is why didn't we see it?
> Is main.cold.1 an alias? Then we probably should look harder/smarter.
> Or does it now cover any of the backtrace addresses?

Maybe because jmp instruction is used instead of call?

0000000000400440 <main>:
  400440:	48 83 ec 08          	sub    $0x8,%rsp
  400444:	83 ff 6f             	cmp    $0x6f,%edi
  400447:	0f 85 e3 ff ff ff    	jne    400430 <main.cold.0>
  40044d:	31 c0                	xor    %eax,%eax
  40044f:	48 83 c4 08          	add    $0x8,%rsp
  400453:	c3                   	retq   
  400454:	66 2e 0f 1f 84 00 00 	nopw   %cs:0x0(%rax,%rax,1)
  40045b:	00 00 00 
  40045e:	66 90                	xchg   %ax,%ax

I'm not expert in libbactrace, so maybe true is somewhere else.

Martin

> 
> If there isn't, or it isn't actually called, then the question is, is
> that actually legal? It seems, at least for C and C++ based programs
> that they should start in 'main'. If not they are not, is that because
> gcc did an illegal transformation? Or does it only look that way
> because we cannot unwind correctly (did it do some tail call)?
> 
> We could just use -freorder-blocks-and-partition. But I would like to
> first really understand why it is necessary.
> 
> If you could maybe post the binary somewhere for inspection that would
> be great.
> 
> Thanks,
> 
> Mark
> 

Re: tests/backtrace-dwarf.c failure due to -freorder-blocks-and-partition

[Mark Wielaard]

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

Hi Martin,

Sorry for dropping this after you provided such an excellent analysis.
So the problem really is that main gets split into main.cold which is
jumped into, so a simple backtrace won't see the "hot" main anymore. If
we would use the debuginfo we might be able to get the "correct"
function name, but that isn't really the purpose of this testcase.

The only problem is that I haven't been able to replicate this in the
testsuite (I do see the partitioning in the standalone testcase you
posted).

Is it just run-backtrace-dwarf that is a problem for you?
In that case, does the attached patch help? It puts all of main in its
own section, which should prevent gcc from partitioning it.

Thanks,

Mark

[-- Attachment #2: Type: text/x-patch, Size: 454 bytes --]

diff --git a/tests/backtrace-dwarf.c b/tests/backtrace-dwarf.c
index e1eb492..35f25ed 100644
--- a/tests/backtrace-dwarf.c
+++ b/tests/backtrace-dwarf.c
@@ -124,6 +124,7 @@ thread_callback (Dwfl_Thread *thread, void *thread_arg)
 }
 
 int
+__attribute__((section(".main"))) /* Defeat -freorder-blocks-and-partition  */
 main (int argc __attribute__ ((unused)), char **argv)
 {
   /* We use no threads here which can interfere with handling a stream.  */

Re: tests/backtrace-dwarf.c failure due to -freorder-blocks-and-partition

[Mark Wielaard]

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

On Thu, 2018-08-30 at 18:15 +0200, Mark Wielaard wrote:
> Sorry for dropping this after you provided such an excellent analysis.
> So the problem really is that main gets split into main.cold which is
> jumped into, so a simple backtrace won't see the "hot" main anymore. If
> we would use the debuginfo we might be able to get the "correct"
> function name, but that isn't really the purpose of this testcase.
> 
> The only problem is that I haven't been able to replicate this in the
> testsuite (I do see the partitioning in the standalone testcase you
> posted).
> 
> Is it just run-backtrace-dwarf that is a problem for you?
> In that case, does the attached patch help? It puts all of main in its
> own section, which should prevent gcc from partitioning it.

I pushed the attached to master assuming it helps and doesn't cause any
issues.

Cheers,

Mark

[-- Attachment #2: Type: text/x-patch, Size: 1392 bytes --]

From a1e892e920bbde60a9daa1f98c105c227ee7427d Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Tue, 11 Sep 2018 12:58:14 +0200
Subject: [PATCH] tests: Add section attribute to defeat
 -freorder-blocks-and-partition.

GCC could partition main into an cold and hot block causing our symbol
lookup to fail in the backtrace-dwarf testcase. Add a section attribute
on "main" so that it will be kept together.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 tests/ChangeLog         | 4 ++++
 tests/backtrace-dwarf.c | 1 +
 2 files changed, 5 insertions(+)

diff --git a/tests/ChangeLog b/tests/ChangeLog
index 2a20ffa..8d70891 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2018-09-11  Mark Wielaard  <mark@klomp.org>
+
+	* backtrace-dwarf.c (main): Add section attribute.
+
 2018-07-24  Mark Wielaard  <mark@klomp.org>
 
 	* run-annobingroup.sh: Add testfile-annobingroup-i386.o tests.
diff --git a/tests/backtrace-dwarf.c b/tests/backtrace-dwarf.c
index e1eb492..35f25ed 100644
--- a/tests/backtrace-dwarf.c
+++ b/tests/backtrace-dwarf.c
@@ -124,6 +124,7 @@ thread_callback (Dwfl_Thread *thread, void *thread_arg)
 }
 
 int
+__attribute__((section(".main"))) /* Defeat -freorder-blocks-and-partition  */
 main (int argc __attribute__ ((unused)), char **argv)
 {
   /* We use no threads here which can interfere with handling a stream.  */
-- 
1.8.3.1