| Message ID | 1398683852.4016.30.camel@bordewijk.wildebeest.org |
|---|---|
| State | New |
| Headers | show |
On Mon, Apr 28, 2014 at 01:17:32PM +0200, Mark Wielaard wrote: > On Tue, 2014-04-22 at 12:31 +0200, Mark Wielaard wrote: > > On Mon, 2014-04-14 at 23:19 +0200, Mark Wielaard wrote: > > > On Fri, 2014-04-11 at 11:03 -0700, Cary Coutant wrote: > > > > >> The DWARF bits are fine with me. > > > > > > > > > > Thanks. Who can approve the other bits? > > > > > > > > You should probably get C and C++ front end approval. I'm not really > > > > sure who needs to review patches in c-family/. Since the part in c/ is > > > > so tiny, maybe all you need is a C++ front end maintainer. Both > > > > Richard Henderson and Jason Merrill are global reviewers, so either of > > > > them could approve the whole thing. > > > > > > Thanks, I added them to the CC. > > > > > > > > When approved should I wait till stage 1 opens before committing? > > > > > > > > Yes. The PR you're fixing is an enhancement request, not a regression, > > > > so it needs to wait. > > > > > > Since stage one just opened up again this seems a good time to re-ask > > > for approval then :) Rebased patch against current trunk attached. > > > > Ping. Tom already pushed his patches to GDB that take advantage of the > > new information if available. > > Ping2. Please let me know if I should ping/cc other people to get this > reviewed. Do you want to add DW_AT_type to DW_TAG_enumeration only if it has explicit underlying type (enum class foo: char { ... };) or even when the underlying type is computed emplicitly (then you'd just use TREE_TYPE of the ENUMERAL_TYPE if non-NULL). Jakub
On Mon, 2014-04-28 at 14:23 +0200, Jakub Jelinek wrote: > On Mon, Apr 28, 2014 at 01:17:32PM +0200, Mark Wielaard wrote: > > Ping2. Please let me know if I should ping/cc other people to get this > > reviewed. > > Do you want to add DW_AT_type to DW_TAG_enumeration only if it has explicit > underlying type (enum class foo: char { ... };) or even when the underlying > type is computed emplicitly (then you'd just use TREE_TYPE of the > ENUMERAL_TYPE if non-NULL). The debugger cares about the actual underlying type used if the language can use multiple. Either explicitly assigned by the user or implicitly as derived by the language/compile flags used. So the lang hook should provide one in both cases, if appropriate. Cheers, Mark
On 04/28/2014 08:37 AM, Mark Wielaard wrote: > On Mon, 2014-04-28 at 14:23 +0200, Jakub Jelinek wrote: >> Do you want to add DW_AT_type to DW_TAG_enumeration only if it has explicit >> underlying type (enum class foo: char { ... };) or even when the underlying >> type is computed emplicitly (then you'd just use TREE_TYPE of the >> ENUMERAL_TYPE if non-NULL). > > The debugger cares about the actual underlying type used if the language > can use multiple. Either explicitly assigned by the user or implicitly > as derived by the language/compile flags used. So the lang hook should > provide one in both cases, if appropriate. Why do you want to do this for C? It seems to me that this is only interesting in C++ because of overload resolution. Jason
On Mon, May 12, 2014 at 11:22:11PM -0400, Jason Merrill wrote: > On 04/28/2014 08:37 AM, Mark Wielaard wrote: > >The debugger cares about the actual underlying type used if the language > >can use multiple. Either explicitly assigned by the user or implicitly > >as derived by the language/compile flags used. So the lang hook should > >provide one in both cases, if appropriate. > > Why do you want to do this for C? It seems to me that this is only > interesting in C++ because of overload resolution. So the debugger doesn't have to guess the properties of the enum's underlying base type, like size, encoding and signedness. Which are implementation defined and might differ between enums even in C depending on whether -fshort-enums was used (which is the default on some arches). Cheers, Mark
On 05/13/2014 03:21 AM, Mark Wielaard wrote: > So the debugger doesn't have to guess the properties of the enum's > underlying base type, like size, encoding and signedness. Well, the enum already has DW_AT_byte_size. It seems to me that it should also have DW_AT_encoding to provide the other two properties you mention. Jason
On Mon, May 19, 2014 at 04:50:35PM -0400, Jason Merrill wrote: > On 05/13/2014 03:21 AM, Mark Wielaard wrote: > >So the debugger doesn't have to guess the properties of the enum's > >underlying base type, like size, encoding and signedness. > > Well, the enum already has DW_AT_byte_size. It seems to me that it should > also have DW_AT_encoding to provide the other two properties you mention. Right, that is the idea, since an enum doesn't provide those attributes, it should have a reference to the underlying base type that provides them. Then it can also drop the DW_AT_byte_size, if it is different from the underlying base type, but that is a followup patch. Thanks, Mark
On 05/20/2014 02:55 AM, Mark Wielaard wrote: > On Mon, May 19, 2014 at 04:50:35PM -0400, Jason Merrill wrote: >> On 05/13/2014 03:21 AM, Mark Wielaard wrote: >>> So the debugger doesn't have to guess the properties of the enum's >>> underlying base type, like size, encoding and signedness. >> >> Well, the enum already has DW_AT_byte_size. It seems to me that it should >> also have DW_AT_encoding to provide the other two properties you mention. > > Right, that is the idea, since an enum doesn't provide those attributes, > it should have a reference to the underlying base type that provides them. Yes, but why is that better than providing them directly? The latter would seem to work better with non-C-family languages like Ada. Jason
On Tue, May 20, 2014 at 10:43:22AM -0400, Jason Merrill wrote: > On 05/20/2014 02:55 AM, Mark Wielaard wrote: > >On Mon, May 19, 2014 at 04:50:35PM -0400, Jason Merrill wrote: > >>On 05/13/2014 03:21 AM, Mark Wielaard wrote: > >>>So the debugger doesn't have to guess the properties of the enum's > >>>underlying base type, like size, encoding and signedness. > >> > >>Well, the enum already has DW_AT_byte_size. It seems to me that it should > >>also have DW_AT_encoding to provide the other two properties you mention. > > > >Right, that is the idea, since an enum doesn't provide those attributes, > >it should have a reference to the underlying base type that provides them. > > Yes, but why is that better than providing them directly? Because that would loose the information of which underlying type is used to implement the enum. And it is probably less efficient to replicate all the attributes for each enum. It is also simply how the DWARF standard specifies these properties. They are not attached to the DW_TAG_enumeration_type directly but specified through the underlying type. Which is what gdb expects now. The DWARF part isn't what this patch is blocked on. That has already been discussed on the DWARF standard list, coordinated with the gdb hackers and approved some months ago. The part that hasn't been reviewed and approved yet is the frontend changes that implement the lang-hook. Thanks, Mark
On 05/20/2014 01:51 PM, Mark Wielaard wrote: > The DWARF part isn't what this patch is blocked on. That has already > been discussed on the DWARF standard list, coordinated with the gdb > hackers and approved some months ago. Fair enough. > The part that hasn't been reviewed > and approved yet is the frontend changes that implement the lang-hook. Rather than define the hook for C, let's have a default version that uses the type_for_size langhook; that should work better for Ada. Jason
> Yes, but why is that better than providing them directly? The latter > would seem to work better with non-C-family languages like Ada. That's correct, enumeration types don't have base types in Ada, i.e. they are their own base types. But if the attributes cannot be expressed in DWARF, then synthesizing a base type on the fly would probably be OK.
commit 603223a974054aa52512ca08f36f1550692240e5
Author: Mark Wielaard <mjw@redhat.com>
Date: Sun Mar 23 12:05:16 2014 +0100
PR debug/16063. Add DW_AT_type to DW_TAG_enumeration.
Add a new lang-hook that provides the underlying base type of an
ENUMERAL_TYPE. Including implementations for C and C++. Use this
enum_underlying_base_type lang-hook in dwarf2out.c to add a DW_AT_type
base type reference to a DW_TAG_enumeration.
gcc/
* dwarf2out.c (gen_enumeration_type_die): Add DW_AT_type if
enum_underlying_base_type defined and DWARF version > 3.
* langhooks.h (struct lang_hooks_for_types): Add
enum_underlying_base_type.
* langhooks-def.h (LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE): New define.
(LANG_HOOKS_FOR_TYPES_INITIALIZER): Add new lang hook.
gcc/c-family/
* c-common.c (c_enum_underlying_base_type): New function.
* c-common.h (c_enum_underlying_base_type): Add declaration.
gcc/c/
* c-objc-common.h (LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE): Define.
gcc/cp/
* cp-lang.c (cxx_enum_underlying_base_type): New function.
(LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE): Define.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b25f1f6..766e0e7 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+2014-03-21 Mark Wielaard <mjw@redhat.com>
+
+ PR debug/16063
+ * dwarf2out.c (gen_enumeration_type_die): Add DW_AT_type if
+ enum_underlying_base_type defined and DWARF version > 3.
+ * langhooks.h (struct lang_hooks_for_types): Add
+ enum_underlying_base_type.
+ * langhooks-def.h (LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE): New define.
+ (LANG_HOOKS_FOR_TYPES_INITIALIZER): Add new lang hook.
+
2014-04-27 Richard Sandiford <rdsandiford@googlemail.com>
* cselib.c (find_slot_memmode): Delete.
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index fb0d102..e652c1b 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,9 @@
+2014-03-21 Mark Wielaard <mjw@redhat.com>
+
+ PR debug/16063
+ * c-common.c (c_enum_underlying_base_type): New function.
+ * c-common.h (c_enum_underlying_base_type): Add declaration.
+
2014-04-25 Marek Polacek <polacek@redhat.com>
PR c/18079
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 0ad955d..6862c6f 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -3906,6 +3906,14 @@ c_register_builtin_type (tree type, const char* name)
registered_builtin_types = tree_cons (0, type, registered_builtin_types);
}
+
+/* The C version of the enum_underlying_base_type langhook. */
+tree
+c_enum_underlying_base_type (const_tree type)
+{
+ return c_common_type_for_size (TYPE_PRECISION (type), TYPE_UNSIGNED (type));
+}
+
/* Print an error message for invalid operands to arith operation
CODE with TYPE0 for operand 0, and TYPE1 for operand 1.
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 57b7dce..25c3272 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -832,6 +832,7 @@ extern void c_common_finish (void);
extern void c_common_parse_file (void);
extern alias_set_type c_common_get_alias_set (tree);
extern void c_register_builtin_type (tree, const char*);
+extern tree c_enum_underlying_base_type (const_tree);
extern bool c_promoting_integer_type_p (const_tree);
extern int self_promoting_args_p (const_tree);
extern tree strip_pointer_operator (tree);
diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index 80841af..1eb3782 100644
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -1,3 +1,8 @@
+2014-03-21 Mark Wielaard <mjw@redhat.com>
+
+ PR debug/16063
+ * c-objc-common.h (LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE): Define.
+
2014-04-25 Marek Polacek <polacek@redhat.com>
PR c/18079
diff --git a/gcc/c/c-objc-common.h b/gcc/c/c-objc-common.h
index 92cf60f..0651db7 100644
--- a/gcc/c/c-objc-common.h
+++ b/gcc/c/c-objc-common.h
@@ -84,6 +84,8 @@ along with GCC; see the file COPYING3. If not see
#define LANG_HOOKS_TO_TARGET_CHARSET c_common_to_target_charset
#undef LANG_HOOKS_EXPR_TO_DECL
#define LANG_HOOKS_EXPR_TO_DECL c_expr_to_decl
+#undef LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE
+#define LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE c_enum_underlying_base_type
/* The C front end's scoping structure is very different from
that expected by the language-independent code; it is best
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 5840461..cd0055a 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,9 @@
+2014-03-21 Mark Wielaard <mjw@redhat.com>
+
+ PR debug/16063
+ * cp-lang.c (cxx_enum_underlying_base_type): New function.
+ (LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE): Define.
+
2014-04-24 Jakub Jelinek <jakub@redhat.com>
* parser.c (cp_parser_omp_atomic): Allow seq_cst before
diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
index c28c07a..6a40d29 100644
--- a/gcc/cp/cp-lang.c
+++ b/gcc/cp/cp-lang.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see
#include "coretypes.h"
#include "tm.h"
#include "tree.h"
+#include "stor-layout.h"
#include "cp-tree.h"
#include "c-family/c-common.h"
#include "langhooks.h"
@@ -40,6 +41,7 @@ static enum classify_record cp_classify_record (tree type);
static tree cp_eh_personality (void);
static tree get_template_innermost_arguments_folded (const_tree);
static tree get_template_argument_pack_elems_folded (const_tree);
+static tree cxx_enum_underlying_base_type (const_tree);
/* Lang hooks common to C++ and ObjC++ are declared in cp/cp-objcp-common.h;
consequently, there should be very few hooks below. */
@@ -81,6 +83,8 @@ static tree get_template_argument_pack_elems_folded (const_tree);
#define LANG_HOOKS_EH_PERSONALITY cp_eh_personality
#undef LANG_HOOKS_EH_RUNTIME_TYPE
#define LANG_HOOKS_EH_RUNTIME_TYPE build_eh_type_type
+#undef LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE
+#define LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE cxx_enum_underlying_base_type
/* Each front end provides its own lang hook initializer. */
struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
@@ -219,5 +223,19 @@ get_template_argument_pack_elems_folded (const_tree t)
return fold_cplus_constants (get_template_argument_pack_elems (t));
}
+/* The C++ version of the enum_underlying_base_type langhook.
+ See also cp/semantics.c (finish_underlying_type). */
+static tree cxx_enum_underlying_base_type (const_tree type)
+{
+ tree underlying_type = ENUM_UNDERLYING_TYPE (type);
+
+ if (! ENUM_FIXED_UNDERLYING_TYPE_P (type))
+ underlying_type
+ = c_common_type_for_mode (TYPE_MODE (underlying_type),
+ TYPE_UNSIGNED (underlying_type));
+
+ return underlying_type;
+}
+
#include "gt-cp-cp-lang.h"
#include "gtype-cp.h"
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 1272326..4beed2d 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -17368,6 +17368,12 @@ gen_enumeration_type_die (tree type, dw_die_ref context_die)
TREE_ASM_WRITTEN (type) = 1;
add_byte_size_attribute (type_die, type);
+ if (lang_hooks.types.enum_underlying_base_type != NULL
+ && (dwarf_version >= 3 || !dwarf_strict))
+ {
+ tree underlying = lang_hooks.types.enum_underlying_base_type (type);
+ add_type_attribute (type_die, underlying, 0, 0, context_die);
+ }
if (TYPE_STUB_DECL (type) != NULL_TREE)
{
add_src_coords_attributes (type_die, TYPE_STUB_DECL (type));
diff --git a/gcc/langhooks-def.h b/gcc/langhooks-def.h
index 95bd379..b8e0d97 100644
--- a/gcc/langhooks-def.h
+++ b/gcc/langhooks-def.h
@@ -173,6 +173,7 @@ extern tree lhd_make_node (enum tree_code);
#define LANG_HOOKS_GET_SUBRANGE_BOUNDS NULL
#define LANG_HOOKS_DESCRIPTIVE_TYPE NULL
#define LANG_HOOKS_RECONSTRUCT_COMPLEX_TYPE reconstruct_complex_type
+#define LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE NULL
#define LANG_HOOKS_FOR_TYPES_INITIALIZER { \
LANG_HOOKS_MAKE_TYPE, \
@@ -191,7 +192,8 @@ extern tree lhd_make_node (enum tree_code);
LANG_HOOKS_GET_ARRAY_DESCR_INFO, \
LANG_HOOKS_GET_SUBRANGE_BOUNDS, \
LANG_HOOKS_DESCRIPTIVE_TYPE, \
- LANG_HOOKS_RECONSTRUCT_COMPLEX_TYPE \
+ LANG_HOOKS_RECONSTRUCT_COMPLEX_TYPE, \
+ LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE \
}
/* Declaration hooks. */
diff --git a/gcc/langhooks.h b/gcc/langhooks.h
index c848b0c..667298b 100644
--- a/gcc/langhooks.h
+++ b/gcc/langhooks.h
@@ -137,6 +137,8 @@ struct lang_hooks_for_types
return values from functions. The argument TYPE is the top of the
chain, and BOTTOM is the new type which we will point to. */
tree (*reconstruct_complex_type) (tree, tree);
+
+ tree (*enum_underlying_base_type) (const_tree);
};
/* Language hooks related to decls and the symbol table. */