Re: [PATCH] dwarf.h: Remove non-existing DW_TAG_mutable_type.

Re: [PATCH] dwarf.h: Remove non-existing DW_TAG_mutable_type.

[Mark Wielaard]

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

After Petr's backends cleanup patch this patch collapses to two
oneliners. Pushed as attached.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-dwarf.h-Remove-non-existing-DW_TAG_mutable_type.patch --]
[-- Type: text/x-patch, Size: 3291 bytes --]

>From af1e6ed9b4c0160a6a5c6f59d9443ae4f6f20a14 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Wed, 18 Jun 2014 10:57:58 +0200
Subject: [PATCH] dwarf.h: Remove non-existing DW_TAG_mutable_type.

The DW_TAG_mutable_type was only mentioned in an early draft of DWARFv3.
But was removed because there are no C++ mutable qualified types. It was
replaced by a new attribute DW_AT_mutable on DW_TAG_member DIEs. The new
attribute is available in dwarf.h.
http://dwarfstd.org/ShowIssue.php?issue=050223.1

DW_TAG_mutable_type was only used internally in some backends (which
just ignored it anyway).  dwarves did use it to turn it into a string
value, libabigail used it and ignored it (patches to remove sent).
GCC, GDB and binutils don't use nor define it.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 backends/ChangeLog    |    5 +++++
 backends/libebl_CPU.h |    4 ++--
 libdw/ChangeLog       |    4 ++++
 libdw/dwarf.h         |    4 ++--
 4 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/backends/ChangeLog b/backends/ChangeLog
index c846a95..a2df83b 100644
--- a/backends/ChangeLog
+++ b/backends/ChangeLog
@@ -1,3 +1,8 @@
+2014-06-18  Mark Wielaard  <mjw@redhat.com>
+
+	* libebl_CPU.h (dwarf_peel_type): Remove DW_TAG_mutable_type
+	handling.
+
 2014-06-17  Mark Wielaard  <mjw@redhat.com>
 
 	* arm_init.c (arm_init): Set func_addr_mask.
diff --git a/backends/libebl_CPU.h b/backends/libebl_CPU.h
index 3ad9258..fa0d4f2 100644
--- a/backends/libebl_CPU.h
+++ b/backends/libebl_CPU.h
@@ -1,5 +1,5 @@
 /* Common interface for libebl modules.
-   Copyright (C) 2000, 2001, 2002, 2003, 2005, 2013 Red Hat, Inc.
+   Copyright (C) 2000, 2001, 2002, 2003, 2005, 2013, 2014 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -60,7 +60,7 @@ dwarf_peel_type (Dwarf_Die *typediep, Dwarf_Attribute *attrp)
   int tag = DWARF_TAG_OR_RETURN (typediep);
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attrp = dwarf_attr_integrate (typediep, DW_AT_type, attrp);
       typediep = dwarf_formref_die (attrp, typediep);
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 780b34d..2884419 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,7 @@
+2014-06-18  Mark Wielaard  <mjw@redhat.com>
+
+	* dwarf.h: Remove DW_TAG_mutable_type.
+
 2014-05-02  Mark Wielaard  <mjw@redhat.com>
 
 	* libdwP.h (__check_build_id): Removed now unused.
diff --git a/libdw/dwarf.h b/libdw/dwarf.h
index 41cae5b..d98a4bd 100644
--- a/libdw/dwarf.h
+++ b/libdw/dwarf.h
@@ -1,5 +1,5 @@
 /* This file defines standard DWARF types, structures, and macros.
-   Copyright (C) 2000-2011 Red Hat, Inc.
+   Copyright (C) 2000-2011, 2014 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -87,7 +87,7 @@ enum
     DW_TAG_unspecified_type = 0x3b,
     DW_TAG_partial_unit = 0x3c,
     DW_TAG_imported_unit = 0x3d,
-    DW_TAG_mutable_type = 0x3e,
+    /* 0x3e reserved.  */
     DW_TAG_condition = 0x3f,
     DW_TAG_shared_type = 0x40,
     DW_TAG_type_unit = 0x41,
