[PATCH] Fix wrong assertions

[PATCH] Fix wrong assertions

[Andreas Schwab]

Both callers and callees are lengths, and the sum of them can validly
equal the total length.

Andreas.

	PR symtab/18392
	* dwarf2-frame-tailcall.c (pretended_chain_levels): Correct
	assertion.
	* dwarf2loc.c (chain_candidate): Likewise.
---
 gdb/dwarf2-frame-tailcall.c | 2 +-
 gdb/dwarf2loc.c             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2-frame-tailcall.c b/gdb/dwarf2-frame-tailcall.c
index b412a5b..f964ab2 100644
--- a/gdb/dwarf2-frame-tailcall.c
+++ b/gdb/dwarf2-frame-tailcall.c
@@ -197,7 +197,7 @@ pretended_chain_levels (struct call_site_chain *chain)
     return chain->length;
 
   chain_levels = chain->callers + chain->callees;
-  gdb_assert (chain_levels < chain->length);
+  gdb_assert (chain_levels <= chain->length);
 
   return chain_levels;
 }
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index d811106..29b265b 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -827,7 +827,7 @@ chain_candidate (struct gdbarch *gdbarch, struct call_site_chain **resultp,
      PC again.  In such case there must be two different code paths to reach
      it, therefore some of the former determined intermediate PCs must differ
      and the unambiguous chain gets shortened.  */
-  gdb_assert (result->callers + result->callees < result->length);
+  gdb_assert (result->callers + result->callees <= result->length);
 }
 
 /* Create and return call_site_chain for CALLER_PC and CALLEE_PC.  All the
-- 
2.4.0

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Fix wrong assertions

[Jan Kratochvil]

On Sat, 09 May 2015 20:56:55 +0200, Andreas Schwab wrote:
> Both callers and callees are lengths, and the sum of them can validly
> equal the total length.

That '<' and not '<=' was there intentional.  Personally I think it needs more
investigation why that can happen.  The idea was that if two solutions exist
neither can be perfect so there have to be some ambiguous enties so there will
be '<' and not '<=' (to fit the ambiguous entries between).

But creating artifical reproducers is a bit difficult and you haven't provided
a reproducer so I cannot say anything much specific.

Personally I do not mind, it is up to the maintainers whether the goal is just
to remove the assertion or verify there aren't some other bugs causing it.


Jan

Re: [PATCH] Fix wrong assertions

[Andreas Schwab]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> But creating artifical reproducers is a bit difficult and you haven't provided
> a reproducer so I cannot say anything much specific.

Break on reload1.c:2917 (which is "return replace_equiv_address_nv (x,
new_rtx);") and try to step into it.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[patch] testcase: tailcall assertion [Re: [PATCH] Fix wrong assertions]

[Jan Kratochvil]

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

On Sat, 09 May 2015 20:56:55 +0200, Andreas Schwab wrote:
> Both callers and callees are lengths, and the sum of them can validly
> equal the total length.

This can happen for functions tail-calling themselves.

This is what GCC generated for cc1.  I was unable to create a .c file
reproducing it (GCC does not generate tail-call call site DWARF for the
self-tail-call) but a hand crafted DWARF can crash GDB like cc1 does.


Thanks,
Jan

[-- Attachment #2: 2 --]
[-- Type: text/plain, Size: 21172 bytes --]

gdb/testsuite/ChangeLog
2015-05-19  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.arch/amd64-tailcall-self.S: New file.
	* gdb.arch/amd64-tailcall-self.c: New file.
	* gdb.arch/amd64-tailcall-self.exp: New file.

diff --git a/gdb/testsuite/gdb.arch/amd64-tailcall-self.S b/gdb/testsuite/gdb.arch/amd64-tailcall-self.S
new file mode 100644
index 0000000..b1ca7c8
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-tailcall-self.S
@@ -0,0 +1,614 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This source file was generated by:
+   gcc -o gdb.arch/amd64-tailcall-self.S -S gdb.arch/amd64-tailcall-self.c -Wall -g -dA -Os
+   */
+
+	.file	"amd64-tailcall-self.c"
+	.text
+.Ltext0:
+	.section	.text.unlikely,"ax",@progbits
+.LCOLDB0:
+	.text
+.LHOTB0:
+	.section	.text.unlikely
+.Ltext_cold0:
+	.text
+	.type	b, @function
+b:
+.LFB0:
+	.file 1 "amd64-tailcall-self.c"
+	# amd64-tailcall-self.c:1
+	.loc 1 1 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# amd64-tailcall-self.c:2
+	.loc 1 2 0
+# SUCC: EXIT [100.0%] 
+	ret
+	.cfi_endproc
+.LFE0:
+	.size	b, .-b
+	.section	.text.unlikely
+.LCOLDE0:
+	.text
+.LHOTE0:
+	.section	.text.unlikely
+.LCOLDB1:
+	.text
+.LHOTB1:
+	.globl	c
+	.type	c, @function
+c:
+.LFB1:
+	# amd64-tailcall-self.c:7
+	.loc 1 7 0
+	.cfi_startproc
+.LVL0:
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# amd64-tailcall-self.c:8
+	.loc 1 8 0
+	leal	1(%rdi), %eax
+# SUCC: EXIT [100.0%] 
+	# amd64-tailcall-self.c:9
+	.loc 1 9 0
+	ret
+	.cfi_endproc
+.LFE1:
+	.size	c, .-c
+	.section	.text.unlikely
+.LCOLDE1:
+	.text
+.LHOTE1:
+	.section	.text.unlikely
+.LCOLDB2:
+	.text
+.LHOTB2:
+	.globl	a
+	.type	a, @function
+a:
+.LFB2:
+	# amd64-tailcall-self.c:11
+	.loc 1 11 0
+	.cfi_startproc
+.LVL1:
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU) 3 [100.0%]  (DFS_BACK,CAN_FALLTHRU)
+.L5:
+	# amd64-tailcall-self.c:12
+	.loc 1 12 0
+#APP
+# 12 "amd64-tailcall-self.c" 1
+	nop;nop;nop
+# 0 "" 2
+	# amd64-tailcall-self.c:13
+	.loc 1 13 0
+#NO_APP
+	movl	i(%rip), %edi
+	leal	-1(%rdi), %eax
+	cmpl	$9, %eax
+# SUCC: 3 [91.0%]  (FALLTHRU,CAN_FALLTHRU) 4 [9.0%]  (CAN_FALLTHRU,LOOP_EXIT)
+	ja	.L4
+# BLOCK 3 freq:9100 seq:1
+# PRED: 2 [91.0%]  (FALLTHRU,CAN_FALLTHRU)
+	# amd64-tailcall-self.c:14
+	.loc 1 14 0
+	orl	$1, %edi
+	movl	%edi, i(%rip)
+# SUCC: 2 [100.0%]  (DFS_BACK,CAN_FALLTHRU)
+	jmp	.L5
+.Lfirsttailcall:
+# BLOCK 4 freq:900 seq:2
+# PRED: 2 [9.0%]  (CAN_FALLTHRU,LOOP_EXIT)
+.L4:
+	# amd64-tailcall-self.c:16
+	.loc 1 16 0
+	cmpl	$0, %edi
+# SUCC: 5 [19.9%]  (FALLTHRU,CAN_FALLTHRU) 6 [80.1%]  (CAN_FALLTHRU)
+	jne	.L3
+# BLOCK 5 freq:179 seq:3
+# PRED: 4 [19.9%]  (FALLTHRU,CAN_FALLTHRU)
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+	# amd64-tailcall-self.c:17
+	.loc 1 17 0
+	jmp	b
+.LVL2:
+# BLOCK 6 freq:721 seq:4
+# PRED: 4 [80.1%]  (CAN_FALLTHRU)
+.L3:
+# SUCC: EXIT [100.0%] 
+	ret
+	.cfi_endproc
+.LFE2:
+	.size	a, .-a
+	.section	.text.unlikely
+.LCOLDE2:
+	.text
+.LHOTE2:
+	.section	.text.unlikely
+.LCOLDB3:
+	.section	.text.startup,"ax",@progbits
+.LHOTB3:
+	.globl	main
+	.type	main, @function
+main:
+.LFB3:
+	# amd64-tailcall-self.c:20
+	.loc 1 20 0
+	.cfi_startproc
+.LVL3:
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# amd64-tailcall-self.c:21
+	.loc 1 21 0
+	call	a
+.LVL4:
+	# amd64-tailcall-self.c:23
+	.loc 1 23 0
+	xorl	%eax, %eax
+# SUCC: EXIT [100.0%] 
+	ret
+	.cfi_endproc
+.LFE3:
+	.size	main, .-main
+	.section	.text.unlikely
+.LCOLDE3:
+	.section	.text.startup
+.LHOTE3:
+	.comm	i,4,4
+	.text
+.Letext0:
+	.section	.text.unlikely
+.Letext_cold0:
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.long	2f-1f	# Length of Compilation Unit Info
+1:
+	.value	0x4	# DWARF version number
+	.long	.Ldebug_abbrev0	# Offset Into Abbrev. Section
+	.byte	0x8	# Pointer Size (in bytes)
+	.uleb128 0x1	# (DIE (0xb) DW_TAG_compile_unit)
+	.long	.LASF4	# DW_AT_producer: "GNU C 4.9.2 20150212 (Red Hat 4.9.2-6) -mtune=generic -march=x86-64 -g -Os"
+	.byte	0x1	# DW_AT_language
+	.long	.LASF5	# DW_AT_name: "amd64-tailcall-self.c"
+	.long	.LASF6	# DW_AT_comp_dir: "/home/jkratoch/t"
+	.long	.Ldebug_ranges0+0	# DW_AT_ranges
+	.quad	0	# DW_AT_low_pc
+	.long	.Ldebug_line0	# DW_AT_stmt_list
+die29:
+	.uleb128 0x2	# (DIE (0x29) DW_TAG_subprogram)
+	.ascii "b\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (amd64-tailcall-self.c)
+	.byte	0x1	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.quad	.LFB0	# DW_AT_low_pc
+	.quad	.LFE0-.LFB0	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x3	# (DIE (0x40) DW_TAG_subprogram)
+			# DW_AT_external
+	.ascii "c\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (amd64-tailcall-self.c)
+	.byte	0x7	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	die6b	# DW_AT_type
+	.quad	.LFB1	# DW_AT_low_pc
+	.quad	.LFE1-.LFB1	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x4	# (DIE (0x5f) DW_TAG_formal_parameter)
+	.ascii "q\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (amd64-tailcall-self.c)
+	.byte	0x7	# DW_AT_decl_line
+	.long	die6b	# DW_AT_type
+	.uleb128 0x1	# DW_AT_location
+	.byte	0x55	# DW_OP_reg5
+	.byte	0	# end of children of DIE 0x40
+die6b:
+	.uleb128 0x5	# (DIE (0x6b) DW_TAG_base_type)
+	.byte	0x4	# DW_AT_byte_size
+	.byte	0x5	# DW_AT_encoding
+	.ascii "int\0"	# DW_AT_name
+die72:
+	.uleb128 0x6	# (DIE (0x72) DW_TAG_subprogram)
+			# DW_AT_external
+	.ascii "a\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (amd64-tailcall-self.c)
+	.byte	0xb	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.quad	.LFB2	# DW_AT_low_pc
+	.quad	.LFE2-.LFB2	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x4	# (DIE (0x8d) DW_TAG_formal_parameter)
+	.ascii "q\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (amd64-tailcall-self.c)
+	.byte	0xb	# DW_AT_decl_line
+	.long	die6b	# DW_AT_type
+	.uleb128 0x4	# DW_AT_location
+	.byte	0xf3	# DW_OP_GNU_entry_value
+	.uleb128 0x1
+	.byte	0x55	# DW_OP_reg5
+	.byte	0x9f	# DW_OP_stack_value
+	.uleb128 0x7	# (DIE () DW_TAG_GNU_call_site)
+	.quad	.Lfirsttailcall	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	die72	# DW_AT_abstract_origin
+	.uleb128 0x7	# (DIE (0x9b) DW_TAG_GNU_call_site)
+	.quad	.LVL2	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	die29	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0x72
+	.uleb128 0x8	# (DIE (0xa9) DW_TAG_subprogram)
+			# DW_AT_external
+	.long	.LASF0	# DW_AT_name: "main"
+	.byte	0x1	# DW_AT_decl_file (amd64-tailcall-self.c)
+	.byte	0x14	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	die6b	# DW_AT_type
+	.quad	.LFB3	# DW_AT_low_pc
+	.quad	.LFE3-.LFB3	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x9	# (DIE (0xca) DW_TAG_formal_parameter)
+	.long	.LASF1	# DW_AT_name: "argc"
+	.byte	0x1	# DW_AT_decl_file (amd64-tailcall-self.c)
+	.byte	0x14	# DW_AT_decl_line
+	.long	die6b	# DW_AT_type
+	.long	.LLST0	# DW_AT_location
+	.uleb128 0x9	# (DIE (0xd9) DW_TAG_formal_parameter)
+	.long	.LASF2	# DW_AT_name: "argv"
+	.byte	0x1	# DW_AT_decl_file (amd64-tailcall-self.c)
+	.byte	0x14	# DW_AT_decl_line
+	.long	diefe	# DW_AT_type
+	.long	.LLST1	# DW_AT_location
+	.uleb128 0xa	# (DIE (0xe8) DW_TAG_GNU_call_site)
+	.quad	.LVL4	# DW_AT_low_pc
+	.long	die72	# DW_AT_abstract_origin
+	.uleb128 0xb	# (DIE (0xf5) DW_TAG_GNU_call_site_parameter)
+	.uleb128 0x1	# DW_AT_location
+	.byte	0x55	# DW_OP_reg5
+	.uleb128 0x3	# DW_AT_GNU_call_site_value
+	.byte	0xf3	# DW_OP_GNU_entry_value
+	.uleb128 0x1
+	.byte	0x55	# DW_OP_reg5
+	.byte	0	# end of children of DIE 0xe8
+	.byte	0	# end of children of DIE 0xa9
+diefe:
+	.uleb128 0xc	# (DIE (0xfe) DW_TAG_pointer_type)
+	.byte	0x8	# DW_AT_byte_size
+	.long	die104	# DW_AT_type
+die104:
+	.uleb128 0xc	# (DIE (0x104) DW_TAG_pointer_type)
+	.byte	0x8	# DW_AT_byte_size
+	.long	die10a	# DW_AT_type
+die10a:
+	.uleb128 0xd	# (DIE (0x10a) DW_TAG_base_type)
+	.byte	0x1	# DW_AT_byte_size
+	.byte	0x6	# DW_AT_encoding
+	.long	.LASF3	# DW_AT_name: "char"
+	.uleb128 0xe	# (DIE (0x111) DW_TAG_variable)
+	.ascii "i\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (amd64-tailcall-self.c)
+	.byte	0x5	# DW_AT_decl_line
+	.long	die6b	# DW_AT_type
+			# DW_AT_external
+	.uleb128 0x9	# DW_AT_location
+	.byte	0x3	# DW_OP_addr
+	.quad	i
+	.byte	0	# end of children of DIE 0xb
+2:
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1	# (abbrev code)
+	.uleb128 0x11	# (TAG: DW_TAG_compile_unit)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x25	# (DW_AT_producer)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x13	# (DW_AT_language)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x1b	# (DW_AT_comp_dir)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x55	# (DW_AT_ranges)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x10	# (DW_AT_stmt_list)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0x2	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x3	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3f	# (DW_AT_external)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x4	# (abbrev code)
+	.uleb128 0x5	# (TAG: DW_TAG_formal_parameter)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x2	# (DW_AT_location)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.byte	0
+	.byte	0
+	.uleb128 0x5	# (abbrev code)
+	.uleb128 0x24	# (TAG: DW_TAG_base_type)
+	.byte	0	# DW_children_no
+	.uleb128 0xb	# (DW_AT_byte_size)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3e	# (DW_AT_encoding)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.byte	0
+	.byte	0
+	.uleb128 0x6	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3f	# (DW_AT_external)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x7	# (abbrev code)
+	.uleb128 0x4109	# (TAG: DW_TAG_GNU_call_site)
+	.byte	0	# DW_children_no
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x2115	# (DW_AT_GNU_tail_call)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x31	# (DW_AT_abstract_origin)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x8	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3f	# (DW_AT_external)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x9	# (abbrev code)
+	.uleb128 0x5	# (TAG: DW_TAG_formal_parameter)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x2	# (DW_AT_location)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0xa	# (abbrev code)
+	.uleb128 0x4109	# (TAG: DW_TAG_GNU_call_site)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x31	# (DW_AT_abstract_origin)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0xb	# (abbrev code)
+	.uleb128 0x410a	# (TAG: DW_TAG_GNU_call_site_parameter)
+	.byte	0	# DW_children_no
+	.uleb128 0x2	# (DW_AT_location)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2111	# (DW_AT_GNU_call_site_value)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.byte	0
+	.byte	0
+	.uleb128 0xc	# (abbrev code)
+	.uleb128 0xf	# (TAG: DW_TAG_pointer_type)
+	.byte	0	# DW_children_no
+	.uleb128 0xb	# (DW_AT_byte_size)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0xd	# (abbrev code)
+	.uleb128 0x24	# (TAG: DW_TAG_base_type)
+	.byte	0	# DW_children_no
+	.uleb128 0xb	# (DW_AT_byte_size)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3e	# (DW_AT_encoding)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.byte	0
+	.byte	0
+	.uleb128 0xe	# (abbrev code)
+	.uleb128 0x34	# (TAG: DW_TAG_variable)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x3f	# (DW_AT_external)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x2	# (DW_AT_location)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.byte	0
+	.byte	0
+	.byte	0
+	.section	.debug_loc,"",@progbits
+.Ldebug_loc0:
+.LLST0:
+	.quad	.LVL3	# Location list begin address (*.LLST0)
+	.quad	.LVL4-1	# Location list end address (*.LLST0)
+	.value	0x1	# Location expression size
+	.byte	0x55	# DW_OP_reg5
+	.quad	.LVL4-1	# Location list begin address (*.LLST0)
+	.quad	.LFE3	# Location list end address (*.LLST0)
+	.value	0x4	# Location expression size
+	.byte	0xf3	# DW_OP_GNU_entry_value
+	.uleb128 0x1
+	.byte	0x55	# DW_OP_reg5
+	.byte	0x9f	# DW_OP_stack_value
+	.quad	0	# Location list terminator begin (*.LLST0)
+	.quad	0	# Location list terminator end (*.LLST0)
+.LLST1:
+	.quad	.LVL3	# Location list begin address (*.LLST1)
+	.quad	.LVL4-1	# Location list end address (*.LLST1)
+	.value	0x1	# Location expression size
+	.byte	0x54	# DW_OP_reg4
+	.quad	.LVL4-1	# Location list begin address (*.LLST1)
+	.quad	.LFE3	# Location list end address (*.LLST1)
+	.value	0x4	# Location expression size
+	.byte	0xf3	# DW_OP_GNU_entry_value
+	.uleb128 0x1
+	.byte	0x54	# DW_OP_reg4
+	.byte	0x9f	# DW_OP_stack_value
+	.quad	0	# Location list terminator begin (*.LLST1)
+	.quad	0	# Location list terminator end (*.LLST1)
+	.section	.debug_aranges,"",@progbits
+	.long	0x3c	# Length of Address Ranges Info
+	.value	0x2	# DWARF Version
+	.long	.Ldebug_info0	# Offset of Compilation Unit Info
+	.byte	0x8	# Size of Address
+	.byte	0	# Size of Segment Descriptor
+	.value	0	# Pad to 16 byte boundary
+	.value	0
+	.quad	.Ltext0	# Address
+	.quad	.Letext0-.Ltext0	# Length
+	.quad	.LFB3	# Address
+	.quad	.LFE3-.LFB3	# Length
+	.quad	0
+	.quad	0
+	.section	.debug_ranges,"",@progbits
+.Ldebug_ranges0:
+	.quad	.Ltext0	# Offset 0
+	.quad	.Letext0
+	.quad	.LFB3	# Offset 0x10
+	.quad	.LFE3
+	.quad	0
+	.quad	0
+	.section	.debug_line,"",@progbits
+.Ldebug_line0:
+	.section	.debug_str,"MS",@progbits,1
+.LASF1:
+	.string	"argc"
+.LASF0:
+	.string	"main"
+.LASF4:
+	.string	"GNU C 4.9.2 20150212 (Red Hat 4.9.2-6) -mtune=generic -march=x86-64 -g -Os"
+.LASF5:
+	.string	"amd64-tailcall-self.c"
+.LASF6:
+	.string	""
+.LASF3:
+	.string	"char"
+.LASF2:
+	.string	"argv"
+	.ident	"GCC: (GNU) 4.9.2 20150212 (Red Hat 4.9.2-6)"
diff --git a/gdb/testsuite/gdb.arch/amd64-tailcall-self.c b/gdb/testsuite/gdb.arch/amd64-tailcall-self.c
new file mode 100644
index 0000000..0b40dcb
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-tailcall-self.c
@@ -0,0 +1,40 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+__attribute__((noinline,noclone)) static void b(void) {
+  asm volatile("");
+}
+
+int i;
+
+__attribute__((noinline,noclone)) int c(int q) {
+  return q+1;
+}
+
+__attribute__((noinline,noclone)) void a(int q) {
+  asm volatile("nop;nop;nop"::"g"(q):"memory");
+  if (i>=1&&i<=10) {
+    i|=1;
+    a(i);
+  } else if (i==0)
+    b();
+}
+
+int main(int argc,char **argv) {
+  a(argc);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-tailcall-self.exp b/gdb/testsuite/gdb.arch/amd64-tailcall-self.exp
new file mode 100644
index 0000000..0803ff2
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-tailcall-self.exp
@@ -0,0 +1,31 @@
+# Copyright (C) 2015 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile .S
+
+if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
+    verbose "Skipping ${testfile}."
+    return
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {}] } {
+    return -1
+}
+
+if ![runto b] {
+    return -1
+}
+
+gdb_test "bt" "#0 +b \\(\\) at \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in a \\(q=<optimized out>\\) at \[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in main \\(\[^\r\n\]*\\) at .*"

