Bug 44126 - wrong location description for DW_AT_vtable_elem_location
Summary: wrong location description for DW_AT_vtable_elem_location
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: debug (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-debug
Depends on:
Blocks:
 
Reported: 2010-05-13 20:37 UTC by Tom Tromey
Modified: 2023-04-03 02:03 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-05-13 20:42:01


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Tromey 2010-05-13 20:37:37 UTC
Consider this simple example:

class K {
public:
  virtual int x() { return 23; }
};

K k;  


This yields this attribute in the dwarf:

<7a>   DW_AT_vtable_elem_location: 2 byte block: 10 0       (DW_OP_constu: 0)

However, this is incorrect.
The location description ought to compute the address of the vtable slot.
Comment 1 Jason Merrill 2010-05-13 20:42:01 UTC
"An entry for a virtual function also has a DW_AT_vtable_elem_location attribute whose value contains a location description yielding the address of the slot for the function within the virtual function table for the enclosing class. The address of an object of the enclosing type is pushed onto the expression stack before the location description is evaluated."

Dodji, want to look at this?
Comment 2 Jakub Jelinek 2010-05-13 21:48:47 UTC
If I understand it well, we should:
--- dwarf2out.c 2010-05-13 23:36:24.000000000 +0200
+++ dwarf2out.c    2010-05-13 23:55:07.422464196 +0200     
@@ -17094,10 +17094,19 @@ add_pure_or_virtual_attribute (dw_die_re
       add_AT_unsigned (die, DW_AT_virtuality, DW_VIRTUALITY_virtual);
 
       if (host_integerp (DECL_VINDEX (func_decl), 0))
-       add_AT_loc (die, DW_AT_vtable_elem_location,
-                   new_loc_descr (DW_OP_constu,
-                                  tree_low_cst (DECL_VINDEX (func_decl), 0),
-                                  0));
+       {
+         dw_loc_descr_ref loc;
+         HOST_WIDE_INT offset 
+           = tree_low_cst (DECL_VINDEX (func_decl), 0)
+             * (POINTER_SIZE / BITS_PER_UNIT);
+         if (DWARF2_ADDR_SIZE == (POINTER_SIZE / BITS_PER_UNIT))
+           loc = new_loc_descr (DW_OP_deref, 0, 0);
+         else
+           loc = new_loc_descr (DW_OP_deref_size,
+                                POINTER_SIZE / BITS_PER_UNIT, 0);
+         add_loc_descr (&loc, new_loc_descr (DW_OP_plus_uconst, offset, 0));
+         add_AT_loc (die, DW_AT_vtable_elem_location, loc);
+       }
 
       /* GNU extension: Record what type this method came from originally.
        * */
       if (debug_info_level > DINFO_LEVEL_TERSE

The consumers should be pushing address of object, so we need to dereference its first pointer to get at vtable + 8 resp. 16 and DECL_VINDEX is number of pointers after that spot (seems to be the case even on ia64).

The problem is that gdb won't grok that, so we have a chicken-and-egg issue.
Comment 3 Dodji Seketeli 2010-05-14 06:41:33 UTC
Subject: Re:  wrong location description for
 DW_AT_vtable_elem_location

> Dodji, want to look at this?
Sure.

Like, Jakub said, we need to synchronize with GDB. I'll test Jakub's
patch ASAP and push the change when GDB is ready to consume it then.
Comment 4 Tom Tromey 2010-06-11 14:53:07 UTC
I think the problem with this patch is that it leaves gdb no way
to determine which approach it should use.  This is important because
there is a lot of existing code compiled with the incorrect approach.

Currently gdb has a heuristic where it uses the incorrect style
if it sees DW_AT_containing_type in the virtual function's DIE.
This appears to be a GNU extension (you can see it at the end of the
patch, in the context -- but there is another instance in dwarf2out.c).
Maybe removing that would help.  I'm not sure if it is needed and/or
useful for some other reason, maybe a little exploration of the history
would help.
Comment 5 Tom Tromey 2010-06-11 15:07:11 UTC
Jakub pointed out that gdb can just look for an isolated
DW_OP_constu to fall back to the old code.
I will write a gdb patch.
Comment 6 Tom Tromey 2010-06-11 20:02:13 UTC
Ok, I committed the gdb change:
http://sourceware.org/ml/gdb-patches/2010-06/msg00287.html
Comment 7 Tom Tromey 2023-04-03 02:03:17 UTC
I happened to be looking in this area and I see that gcc still
generates the old, incorrect form.