-- 
1.7.1

Re: [PATCH] dwarf.h: Remove non-existing DW_TAG_mutable_type.

[Mark Wielaard]

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

On Wed, 2014-06-18 at 11:48 -0700, Josh Stone wrote:
> On 06/18/2014 11:38 AM, Mark Wielaard wrote:
> > And a future version of DWARF might
> > use the number for a completely different purpose.
> 
> The issue you linked says this:
> 
>    Remove DW_TAG_mutable_type from Appendix A (Figure 17)
>    (Make sure not to reassign the number 0x3e to another tag.
>     Perhaps we should leave the table entry in there with
>     an "unused" marker in place of DW_TAG_mutable_type.
> 
> So it seems they intend to leave that number out forever.

They might and that might be a good thing. Although I don't know of any
producer that ever produced it since it was for a concept that didn't
really exist in any language. Note that the extra comments didn't end up
in the final spec. All that was officially done was remove the name. All
I am proposing is we do the same since the name should never have been
there in the first place. I'll add a comment instead saying the value is
reserved.

Cheers,

Mark

Re: [PATCH] dwarf.h: Remove non-existing DW_TAG_mutable_type.

[Roland McGrath]

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

I think it's fine to remove it and just leave a comment about the value
being reserved.

Re: [PATCH] dwarf.h: Remove non-existing DW_TAG_mutable_type.

[Josh Stone]

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

On 06/18/2014 11:38 AM, Mark Wielaard wrote:
> And a future version of DWARF might
> use the number for a completely different purpose.

The issue you linked says this:

   Remove DW_TAG_mutable_type from Appendix A (Figure 17)
   (Make sure not to reassign the number 0x3e to another tag.
    Perhaps we should leave the table entry in there with
    an "unused" marker in place of DW_TAG_mutable_type.

So it seems they intend to leave that number out forever.

Re: [PATCH] dwarf.h: Remove non-existing DW_TAG_mutable_type.

[Mark Wielaard]

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

On Wed, 2014-06-18 at 08:48 -0700, Josh Stone wrote:
> It's fine to patch its use out, in elfutils and others, but why take the
> API hit of removing the constant?  The number is still allocated, so it
> won't do any harm to leave it there.

It is not an ABI break. But technically it is indeed not backwards
source compatible. Problem with keeping it around is that I haven't seen
any correct usage of it. Also other dwarf.h headers, like the one from
binutils, don't define it either. And a future version of DWARF might
use the number for a completely different purpose. So IMHO it is much
better to just get rid of it.

> For example, libdwarf just commented on the removal:
> >         /*  Do not use DW_TAG_mutable_type */
> > #define DW_TAG_mutable_type 0x3e /* Withdrawn from DWARF3 by DWARF3f. */

I think the problem with doing that is that people won't read the header
file and will miss the comment thinking that if their code compiles it
is correct, while the fact they are using the define/constant means
their code is actually broken.

If there would be a good way to poison the define to cause a compiler
warning that would be good. Then we could deprecate it in the next
release and remove it in the one after. But I don't know of a way to do
that.

Thanks,

Mark

Re: [PATCH] dwarf.h: Remove non-existing DW_TAG_mutable_type.

[Josh Stone]

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

On 06/18/2014 02:48 AM, Mark Wielaard wrote:
> The DW_TAG_mutable_type was only mentioned in an early draft of DWARFv3.
> But was removed because there are no C++ mutable qualified types. It was
> replaced by a new attribute DW_AT_mutable on DW_TAG_member DIEs. The new
> attribute is available in dwarf.h.
> http://dwarfstd.org/ShowIssue.php?issue=050223.1
> 
> DW_TAG_mutable_type was only used internally in some backends (which
> just ignored it anyway). dwarves did use it to turn it into a string
> value, libabigail used it and ignored it (patches to remove sent).
> GCC, GDB and binutils don't use nor define it.

It's fine to patch its use out, in elfutils and others, but why take the
API hit of removing the constant?  The number is still allocated, so it
won't do any harm to leave it there.

For example, libdwarf just commented on the removal:
>         /*  Do not use DW_TAG_mutable_type */
> #define DW_TAG_mutable_type 0x3e /* Withdrawn from DWARF3 by DWARF3f. */

[PATCH] dwarf.h: Remove non-existing DW_TAG_mutable_type.

[Mark Wielaard]

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

The DW_TAG_mutable_type was only mentioned in an early draft of DWARFv3.
But was removed because there are no C++ mutable qualified types. It was
replaced by a new attribute DW_AT_mutable on DW_TAG_member DIEs. The new
attribute is available in dwarf.h.
http://dwarfstd.org/ShowIssue.php?issue=050223.1

DW_TAG_mutable_type was only used internally in some backends (which
just ignored it anyway). dwarves did use it to turn it into a string
value, libabigail used it and ignored it (patches to remove sent).
GCC, GDB and binutils don't use nor define it.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 backends/alpha_retval.c  |    2 +-
 backends/arm_retval.c    |    2 +-
 backends/i386_retval.c   |    2 +-
 backends/ia64_retval.c   |    2 +-
 backends/libebl_CPU.h    |    2 +-
 backends/ppc64_retval.c  |    2 +-
 backends/ppc_retval.c    |    2 +-
 backends/s390_retval.c   |    2 +-
 backends/sh_retval.c     |    2 +-
 backends/sparc_retval.c  |    2 +-
 backends/tilegx_retval.c |    2 +-
 backends/x86_64_retval.c |    2 +-
 libdw/dwarf.h            |    1 -
 13 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/backends/alpha_retval.c b/backends/alpha_retval.c
index 6dfa694..fddb47f 100644
--- a/backends/alpha_retval.c
+++ b/backends/alpha_retval.c
@@ -82,7 +82,7 @@ alpha_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
   /* Follow typedefs and qualifiers to get to the actual type.  */
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attr = dwarf_attr_integrate (typedie, DW_AT_type, &attr_mem);
       typedie = dwarf_formref_die (attr, &die_mem);
diff --git a/backends/arm_retval.c b/backends/arm_retval.c
index 222f755..92e3631 100644
--- a/backends/arm_retval.c
+++ b/backends/arm_retval.c
@@ -78,7 +78,7 @@ arm_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
   /* Follow typedefs and qualifiers to get to the actual type.  */
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attr = dwarf_attr_integrate (typedie, DW_AT_type, &attr_mem);
       typedie = dwarf_formref_die (attr, &die_mem);
diff --git a/backends/i386_retval.c b/backends/i386_retval.c
index 90678c3..dc0c5e6 100644
--- a/backends/i386_retval.c
+++ b/backends/i386_retval.c
@@ -82,7 +82,7 @@ i386_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
   /* Follow typedefs and qualifiers to get to the actual type.  */
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attr = dwarf_attr_integrate (typedie, DW_AT_type, &attr_mem);
       typedie = dwarf_formref_die (attr, &die_mem);
diff --git a/backends/ia64_retval.c b/backends/ia64_retval.c
index ac0d8c3..d259a80 100644
--- a/backends/ia64_retval.c
+++ b/backends/ia64_retval.c
@@ -253,7 +253,7 @@ ia64_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
   /* Follow typedefs and qualifiers to get to the actual type.  */
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attr = dwarf_attr (typedie, DW_AT_type, &attr_mem);
       typedie = dwarf_formref_die (attr, &die_mem);
diff --git a/backends/libebl_CPU.h b/backends/libebl_CPU.h
index 3ad9258..344a70e 100644
--- a/backends/libebl_CPU.h
+++ b/backends/libebl_CPU.h
@@ -60,7 +60,7 @@ dwarf_peel_type (Dwarf_Die *typediep, Dwarf_Attribute *attrp)
   int tag = DWARF_TAG_OR_RETURN (typediep);
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attrp = dwarf_attr_integrate (typediep, DW_AT_type, attrp);
       typediep = dwarf_formref_die (attrp, typediep);
diff --git a/backends/ppc64_retval.c b/backends/ppc64_retval.c
index c5c3b6f..042859f 100644
--- a/backends/ppc64_retval.c
+++ b/backends/ppc64_retval.c
@@ -92,7 +92,7 @@ ppc64_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
   /* Follow typedefs and qualifiers to get to the actual type.  */
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attr = dwarf_attr_integrate (typedie, DW_AT_type, &attr_mem);
       typedie = dwarf_formref_die (attr, &die_mem);
diff --git a/backends/ppc_retval.c b/backends/ppc_retval.c
index 7ca0c18..c289440 100644
--- a/backends/ppc_retval.c
+++ b/backends/ppc_retval.c
@@ -104,7 +104,7 @@ ppc_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
   /* Follow typedefs and qualifiers to get to the actual type.  */
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attr = dwarf_attr_integrate (typedie, DW_AT_type, &attr_mem);
       typedie = dwarf_formref_die (attr, &die_mem);
diff --git a/backends/s390_retval.c b/backends/s390_retval.c
index b671ee8..f830f0e 100644
--- a/backends/s390_retval.c
+++ b/backends/s390_retval.c
@@ -83,7 +83,7 @@ s390_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
   /* Follow typedefs and qualifiers to get to the actual type.  */
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attr = dwarf_attr_integrate (typedie, DW_AT_type, &attr_mem);
       typedie = dwarf_formref_die (attr, &die_mem);
diff --git a/backends/sh_retval.c b/backends/sh_retval.c
index 1166231..c45c91a 100644
--- a/backends/sh_retval.c
+++ b/backends/sh_retval.c
@@ -80,7 +80,7 @@ sh_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
   /* Follow typedefs and qualifiers to get to the actual type.  */
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attr = dwarf_attr_integrate (typedie, DW_AT_type, &attr_mem);
       typedie = dwarf_formref_die (attr, &die_mem);
diff --git a/backends/sparc_retval.c b/backends/sparc_retval.c
index dcd3785..91b97fd 100644
--- a/backends/sparc_retval.c
+++ b/backends/sparc_retval.c
@@ -87,7 +87,7 @@ sparc_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
   /* Follow typedefs and qualifiers to get to the actual type.  */
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attr = dwarf_attr_integrate (typedie, DW_AT_type, &attr_mem);
       typedie = dwarf_formref_die (attr, &die_mem);
diff --git a/backends/tilegx_retval.c b/backends/tilegx_retval.c
index e14cc51..25fbc9a 100644
--- a/backends/tilegx_retval.c
+++ b/backends/tilegx_retval.c
@@ -74,7 +74,7 @@ tilegx_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
   /* Follow typedefs and qualifiers to get to the actual type.  */
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attr = dwarf_attr_integrate (typedie, DW_AT_type, &attr_mem);
       typedie = dwarf_formref_die (attr, &die_mem);
diff --git a/backends/x86_64_retval.c b/backends/x86_64_retval.c
index f3e9f2b..5734afc 100644
--- a/backends/x86_64_retval.c
+++ b/backends/x86_64_retval.c
@@ -96,7 +96,7 @@ x86_64_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
   /* Follow typedefs and qualifiers to get to the actual type.  */
   while (tag == DW_TAG_typedef
 	 || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type
-	 || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type)
+	 || tag == DW_TAG_restrict_type)
     {
       attr = dwarf_attr_integrate (typedie, DW_AT_type, &attr_mem);
       typedie = dwarf_formref_die (attr, &die_mem);
diff --git a/libdw/dwarf.h b/libdw/dwarf.h
index 41cae5b..a8d2ec9 100644
--- a/libdw/dwarf.h
+++ b/libdw/dwarf.h
@@ -87,7 +87,6 @@ enum
     DW_TAG_unspecified_type = 0x3b,
     DW_TAG_partial_unit = 0x3c,
     DW_TAG_imported_unit = 0x3d,
-    DW_TAG_mutable_type = 0x3e,
     DW_TAG_condition = 0x3f,
     DW_TAG_shared_type = 0x40,
     DW_TAG_type_unit = 0x41,
-- 
1.7.1