Re: [PATCH] Fix wrong assertions

[Yao Qi]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> That '<' and not '<=' was there intentional.  Personally I think it needs more
> investigation why that can happen.  The idea was that if two solutions exist
> neither can be perfect so there have to be some ambiguous enties so there will
> be '<' and not '<=' (to fit the ambiguous entries between).
>
> But creating artifical reproducers is a bit difficult and you haven't provided
> a reproducer so I cannot say anything much specific.
>
> Personally I do not mind, it is up to the maintainers whether the goal is just
> to remove the assertion or verify there aren't some other bugs causing it.

I can reproduce this internal error via Jan's test case, and spend some
time investigating it, but still unable to fully understand the code.

Jan, since you wrote this piece of code, please help me to understand
it.

(gdb) set debug entry-values 1
(gdb) bt
tailcall: initial: 0x40052e(a)
tailcall: compare: 0x400527(a) 0x40052e(a)
tailcall: reduced: | 0x40052e(a)
gdb/git/gdb/dwarf2loc.c:834: internal-error: chain_candidate: Assertion `result->callers + result->callees < result->length' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) y

I don't know why we need do intersection in chain_candidate, as the
comments say:

/* Intersect RESULTP with CHAIN to keep RESULTP unambiguous, keep in RESULTP
   only top callers and bottom callees which are present in both.  GDBARCH is
   used only for ENTRY_VALUES_DEBUG.  RESULTP is NULL after return if there are
   no remaining possibilities to provide unambiguous non-trivial result.
   RESULTP should point to NULL on the first (initialization) call.  Caller is
   responsible for xfree of any RESULTP data.  */

What do you mean by "ambiguous" here?  Is it ambiguous if we can get
more than one call chain path from caller_pc to callee_pc?  For example,
main tail calls a, a tail call b and c, b and c tail call d, when GDB
unwinds from d, there are two chains, main -> a -> b -> d, and main -> a
-> c -> d.  Are they ambiguous by your definition?

Further, what is "partially ambiguous result" in the comments below?

/* Determined tail calls for constructing virtual tail call frames.  */

struct call_site_chain
  {
    /* Initially CALLERS == CALLEES == LENGTH.  For partially ambiguous result
       CALLERS + CALLEES < LENGTH.  */
    int callers, callees, length;

    /* Variably sized array with LENGTH elements.  Later [0..CALLERS-1] contain
       top (GDB "prev") sites and [LENGTH-CALLEES..LENGTH-1] contain bottom
       (GDB "next") sites.  One is interested primarily in the PC field.  */
    struct call_site *call_site[1];
  };

I am confused by the usage of the variable-sized array call_site,
elements from 0 to CALLERS-1 are top sites, and elements from
LENGTH-CALLEES to LENGTH-1 are bottom sites, so I conclude that
CALLERS-1 < LENGTH-CALLEES, then CALLERS + CALLEES < LENGTH + 1,
then CALLERS + CALLEES =< LENGTH.  Is it right?

I still have other questions, but I'd like to stop here, otherwise this
mail will be too long.  Your answers to these questions may help to
answer the rest of my questions.

-- 
Yao (齐尧)

Re: [PATCH] Fix wrong assertions

[Jan Kratochvil]

On Fri, 29 May 2015 11:31:18 +0200, Yao Qi wrote:
> spend some
> time investigating it, but still unable to fully understand the code.

I admit the code comments are not too great, I did not notice that when
writing them.


> (gdb) set debug entry-values 1
> (gdb) bt
> tailcall: initial: 0x40052e(a)
> tailcall: compare: 0x400527(a) 0x40052e(a)
> tailcall: reduced: | 0x40052e(a)
> gdb/git/gdb/dwarf2loc.c:834: internal-error: chain_candidate: Assertion `result->callers + result->callees < result->length' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n) y
> 
> I don't know why we need do intersection in chain_candidate, as the
> comments say:
> 
> /* Intersect RESULTP with CHAIN to keep RESULTP unambiguous, keep in RESULTP
>    only top callers and bottom callees which are present in both.  GDBARCH is
>    used only for ENTRY_VALUES_DEBUG.  RESULTP is NULL after return if there are
>    no remaining possibilities to provide unambiguous non-trivial result.
>    RESULTP should point to NULL on the first (initialization) call.  Caller is
>    responsible for xfree of any RESULTP data.  */
> 
> What do you mean by "ambiguous" here?  Is it ambiguous if we can get
> more than one call chain path from caller_pc to callee_pc?

