public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFC] While processing a struct die, store the method's address in its fn_field
@ 2014-11-24 15:55 Siva Chandra
  2014-11-24 20:22 ` Doug Evans
  0 siblings, 1 reply; 14+ messages in thread
From: Siva Chandra @ 2014-11-24 15:55 UTC (permalink / raw)
  To: gdb-patches

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

[The tests in this patch depend on this patch:
https://sourceware.org/ml/gdb-patches/2014-11/msg00479.html.  Also, it
adds a new dwarf2 test which I am not very sure I got it right.]

While processing a struct die, store the method's address in its fn_field.

This enables calling the method when its physname is missing and its
minsym cannot be discovered, but its address (DW_AT_low_pc) is available.
For example, this happens for the operator() methods of c++11 lambdas.
Consider the following C++ code:

int
main ()
{
  auto lambda = [] (int j) { return j + 113; };
  return lambda (-113);
}

When compiled with g++, the DWARF corresponding to the lambda's operator()
shows up under the DWARF for the function main as follows:

DW_TAG_subprogram
  DW_AT_name                  "operator()"
  DW_AT_type                  <0x0000002d>
  DW_AT_artificial            yes(1)
  DW_AT_low_pc                0x00400566
  DW_AT_high_pc               <offset-from-lowpc>19
  DW_AT_frame_base            len 0x0001: 9c: DW_OP_call_frame_cfa
  DW_AT_object_pointer        <0x0000010f>
  DW_AT_GNU_all_call_sites    yes(1)
  DW_TAG_pointer_type
    DW_AT_byte_size             0x00000008
    DW_AT_type                  <0x000000b9>
  DW_TAG_formal_parameter
    DW_AT_name                  "__closure"
    DW_AT_type                  <0x0000011b>
    DW_AT_artificial            yes(1)
    DW_AT_location              len 0x0002: 9168: DW_OP_fbreg -24
  DW_TAG_const_type
    DW_AT_type                  <0x00000109>
  DW_TAG_formal_parameter
    DW_AT_name                  "j"
    DW_AT_decl_file             0x00000001
    DW_AT_decl_line             0x00000004
    DW_AT_type                  <0x0000002d>
    DW_AT_location              len 0x0002: 9164:DW_OP_fbreg -28

There is no physname and the minsym corresponding to the the operator()
method does not demangle to its qualified name as specified by the DWARF.
However, since DW_AT_low_pc is available, it can be used to create a value
corresponding to the method in value.c:value_fn_field and subsequently be
passed to call_function_by_hand to invoke it.

gdb/ChangeLog:

2014-11-24  Siva Chandra Reddy  <sivachandra@google.com>

        * dwarf2read.c (dwarf2_add_member_fn): Note the methods address
        if its DT_AT_low_pc is available.
        * gdbtypes.h (struct fn_field): New field ADDR.
        (TYPE_FN_FIELD_ADDR): New macro.
        * value.c (value_fn_field): Use address of the method if
        available.

gdb/testsuite/ChangeLog:

2014-11-24  Siva Chandra Reddy  <sivachandra@google.com>

        * gdb.dwarf2/dw2-member-function-addr.S: New file.
        * gdb.dwarf2/dw2-member-function-addr.exp: New file.

[-- Attachment #2: member-function-addr-v1.txt --]
[-- Type: text/plain, Size: 18167 bytes --]

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0790388..3a36cb1 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -12654,6 +12654,11 @@ dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
   /* Fill in the member function field info.  */
   fnp = &new_fnfield->fnfield;
 
+  /* Set the address of the method if die has DW_AT_low_pc.  */
+  attr = dwarf2_attr (die, DW_AT_low_pc, cu);
+  if (attr)
+    fnp->addr = attr_value_as_address (attr);
+
   /* Delay processing of the physname until later.  */
   if (cu->language == language_cplus || cu->language == language_java)
     {
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index d32c97c..d98887a 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -885,6 +885,9 @@ struct cplus_struct_type
 
 	struct fn_field
 	  {
+	    /* For methods which are not virtual, then this is the
+	       real address of the method if it is non-zero.  */
+	    CORE_ADDR addr;
 
 	    /* * If is_stub is clear, this is the mangled name which
 	       we can look up to find the address of the method
@@ -1344,6 +1347,7 @@ extern void allocate_gnat_aux_type (struct type *);
 
 #define TYPE_FN_FIELD(thisfn, n) (thisfn)[n]
 #define TYPE_FN_FIELD_PHYSNAME(thisfn, n) (thisfn)[n].physname
+#define TYPE_FN_FIELD_ADDR(thisfn, n) (thisfn)[n].addr
 #define TYPE_FN_FIELD_TYPE(thisfn, n) (thisfn)[n].type
 #define TYPE_FN_FIELD_ARGS(thisfn, n) TYPE_FIELDS ((thisfn)[n].type)
 #define TYPE_FN_FIELD_CONST(thisfn, n) ((thisfn)[n].is_const)
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-member-function-addr.S b/gdb/testsuite/gdb.dwarf2/dw2-member-function-addr.S
new file mode 100644
index 0000000..a89aa27
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-member-function-addr.S
@@ -0,0 +1,460 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 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/>.  */
+
+/* The contents below were generated by compiling the code below with
+       g++ -g-dA -S -std=c++11 lambda.cc
+
+   int
+   main ()
+   {
+     auto lambda = [] (int j) { return j + 113; };
+     return lambda (-113);
+   }  */
+
+	.file	"lambda.cc"
+	.text
+.Ltext0:
+	.align 2
+	.type	_ZZ4mainENKUliE_clEi, @function
+_ZZ4mainENKUliE_clEi:
+.LFB1:
+	.file 1 "lambda.cc"
+	# lambda.cc:4
+	.loc 1 4 0
+	.cfi_startproc
+# BLOCK 2 seq:0
+# PRED: ENTRY (FALLTHRU)
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset 6, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register 6
+	movq	%rdi, -8(%rbp)
+	movl	%esi, -12(%rbp)
+.LBB2:
+	# lambda.cc:4
+	.loc 1 4 0
+	movl	-12(%rbp), %eax
+	addl	$113, %eax
+.LBE2:
+	popq	%rbp
+	.cfi_def_cfa 7, 8
+# SUCC: EXIT [100.0%]
+	ret
+	.cfi_endproc
+.LFE1:
+	.size	_ZZ4mainENKUliE_clEi, .-_ZZ4mainENKUliE_clEi
+	.globl	main
+	.type	main, @function
+main:
+.LFB0:
+	# lambda.cc:3
+	.loc 1 3 0
+	.cfi_startproc
+# BLOCK 2 seq:0
+# PRED: ENTRY (FALLTHRU)
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset 6, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register 6
+	subq	$16, %rsp
+.LBB3:
+	# lambda.cc:5
+	.loc 1 5 0
+	leaq	-1(%rbp), %rax
+	movl	$-113, %esi
+	movq	%rax, %rdi
+	call	_ZZ4mainENKUliE_clEi
+.LBE3:
+	# lambda.cc:6
+	.loc 1 6 0
+	leave
+	.cfi_def_cfa 7, 8
+# SUCC: EXIT [100.0%]
+	ret
+	.cfi_endproc
+.LFE0:
+	.size	main, .-main
+.Letext0:
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.long	0x115	# Length of Compilation Unit Info
+	.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	.LASF1	# DW_AT_producer: "GNU C++ 4.8.2 -mtune=generic -march=x86-64 -g -std=c++11 -fstack-protector"
+	.byte	0x4	# DW_AT_language
+	.long	.LASF2	# DW_AT_name: "lambda.cc"
+	.long	.LASF3	# DW_AT_comp_dir: "/home/sivachandra/LAB/c++"
+	.quad	.Ltext0	# DW_AT_low_pc
+	.quad	.Letext0-.Ltext0	# DW_AT_high_pc
+	.long	.Ldebug_line0	# DW_AT_stmt_list
+	.uleb128 0x2	# (DIE (0x2d) DW_TAG_base_type)
+	.byte	0x4	# DW_AT_byte_size
+	.byte	0x5	# DW_AT_encoding
+	.ascii "int\0"	# DW_AT_name
+	.uleb128 0x3	# (DIE (0x34) DW_TAG_subprogram)
+			# DW_AT_external
+	.long	.LASF4	# DW_AT_name: "main"
+	.byte	0x1	# DW_AT_decl_file (lambda.cc)
+	.byte	0x2	# DW_AT_decl_line
+	.long	0x2d	# DW_AT_type
+	.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_tail_call_sites
+	.uleb128 0x4	# (DIE (0x51) DW_TAG_lexical_block)
+	.quad	.LBB3	# DW_AT_low_pc
+	.quad	.LBE3-.LBB3	# DW_AT_high_pc
+	.uleb128 0x5	# (DIE (0x62) DW_TAG_variable)
+	.long	.LASF5	# DW_AT_name: "lambda"
+	.byte	0x1	# DW_AT_decl_file (lambda.cc)
+	.byte	0x4	# DW_AT_decl_line
+	.long	0x70	# DW_AT_type
+	.uleb128 0x2	# DW_AT_location
+	.byte	0x91	# DW_OP_fbreg
+	.sleb128 -17
+	.uleb128 0x6	# (DIE (0x70) DW_TAG_structure_type)
+	.long	.LASF6	# DW_AT_name: "__lambda0"
+	.byte	0x1	# DW_AT_byte_size
+	.byte	0x1	# DW_AT_decl_file (lambda.cc)
+	.byte	0x4	# DW_AT_decl_line
+	.uleb128 0x7	# (DIE (0x78) DW_TAG_subprogram)
+	.long	.LASF0	# DW_AT_name: "<lambda>"
+			# DW_AT_artificial
+			# DW_AT_declaration
+	.long	0x85	# DW_AT_object_pointer
+	.long	0x9c	# DW_AT_sibling
+	.uleb128 0x8	# (DIE (0x85) DW_TAG_formal_parameter)
+	.long	0x8a	# DW_AT_type
+			# DW_AT_artificial
+	.uleb128 0x9	# (DIE (0x8a) DW_TAG_pointer_type)
+	.byte	0x8	# DW_AT_byte_size
+	.long	0x70	# DW_AT_type
+	.uleb128 0xa	# (DIE (0x90) DW_TAG_formal_parameter)
+	.long	0x95	# DW_AT_type
+	.uleb128 0xb	# (DIE (0x95) DW_TAG_rvalue_reference_type)
+	.byte	0x8	# DW_AT_byte_size
+	.long	0x70	# DW_AT_type
+	.byte	0	# end of children of DIE 0x78
+	.uleb128 0x7	# (DIE (0x9c) DW_TAG_subprogram)
+	.long	.LASF0	# DW_AT_name: "<lambda>"
+			# DW_AT_artificial
+			# DW_AT_declaration
+	.long	0xa9	# DW_AT_object_pointer
+	.long	0xbf	# DW_AT_sibling
+	.uleb128 0x8	# (DIE (0xa9) DW_TAG_formal_parameter)
+	.long	0x8a	# DW_AT_type
+			# DW_AT_artificial
+	.uleb128 0xa	# (DIE (0xae) DW_TAG_formal_parameter)
+	.long	0xb3	# DW_AT_type
+	.uleb128 0xc	# (DIE (0xb3) DW_TAG_reference_type)
+	.byte	0x8	# DW_AT_byte_size
+	.long	0xb9	# DW_AT_type
+	.uleb128 0xd	# (DIE (0xb9) DW_TAG_const_type)
+	.long	0x70	# DW_AT_type
+	.byte	0	# end of children of DIE 0x9c
+	.uleb128 0x7	# (DIE (0xbf) DW_TAG_subprogram)
+	.long	.LASF0	# DW_AT_name: "<lambda>"
+			# DW_AT_artificial
+			# DW_AT_declaration
+	.long	0xcc	# DW_AT_object_pointer
+	.long	0xd2	# DW_AT_sibling
+	.uleb128 0x8	# (DIE (0xcc) DW_TAG_formal_parameter)
+	.long	0x8a	# DW_AT_type
+			# DW_AT_artificial
+	.byte	0	# end of children of DIE 0xbf
+	.uleb128 0xe	# (DIE (0xd2) DW_TAG_subprogram)
+	.long	.LASF7	# DW_AT_name: "operator()"
+	.long	0x2d	# DW_AT_type
+			# DW_AT_artificial
+	.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
+	.long	0xf7	# DW_AT_object_pointer
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x9	# (DIE (0xf1) DW_TAG_pointer_type)
+	.byte	0x8	# DW_AT_byte_size
+	.long	0xb9	# DW_AT_type
+	.uleb128 0xf	# (DIE (0xf7) DW_TAG_formal_parameter)
+	.long	.LASF8	# DW_AT_name: "__closure"
+	.long	0x103	# DW_AT_type
+			# DW_AT_artificial
+	.uleb128 0x2	# DW_AT_location
+	.byte	0x91	# DW_OP_fbreg
+	.sleb128 -24
+	.uleb128 0xd	# (DIE (0x103) DW_TAG_const_type)
+	.long	0xf1	# DW_AT_type
+	.uleb128 0x10	# (DIE (0x108) DW_TAG_formal_parameter)
+	.ascii "j\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (lambda.cc)
+	.byte	0x4	# DW_AT_decl_line
+	.long	0x2d	# DW_AT_type
+	.uleb128 0x2	# DW_AT_location
+	.byte	0x91	# DW_OP_fbreg
+	.sleb128 -28
+	.byte	0	# end of children of DIE 0xd2
+	.byte	0	# end of children of DIE 0x70
+	.byte	0	# end of children of DIE 0x51
+	.byte	0	# end of children of DIE 0x34
+	.byte	0	# end of children of DIE 0xb
+	.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 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x10	# (DW_AT_stmt_list)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0x2	# (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 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 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 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 0x2116	# (DW_AT_GNU_all_tail_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x4	# (abbrev code)
+	.uleb128 0xb	# (TAG: DW_TAG_lexical_block)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.byte	0
+	.byte	0
+	.uleb128 0x5	# (abbrev code)
+	.uleb128 0x34	# (TAG: DW_TAG_variable)
+	.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 0x18	# (DW_FORM_exprloc)
+	.byte	0
+	.byte	0
+	.uleb128 0x6	# (abbrev code)
+	.uleb128 0x13	# (TAG: DW_TAG_structure_type)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0xb	# (DW_AT_byte_size)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.byte	0
+	.byte	0
+	.uleb128 0x7	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x34	# (DW_AT_artificial)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x3c	# (DW_AT_declaration)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x64	# (DW_AT_object_pointer)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x1	# (DW_AT_sibling)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x8	# (abbrev code)
+	.uleb128 0x5	# (TAG: DW_TAG_formal_parameter)
+	.byte	0	# DW_children_no
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x34	# (DW_AT_artificial)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x9	# (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 0xa	# (abbrev code)
+	.uleb128 0x5	# (TAG: DW_TAG_formal_parameter)
+	.byte	0	# DW_children_no
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0xb	# (abbrev code)
+	.uleb128 0x42	# (TAG: DW_TAG_rvalue_reference_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 0xc	# (abbrev code)
+	.uleb128 0x10	# (TAG: DW_TAG_reference_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 0x26	# (TAG: DW_TAG_const_type)
+	.byte	0	# DW_children_no
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0xe	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x34	# (DW_AT_artificial)
+	.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 0x64	# (DW_AT_object_pointer)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0xf	# (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 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x34	# (DW_AT_artificial)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x2	# (DW_AT_location)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.byte	0
+	.byte	0
+	.uleb128 0x10	# (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
+	.byte	0
+	.section	.debug_aranges,"",@progbits
+	.long	0x2c	# 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	0
+	.quad	0
+	.section	.debug_line,"",@progbits
+.Ldebug_line0:
+	.section	.debug_str,"MS",@progbits,1
+.LASF3:
+	.string	"/home/sivachandra/LAB/c++"
+.LASF6:
+	.string	"__lambda0"
+.LASF7:
+	.string	"operator()"
+.LASF2:
+	.string	"lambda.cc"
+.LASF0:
+	.string	"<lambda>"
+.LASF1:
+	.string	"GNU C++ 4.8.2 -mtune=generic -march=x86-64 -g -std=c++11 -fstack-protector"
+.LASF4:
+	.string	"main"
+.LASF5:
+	.string	"lambda"
+.LASF8:
+	.string	"__closure"
+	.ident	"GCC: (Ubuntu 4.8.2-19ubuntu1) 4.8.2"
+	.section	.note.GNU-stack,"",@progbits
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-member-function-addr.exp b/gdb/testsuite/gdb.dwarf2/dw2-member-function-addr.exp
new file mode 100644
index 0000000..9356e35
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-member-function-addr.exp
@@ -0,0 +1,33 @@
+# Copyright 2014 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/>.
+
+load_lib dwarf.exp
+
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile .S
+
+if {[prepare_for_testing_full $testfile.exp \
+	 [list $testfile debug $srcfile nodebug]]} {
+    return -1
+}
+
+clean_restart $testfile
+
+gdb_test "break main" "Breakpoint 1 .*" "break main"
+gdb_test "run" "Starting program: .*Breakpoint 1.*" "run"
+gdb_test "p lambda(10)" ".* = 123" "p lambda()"
diff --git a/gdb/value.c b/gdb/value.c
index ecfb154..7c83b81 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3068,24 +3068,30 @@ value_fn_field (struct value **arg1p, struct fn_field *f,
   struct value *v;
   struct type *ftype = TYPE_FN_FIELD_TYPE (f, j);
   const char *physname = TYPE_FN_FIELD_PHYSNAME (f, j);
-  struct symbol *sym;
+  const CORE_ADDR addr = TYPE_FN_FIELD_ADDR (f, j);
+  struct symbol *sym = NULL;
   struct bound_minimal_symbol msym;
 
-  sym = lookup_symbol (physname, 0, VAR_DOMAIN, 0);
-  if (sym != NULL)
+  if (!addr)
     {
-      memset (&msym, 0, sizeof (msym));
-    }
-  else
-    {
-      gdb_assert (sym == NULL);
-      msym = lookup_bound_minimal_symbol (physname);
-      if (msym.minsym == NULL)
-	return NULL;
+      sym = lookup_symbol (physname, 0, VAR_DOMAIN, 0);
+      if (sym != NULL)
+	{
+	  memset (&msym, 0, sizeof (msym));
+	}
+      else
+	{
+	  gdb_assert (sym == NULL);
+	  msym = lookup_bound_minimal_symbol (physname);
+	  if (msym.minsym == NULL)
+	    return NULL;
+	}
     }
 
   v = allocate_value (ftype);
-  if (sym)
+  if (addr)
+    set_value_address (v, f->addr);
+  else if (sym)
     {
       set_value_address (v, BLOCK_START (SYMBOL_BLOCK_VALUE (sym)));
     }

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-24 15:55 [RFC] While processing a struct die, store the method's address in its fn_field Siva Chandra
@ 2014-11-24 20:22 ` Doug Evans
  2014-11-24 20:28   ` Doug Evans
  2014-11-25 15:00   ` Siva Chandra
  0 siblings, 2 replies; 14+ messages in thread
