-mfentry segfault on GCC 7

[Taeung]

Hello,

When building uftrace by gcc 7, I found the segmentation fault at libmcount/mcount.c:515.
It is similar to #91

gcc : 7.0.1
git-branch : master

• dynamic tracing test environment

$ gcc -dumpversion
7.0.1

$ git status | head -1
On branch master

$ sudo make clean && sudo make install

• Segmentation fault at libmcount/mcount.c:515.
  (This situation is same whatever target problem(abc.dyn) is built by e.g. gcc 5,6,7)

$ gcc -pg -mfentry -mnop-mcount -o abc.dyn tests/s-abc.c

$ uftrace -P . abc.dyn
child terminated by signal: 11: Segmentation fault
# DURATION    TID     FUNCTION
   0.668 us [ 8281] | __monstartup();
   0.500 us [ 8281] | __cxa_atexit();

• Check core file with gdb

$ gdb abc.dyn core
...
Reading symbols from abc.dyn...(no debugging symbols found)...done.
[New LWP 8281]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `abc.dyn'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007fb4135f05c2 in mcount_entry (parent_loc=0x7fffaaef5948, child=4195955, 
    regs=0x7fffaaef5910) at /home/taeung/git/uftrace/libmcount/mcount.c:515
515		struct ftrace_trigger tr = {

[namhyung]

Thanks, the fix would be same as the x-ray:

diff --git a/arch/x86_64/fentry.S b/arch/x86_64/fentry.S
index 026168a..8860cf5 100644
--- a/arch/x86_64/fentry.S
+++ b/arch/x86_64/fentry.S
@@ -4,23 +4,33 @@
 /* stack frame (with -pg): return addr = 8(%rbp), prev fp = 0(%rbp) */
 /* stack frame (with -fentry): return addr = (%rsp), prev fp = 8(%rsp) */
 
-.globl __fentry__
+.global __fentry__
+.type __fentry__, @function
 __fentry__:
-	pushq %rdi
-	pushq %rsi
-	pushq %rdx
-	pushq %rcx
-	pushq %r8
-	pushq %r9
-	pushq %rax
+	.cfi_startproc
+	sub $48, %rsp
+	.cfi_adjust_cfa_offset 48
+
+	movq %rdi, 40(%rsp)
+	.cfi_offset rdi, -24
+	movq %rsi, 32(%rsp)
+	.cfi_offset rsi, -32
+	movq %rdx, 24(%rsp)
+	.cfi_offset rdx, -40
+	movq %rcx, 16(%rsp)
+	.cfi_offset rcx, -48
+	movq %r8, 8(%rsp)
+	.cfi_offset r8, -56
+	movq %r9, 0(%rsp)
+	.cfi_offset r9, -64
 
 	/* child ip */
-	movq 56(%rsp), %rsi
+	movq 48(%rsp), %rsi
 	/* parent ip */
-	lea 64(%rsp), %rdi
+	lea 56(%rsp), %rdi
 
 	/* mcount_args */
-	lea 8(%rsp), %rdx
+	movq %rsp, %rdx
 
 	call mcount_entry
 	cmpq $0, %rax
@@ -30,25 +40,35 @@ __fentry__:
 	movabs $fentry_return@GOTOFF, %rdx
 	lea _GLOBAL_OFFSET_TABLE_(%rip), %rcx
 	add %rcx, %rdx
-	movq %rdx, 64(%rsp)
+	movq %rdx, 56(%rsp)
 1:
-	popq %rax
-	popq %r9
-	popq %r8
-	popq %rcx
-	popq %rdx
-	popq %rsi
-	popq %rdi
+	movq 0(%rsp), %r9
+	movq 8(%rsp), %r8
+	movq 16(%rsp), %rcx
+	movq 24(%rsp), %rdx
+	movq 32(%rsp), %rsi
+	movq 40(%rsp), %rdi
+
+	add $48, %rsp
+	.cfi_adjust_cfa_offset -48
+
 	retq
+	.cfi_endproc
 
-.type __fentry__, @function
 .size __fentry__, .-__fentry__
 
 
+.global fentry_return
+.type fentry_return, @function
 fentry_return:
+	.cfi_startproc
 	sub  $24, %rsp
+	.cfi_def_cfa_offset 24
+
 	movq %rdx, 8(%rsp)
+	.cfi_offset rdx, -16
 	movq %rax, 0(%rsp)
+	.cfi_offset rax, -24
 
 	/* set the first argument of mcount_exit as pointer to return values */
 	movq %rsp, %rdi
@@ -59,8 +79,11 @@ fentry_return:
 
 	movq 0(%rsp), %rax
 	movq 8(%rsp), %rdx
+
 	add  $16, %rsp
+	.cfi_def_cfa_offset 16
+
 	retq
+	.cfi_endproc
 
-.type fentry_return, @function
 .size fentry_return, .-fentry_return

[Taeung]

Okey, I'll test your patch !!

Frankly speaking, I tried to imitate your before patches on arch/x86_64/*.S but I failed to fix this .. 😟

[Taeung]

@namhyung Great! It is fine !

[namhyung]

Thank you so much!

But I think the stack pointer is not aligned on the return path (fentry_return) too. Maybe there's no movaps (or similar) instructions there yet. So I think it also needs to be changed..

[namhyung]

Could you please test check/stack-align-fix branch working on GCC 7 (with or without dynamic tracing)?

[Taeung]

Sure, I tested check/stack-align-fix, @namhyung

• check/stack-align-fix Test environment

$ git status | head -1
On branch check/stack-align-fix

$ gcc -dumpversion
7.0.1

$ sudo make clean && sudo make install

• dynamic tracing work fine.

$ ./runtest.py dynamic
Test case                 pg             finstrument-fu
------------------------: O0 O1 O2 O3 Os O0 O1 O2 O3 Os
136 dynamic             : OK OK OK OK OK OK OK OK OK OK
...

• normal tracing work right.

$ ./runtest.py basic
Test case                 pg             finstrument-fu
------------------------: O0 O1 O2 O3 Os O0 O1 O2 O3 Os
001 basic               : OK OK OK OK OK OK OK OK OK OK
...

[Taeung]

@namhyung Would you mind if I ask you some doc or link to precisely understand baf37fc6 ?

[namhyung]

Well, it's same as #91. You found the problem since GCC 7 added a movaps in the entry path. The exit path has same problem (rsp % 16 != 0) but it didn't crash as GCC didn't add the movaps (or similar) instruction in the return path of mcount/plthook/fentry. But it still needs to be fixed for safety.

[Taeung]

Thank you for detailed explanation !!, @namhyung