Yes.


> For example,
> main tail calls a, a tail call b and c, b and c tail call d, when GDB
> unwinds from d, there are two chains, main -> a -> b -> d, and main -> a
> -> c -> d.  Are they ambiguous by your definition?

Those two chains are ambigous as it could be also the other chain.

Chain intersecting those two chains is:
	main -> a -> <???> -> d


> Further, what is "partially ambiguous result" in the comments below?

The terminology seems bogus there.

"partially ambiguous" was meant the chain:
	main -> a -> <???> -> d
An intersection of all possible chains.


> /* Determined tail calls for constructing virtual tail call frames.  */
> 
> struct call_site_chain
>   {
>     /* Initially CALLERS == CALLEES == LENGTH.  For partially ambiguous result
>        CALLERS + CALLEES < LENGTH.  */
>     int callers, callees, length;
> 
>     /* Variably sized array with LENGTH elements.  Later [0..CALLERS-1] contain
>        top (GDB "prev") sites and [LENGTH-CALLEES..LENGTH-1] contain bottom
>        (GDB "next") sites.  One is interested primarily in the PC field.  */
>     struct call_site *call_site[1];
>   };
> 
> I am confused by the usage of the variable-sized array call_site,
> elements from 0 to CALLERS-1 are top sites, and elements from
> LENGTH-CALLEES to LENGTH-1 are bottom sites, so I conclude that
> CALLERS-1 < LENGTH-CALLEES, then CALLERS + CALLEES < LENGTH + 1,
> then CALLERS + CALLEES =< LENGTH.  Is it right?