From: Doug Evans @ 2014-11-24 20:22 UTC (permalink / raw)
  To: Siva Chandra; +Cc: gdb-patches

On Mon, Nov 24, 2014 at 7:54 AM, Siva Chandra <sivachandra@google.com> wrote:
> [The tests in this patch depend on this patch:
> https://sourceware.org/ml/gdb-patches/2014-11/msg00479.html.  Also, it
> adds a new dwarf2 test which I am not very sure I got it right.]
>
> While processing a struct die, store the method's address in its fn_field.
>
> This enables calling the method when its physname is missing and its
> minsym cannot be discovered, but its address (DW_AT_low_pc) is available.
> For example, this happens for the operator() methods of c++11 lambdas.
> Consider the following C++ code:
>
> int
> main ()
> {
>   auto lambda = [] (int j) { return j + 113; };
>   return lambda (-113);
> }
>
> When compiled with g++, the DWARF corresponding to the lambda's operator()
> shows up under the DWARF for the function main as follows:
>
> DW_TAG_subprogram
>   DW_AT_name                  "operator()"
>   DW_AT_type                  <0x0000002d>
>   DW_AT_artificial            yes(1)
>   DW_AT_low_pc                0x00400566
>   DW_AT_high_pc               <offset-from-lowpc>19
>   DW_AT_frame_base            len 0x0001: 9c: DW_OP_call_frame_cfa
>   DW_AT_object_pointer        <0x0000010f>
>   DW_AT_GNU_all_call_sites    yes(1)
>   DW_TAG_pointer_type
>     DW_AT_byte_size             0x00000008
>     DW_AT_type                  <0x000000b9>
>   DW_TAG_formal_parameter
>     DW_AT_name                  "__closure"
>     DW_AT_type                  <0x0000011b>
>     DW_AT_artificial            yes(1)
>     DW_AT_location              len 0x0002: 9168: DW_OP_fbreg -24
>   DW_TAG_const_type
>     DW_AT_type                  <0x00000109>
>   DW_TAG_formal_parameter
>     DW_AT_name                  "j"
>     DW_AT_decl_file             0x00000001
>     DW_AT_decl_line             0x00000004
>     DW_AT_type                  <0x0000002d>
>     DW_AT_location              len 0x0002: 9164:DW_OP_fbreg -28
>
> There is no physname and the minsym corresponding to the the operator()
> method does not demangle to its qualified name as specified by the DWARF.
> However, since DW_AT_low_pc is available, it can be used to create a value
> corresponding to the method in value.c:value_fn_field and subsequently be
> passed to call_function_by_hand to invoke it.
>
> gdb/ChangeLog:
>
> 2014-11-24  Siva Chandra Reddy  <sivachandra@google.com>
>
>         * dwarf2read.c (dwarf2_add_member_fn): Note the methods address
>         if its DT_AT_low_pc is available.
>         * gdbtypes.h (struct fn_field): New field ADDR.
>         (TYPE_FN_FIELD_ADDR): New macro.
>         * value.c (value_fn_field): Use address of the method if
>         available.
>
> gdb/testsuite/ChangeLog:
>
> 2014-11-24  Siva Chandra Reddy  <sivachandra@google.com>
>
>         * gdb.dwarf2/dw2-member-function-addr.S: New file.
>         * gdb.dwarf2/dw2-member-function-addr.exp: New file.

