diff mbox

Handle volatile array types in dwarf2read.c.

Message ID 1404238306.3766.17.camel@bordewijk.wildebeest.org
State New
Headers show

Commit Message

Mark Wielaard July 1, 2014, 6:11 p.m. UTC
On Tue, 2014-07-01 at 10:04 -0600, Tom Tromey wrote:
> Mark> +   the cv-qualifiers.  This is mimics section 6.7.3 of the C99
> Mark> +   specification.  */
> 
> "This is" -> "This".

Fixed.

> Mark> +static struct type *
> Mark> +add_array_cv_type (struct die_info *die, struct dwarf2_cu *cu,
> Mark> +		   struct type *base_type, int cnst, int voltl)
> 
> gdb usually puts a blank line between the intro comment and the function
> head.

Fixed.

> Mark> +  while (TYPE_CODE (TYPE_TARGET_TYPE (inner_array)) == TYPE_CODE_ARRAY)
> Mark> +    {
> Mark> +      TYPE_TARGET_TYPE (inner_array) =
> Mark> +	copy_type (TYPE_TARGET_TYPE (inner_array));
> Mark> +      inner_array = TYPE_TARGET_TYPE (inner_array);
> Mark> +    }
> 
> I think this may be overly eager in the case where the underlying type
> already has the needed qualifications.

But we have to look anyway. This is just the existing code, moved into
its own function. What do you suggest should be changed?

> Mark> +  cnst |= TYPE_CONST (el_type);
> Mark> +  voltl |= TYPE_VOLATILE (el_type);
> Mark> +  TYPE_TARGET_TYPE (inner_array) = make_cv_type (cnst, voltl, el_type, NULL);
> 
> make_cv_type preserves the already existing qualifiers so you don't need
> to track "cnst" and "voltl".  You can just pass in the ones you want to
> add.

The original code already did it this way. And I don't think it is true
that make_cv_type preserves existing const or volatile qualifiers. The
first thing make_cv_type does is mask away the old const and volatile
flags. Without explicitly preserving the existing flags 2 of the new
testcases, the const volatile arrays tests, fail (note one of the
qualifiers is missing):

ptype vindictive
type = const char [2]
(gdb) FAIL: gdb.base/volatile.exp: ptype vindictive
ptype vegetation
type = const unsigned char [2]
(gdb) FAIL: gdb.base/volatile.exp: ptype vegetation

> Mark> +  /* In case the volatile qualifier is applied to an array type, the
> Mark> +     element type is so qualified, not the array type (section 6.7.3
> Mark> +     of C99).  */
> Mark> +  if (TYPE_CODE (base_type) == TYPE_CODE_ARRAY)
> Mark> +    return add_array_cv_type (die, cu, base_type, 0, 1);
> 
> I wonder about typedefs to array type.
> Calling check_typedef here is probably not so great but I assume we can
> just ignore incomplete types.

I am not sure I understand precisely what you are wondering about. Do
you have an example? Then I can add a testcase for it. This is just
precisely as is done for the const case. So the issue you are thinking
about is probably the same for both cases and might already be existing.

Thanks,

Mark

Comments

Tom Tromey July 1, 2014, 6:24 p.m. UTC | #1
Mark> But we have to look anyway. This is just the existing code, moved into
Mark> its own function. What do you suggest should be changed?

It's fine to leave it.

Tom> make_cv_type preserves the already existing qualifiers so you don't need
Tom> to track "cnst" and "voltl".  You can just pass in the ones you want to
Tom> add.

Mark> The original code already did it this way. And I don't think it is true
Mark> that make_cv_type preserves existing const or volatile qualifiers.

Wow, I wonder how long I've had the incorrect view of make_cv_type.
Hopefully not too long.  Thanks for pointing that out.

Tom> I wonder about typedefs to array type.
Tom> Calling check_typedef here is probably not so great but I assume we can
Tom> just ignore incomplete types.

Mark> I am not sure I understand precisely what you are wondering about. Do
Mark> you have an example? Then I can add a testcase for it. This is just
Mark> precisely as is done for the const case. So the issue you are thinking
Mark> about is probably the same for both cases and might already be existing.

Well, I was thinking this:

    typedef int atype[23];
    const atype a;

However, gcc omits the typedef from the DWARF, so I suppose some
hand-crafted DWARF would have to be written.


This patch is ok.  If there's a further bug and you want to fix it, it's
fine to do that separately.

Tom
Mark Wielaard July 1, 2014, 8:15 p.m. UTC | #2
On Tue, 2014-07-01 at 12:24 -0600, Tom Tromey wrote:
> This patch is ok.

Thanks, pushed.