Yes, that is right.  Initially there is some chain (let's say the longest one
but that doe snot matter).  Consequently its elements from the middle are
being removed and there remains only some few unambiguous top and bottom ones.

The original idea why the comparison should be sharp ("<") was that if there
are multiple chains like (0xaddr show jmp instruction address):
	main(0x100) -> a(0x200) -> d(0x400)
	main(0x100) -> a(0x200) -> c(0x300) -> d(0x400)
then - such situation cannot exist - if two jmp instructions in "a" have the
same address they must also jump to the same address (*).

(*) jump to a computed address would be never considered for the DWARF
    tail-call records.

So there could be:
	main(0x100) -> a(0x200) -> d(0x400)
	main(0x100) -> a(0x270) -> c(0x300) -> d(0x400)
But then "a" frame itself is ambiguous and it must not be displayed.

I did not realize that there can be self-tail-call:
	main(0x100) -> a(0x200) -> d(0x400)
	main(0x100) -> a(0x280) -> a(0x200) -> d(0x400)
which intersects to:
	main(0x100) -> <???>? -> a(0x200) -> d(0x400)
And so if the first chain was chosen the
	main(0x100) -> a(0x200) -> d(0x400)
then the final intersection has callers+callees==length.

Originally the patchset tried to display the "ambiguous" part <???> in
backtrace creating a bogus frame there but GDB had too many problems with such
a frame.  So currently no such frame is created although still backtrace could
annotate it somehow there are "ambiguous" frames between these two frames.