Hi.

[I have to admit I hadn't realized gdb used low_pc for the entry point.
Or more likely forgotten. :-)
There's nothing to say that low_pc is the entry point,
but gdb has been using this since forever I'm guessing,
and if it wasn't the entry point presumably the compiler could emit
a DW_TAG_entry_point record.]

Adding new fields to types or symbols is a big deal.
I'd like to understand why things are failing better.
For normal functions gdb gets the start address
from BLOCK_START (SYMBOL_BLOCK_VALUE (sym)).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-24 20:22 ` Doug Evans
@ 2014-11-24 20:28   ` Doug Evans
  2014-11-26  4:36     ` Doug Evans
  2014-11-25 15:00   ` Siva Chandra
  1 sibling, 1 reply; 14+ messages in thread
From: Doug Evans @ 2014-11-24 20:28 UTC (permalink / raw)
  To: Siva Chandra; +Cc: gdb-patches

On Mon, Nov 24, 2014 at 12:22 PM, Doug Evans <dje@google.com> wrote:
> On Mon, Nov 24, 2014 at 7:54 AM, Siva Chandra <sivachandra@google.com> wrote:
>> [The tests in this patch depend on this patch:
>> https://sourceware.org/ml/gdb-patches/2014-11/msg00479.html.  Also, it
>> adds a new dwarf2 test which I am not very sure I got it right.]
>>
>> While processing a struct die, store the method's address in its fn_field.
>>
>> This enables calling the method when its physname is missing and its
>> minsym cannot be discovered, but its address (DW_AT_low_pc) is available.
>> For example, this happens for the operator() methods of c++11 lambdas.
>> Consider the following C++ code:
>>
>> int
>> main ()
>> {
>>   auto lambda = [] (int j) { return j + 113; };
>>   return lambda (-113);
>> }
>>
>> When compiled with g++, the DWARF corresponding to the lambda's operator()
>> shows up under the DWARF for the function main as follows:
>>
>> DW_TAG_subprogram
>>   DW_AT_name                  "operator()"
>>   DW_AT_type                  <0x0000002d>
>>   DW_AT_artificial            yes(1)
>>   DW_AT_low_pc                0x00400566
>>   DW_AT_high_pc               <offset-from-lowpc>19
>>   DW_AT_frame_base            len 0x0001: 9c: DW_OP_call_frame_cfa
>>   DW_AT_object_pointer        <0x0000010f>
>>   DW_AT_GNU_all_call_sites    yes(1)
>>   DW_TAG_pointer_type
>>     DW_AT_byte_size             0x00000008
>>     DW_AT_type                  <0x000000b9>
>>   DW_TAG_formal_parameter
>>     DW_AT_name                  "__closure"
>>     DW_AT_type                  <0x0000011b>
>>     DW_AT_artificial            yes(1)
>>     DW_AT_location              len 0x0002: 9168: DW_OP_fbreg -24
>>   DW_TAG_const_type
>>     DW_AT_type                  <0x00000109>
>>   DW_TAG_formal_parameter
>>     DW_AT_name                  "j"
>>     DW_AT_decl_file             0x00000001
>>     DW_AT_decl_line             0x00000004
>>     DW_AT_type                  <0x0000002d>
>>     DW_AT_location              len 0x0002: 9164:DW_OP_fbreg -28
>>
>> There is no physname and the minsym corresponding to the the operator()
>> method does not demangle to its qualified name as specified by the DWARF.
>> However, since DW_AT_low_pc is available, it can be used to create a value
>> corresponding to the method in value.c:value_fn_field and subsequently be
>> passed to call_function_by_hand to invoke it.
>>
>> gdb/ChangeLog:
>>
>> 2014-11-24  Siva Chandra Reddy  <sivachandra@google.com>
>>
>>         * dwarf2read.c (dwarf2_add_member_fn): Note the methods address
>>         if its DT_AT_low_pc is available.
>>         * gdbtypes.h (struct fn_field): New field ADDR.
>>         (TYPE_FN_FIELD_ADDR): New macro.
>>         * value.c (value_fn_field): Use address of the method if
>>         available.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 2014-11-24  Siva Chandra Reddy  <sivachandra@google.com>
>>
>>         * gdb.dwarf2/dw2-member-function-addr.S: New file.
>>         * gdb.dwarf2/dw2-member-function-addr.exp: New file.
>
> Hi.
>
> [I have to admit I hadn't realized gdb used low_pc for the entry point.
> Or more likely forgotten. :-)
> There's nothing to say that low_pc is the entry point,
> but gdb has been using this since forever I'm guessing,
> and if it wasn't the entry point presumably the compiler could emit
> a DW_TAG_entry_point record.]
>
> Adding new fields to types or symbols is a big deal.
> I'd like to understand why things are failing better.
> For normal functions gdb gets the start address
> from BLOCK_START (SYMBOL_BLOCK_VALUE (sym)).

