Re: [PATCH] DWARFv5. Handle DW_TAG_atomic_type _Atomic type modifier.

[Tom Tromey <tromey@redhat.com>]

>>>>> "Mark" == Mark Wielaard <mjw@redhat.com> writes:

Mark> This prototype patch matches the experimental patch to GCC:
Mark> https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01677.html

Thanks, Mark.

Mark> Since there is not even a draft of DWARFv5 I don't recommend adopting
Mark> this patch. All details may change in the future. I am mainly doing it
Mark> to give better feedback on the DWARFv5 proposals (in this case the
Mark> feedback would be that it is unfortunate we cannot easily do this as a
Mark> vendor extension with DW_TAG_GNU_atomic_type since that would break
Mark> backward compatibility).

I don't understand this bit.  It's reasonably normal to add a new GNU tag.

Mark> Is there a recommended way for doing/keeping these kind of
Mark> speculative patches?

Just hosting it on a public git somewhere.  I suppose we could resurrect
archer.git, though there are plenty of hosting services now.

Some nits follow.  The patch looks good overall.

Mark> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
Mark> index ba64256..50bc194 100644
Mark> --- a/gdb/dwarf2read.c
Mark> +++ b/gdb/dwarf2read.c
Mark> @@ -4286,9 +4286,9 @@ error_check_comp_unit_head (struct comp_unit_head *header,
Mark>    bfd *abfd = get_section_bfd_owner (section);
Mark>    const char *filename = get_section_file_name (section);
 
Mark> -  if (header->version != 2 && header->version != 3 && header->version != 4)
Mark> +  if (header->version < 2 || header->version > 5)
Mark>      error (_("Dwarf Error: wrong version in compilation unit header "
Mark> -	   "(is %d, should be 2, 3, or 4) [in module %s]"), header->version,
Mark> +	   "(is %d, should be 2, 3, 4 or 5) [in module %s]"), header->version,

Normally this should be a separate patch.  For one thing, then it could
go in even if _Atomic doesn't appear in DWARF 5.

Mark> +static struct type *
Mark> +read_tag_atomic_type (struct die_info *die, struct dwarf2_cu *cu)

gdb requires a comment before a new function.

Mark> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-atomic.S b/gdb/testsuite/gdb.dwarf2/dw2-atomic.S

FWIW it's simpler and better to write this using the DWARF assembler in
the test suite.  enum-type.exp is an ok example of the kind of thing
you'd want to do -- just describing a new type and then testing it with
ptype.  Using the DWARF assembler would let you drop the x86-64-only
part of the test.

Mark> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-error.exp b/gdb/testsuite/gdb.dwarf2/dw2-atomic.exp

Hah, this confused me for a while, until I realized that git was
treating it like a copy.

Mark>  # First test that reading symbols fails.
Mark>  gdb_test "file $binfile" \
Mark> -    "Reading symbols.*Dwarf Error: wrong version in compilation unit header .is 153, should be 2, 3, or 4.*" \
Mark> +    "Reading symbols.*Dwarf Error: wrong version in compilation unit header .is 153, should be 2, 3, 4 or 5.*" \
Mark>      "file $testfile"

Should be in the separate DWARF 5 patch.

Tom