Jan

Re: [PATCH] Fix wrong assertions

[Yao Qi]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Hi, Jan,
thanks for your explanations... they are very helpful.

>> Further, what is "partially ambiguous result" in the comments below?
>
> The terminology seems bogus there.
>
> "partially ambiguous" was meant the chain:
> 	main -> a -> <???> -> d
> An intersection of all possible chains.
>

Sounds like "partially ambiguous" is equivalent to "ambiguous".

>
>> /* Determined tail calls for constructing virtual tail call frames.  */
>> 
>> struct call_site_chain
>>   {
>>     /* Initially CALLERS == CALLEES == LENGTH.  For partially ambiguous result
>>        CALLERS + CALLEES < LENGTH.  */
>>     int callers, callees, length;
>> 
>>     /* Variably sized array with LENGTH elements.  Later [0..CALLERS-1] contain
>>        top (GDB "prev") sites and [LENGTH-CALLEES..LENGTH-1] contain bottom
>>        (GDB "next") sites.  One is interested primarily in the PC field.  */
>>     struct call_site *call_site[1];
>>   };
>> 
>> I am confused by the usage of the variable-sized array call_site,
>> elements from 0 to CALLERS-1 are top sites, and elements from
>> LENGTH-CALLEES to LENGTH-1 are bottom sites, so I conclude that
>> CALLERS-1 < LENGTH-CALLEES, then CALLERS + CALLEES < LENGTH + 1,
>> then CALLERS + CALLEES =< LENGTH.  Is it right?
>
> Yes, that is right.  Initially there is some chain (let's say the longest one

If that is right, the assert below is too strict, isn't?

  /* See call_site_find_chain_1 why there is no way to reach the bottom callee
     PC again.  In such case there must be two different code paths to reach
     it, therefore some of the former determined intermediate PCs must differ
     and the unambiguous chain gets shortened.  */
  gdb_assert (result->callers + result->callees < result->length);

> but that doe snot matter).  Consequently its elements from the middle are
> being removed and there remains only some few unambiguous top and
> bottom ones.