Bleah, hit Send too soon.

Nits:

1) The testcase is amd64-linux specific, which is ok, but the .exp file
needs a test to check for this.  grep for x86_64 in gdb.dwarf2/*.exp.

2) Please massage the assembler output and change things like
this to something more generic.  "/tmp" or some such.

+       .long   .LASF3  # DW_AT_comp_dir: "/home/sivachandra/LAB/c++"

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-24 20:22 ` Doug Evans
  2014-11-24 20:28   ` Doug Evans
@ 2014-11-25 15:00   ` Siva Chandra
  2014-11-25 22:10     ` Doug Evans
  1 sibling, 1 reply; 14+ messages in thread
From: Siva Chandra @ 2014-11-25 15:00 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On Mon, Nov 24, 2014 at 12:22 PM, Doug Evans <dje@google.com> wrote:
> Adding new fields to types or symbols is a big deal.
> I'd like to understand why things are failing better.
> For normal functions gdb gets the start address
> from BLOCK_START (SYMBOL_BLOCK_VALUE (sym)).

I will try, but tl;dr.

Consider a simple class definition like this which has a method which
is not inlined:

class A
{
public:
  int method (int a);
};

int
A::method (int a)
{
  return a * a;
}

The DWARF for the class comes out like this:

< 1><0x0000002d>    DW_TAG_class_type
                      DW_AT_name                  "A"
                      DW_AT_byte_size             0x00000001
                      DW_AT_decl_file             0x00000001 /tmp
                      DW_AT_decl_line             0x00000001
                      DW_AT_sibling               <0x00000057>
< 2><0x00000037>      DW_TAG_subprogram
                        DW_AT_external              yes(1)
                        DW_AT_name                  "method"
                        DW_AT_decl_file             0x00000001 /tmp
                        DW_AT_decl_line             0x00000004
                        DW_AT_linkage_name          "_ZN1A6methodEi"
                        DW_AT_type                  <0x00000057>
                        DW_AT_accessibility         DW_ACCESS_public
                        DW_AT_declaration           yes(1)
                        DW_AT_object_pointer        <0x0000004b>
< 3><0x0000004b>        DW_TAG_formal_parameter
                          DW_AT_type                  <0x0000005e>
                          DW_AT_artificial            yes(1)
< 3><0x00000050>        DW_TAG_formal_parameter
                          DW_AT_type                  <0x00000057>

Notice that there is a linkage name for the method named "method". So,
one can demangle the linkage name and get to the symbol. If this
fails, one can use the linkage name to get to the minsym. The issue
with lambdas is that their operator() methods do not have a linkage
name. Not exactly true: they have a linkage name but the DWARF doesn't
specify it. Consider this example:

$> cat lambda.cc

int
main ()
{
  auto lambda = [] (int j) { return j + 113; };
  return lambda (-113);
}

$> g++ -g -std=c++11 lambda.cc -o lambda

The DWARF for the lambda struct come out like this:

< 3><0x00000070>        DW_TAG_structure_type
                          DW_AT_name                  "<lambda(int)>"
                          DW_AT_byte_size             0x00000001
                          DW_AT_decl_file             0x00000001 /tmp
                          DW_AT_decl_line             0x00000004
< 4><0x00000078>          DW_TAG_subprogram
                            DW_AT_name                  "<lambda>"
                            DW_AT_artificial            yes(1)
                            DW_AT_declaration           yes(1)
                            DW_AT_object_pointer        <0x00000085>
                            DW_AT_sibling               <0x0000009c>
< 5><0x00000085>            DW_TAG_formal_parameter
                              DW_AT_type                  <0x0000008a>
                              DW_AT_artificial            yes(1)
< 5><0x0000008a>            DW_TAG_pointer_type
                              DW_AT_byte_size             0x00000008
                              DW_AT_type                  <0x00000070>
< 5><0x00000090>            DW_TAG_formal_parameter
                              DW_AT_type                  <0x00000095>
< 5><0x00000095>            DW_TAG_rvalue_reference_type
                              DW_AT_byte_size             0x00000008
                              DW_AT_type                  <0x00000070>
< 4><0x0000009c>          DW_TAG_subprogram
                            DW_AT_name                  "<lambda>"
                            DW_AT_artificial            yes(1)
                            DW_AT_declaration           yes(1)
                            DW_AT_object_pointer        <0x000000a9>
                            DW_AT_sibling               <0x000000bf>
< 5><0x000000a9>            DW_TAG_formal_parameter
                              DW_AT_type                  <0x0000008a>
                              DW_AT_artificial            yes(1)
< 5><0x000000ae>            DW_TAG_formal_parameter
                              DW_AT_type                  <0x000000b3>
< 5><0x000000b3>            DW_TAG_reference_type
                              DW_AT_byte_size             0x00000008
                              DW_AT_type                  <0x000000b9>
< 5><0x000000b9>            DW_TAG_const_type
                              DW_AT_type                  <0x00000070>
< 4><0x000000bf>          DW_TAG_subprogram
                            DW_AT_name                  "<lambda>"
                            DW_AT_artificial            yes(1)
                            DW_AT_declaration           yes(1)
                            <Unknown AT value 0x211a>   yes(1)
                            DW_AT_object_pointer        <0x000000cc>
                            DW_AT_sibling               <0x000000d2>
< 5><0x000000cc>            DW_TAG_formal_parameter
                              DW_AT_type                  <0x0000008a>
                              DW_AT_artificial            yes(1)
< 4><0x000000d2>          DW_TAG_subprogram
                            DW_AT_name                  "~<lambda>"
                            DW_AT_artificial            yes(1)
                            DW_AT_declaration           yes(1)
                            DW_AT_object_pointer        <0x000000df>
                            DW_AT_sibling               <0x000000ea>
< 5><0x000000df>            DW_TAG_formal_parameter
                              DW_AT_type                  <0x0000008a>
                              DW_AT_artificial            yes(1)
< 5><0x000000e4>            DW_TAG_formal_parameter
                              DW_AT_type                  <0x0000002d>
                              DW_AT_artificial            yes(1)
< 4><0x000000ea>          DW_TAG_subprogram
                            DW_AT_name                  "operator()"
                            DW_AT_type                  <0x0000002d>
                            DW_AT_artificial            yes(1)
                            DW_AT_low_pc                0x00400566
                            DW_AT_high_pc               <offset-from-lowpc>19
                            DW_AT_frame_base            len 0x0001:
9c: DW_OP_call_frame_cfa
                            DW_AT_object_pointer        <0x0000010f>
                            DW_AT_GNU_all_call_sites    yes(1)
< 5><0x00000109>            DW_TAG_pointer_type
                              DW_AT_byte_size             0x00000008
                              DW_AT_type                  <0x000000b9>
< 5><0x0000010f>            DW_TAG_formal_parameter
                              DW_AT_name                  "__closure"
                              DW_AT_type                  <0x0000011b>
                              DW_AT_artificial            yes(1)
                              DW_AT_location              len 0x0002:
9168: DW_OP_fbreg -24
< 5><0x0000011b>            DW_TAG_const_type
                              DW_AT_type                  <0x00000109>
< 5><0x00000120>            DW_TAG_formal_parameter
                              DW_AT_name                  "j"
                              DW_AT_decl_file             0x00000001 /tmp
                              DW_AT_decl_line             0x00000004
                              DW_AT_type                  <0x0000002d>
                              DW_AT_location              len 0x0002:
9164: DW_OP_fbreg -28

Notice that the operator() method has no linkage name specified in the
DWARF. But, it has a DW_AT_low_pc. Lets grep the binary for this low
pc value:

$> readelf -a lambda | grep 400566
    41: 0000000000400566    19 FUNC    LOCAL  DEFAULT   12 _ZZ4mainENKUliE_clEi

So then, there is an ELF symbol for the operator() method! Lets
demangle the name:

(gdb) maintenance demangle _ZZ4mainENKUliE_clEi
main::{lambda(int)#1}::operator()(int) const

This name is not the same as the name of the lambda structure given by
DWARF! If I use clang to compile, the demangled ELF symbol for the
operator() turns out to be "main::$_0::operator()(int) const", which
seems to indicate that gcc and clang have their own way of
differentiating the different lambdas that can occur in a CU. Hence,
GDB should probably not even attempt to generate a name, mangle it and
lookup for a symbol with that name. However, low_pc is immediately
available in the DWARF generated by GCC, so why not use it? [Clang
actually does not put out the low pc value for the subprogram die
under a structure die, but it puts out a separate (not under a
structure die) subprogram die for the operator() method with the low
pc value.]

Thanks,
Siva Chandra

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-25 15:00   ` Siva Chandra
@ 2014-11-25 22:10     ` Doug Evans
  2014-11-25 23:37       ` Siva Chandra
  0 siblings, 1 reply; 14+ messages in thread
From: Doug Evans @ 2014-11-25 22:10 UTC (permalink / raw)
  To: Siva Chandra; +Cc: gdb-patches

On Tue, Nov 25, 2014 at 7:00 AM, Siva Chandra <sivachandra@google.com> wrote:
> On Mon, Nov 24, 2014 at 12:22 PM, Doug Evans <dje@google.com> wrote:
>> Adding new fields to types or symbols is a big deal.
>> I'd like to understand why things are failing better.
>> For normal functions gdb gets the start address
>> from BLOCK_START (SYMBOL_BLOCK_VALUE (sym)).
>
> I will try, but tl;dr.
>
> Consider a simple class definition like this which has a method which
> is not inlined:
>
> class A
> {
> public:
>   int method (int a);
> };
>
> int
> A::method (int a)
> {
>   return a * a;
> }
>
> The DWARF for the class comes out like this:
>
> < 1><0x0000002d>    DW_TAG_class_type
>                       DW_AT_name                  "A"
>                       DW_AT_byte_size             0x00000001
>                       DW_AT_decl_file             0x00000001 /tmp
>                       DW_AT_decl_line             0x00000001
>                       DW_AT_sibling               <0x00000057>
> < 2><0x00000037>      DW_TAG_subprogram
>                         DW_AT_external              yes(1)
>                         DW_AT_name                  "method"
>                         DW_AT_decl_file             0x00000001 /tmp
>                         DW_AT_decl_line             0x00000004
>                         DW_AT_linkage_name          "_ZN1A6methodEi"
>                         DW_AT_type                  <0x00000057>
>                         DW_AT_accessibility         DW_ACCESS_public
>                         DW_AT_declaration           yes(1)
>                         DW_AT_object_pointer        <0x0000004b>
> < 3><0x0000004b>        DW_TAG_formal_parameter
>                           DW_AT_type                  <0x0000005e>
>                           DW_AT_artificial            yes(1)
> < 3><0x00000050>        DW_TAG_formal_parameter
>                           DW_AT_type                  <0x00000057>
>
> Notice that there is a linkage name for the method named "method". So,
> one can demangle the linkage name and get to the symbol. If this
> fails, one can use the linkage name to get to the minsym. The issue
> with lambdas is that their operator() methods do not have a linkage
> name. Not exactly true: they have a linkage name but the DWARF doesn't
> specify it. Consider this example:
>
> $> cat lambda.cc
>
> int
> main ()
> {
>   auto lambda = [] (int j) { return j + 113; };
>   return lambda (-113);
> }
>
> $> g++ -g -std=c++11 lambda.cc -o lambda
>
> The DWARF for the lambda struct come out like this:
>
> < 3><0x00000070>        DW_TAG_structure_type
>                           DW_AT_name                  "<lambda(int)>"
>                           DW_AT_byte_size             0x00000001
>                           DW_AT_decl_file             0x00000001 /tmp
>                           DW_AT_decl_line             0x00000004
> < 4><0x00000078>          DW_TAG_subprogram
>                             DW_AT_name                  "<lambda>"
>                             DW_AT_artificial            yes(1)
>                             DW_AT_declaration           yes(1)
>                             DW_AT_object_pointer        <0x00000085>
>                             DW_AT_sibling               <0x0000009c>
> < 5><0x00000085>            DW_TAG_formal_parameter
>                               DW_AT_type                  <0x0000008a>
>                               DW_AT_artificial            yes(1)
> < 5><0x0000008a>            DW_TAG_pointer_type
>                               DW_AT_byte_size             0x00000008
>                               DW_AT_type                  <0x00000070>
> < 5><0x00000090>            DW_TAG_formal_parameter
>                               DW_AT_type                  <0x00000095>
> < 5><0x00000095>            DW_TAG_rvalue_reference_type
>                               DW_AT_byte_size             0x00000008
>                               DW_AT_type                  <0x00000070>
> < 4><0x0000009c>          DW_TAG_subprogram
>                             DW_AT_name                  "<lambda>"
>                             DW_AT_artificial            yes(1)
>                             DW_AT_declaration           yes(1)
>                             DW_AT_object_pointer        <0x000000a9>
>                             DW_AT_sibling               <0x000000bf>
> < 5><0x000000a9>            DW_TAG_formal_parameter
>                               DW_AT_type                  <0x0000008a>
>                               DW_AT_artificial            yes(1)
> < 5><0x000000ae>            DW_TAG_formal_parameter
>                               DW_AT_type                  <0x000000b3>
> < 5><0x000000b3>            DW_TAG_reference_type
>                               DW_AT_byte_size             0x00000008
>                               DW_AT_type                  <0x000000b9>
> < 5><0x000000b9>            DW_TAG_const_type
>                               DW_AT_type                  <0x00000070>
> < 4><0x000000bf>          DW_TAG_subprogram
>                             DW_AT_name                  "<lambda>"
>                             DW_AT_artificial            yes(1)
>                             DW_AT_declaration           yes(1)
>                             <Unknown AT value 0x211a>   yes(1)
>                             DW_AT_object_pointer        <0x000000cc>
>                             DW_AT_sibling               <0x000000d2>
> < 5><0x000000cc>            DW_TAG_formal_parameter
>                               DW_AT_type                  <0x0000008a>
>                               DW_AT_artificial            yes(1)
> < 4><0x000000d2>          DW_TAG_subprogram
>                             DW_AT_name                  "~<lambda>"
>                             DW_AT_artificial            yes(1)
>                             DW_AT_declaration           yes(1)
>                             DW_AT_object_pointer        <0x000000df>
>                             DW_AT_sibling               <0x000000ea>
> < 5><0x000000df>            DW_TAG_formal_parameter
>                               DW_AT_type                  <0x0000008a>
>                               DW_AT_artificial            yes(1)
> < 5><0x000000e4>            DW_TAG_formal_parameter
>                               DW_AT_type                  <0x0000002d>
>                               DW_AT_artificial            yes(1)
> < 4><0x000000ea>          DW_TAG_subprogram
>                             DW_AT_name                  "operator()"
>                             DW_AT_type                  <0x0000002d>
>                             DW_AT_artificial            yes(1)
>                             DW_AT_low_pc                0x00400566
>                             DW_AT_high_pc               <offset-from-lowpc>19
>                             DW_AT_frame_base            len 0x0001:
> 9c: DW_OP_call_frame_cfa
>                             DW_AT_object_pointer        <0x0000010f>
>                             DW_AT_GNU_all_call_sites    yes(1)
> < 5><0x00000109>            DW_TAG_pointer_type
>                               DW_AT_byte_size             0x00000008
>                               DW_AT_type                  <0x000000b9>
> < 5><0x0000010f>            DW_TAG_formal_parameter
>                               DW_AT_name                  "__closure"
>                               DW_AT_type                  <0x0000011b>
>                               DW_AT_artificial            yes(1)
>                               DW_AT_location              len 0x0002:
> 9168: DW_OP_fbreg -24
> < 5><0x0000011b>            DW_TAG_const_type
>                               DW_AT_type                  <0x00000109>
> < 5><0x00000120>            DW_TAG_formal_parameter
>                               DW_AT_name                  "j"
>                               DW_AT_decl_file             0x00000001 /tmp
>                               DW_AT_decl_line             0x00000004
>                               DW_AT_type                  <0x0000002d>
>                               DW_AT_location              len 0x0002:
> 9164: DW_OP_fbreg -28
>
> Notice that the operator() method has no linkage name specified in the
> DWARF. But, it has a DW_AT_low_pc. Lets grep the binary for this low
> pc value:
>
> $> readelf -a lambda | grep 400566
>     41: 0000000000400566    19 FUNC    LOCAL  DEFAULT   12 _ZZ4mainENKUliE_clEi
>
> So then, there is an ELF symbol for the operator() method! Lets
> demangle the name:
>
> (gdb) maintenance demangle _ZZ4mainENKUliE_clEi
> main::{lambda(int)#1}::operator()(int) const
>
> This name is not the same as the name of the lambda structure given by
> DWARF! If I use clang to compile, the demangled ELF symbol for the
> operator() turns out to be "main::$_0::operator()(int) const", which
> seems to indicate that gcc and clang have their own way of
> differentiating the different lambdas that can occur in a CU. Hence,
> GDB should probably not even attempt to generate a name, mangle it and
> lookup for a symbol with that name. However, low_pc is immediately
> available in the DWARF generated by GCC, so why not use it?

For functions/methods gdb already uses lowpc as the start address.
[IOW it doesn't use the linkage name, though even recently
I'd forgotten this and thought otherwise.
Also, GDB doesn't take a demangled name, mangle it,
and then try to look that up in the ELF symbol table.]

If I do "mt print symbols foo.sym", then I see this:

Symtab for file lambda.cc
Compilation directory is /home/dje/src/play
Read from object file /home/dje/ruffies-copy/src/play/lambda.x64 (0x288bf00)
Language: c++

Line table:

 line 6 at 0x4006a6
 line 6 at 0x4006b1
 line 5 at 0x4006b9
 line 7 at 0x4006c1
 line 8 at 0x4006d3
 line 0 at 0x4006d5

Blockvector:

block #000, object at 0x277ee90, 1 syms/buckets in 0x4006a6..0x4006d5
 int main(); block object 0x277edc0, 0x4006b9..0x4006d5 section .text
  block #001, object at 0x277ee20 under 0x277ee90, 1 syms/buckets in
0x4006a6..0x4006d5
   typedef int int;
        block #002, object at 0x277ec80 under 0x277ed50, 3
syms/buckets in 0x4006a6..0x4006b9, function
<lambda(int)>::operator()(int) const
         int <lambda(int)>::operator()(int) const; block object
0x277ec80, 0x4006a6..0x4006b9
         const struct <lambda(int)> {
         } * const __closure; computed at runtime
         int j; computed at runtime
    block #003, object at 0x277edc0 under 0x277ee20, 0 syms/buckets in
0x4006b9..0x4006d5, function main()
      block #004, object at 0x277ed50 under 0x277edc0, 1 syms/buckets
in 0x4006c1..0x4006d3
       struct <lambda(int)> {
       } lambda; computed at runtime
       struct <lambda(int)> {
       };

Note that there is a block for "function
<<lambda(int)>::operator()(int) const>" with a start address of
0x4006a6.

(gdb) ! nm lambda.x64 | grep ZZ4
00000000004006a6 t _ZZ4mainENKUliE_clEi

So I think(!) the existing mechanism of using
BLOCK_START (SYMBOL_BLOCK_VALUE (sym))
should work here, we just need to make sure
the various pieces are glued together.
But we don't AFAICT need a new field to record low_pc,
it is already recorded.

>  [Clang
> actually does not put out the low pc value for the subprogram die
> under a structure die, but it puts out a separate (not under a
> structure die) subprogram die for the operator() method with the low
> pc value.]

DWARF can be hard to read at times.
Note that GCC can do this too.  E.g.,
It will output a declaration in the proper context
(e.g., inside a class definition for a method) without DW_AT_low_pc
and then it will output another DIE at the top level with DW_AT_low_pc
and with a DW_AT_specification referring back to previous DIE in its context.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-25 22:10     ` Doug Evans
@ 2014-11-25 23:37       ` Siva Chandra
  2014-11-26  3:22         ` Doug Evans
  0 siblings, 1 reply; 14+ messages in thread
From: Siva Chandra @ 2014-11-25 23:37 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On Tue, Nov 25, 2014 at 2:09 PM, Doug Evans <dje@google.com> wrote:
> For functions/methods gdb already uses lowpc as the start address.

May be I am reading wrong somewhere, but if I am correct, GDB
currently gets to a low pc (the address of the method) of a method via
its symbol. So, I agree that GDB uses low pc anyway.

> [IOW it doesn't use the linkage name, though even recently
> I'd forgotten this and thought otherwise.
> Also, GDB doesn't take a demangled name, mangle it,
> and then try to look that up in the ELF symbol table.]

May be I messed up some terminology somewhere, symbol handling side of
GDB is new to me. But, looking at value_fn_field, it appears to me
that GDB gets a method's address via its linkage name (first looking
up its symbol/minsym). Also, value_fn_field uses lookup_symbol which
uses lookup_symbol_in_language which demangles the linkage name. Based
on this, I concluded that GDB cannot get the address of a method if
the linkage name is missing. This is my understanding now, which could
be wrong ofcourse!

>>  [Clang
>> actually does not put out the low pc value for the subprogram die
>> under a structure die, but it puts out a separate (not under a
>> structure die) subprogram die for the operator() method with the low
>> pc value.]
>
> DWARF can be hard to read at times.
> Note that GCC can do this too.  E.g.,
> It will output a declaration in the proper context
> (e.g., inside a class definition for a method) without DW_AT_low_pc
> and then it will output another DIE at the top level with DW_AT_low_pc
> and with a DW_AT_specification referring back to previous DIE in its context.

Yes, Clang takes this exact route and things work fine with a Clang
generated binary. GCC does not generate the top level DIE at all, but
stuffs in DW_AT_low_pc into the DIE under the class definition and
does not specify the linkage name. So, there is no DIE specifying the
linkage name of operator(). IMO however, what GCC is emitting is
logical sufficient. In which case, GDB needs to store the low pc value
somewhere, and my patch puts it in the type struct.

Thanks,
Siva Chandra

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-25 23:37       ` Siva Chandra
@ 2014-11-26  3:22         ` Doug Evans
  2014-11-26  3:58           ` Siva Chandra
  2014-11-26  7:31           ` Siva Chandra
  0 siblings, 2 replies; 14+ messages in thread
From: Doug Evans @ 2014-11-26  3:22 UTC (permalink / raw)
  To: Siva Chandra; +Cc: gdb-patches

On Tue, Nov 25, 2014 at 3:37 PM, Siva Chandra <sivachandra@google.com> wrote:
> On Tue, Nov 25, 2014 at 2:09 PM, Doug Evans <dje@google.com> wrote:
>> For functions/methods gdb already uses lowpc as the start address.
>
> May be I am reading wrong somewhere, but if I am correct, GDB
> currently gets to a low pc (the address of the method) of a method via
> its symbol. So, I agree that GDB uses low pc anyway.

GDB's symbol handling is pretty confusing.

A better way to phrase this:

"GDB currently gets to a low pc (the address of the method) of a method via
its symbol"

is to write it as:

"One way GDB currently gets to a low pc (the address of the method)
of a method via its ELF symbol."

[Not only is "symtab" ambiguous in GDB, so is "symbol" ...]

GDB tries a myriad of different ways to look up a symbol.
If one way doesn't work it tries another.
If that doesn't work it tries another.
And on and on.

Typically the last way GDB tries is to look the symbol up
in the minsym symtab (the ELF symbol table).
Note that this works for demangled c++ symbols
not because we mangle them and then look up the
linkage name (note: even "linkage name"
is ambiguous in GDB!), but rather because we pre-demangle
ELF symbols (*1) and match them that way.
So while gdb uses minsyms for lookup, it does that
as a last resort (as always there are exceptions,
e.g., LOC_UNRESOLVED).

(*1): At a cost of 10 of the 13 seconds of GDB startup time and
a cost of 480MB of memory, for one of my benchmarks.
Besides the performance cost, looking up minsyms
this way hides bugs: PRs 17602, 17603.
IOW, what if one of the preceding lookups *should* have worked?

One useful experiment that I do from time to time is
hack out minsyms, and see what happens.
E.g., What still works? What broke? How much faster is gdb?

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 19aaed3..655872f 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1126,6 +1126,8 @@ elf_read_minimal_symbols (struct objfile
*objfile, int symfile_flags,
   set_objfile_data (objfile, dbx_objfile_data_key, dbx);
   make_cleanup (free_elfinfo, (void *) objfile);

+  if (0) {
+
   /* Process the normal ELF symbol table first.  This may write some
      chain of info into the dbx_symfile_info of the objfile, which can
      later be used by elfstab_offset_sections.  */