> Well, I was thinking this:
> 
>     typedef int atype[23];
>     const atype a;
> 
> However, gcc omits the typedef from the DWARF, so I suppose some
> hand-crafted DWARF would have to be written.

I tried in a couple of ways, but GCC always happily optimizes the
unnecessary typedefs away from the DWARF, so it probably is not an issue
in practice if GDB doesn't completely handle that case.

Cheers,

Mark
diff mbox

Patch

commit 8782e0bef99e1c282364d283f5cbf6cc32f437ee
Author: Mark Wielaard <mjw@redhat.com>
Date:   Mon Jun 30 23:21:52 2014 +0200

    Handle volatile array types in dwarf2read.c.
    
    read_tag_const_type propagates the cv-qualifier to the array element type,
    but read_tag_volatile_type didn't. Make sure that both cv-qualifiers that
    apply to array types are handled the same.
    
    gdb/ChangeLog
    
    	* dwarf2read.c (add_array_cv_type): New function.
    	(read_tag_const_type): Call add_array_cv_type for TYPE_CODE_ARRAY.
    	(read_tag_volatile_type): Likewise.
    
    gdb/testsuite/ChangeLog
    
    	* gdb.base/constvars.c (violent, violet, vips, virgen, vulgar,
    	vulture, vilify, villar): New volatile array constants.
    	(vindictive, vegetation): New const volatile array constants.
    	* gdb.base/volatile.exp: Test volatile and const volatile array
    	types.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8d304ac..ccfae0c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@ 
+2014-06-30  Mark Wielaard  <mjw@redhat.com>
+
+	* dwarf2read.c (add_array_cv_type): New function.
+	(read_tag_const_type): Call add_array_cv_type for TYPE_CODE_ARRAY.
+	(read_tag_volatile_type): Likewise.
+
 2014-06-30  Tom Tromey  <tromey@redhat.com>
 
 	* symtab.c (operator_chars): Make parameters and return type
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index fc4f7cb..8b600fb 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14097,6 +14097,36 @@  read_tag_reference_type (struct die_info *die, struct dwarf2_cu *cu)
   return set_die_type (die, type, cu);
 }
 
+/* Add the given cv-qualifiers to the element type of the array.  GCC
+   outputs DWARF type qualifiers that apply to an array, not the
+   element type.  But GDB relies on the array element type to carry
+   the cv-qualifiers.  This mimics section 6.7.3 of the C99
+   specification.  */
+
+static struct type *
+add_array_cv_type (struct die_info *die, struct dwarf2_cu *cu,
+		   struct type *base_type, int cnst, int voltl)
+{
+  struct type *el_type, *inner_array;
+
+  base_type = copy_type (base_type);
+  inner_array = base_type;
+
+  while (TYPE_CODE (TYPE_TARGET_TYPE (inner_array)) == TYPE_CODE_ARRAY)
+    {
+      TYPE_TARGET_TYPE (inner_array) =
+	copy_type (TYPE_TARGET_TYPE (inner_array));
+      inner_array = TYPE_TARGET_TYPE (inner_array);
+    }
+
+  el_type = TYPE_TARGET_TYPE (inner_array);
+  cnst |= TYPE_CONST (el_type);
+  voltl |= TYPE_VOLATILE (el_type);
+  TYPE_TARGET_TYPE (inner_array) = make_cv_type (cnst, voltl, el_type, NULL);
+
+  return set_die_type (die, base_type, cu);
+}
+
 static struct type *
 read_tag_const_type (struct die_info *die, struct dwarf2_cu *cu)
 {
@@ -14112,25 +14142,7 @@  read_tag_const_type (struct die_info *die, struct dwarf2_cu *cu)
   /* In case the const qualifier is applied to an array type, the element type
      is so qualified, not the array type (section 6.7.3 of C99).  */
   if (TYPE_CODE (base_type) == TYPE_CODE_ARRAY)
-    {
-      struct type *el_type, *inner_array;
-
-      base_type = copy_type (base_type);
-      inner_array = base_type;
-
-      while (TYPE_CODE (TYPE_TARGET_TYPE (inner_array)) == TYPE_CODE_ARRAY)
-	{
-	  TYPE_TARGET_TYPE (inner_array) =
-	    copy_type (TYPE_TARGET_TYPE (inner_array));
-	  inner_array = TYPE_TARGET_TYPE (inner_array);
-	}
-
-      el_type = TYPE_TARGET_TYPE (inner_array);
-      TYPE_TARGET_TYPE (inner_array) =
-	make_cv_type (1, TYPE_VOLATILE (el_type), el_type, NULL);
-
-      return set_die_type (die, base_type, cu);
-    }
+    return add_array_cv_type (die, cu, base_type, 1, 0);
 
   cv_type = make_cv_type (1, TYPE_VOLATILE (base_type), base_type, 0);
   return set_die_type (die, cv_type, cu);
@@ -14148,6 +14160,12 @@  read_tag_volatile_type (struct die_info *die, struct dwarf2_cu *cu)
   if (cv_type)
     return cv_type;
 
+  /* In case the volatile qualifier is applied to an array type, the
+     element type is so qualified, not the array type (section 6.7.3
+     of C99).  */
+  if (TYPE_CODE (base_type) == TYPE_CODE_ARRAY)
+    return add_array_cv_type (die, cu, base_type, 0, 1);
+
   cv_type = make_cv_type (TYPE_CONST (base_type), 1, base_type, 0);
   return set_die_type (die, cv_type, cu);
 }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 751848f..90be171 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@ 