If there is no call sites removed from the chain during the intersection,
CALLERS + CALLEES == LENGTH, right?  in function chain_candidate,
result->length is set by the length of a chain.  If this chain is the
shortest one, CALLERS + CALLEES == LENGTH otherwise,
CALLERS + CALLEES < LENGTH.  Is it right?  If so, we need to relax the
condition in the assert and update the comments.

>
> The original idea why the comparison should be sharp ("<") was that if there
> are multiple chains like (0xaddr show jmp instruction address):
> 	main(0x100) -> a(0x200) -> d(0x400)
> 	main(0x100) -> a(0x200) -> c(0x300) -> d(0x400)
> then - such situation cannot exist - if two jmp instructions in "a" have the
> same address they must also jump to the same address (*).
>
> (*) jump to a computed address would be never considered for the DWARF
>     tail-call records.
>
> So there could be:
> 	main(0x100) -> a(0x200) -> d(0x400)
> 	main(0x100) -> a(0x270) -> c(0x300) -> d(0x400)
> But then "a" frame itself is ambiguous and it must not be displayed.
>
> I did not realize that there can be self-tail-call:
> 	main(0x100) -> a(0x200) -> d(0x400)
> 	main(0x100) -> a(0x280) -> a(0x200) -> d(0x400)
> which intersects to:
> 	main(0x100) -> <???>? -> a(0x200) -> d(0x400)
> And so if the first chain was chosen the
> 	main(0x100) -> a(0x200) -> d(0x400)
> then the final intersection has callers+callees==length.

