Bug 15407 - [x86_64] Partial frame info in sysdeps/x86_64/start.S
Summary: [x86_64] Partial frame info in sysdeps/x86_64/start.S
Status: ASSIGNED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.18
: P2 normal
Target Milestone: ---
Assignee: Jan Kratochvil
URL:
Keywords:
: 14763 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-04-26 17:20 UTC by H.J. Lu
Modified: 2015-11-04 21:59 UTC (History)
6 users (show)

See Also:
Host: x86_64-*-*
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2013-04-26 17:20:40 UTC
The commit:

commit 6a1bd2a100c958d30bbfe8c9b8f9071d24b7c3f4
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date:   Fri Mar 16 20:49:23 2012 +0100

      * sysdeps/x86_64/elf/start.S: Include <sysdep.h>.
      (_start): Add cfi_startproc, cfi_undefined for rip and cfi_endproc.

adds partial frame info in sysdeps/x86_64/start.S:

[hjl@gnu-6 build-x86_64-linux]$ objdump -drw csu/start.o

csu/start.o:     file format elf64-x86-64


Disassembly of section .text:

0000000000000000 <_start>:
   0:	31 ed                	xor    %ebp,%ebp
   2:	49 89 d1             	mov    %rdx,%r9
   5:	5e                   	pop    %rsi
   6:	48 89 e2             	mov    %rsp,%rdx
   9:	48 83 e4 f0          	and    $0xfffffffffffffff0,%rsp
   d:	50                   	push   %rax
   e:	54                   	push   %rsp
   f:	49 c7 c0 00 00 00 00 	mov    $0x0,%r8	12: R_X86_64_32S	__libc_csu_fini
  16:	48 c7 c1 00 00 00 00 	mov    $0x0,%rcx	19: R_X86_64_32S	__libc_csu_init
  1d:	48 c7 c7 00 00 00 00 	mov    $0x0,%rdi	20: R_X86_64_32S	main
  24:	e8 00 00 00 00       	callq  29 <_start+0x29>	25: R_X86_64_PC32	__libc_start_main-0x4
  29:	f4                   	hlt    
[hjl@gnu-6 build-x86_64-linux]$ readelf -wf csu/start.o
Contents of the .eh_frame section:

00000000 00000014 00000000 CIE
  Version:               1
  Augmentation:          "zR"
  Code alignment factor: 1
  Data alignment factor: -8
  Return address column: 16
  Augmentation data:     1b

  DW_CFA_def_cfa: r7 (rsp) ofs 8
  DW_CFA_offset: r16 (rip) at cfa-8
  DW_CFA_undefined: r16 (rip)

00000018 00000014 0000001c FDE cie=00000000 pc=00000000..0000002a
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop

[hjl@gnu-6 build-x86_64-linux]$ 

Since it ignores stack ops, stack unwind fails in certain cases.
exception_static_test in gold is one example.
Comment 1 H.J. Lu 2013-04-26 19:20:19 UTC
Can't GDB stop unwind when frame pointer is 0?
Comment 2 H.J. Lu 2013-04-26 19:28:03 UTC
i386 crt1.o doesn't have frame info and GDB doesn't have
run away unwind:

Attaching to process 30992
Reading symbols from /export/home/hjl/bugs/libc/start/x...(no debugging symbols found)...done.
Reading symbols from /lib/libc.so.6...(no debugging symbols found)...done.
Loaded symbols for /lib/libc.so.6
Reading symbols from /lib/ld-linux.so.2...(no debugging symbols found)...done.
Loaded symbols for /lib/ld-linux.so.2
0xf7ffd430 in __kernel_vsyscall ()
#0  0xf7ffd430 in __kernel_vsyscall ()
#1  0x4abb8eb6 in __pause_nocancel () from /lib/libc.so.6
#2  0x08048407 in ?? ()
#3  0x4ab1a835 in __libc_start_main () from /lib/libc.so.6
#4  0x08048321 in ?? ()
Missing separate debuginfos, use: debuginfo-install glibc-2.16-30.1.fc18.i686
(gdb)
Comment 3 Jan Kratochvil 2013-04-26 19:31:57 UTC
(In reply to comment #1)
> Can't GDB stop unwind when frame pointer is 0?

While GDB is not 100% zero-correct an address 0 is a perfectly valid address, for both SP and PC, at least in embedded targets.
Comment 4 Jakub Jelinek 2013-04-26 19:40:16 UTC
For a return address (which ought to be after some instruction)?
Comment 5 Roland McGrath 2013-04-26 21:22:36 UTC
HJ, do you think you could produce a (preferably C only, not C++) test case that shows the same problems that gold/exception_static_test experiences?
Comment 6 H.J. Lu 2013-04-26 21:30:28 UTC
(In reply to comment #5)
> HJ, do you think you could produce a (preferably C only, not C++) test case
> that shows the same problems that gold/exception_static_test experiences?

The problem only shows up with C++ exception when linked with gold:

[hjl@gnu-6 testsuite]$ cat /tmp/x.cc 
void
f1()
{
  throw 0;
}

bool
t1()
{
  int i;
  try
    {
      i = 0;
      f1();
      i = 1;
    }
  catch (...)
    {
      return i == 0;
    }

  return false;
}

int
main()
{
  return !t1();
}
[hjl@gnu-6 testsuite]$ g++ -static  /tmp/x.cc -fuse-ld=gold
[hjl@gnu-6 testsuite]$ ./a.out 
Aborted
[hjl@gnu-6 testsuite]$
Comment 7 Roland McGrath 2013-04-26 21:35:10 UTC
But you can investigate the details of the failure, discern what aspect of the inaccurate CFI is problematic, and then write a different test that will demonstrate that wrong unwinding.
Comment 8 H.J. Lu 2013-04-26 21:58:51 UTC
--build-id --no-add-needed --hash-style=gnu -m elf_x86_64 -static -fuse-ld=gold /usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64/crt1.o /usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64/crti.o /usr/lib/gcc/x86_64-redhat-linux/4.7.2/crtbeginT.o -L/usr/lib/gcc/x86_64-redhat-linux/4.7.2 -L/usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64 -L/lib/../lib64 -L/usr/lib/../lib64 -L/usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../.. foo.o -lstdc++ -lm --start-group -lgcc -lgcc_eh -lc --end-group /usr/lib/gcc/x86_64-redhat-linux/4.7.2/crtend.o /usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64/crtn.o

is passed to linker. BFD linker puts the bad eh_frame section at
very beginning:

Contents of the .eh_frame section:

00000000 00000014 00000000 CIE
  Version:               1
  Augmentation:          "zR"
  Code alignment factor: 1
  Data alignment factor: -8
  Return address column: 16
  Augmentation data:     1b

  DW_CFA_def_cfa: r7 (rsp) ofs 8
  DW_CFA_offset: r16 (rip) at cfa-8
  DW_CFA_undefined: r16 (rip)

00000018 00000014 0000001c FDE cie=00000000 pc=00400d8c..00400db6
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop

while gold puts it in the middle:

000099b0 0000004c 000099b4 FDE cie=00000000 pc=004928d0..00492b12
  DW_CFA_advance_loc: 34 to 004928f2
  DW_CFA_def_cfa_offset: 16
  DW_CFA_offset: r14 (r14) at cfa-16
  DW_CFA_advance_loc: 5 to 004928f7
  DW_CFA_def_cfa_offset: 24
  DW_CFA_offset: r13 (r13) at cfa-24
  DW_CFA_advance_loc: 5 to 004928fc
  DW_CFA_def_cfa_offset: 32
  DW_CFA_offset: r12 (r12) at cfa-32
  DW_CFA_advance_loc: 4 to 00492900
  DW_CFA_def_cfa_offset: 40
  DW_CFA_offset: r6 (rbp) at cfa-40
  DW_CFA_advance_loc: 1 to 00492901
  DW_CFA_def_cfa_offset: 48
  DW_CFA_offset: r3 (rbx) at cfa-48
  DW_CFA_advance_loc: 4 to 00492905
  DW_CFA_def_cfa_offset: 128
  DW_CFA_advance_loc2: 382 to 00492a83
  DW_CFA_remember_state
  DW_CFA_def_cfa_offset: 48
  DW_CFA_advance_loc: 1 to 00492a84
  DW_CFA_restore: r3 (rbx)
  DW_CFA_def_cfa_offset: 40
  DW_CFA_advance_loc: 1 to 00492a85
  DW_CFA_restore: r6 (rbp)
  DW_CFA_def_cfa_offset: 32
  DW_CFA_advance_loc: 2 to 00492a87
  DW_CFA_restore: r12 (r12)
  DW_CFA_def_cfa_offset: 24
  DW_CFA_advance_loc: 2 to 00492a89
  DW_CFA_restore: r13 (r13)
  DW_CFA_def_cfa_offset: 16
  DW_CFA_advance_loc: 2 to 00492a8b
  DW_CFA_restore: r14 (r14)
  DW_CFA_def_cfa_offset: 8
  DW_CFA_advance_loc: 5 to 00492a90
  DW_CFA_restore_state
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop

00009a00 00000024 00009a04 FDE cie=00000000 pc=004002e0..004003b0
  DW_CFA_def_cfa_offset: 16
  DW_CFA_advance_loc: 6 to 004002e6
  DW_CFA_def_cfa_offset: 24
  DW_CFA_advance_loc: 10 to 004002f0
  DW_CFA_def_cfa_expression (DW_OP_breg7 (rsp): 8; DW_OP_breg16 (rip): 0; DW_OP_lit15; DW_OP_and; DW_OP_lit11; DW_OP_ge; DW_OP_lit3; DW_OP_shl; DW_OP_plus)
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop

00009a28 00000014 00000000 CIE
  Version:               1
  Augmentation:          "zR"
  Code alignment factor: 1
  Data alignment factor: -8
  Return address column: 16
  Augmentation data:     1b

  DW_CFA_def_cfa: r7 (rsp) ofs 8
  DW_CFA_offset: r16 (rip) at cfa-8
  DW_CFA_undefined: r16 (rip)

00009a40 00000014 0000001c FDE cie=00009a28 pc=00400d9c..00400dc6
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop

00009a58 00000014 00000000 CIE
  Version:               1
  Augmentation:          "zR"
  Code alignment factor: 1
  Data alignment factor: -8
  Return address column: 16
  Augmentation data:     1b

  DW_CFA_def_cfa: r7 (rsp) ofs 8
  DW_CFA_offset: r16 (rip) at cfa-8
  DW_CFA_def_cfa_offset: 24

00009a70 00000014 0000001c FDE cie=00009a58 pc=00451540..004515a1
  DW_CFA_advance_loc: 4 to 00451544

The extra frame info comes from the first entry in PLT since gold
doesn't skip it in static executable.  I think this combination
leads to the problem.

Dynamic executables are OK since they have GNU_EH_FRAME.
Comment 9 Roland McGrath 2013-04-26 22:17:34 UTC
I don't understand how the extra FDEs gold emits would be related.
All the FDEs are for disjoint PC ranges, so why would the unwinder even so those others?  Please investigate further what exactly is really going on.  The only thing I can imagine (other than unrelated unwinder bugs) is that the bad CFI for _start is causing the unwinder to think that a PC in one of the PLT ranges is part of the call stack, but given that it's cfi_undefined (rip), I can't think of a way that could be true.
Comment 10 Jan Kratochvil 2013-04-27 16:31:15 UTC
$ gdb -ex 'b __register_frame_info_bases' -ex 'b __register_frame_info_table_bases' -ex 'b _Unwind_Find_registered_FDE' -ex r ./hjl
Breakpoint 1, __register_frame_info_bases (begin=0x4c29d0, ob=0x0, tbase=0x0, dbase=0x0) at ../../../gcc48/libgcc/unwind-dw2-fde.c:81
80	  /* If .eh_frame is empty, don't register at all.  */
81	  if ((const uword *) begin == 0 || *(const uword *) begin == 0)
82	    return;
(gdb) p *(const uword *) begin
$2 = 0
(gdb) up
#1  0x0000000000400fdd in frame_dummy ()
(gdb) disas
Dump of assembler code for function frame_dummy:
   0x0000000000400fd3 <+19>:	mov    $0x4c29d0,%edi
   0x0000000000400fd8 <+24>:	callq  0x40f7d0 <__register_frame_info>
=> 0x0000000000400fdd <+29>:	cmpq   $0x0,0xc4103(%rip)        # 0x4c50e8
End of assembler dump.
(gdb) p &__EH_FRAME_BEGIN__
$3 = (<data variable, no debug info> *) 0x4c29d0
(gdb) p &__FRAME_END__
$4 = (<data variable, no debug info> *) 0x4c29d0

With gold there are no static frames registered, I did not investigate it more but _start in glibc seems unrelated to it.  Does really reverting the glibc patch of mine fix it for you?  (I did not test reverting it.)

The Roland's posted patch looks right; but it seems unrelated.
Comment 11 H.J. Lu 2013-04-29 16:11:31 UTC
We have

[hjl@gnu-6 start-2]$ g++ foo.o -v   
...
 /usr/libexec/gcc/x86_64-redhat-linux/4.7.2/collect2 --build-id --no-add-needed --eh-frame-hdr --hash-style=gnu -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2 /usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64/crt1.o /usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64/crti.o /usr/lib/gcc/x86_64-redhat-linux/4.7.2/crtbegin.o -L/usr/lib/gcc/x86_64-redhat-linux/4.7.2 -L/usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64 -L/lib/../lib64 -L/usr/lib/../lib64 -L/usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../.. foo.o -lstdc++ -lm -lgcc_s -lgcc -lc -lgcc_s -lgcc /usr/lib/gcc/x86_64-redhat-linux/4.7.2/crtend.o /usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64/crtn.o
[hjl@gnu-6 start-2]$ 

All EH frame info should be between crtbegin[ST].o and crtend.o. When
EH frame is added in crt1.o, it is placed before crtbegin.o.  It isn't a
problem for crtbegin[S].o since it doesn't define __EH_FRAME_BEGIN__
and bfd linker effectively ignores EH frame in crt1.o.  When gold sees EH
frame before crtbeginT.o, which is used to mark the start of EH frame with
__EH_FRAME_BEGIN__, it places .eh_frame section in crtbeginT.o to the end
of output .eh_frame section.  It is quite odd to place any EH frame info
before __EH_FRAME_BEGIN__.
Comment 12 H.J. Lu 2013-04-29 16:53:00 UTC
*** Bug 14763 has been marked as a duplicate of this bug. ***
Comment 13 Jan Kratochvil 2013-04-30 15:36:38 UTC
I think the markers like __EH_FRAME_BEGIN__ could be moved from crtbegin.o to some new crtbegin1.o linking it then in the order:

/usr/lib/gcc/x86_64-redhat-linux/4.7.2/crtbegin1.o
/usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64/crt1.o
/usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64/crti.o
/usr/lib/gcc/x86_64-redhat-linux/4.7.2/crtbegin.o

I will try to code it in GCC, I hope there is an agreement for it.
Comment 14 H.J. Lu 2013-04-30 15:45:49 UTC
(In reply to comment #13)
> I think the markers like __EH_FRAME_BEGIN__ could be moved from crtbegin.o to
> some new crtbegin1.o linking it then in the order:
> 
> /usr/lib/gcc/x86_64-redhat-linux/4.7.2/crtbegin1.o
> /usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64/crt1.o
> /usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../lib64/crti.o
> /usr/lib/gcc/x86_64-redhat-linux/4.7.2/crtbegin.o
> 
> I will try to code it in GCC, I hope there is an agreement for it.

I don't know if it works for gold.  Or we can use --eh-frame-hdr
for static executables:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54568
Comment 15 Jan Kratochvil 2013-05-05 18:40:08 UTC
FYI that I am working on it:
  http://people.redhat.com/jkratoch/crt.patch

Trying now to test if I have not broken:
  --target=i686-pc-linux-gnu --enable-targets=all
Comment 16 Jan Kratochvil 2013-05-06 17:23:25 UTC
[patch] Support .eh_frame in crt1 x86_64 glibc (PR libc/15407)
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00275.html
Comment 17 H.J. Lu 2013-05-09 15:25:55 UTC
i386 crt1.o doesn't have EH frame info.  Why isn't it a problem?
Comment 18 Jan Kratochvil 2013-05-10 16:33:43 UTC
Because GDB handles i386 vs. x86_64 non-CFI frame pointer register differently.
Probably because historically i386 used -fno-omit-frame-pointer even for -O code while x86_64 always used -fomit-frame-pointer for -O code.

i386_frame_cache_1:
  get_frame_register (this_frame, I386_EBP_REGNUM, buf);
  cache->base = extract_unsigned_integer (buf, 4, byte_order);

amd64_frame_cache_1:
  if (cache->frameless_p)
          get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
          cache->base = extract_unsigned_integer (buf, 8, byte_order)
  else
      get_frame_register (this_frame, AMD64_RBP_REGNUM, buf);
      cache->base = extract_unsigned_integer (buf, 8, byte_order);

And _start is FRAMELESS_P because one cannot find (for stripped binary) its function boundaries.

Besides that every instruction should have CFI, otherwise it makes unwinders more complicated and less reliable.  i386 _start should also get CFI.
Comment 19 H.J. Lu 2013-05-10 16:59:17 UTC
How does GDB know FRAMELESS_P is true for x86-64 _start?
There is no difference between i386 and x86-64 _start
in stack frame info. It seems to me that FRAMELESS_P should
be false if there is no CFI.
Comment 20 Jan Kratochvil 2013-05-10 17:12:50 UTC
All GDBs out there behave as described above.

IIUC you are proposing to revert the glibc x86_64 _start CFI patch and change GDB x86_64 to consider non-CFI code as using %rbp frame pointer.

I do not find it safe, x86_64 code may use %rbp for arbitrary data - even zero - which will lead to quiet backtrace stop if CFI cannot be found for such code.  Currently GDB will do unwind into some garbage address so the user sees there is something broken (due to CFI failed to be found).

i386 already is not safe for accidental %ebp==0 non-CFI unwind stops but i386 is obsoleted by x86_64 + x32 so it is IMO not much worth trying to improve anything there anymore.
Comment 21 H.J. Lu 2013-05-10 17:22:50 UTC
The normal x86-64 codes always have CFI and GDB doesn't need to worry
about zero BP.  For codes without CFI, there are 3 cases:

1. _start.  GDB can use BP
2. It does have BP.  GDB can use BP.
3. It doesn't have BP.  There is no-win for GDB.

It seems to me that it is OK for GDB to use BP if there
is no CFI.
Comment 22 Jan Kratochvil 2013-05-10 17:32:15 UTC
(In reply to comment #21)
> 3. It doesn't have BP.  There is no-win for GDB.

The win is to unwind some bogus record like

#7  0x0000000000000000 in ?? ()

If GDB does not say anything bad and GDB finishes the backtrace it is GDB bug.
Comment 23 H.J. Lu 2013-05-10 17:44:57 UTC
For codes without CFI, what does GDB today do for

1. _start.
2. It does have BP.
3. It doesn't have BP.

What will GDB do if it is changed to assume there is BP?
Comment 24 Jan Kratochvil 2013-05-10 18:26:07 UTC
(In reply to comment #23)
> For codes without CFI, what does GDB today do for
> 
> 1. _start.

Incorrectly performs the unwind not stopping here, as discussed above.


> 2. It does have BP.

Incorrectly unwinds stripped code as GDB cannot find start of the function (and therefore its prologue).  For non-stripped (ELF symbols) code unwinding works.


> 3. It doesn't have BP.

Attempts to re-sync backtrace:

(gdb) run
Program received signal SIGTRAP, Trace/breakpoint trap.
(gdb) bt
#0  0x0000000000400529 in f ()
#1  0x6f77206f6c6c6568 in ?? ()
#2  0x0000000000646c72 in ?? ()
#3  0x0000000000400533 in main ()


> What will GDB do if it is changed to assume there is BP?

2. It does have BP.

Correctly unwinds stripped code.  But such code does not exist, stripped code is -O2 code and that was always using -fomit-frame-pointer on x86_64.


3. It doesn't have BP.

It will incorrectly stop backtrace if GDB accidentally finds %rbp == 0 which:

 * Does not indicate to user something went wrong during unwinding.

 * Does not try to re-sync backtrace for a possibly later found correct frame
   on stack.

(gdb) run
Program received signal SIGTRAP, Trace/breakpoint trap.
(gdb) bt
#0  0x0000000000400529 in f ()


void *p;
static void g(void) {
  p=p;
}
static void f(void) {
  char a[]="hello world";
  p=a;
  g();
  asm volatile ("int3");
}
int main() {
  f();
  return 0;
}

-fomit-frame-pointer -fno-asynchronous-unwind-tables
Comment 25 H.J. Lu 2013-05-10 20:33:54 UTC
(In reply to comment #24)

> void *p;
> static void g(void) {
>   p=p;
> }
> static void f(void) {
>   char a[]="hello world";
>   p=a;
>   g();
>   asm volatile ("int3");
> }
> int main() {
>   f();
>   return 0;
> }
> 
> -fomit-frame-pointer -fno-asynchronous-unwind-tables

Re-sync backtrace works by guess.  However, people are
using -fno-mit-frame-pointer -fno-asynchronous-unwind-tables
to get faster stack unwind.
Comment 26 Jan Kratochvil 2013-05-11 05:27:36 UTC
(In reply to comment #25)
> However, people are using -fno-mit-frame-pointer
> -fno-asynchronous-unwind-tables to get faster stack unwind.

In such case their binary is not stripped, therefore GDB can find start of the function and its prologue, therefore what is default FRAMELESS_P does not matter in such case.