@@ -1213,6 +1215,8 @@ elf_read_minimal_symbols (struct objfile
*objfile, int symfile_flags,
                       synth_symbol_table, 1);
     }

+  }
+
   /* Install any minimal symbols that have been collected as the current
      minimal symbols for this objfile.  The debug readers below this point
      should not generate new minimal symbols; if they do it's their

[N.B. That patch may not apply properly due to cut-n-paste errors.]

btw, One interesting thing this experiment shows is that (currently)
"start" != "tb main; run".
IIRC, "start" needs the minsym for main, "tb main; run" does not.


>> [IOW it doesn't use the linkage name, though even recently
>> I'd forgotten this and thought otherwise.
>> Also, GDB doesn't take a demangled name, mangle it,
>> and then try to look that up in the ELF symbol table.]
>
> May be I messed up some terminology somewhere, symbol handling side of
> GDB is new to me. But, looking at value_fn_field, it appears to me
> that GDB gets a method's address via its linkage name (first looking
> up its symbol/minsym). Also, value_fn_field uses lookup_symbol which
> uses lookup_symbol_in_language which demangles the linkage name. Based
> on this, I concluded that GDB cannot get the address of a method if
> the linkage name is missing. This is my understanding now, which could
> be wrong ofcourse!

fwiw, GDB is bad enough that I often can't trust just reading the code.
I often punt on that and actually single step through
the relevant code to REALLY see what is happening.

While lookup_symbol_in_language will demangle a mangled
name for lookup, a good first question to answer is what name
is being passed?  Typically it is not a mangled name.
[In another email I mentioned that the handling of lookup
of mangled vs demangled symbols is a cleanup topic in itself. :-)]

>>>  [Clang
>>> actually does not put out the low pc value for the subprogram die
>>> under a structure die, but it puts out a separate (not under a
>>> structure die) subprogram die for the operator() method with the low
>>> pc value.]
>>
>> DWARF can be hard to read at times.
>> Note that GCC can do this too.  E.g.,
>> It will output a declaration in the proper context
>> (e.g., inside a class definition for a method) without DW_AT_low_pc
>> and then it will output another DIE at the top level with DW_AT_low_pc
>> and with a DW_AT_specification referring back to previous DIE in its context.
>
> Yes, Clang takes this exact route and things work fine with a Clang
> generated binary. GCC does not generate the top level DIE at all, but
> stuffs in DW_AT_low_pc into the DIE under the class definition and
> does not specify the linkage name. So, there is no DIE specifying the
> linkage name of operator(). IMO however, what GCC is emitting is
> logical sufficient. In which case, GDB needs to store the low pc value
> somewhere, and my patch puts it in the type struct.

GDB already has a place to store the start address for functions.
Why is that not working here?

Note that in my previous message, where I dumped the symbols,
there is a symbol for operator() and it has the correct start address.
IOW, GDB already has the start address.
I still don't see the need for the new field.
It seems to me that what's wrong is the lookup.
Why isn't GDB finding that symbol, and how do we fix that?

btw, when I try your patch I get one fail:

p lambda(10)^M
Invalid data type for function to be called.^M
(gdb) FAIL: gdb.dwarf2/dw2-member-function-addr.exp: p lambda()

Do you see this?
I see this with clang too.

I think the problem we need to solve is connecting the object
to its operator().

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-26  3:22         ` Doug Evans
@ 2014-11-26  3:58           ` Siva Chandra
  2014-11-26  5:19             ` Doug Evans
  2014-11-26  7:31           ` Siva Chandra
  1 sibling, 1 reply; 14+ messages in thread
From: Siva Chandra @ 2014-11-26  3:58 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

I will go over your comments in detail and respond. One clarification
until then ...

On Tue, Nov 25, 2014 at 7:22 PM, Doug Evans <dje@google.com> wrote:
> btw, when I try your patch I get one fail:
>
> p lambda(10)^M
> Invalid data type for function to be called.^M
> (gdb) FAIL: gdb.dwarf2/dw2-member-function-addr.exp: p lambda()

As mentioned in my very first email, the tests depend on
https://sourceware.org/ml/gdb-patches/2014-11/msg00479.html. We can
remove this dependency by invoking the operator() explicitly
"lambda.operator()(10)".

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-24 20:28   ` Doug Evans
@ 2014-11-26  4:36     ` Doug Evans
  2014-11-26  4:46       ` Siva Chandra
  0 siblings, 1 reply; 14+ messages in thread
From: Doug Evans @ 2014-11-26  4:36 UTC (permalink / raw)
  To: Siva Chandra; +Cc: gdb-patches

On Mon, Nov 24, 2014 at 12:27 PM, Doug Evans <dje@google.com> wrote:
> On Mon, Nov 24, 2014 at 12:22 PM, Doug Evans <dje@google.com> wrote:
>> On Mon, Nov 24, 2014 at 7:54 AM, Siva Chandra <sivachandra@google.com> wrote:
>>> [The tests in this patch depend on this patch:
>>> https://sourceware.org/ml/gdb-patches/2014-11/msg00479.html.  Also, it
>>> adds a new dwarf2 test which I am not very sure I got it right.]
>>>
>>> While processing a struct die, store the method's address in its fn_field.
>>>
>>> This enables calling the method when its physname is missing and its
>>> minsym cannot be discovered, but its address (DW_AT_low_pc) is available.
>>> For example, this happens for the operator() methods of c++11 lambdas.
>>> Consider the following C++ code:
>>>
>>> int
>>> main ()
>>> {
>>>   auto lambda = [] (int j) { return j + 113; };
>>>   return lambda (-113);
>>> }
>>>
>>> When compiled with g++, the DWARF corresponding to the lambda's operator()
>>> shows up under the DWARF for the function main as follows:
>>>
>>> DW_TAG_subprogram
>>>   DW_AT_name                  "operator()"
>>>   DW_AT_type                  <0x0000002d>
>>>   DW_AT_artificial            yes(1)
>>>   DW_AT_low_pc                0x00400566
>>>   DW_AT_high_pc               <offset-from-lowpc>19
>>>   DW_AT_frame_base            len 0x0001: 9c: DW_OP_call_frame_cfa
>>>   DW_AT_object_pointer        <0x0000010f>
>>>   DW_AT_GNU_all_call_sites    yes(1)
>>>   DW_TAG_pointer_type
>>>     DW_AT_byte_size             0x00000008
>>>     DW_AT_type                  <0x000000b9>
>>>   DW_TAG_formal_parameter
>>>     DW_AT_name                  "__closure"
>>>     DW_AT_type                  <0x0000011b>
>>>     DW_AT_artificial            yes(1)
>>>     DW_AT_location              len 0x0002: 9168: DW_OP_fbreg -24
>>>   DW_TAG_const_type
>>>     DW_AT_type                  <0x00000109>
>>>   DW_TAG_formal_parameter
>>>     DW_AT_name                  "j"
>>>     DW_AT_decl_file             0x00000001
>>>     DW_AT_decl_line             0x00000004
>>>     DW_AT_type                  <0x0000002d>
>>>     DW_AT_location              len 0x0002: 9164:DW_OP_fbreg -28
>>>
>>> There is no physname and the minsym corresponding to the the operator()
>>> method does not demangle to its qualified name as specified by the DWARF.
>>> However, since DW_AT_low_pc is available, it can be used to create a value
>>> corresponding to the method in value.c:value_fn_field and subsequently be
>>> passed to call_function_by_hand to invoke it.
>>>
>>> gdb/ChangeLog:
>>>
>>> 2014-11-24  Siva Chandra Reddy  <sivachandra@google.com>
>>>
>>>         * dwarf2read.c (dwarf2_add_member_fn): Note the methods address
>>>         if its DT_AT_low_pc is available.
>>>         * gdbtypes.h (struct fn_field): New field ADDR.
>>>         (TYPE_FN_FIELD_ADDR): New macro.
>>>         * value.c (value_fn_field): Use address of the method if
>>>         available.
>>>
>>> gdb/testsuite/ChangeLog:
>>>
>>> 2014-11-24  Siva Chandra Reddy  <sivachandra@google.com>
>>>
>>>         * gdb.dwarf2/dw2-member-function-addr.S: New file.
>>>         * gdb.dwarf2/dw2-member-function-addr.exp: New file.
>>
>> Hi.
>>
>> [I have to admit I hadn't realized gdb used low_pc for the entry point.
>> Or more likely forgotten. :-)
>> There's nothing to say that low_pc is the entry point,
>> but gdb has been using this since forever I'm guessing,
>> and if it wasn't the entry point presumably the compiler could emit
>> a DW_TAG_entry_point record.]
>>
>> Adding new fields to types or symbols is a big deal.
>> I'd like to understand why things are failing better.
>> For normal functions gdb gets the start address
>> from BLOCK_START (SYMBOL_BLOCK_VALUE (sym)).
>
> Bleah, hit Send too soon.
>
> Nits:
>
> 1) The testcase is amd64-linux specific, which is ok, but the .exp file
> needs a test to check for this.  grep for x86_64 in gdb.dwarf2/*.exp.
>
> 2) Please massage the assembler output and change things like
> this to something more generic.  "/tmp" or some such.
>
> +       .long   .LASF3  # DW_AT_comp_dir: "/home/sivachandra/LAB/c++"

Hi.
While going through this is occurs to me: Do we *need* an assembler
testcase here?
If we're just testing lambda hand-callability, it would be preferable
to just have a .cc test.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-26  4:36     ` Doug Evans
@ 2014-11-26  4:46       ` Siva Chandra
  2014-11-26  5:05         ` Doug Evans
  0 siblings, 1 reply; 14+ messages in thread
From: Siva Chandra @ 2014-11-26  4:46 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On Tue, Nov 25, 2014 at 8:36 PM, Doug Evans <dje@google.com> wrote:
> Hi.
> While going through this is occurs to me: Do we *need* an assembler
> testcase here?
> If we're just testing lambda hand-callability, it would be preferable
> to just have a .cc test.

I preferred an assembler testcase because the problem the patch is
trying to solve does not occur if Clang is the compiler.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-26  4:46       ` Siva Chandra
@ 2014-11-26  5:05         ` Doug Evans
  0 siblings, 0 replies; 14+ messages in thread
From: Doug Evans @ 2014-11-26  5:05 UTC (permalink / raw)
  To: Siva Chandra; +Cc: gdb-patches

On Tue, Nov 25, 2014 at 8:46 PM, Siva Chandra <sivachandra@google.com> wrote:
> On Tue, Nov 25, 2014 at 8:36 PM, Doug Evans <dje@google.com> wrote:
>> Hi.
>> While going through this is occurs to me: Do we *need* an assembler
>> testcase here?
>> If we're just testing lambda hand-callability, it would be preferable
>> to just have a .cc test.
>
> I preferred an assembler testcase because the problem the patch is
> trying to solve does not occur if Clang is the compiler.

GDB's debug info readers are sometimes a pain to work with because
they have to handle different compilers, and sometimes compiler bugs,
but in general we only go to assembler if there's something specific
in the assembler we need.  Here the fact that the .cc fails with gcc
is enough I think.  [assember tests bring their own pain, and are not
to be added lightly]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-26  3:58           ` Siva Chandra
@ 2014-11-26  5:19             ` Doug Evans
  0 siblings, 0 replies; 14+ messages in thread
From: Doug Evans @ 2014-11-26  5:19 UTC (permalink / raw)
  To: Siva Chandra; +Cc: gdb-patches

On Tue, Nov 25, 2014 at 7:58 PM, Siva Chandra <sivachandra@google.com> wrote:
> I will go over your comments in detail and respond. One clarification
> until then ...
>
> On Tue, Nov 25, 2014 at 7:22 PM, Doug Evans <dje@google.com> wrote:
>> btw, when I try your patch I get one fail:
>>
>> p lambda(10)^M
>> Invalid data type for function to be called.^M
>> (gdb) FAIL: gdb.dwarf2/dw2-member-function-addr.exp: p lambda()
>
> As mentioned in my very first email, the tests depend on
> https://sourceware.org/ml/gdb-patches/2014-11/msg00479.html. We can
> remove this dependency by invoking the operator() explicitly
> "lambda.operator()(10)".

Ah righto.

Digging a bit deeper, I see the lookup_symbol call in value_fn_field
gets passed NULL for block.
That means that lookup_local_symbol won't do anything, but what we're
looking for (as can be seen in the symbol dump) is not in STATIC_BLOCK
(block #001) nor GLOBAL_BLOCK (block #000).

If I manually hack the value passed to lookup_symbol to be a block with
__lambda0::operator()(int) const,
and disable your patch, then "p lambda(5)" works.

So I think we need to figure out how to pass a usable value for block
to lookup_symbol in value_fn_field.  Also, I wouldn't preclude a gcc
bug here and/or a bug in dwarf2read.c where the lambda should have
been in a different block than the one it was put in.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-26  3:22         ` Doug Evans
  2014-11-26  3:58           ` Siva Chandra
@ 2014-11-26  7:31           ` Siva Chandra
  2014-11-26 21:11             ` Doug Evans
  1 sibling, 1 reply; 14+ messages in thread
From: Siva Chandra @ 2014-11-26  7:31 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

Phew...

On Tue, Nov 25, 2014 at 7:22 PM, Doug Evans <dje@google.com> wrote:
> fwiw, GDB is bad enough that I often can't trust just reading the code.
> I often punt on that and actually single step through
> the relevant code to REALLY see what is happening.

I single stepped through the code and now see everything that you said here!

I believed this in gdbtypes.h:

 889             /* * If is_stub is clear, this is the mangled name which
 890                we can look up to find the address of the method
 891                (FIXME: it would be cleaner to have a pointer to the
 892                struct symbol here instead).
 893
 894                If is_stub is set, this is the portion of the mangled
 895                name which specifies the arguments.  For example, "ii",
 896                if there are two int arguments, or "" if there are no
 897                arguments.  See gdb_mangle_name for the conversion from
 898                this format to the one used if is_stub is clear.  */
 899
 900             const char *physname;