What are the definitions of CALLERS, CALLEES, top and bottom? given this example?

-- 
Yao (齐尧)

Re: [PATCH] Fix wrong assertions

[Jan Kratochvil]

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

On Fri, 29 May 2015 15:43:19 +0200, Yao Qi wrote:
> Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> > The terminology seems bogus there.
> >
> > "partially ambiguous" was meant the chain:
> > 	main -> a -> <???> -> d
> > An intersection of all possible chains.
> 
> Sounds like "partially ambiguous" is equivalent to "ambiguous".

Yes, probably, I am not sure how to call it all myself.


> If that is right, the assert below is too strict, isn't?

Yes, it is too strict, this is why I agree with the fix by Andreas.


>   /* See call_site_find_chain_1 why there is no way to reach the bottom callee
>      PC again.  In such case there must be two different code paths to reach
>      it, therefore some of the former determined intermediate PCs must differ
>      and the unambiguous chain gets shortened.  */
>   gdb_assert (result->callers + result->callees < result->length);
> 
> > but that doe snot matter).  Consequently its elements from the middle are
> > being removed and there remains only some few unambiguous top and
> > bottom ones.
> 
> If there is no call sites removed from the chain during the intersection,
> CALLERS + CALLEES == LENGTH, right?

Just I expected there always has to be some site removed from the chain.
I do not find obvious it does not have to.  But maybe someone else finds it
obvious.


> in function chain_candidate,
> result->length is set by the length of a chain.  If this chain is the
> shortest one, CALLERS + CALLEES == LENGTH otherwise,
> CALLERS + CALLEES < LENGTH.  Is it right?

It is right now.  But when one does not think about self-tail-calls then even
the shortest one will get one frame removed.


> If so, we need to relax the
> condition in the assert and update the comments.

Yes, attached with updated comment.


> > I did not realize that there can be self-tail-call:
> > 	main(0x100) -> a(0x200) -> d(0x400)
> > 	main(0x100) -> a(0x280) -> a(0x200) -> d(0x400)
> > which intersects to:
> > 	main(0x100) -> <???>? -> a(0x200) -> d(0x400)
> > And so if the first chain was chosen the
> > 	main(0x100) -> a(0x200) -> d(0x400)
> > then the final intersection has callers+callees==length.
> 
> What are the definitions of CALLERS, CALLEES, top and bottom? given this example?

top=CALLERS=main(0x100), therefore 1
bottom=CALLEES=d(0x400), therefore 1

top = topmost, where you can go by GDB "up" commands, also called "prev" in
struct frame_info.

bottom = bottommost, where you can go by GDB "down" commands, also called
"next" in struct frame_info.


Jan

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

2015-05-29  Andreas Schwab  <schwab@linux-m68k.org>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR symtab/18392
	* dwarf2-frame-tailcall.c (pretended_chain_levels): Correct
	assertion.
	* dwarf2loc.c (chain_candidate): Likewise.