+2014-06-30  Mark Wielaard  <mjw@redhat.com>
+
+	* gdb.base/constvars.c (violent, violet, vips, virgen, vulgar,
+	vulture, vilify, villar): New volatile array constants.
+	(vindictive, vegetation): New const volatile array constants.
+	* gdb.base/volatile.exp: Test volatile and const volatile array
+	types.
+
 2014-06-30  Andreas Arnez  <arnez@linux.vnet.ibm.com>
 
 	* gdb.base/watchpoint-reuse-slot.exp: Handle the case that the
diff --git a/gdb/testsuite/gdb.base/constvars.c b/gdb/testsuite/gdb.base/constvars.c
index 4228822..60cca2a 100644
--- a/gdb/testsuite/gdb.base/constvars.c
+++ b/gdb/testsuite/gdb.base/constvars.c
@@ -127,6 +127,16 @@  main (void)
   volatile float          * volatile vitality = &vacuity;
   volatile double         * volatile voracity = &vertigo;
 
+  /* volatile arrays */
+  volatile char violent[2] = {vox, vox};
+  volatile unsigned char violet[2] = {victuals, victuals};
+  volatile short vips[2] = {vixen, vixen};
+  volatile unsigned short virgen[2] = {vitriol, vitriol};
+  volatile long vulgar[2] = {vellum, vellum};
+  volatile unsigned long vulture[2] = {valve, valve};
+  volatile float vilify[2] = {vacuity, vacuity};
+  volatile double villar[2] = {vertigo, vertigo};
+
   /* const volatile vars */
 
   const volatile char           victor = 'Y';
@@ -177,6 +187,10 @@  main (void)
   const volatile char              * const volatile vagary = &victor;
   const volatile unsigned char     * const volatile vendor = &vicar;
   
+  /* const volatile arrays */
+  const volatile char vindictive[2] = {victor, victor};
+  const volatile unsigned char vegetation[2] = {vicar, vicar};
+
   /* various structs with const members */
 
   struct crass { char * const ptr; } crass = { lamprey };
diff --git a/gdb/testsuite/gdb.base/volatile.exp b/gdb/testsuite/gdb.base/volatile.exp
index 7cd7254..0e106fc 100644
--- a/gdb/testsuite/gdb.base/volatile.exp
+++ b/gdb/testsuite/gdb.base/volatile.exp
@@ -229,6 +229,30 @@  gdb_test "ptype vagary" "type = const volatile char \\* const volatile.*"
 local_compiler_xfail_check
 gdb_test "ptype vendor" "type = const volatile unsigned char \\* const volatile.*"
 
+# volatile arrays
+local_compiler_xfail_check
+gdb_test "ptype violent" "type = volatile char \\\[2\\\]"
+local_compiler_xfail_check
+gdb_test "ptype violet" "type = volatile unsigned char \\\[2\\\]"
+local_compiler_xfail_check
+gdb_test "ptype vips" "type = volatile short( int)? \\\[2\\\]"
+local_compiler_xfail_check
+gdb_test "ptype virgen" "type = volatile (unsigned short|short unsigned)( int)? \\\[2\\\]"
+local_compiler_xfail_check
+gdb_test "ptype vulgar" "type = volatile long( int)? \\\[2\\\]"
+local_compiler_xfail_check
+gdb_test "ptype vulture" "type = volatile (unsigned long|long unsigned)( int)? \\\[2\\\]"
+local_compiler_xfail_check
+gdb_test "ptype vilify" "type = volatile float \\\[2\\\]"
+local_compiler_xfail_check
+gdb_test "ptype villar" "type = volatile double \\\[2\\\]"
+
+# const volatile arrays
+local_compiler_xfail_check
+gdb_test "ptype vindictive" "type = const volatile char \\\[2\\\]"
+local_compiler_xfail_check
+gdb_test "ptype vegetation" "type = const volatile unsigned char \\\[2\\\]"
+
 # test function parameters
 local_compiler_xfail_check
 local_compiler_xfail_check_2