Rest of what I thought I understood fell out from this belief! And the
names look so enticing.

Resetting my mind ...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] While processing a struct die, store the method's address in its fn_field
  2014-11-26  7:31           ` Siva Chandra
@ 2014-11-26 21:11             ` Doug Evans
  0 siblings, 0 replies; 14+ messages in thread
From: Doug Evans @ 2014-11-26 21:11 UTC (permalink / raw)
  To: Siva Chandra; +Cc: gdb-patches

On Tue, Nov 25, 2014 at 11:31 PM, Siva Chandra <sivachandra@google.com> wrote:
> Phew...
>
> On Tue, Nov 25, 2014 at 7:22 PM, Doug Evans <dje@google.com> wrote:
>> fwiw, GDB is bad enough that I often can't trust just reading the code.
>> I often punt on that and actually single step through
>> the relevant code to REALLY see what is happening.
>
> I single stepped through the code and now see everything that you said here!
>
> I believed this in gdbtypes.h:
>
>  889             /* * If is_stub is clear, this is the mangled name which
>  890                we can look up to find the address of the method
>  891                (FIXME: it would be cleaner to have a pointer to the
>  892                struct symbol here instead).
>  893
>  894                If is_stub is set, this is the portion of the mangled
>  895                name which specifies the arguments.  For example, "ii",
>  896                if there are two int arguments, or "" if there are no
>  897                arguments.  See gdb_mangle_name for the conversion from
>  898                this format to the one used if is_stub is clear.  */
>  899
>  900             const char *physname;
>
> Rest of what I thought I understood fell out from this belief! And the
> names look so enticing.
>
> Resetting my mind ...

Zoinks!  Yeah, that comment needs some TLC.

[btw, the term "physname" is in a transition mode.
In some places it means "mangled name".
In other places it means "demangled name".
It's not clear to me what we want to transition to though.]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-11-26 21:11 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-24 15:55 [RFC] While processing a struct die, store the method's address in its fn_field Siva Chandra
2014-11-24 20:22 ` Doug Evans
2014-11-24 20:28   ` Doug Evans
2014-11-26  4:36     ` Doug Evans
2014-11-26  4:46       ` Siva Chandra
2014-11-26  5:05         ` Doug Evans
2014-11-25 15:00   ` Siva Chandra
2014-11-25 22:10     ` Doug Evans
2014-11-25 23:37       ` Siva Chandra
2014-11-26  3:22         ` Doug Evans
2014-11-26  3:58           ` Siva Chandra
2014-11-26  5:19             ` Doug Evans
2014-11-26  7:31           ` Siva Chandra
2014-11-26 21:11             ` Doug Evans

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).