diff --git a/gdb/dwarf2-frame-tailcall.c b/gdb/dwarf2-frame-tailcall.c
index b412a5b..f964ab2 100644
--- a/gdb/dwarf2-frame-tailcall.c
+++ b/gdb/dwarf2-frame-tailcall.c
@@ -197,7 +197,7 @@ pretended_chain_levels (struct call_site_chain *chain)
     return chain->length;
 
   chain_levels = chain->callers + chain->callees;
-  gdb_assert (chain_levels < chain->length);
+  gdb_assert (chain_levels <= chain->length);
 
   return chain_levels;
 }
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 3aa8ddd..68d6cb4 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -825,9 +825,9 @@ chain_candidate (struct gdbarch *gdbarch, struct call_site_chain **resultp,
 
   /* See call_site_find_chain_1 why there is no way to reach the bottom callee
      PC again.  In such case there must be two different code paths to reach
-     it, therefore some of the former determined intermediate PCs must differ
-     and the unambiguous chain gets shortened.  */
-  gdb_assert (result->callers + result->callees < result->length);
+     it.  Still it may CALLERS+CALLEES==LENGTH in the case of optional
+     tail-call calling itself.  */
+  gdb_assert (result->callers + result->callees <= result->length);
 }
 
 /* Create and return call_site_chain for CALLER_PC and CALLEE_PC.  All the

Re: [PATCH] Fix wrong assertions

[Yao Qi]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

>> > I did not realize that there can be self-tail-call:
>> > 	main(0x100) -> a(0x200) -> d(0x400)
>> > 	main(0x100) -> a(0x280) -> a(0x200) -> d(0x400)
>> > which intersects to:
>> > 	main(0x100) -> <???>? -> a(0x200) -> d(0x400)
>> > And so if the first chain was chosen the
>> > 	main(0x100) -> a(0x200) -> d(0x400)
>> > then the final intersection has callers+callees==length.
>> 
>> What are the definitions of CALLERS, CALLEES, top and bottom? given
>> this example?
>
> top=CALLERS=main(0x100), therefore 1
> bottom=CALLEES=d(0x400), therefore 1
>
> top = topmost, where you can go by GDB "up" commands, also called "prev" in
> struct frame_info.
>
> bottom = bottommost, where you can go by GDB "down" commands, also called
> "next" in struct frame_info.

OK, I understand what does top/bottom mean.  Since they are numeric
values, what does these number mean?  for example, if CALLERS is 3 and
CALLEES is 2, what does the chain look like?

The code change in the patch looks reasonable to me, but comments change
doesn't, probably because I don't fully understand it.  I'll take a
deeper look next Monday.

-- 
Yao (齐尧)

Re: [PATCH] Fix wrong assertions

[Jan Kratochvil]

On Fri, 29 May 2015 18:33:01 +0200, Yao Qi wrote:
> OK, I understand what does top/bottom mean.  Since they are numeric
> values, what does these number mean?

CALLERS and CALLEES together with LENGTH say what data is at what indexes of
CALL_SITE:

struct call_site_chain
  {
    /* Initially CALLERS == CALLEES == LENGTH.  For partially ambiguous result
       CALLERS + CALLEES < LENGTH.  */
    int callers, callees, length;

    /* Variably sized array with LENGTH elements.  Later [0..CALLERS-1] contain
       top (GDB "prev") sites and [LENGTH-CALLEES..LENGTH-1] contain bottom
       (GDB "next") sites.  One is interested primarily in the PC field.  */
    struct call_site *call_site[1];
  };


> for example, if CALLERS is 3 and
> CALLEES is 2, what does the chain look like?

main(0x100) -> x(0x150) -> y(0x200) -> <???>? -> a(0x200) -> d(0x400)

And if LENGTH is 7 then:
	call_site[0] = main(0x100)
	call_site[1] = x(0x150)
	call_site[2] = y(0x200)
	call_site[3] = garbage
	call_site[4] = garbage
	call_site[5] = a(0x200)
	call_site[6] = d(0x400)


Thanks,
Jan

Re: [PATCH] Fix wrong assertions

[Yao Qi]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index 3aa8ddd..68d6cb4 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -825,9 +825,9 @@ chain_candidate (struct gdbarch *gdbarch, struct call_site_chain **resultp,
>  
>    /* See call_site_find_chain_1 why there is no way to reach the bottom callee
>       PC again.  In such case there must be two different code paths to reach
> -     it, therefore some of the former determined intermediate PCs must differ
> -     and the unambiguous chain gets shortened.  */
> -  gdb_assert (result->callers + result->callees < result->length);
> +     it.  Still it may CALLERS+CALLEES==LENGTH in the case of optional
> +     tail-call calling itself.  */
> +  gdb_assert (result->callers + result->callees <= result->length);

I am not a native English speaker, but I can't parse the comment.  How
about "CALLERS + CALLEES equal to LENGTH in the case of self tail-call"?

Otherwise, the patch is OK to me.

-- 
Yao (齐尧)

[commit] [PATCH] Fix wrong assertions

[Jan Kratochvil]

On Mon, 01 Jun 2015 13:35:02 +0200, Yao Qi wrote:
> I am not a native English speaker, but I can't parse the comment.  How
> about "CALLERS + CALLEES equal to LENGTH in the case of self tail-call"?

Used your comment part.


> Otherwise, the patch is OK to me.

Checked in:
	e0619de699ae6e86d8b93fa96a7668aef2e9636a


Thanks,
Jan