[commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Jan Kratochvil]

Hi,

FIELD_LOC_KIND_BITPOS is 0 and some code relied on it, using only
	TYPE_FIELD_BITPOS (type, n) = foo;
instead of
	SET_FIELD_BITPOS (TYPE_FIELD (type, n), foo);

This caused in consequence with TYPE_FIELD_ENUM from:
	[PATCH] Allow 64-bit enum values
and a sanity check
	-#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS (TYPE_FIELD (thistype, n))
	-#define TYPE_FIELD_ENUMVAL(thistype, n) FIELD_ENUMVAL (TYPE_FIELD (thistype, n))
	+#define TYPE_FIELD_BITPOSL(thistype, n) FIELD_BITPOS (TYPE_FIELD (thistype, n))
	+#define TYPE_FIELD_BITPOS(thistype, n) ({ gdb_assert (TYPE_CODE (thistype) != TYPE_CODE_ENUM); TYPE_FIELD_BITPOSL(thistype, n); })
	+#define TYPE_FIELD_ENUMVALL(thistype, n) FIELD_ENUMVAL (TYPE_FIELD (thistype, n))
	+#define TYPE_FIELD_ENUMVAL(thistype, n) ({ gdb_assert (TYPE_CODE (thistype) == TYPE_CODE_ENUM); TYPE_FIELD_ENUMVALL(thistype, n); })
regression with -gstabs+:
	 file^M
	 No executable file now.^M
	 Discard symbol table from `/unsafegdb/testsuite.unix.-m64/gdb.python/py-value'? (y or n) y^M
	-Error in re-setting breakpoint 1: No symbol table is loaded.  Use the "file" command.^M
	-Error in re-setting breakpoint 2: No symbol table is loaded.  Use the "file" command.^M
	-No symbol file now.^M
	-(gdb) PASS: gdb.python/py-value.exp: Discard the symbols
	-python castval = arg0.cast(ptrtype.pointer())^M
	+gdbtypes.c:3403: internal-error: copy_type_recursive: Assertion `TYPE_CODE (type) != TYPE_CODE_ENUM' failed.^M
	+A problem internal to GDB has been detected,^M
	+further debugging may prove unreliable.^M
	+Quit this debugging session? (y or n) FAIL: gdb.python/py-value.exp: Discard the symbols (GDB internal error)

No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu and with -gstabs+.

Checked in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2012-04/msg00108.html

--- src/gdb/ChangeLog	2012/04/16 11:24:43	1.14107
+++ src/gdb/ChangeLog	2012/04/17 12:43:16	1.14108
@@ -1,3 +1,12 @@
+2012-04-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	Do not rely on FIELD_LOC_KIND_BITPOS being zero.
+	* ada-lang.c (ada_template_to_fixed_record_type_1): Replace
+	TYPE_FIELD_BITPOS used as lvalue by SET_FIELD_BITPOS.
+	* gdbtypes.c (append_flags_type_flag): Likewise, twice.
+	* jv-lang.c (java_link_class_type): Likewise, once.
+	* stabsread.c (read_enum_type): Likewise.
+
 2012-04-16  Yao Qi  <yao@codesourcery.com>
 
 	* common/agent.c (agent_run_command): Add one more parameter `len'.
--- src/gdb/ada-lang.c	2012/03/29 23:30:55	1.363
+++ src/gdb/ada-lang.c	2012/04/17 12:43:19	1.364
@@ -7445,7 +7445,7 @@
     {
       off = align_value (off, field_alignment (type, f))
 	+ TYPE_FIELD_BITPOS (type, f);
-      TYPE_FIELD_BITPOS (rtype, f) = off;
+      SET_FIELD_BITPOS (TYPE_FIELD (rtype, f), off);
       TYPE_FIELD_BITSIZE (rtype, f) = 0;
 
       if (ada_is_variant_part (type, f))
--- src/gdb/gdbtypes.c	2012/02/07 04:48:20	1.226
+++ src/gdb/gdbtypes.c	2012/04/17 12:43:20	1.227
@@ -3606,12 +3606,12 @@
   if (name)
     {
       TYPE_FIELD_NAME (type, bitpos) = xstrdup (name);
-      TYPE_FIELD_BITPOS (type, bitpos) = bitpos;
+      SET_FIELD_BITPOS (TYPE_FIELD (type, bitpos), bitpos);
     }
   else
     {
       /* Don't show this field to the user.  */
-      TYPE_FIELD_BITPOS (type, bitpos) = -1;
+      SET_FIELD_BITPOS (TYPE_FIELD (type, bitpos), -1);
     }
 }
 
--- src/gdb/jv-lang.c	2012/03/02 19:29:00	1.101
+++ src/gdb/jv-lang.c	2012/04/17 12:43:20	1.102
@@ -480,7 +480,7 @@
       if (accflags & 0x0008)	/* ACC_STATIC */
 	SET_FIELD_PHYSADDR (TYPE_FIELD (type, i), boffset);
       else
-	TYPE_FIELD_BITPOS (type, i) = 8 * boffset;
+	SET_FIELD_BITPOS (TYPE_FIELD (type, i), 8 * boffset);
       if (accflags & 0x8000)	/* FIELD_UNRESOLVED_FLAG */
 	{
 	  TYPE_FIELD_TYPE (type, i) = get_java_object_type ();	/* FIXME */
--- src/gdb/stabsread.c	2012/03/13 16:29:16	1.144
+++ src/gdb/stabsread.c	2012/04/17 12:43:20	1.145
@@ -3730,7 +3730,7 @@
 
 	  SYMBOL_TYPE (xsym) = type;
 	  TYPE_FIELD_NAME (type, n) = SYMBOL_LINKAGE_NAME (xsym);
-	  TYPE_FIELD_BITPOS (type, n) = SYMBOL_VALUE (xsym);
+	  SET_FIELD_BITPOS (TYPE_FIELD (type, n), SYMBOL_VALUE (xsym));
 	  TYPE_FIELD_BITSIZE (type, n) = 0;
 	}
       if (syms == osyms)

Re: [commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Pedro Alves]

On 04/17/2012 01:44 PM, Jan Kratochvil wrote:

> Hi,
> 
> FIELD_LOC_KIND_BITPOS is 0 and some code relied on it, using only
> 	TYPE_FIELD_BITPOS (type, n) = foo;
> instead of
> 	SET_FIELD_BITPOS (TYPE_FIELD (type, n), foo);
> 


We could make the compiler catch these by making the macro return an rvalue.

---

 gdb/gdbtypes.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 07c3a86..9901439 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1110,7 +1110,7 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_FIELD_TYPE(thistype, n) FIELD_TYPE(TYPE_FIELD(thistype, n))
 #define TYPE_FIELD_NAME(thistype, n) FIELD_NAME(TYPE_FIELD(thistype, n))
 #define TYPE_FIELD_LOC_KIND(thistype, n) FIELD_LOC_KIND (TYPE_FIELD (thistype, n))
-#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_BITPOS(thistype, n) (FIELD_BITPOS (TYPE_FIELD (thistype, n)) + 0)
 #define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_STATIC_PHYSNAME (TYPE_FIELD (thistype, n))
 #define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_STATIC_PHYSADDR (TYPE_FIELD (thistype, n))
 #define TYPE_FIELD_DWARF_BLOCK(thistype, n) FIELD_DWARF_BLOCK (TYPE_FIELD (thistype, n))

Re: [commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Jan Kratochvil]

On Tue, 17 Apr 2012 15:00:14 +0200, Pedro Alves wrote:
> We could make the compiler catch these by making the macro return an rvalue.

With C++ getters and constructors-with-parameters such problem would have
never exist.


Jan

Re: [commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Pedro Alves]

On 04/17/2012 02:15 PM, Jan Kratochvil wrote:

> On Tue, 17 Apr 2012 15:00:14 +0200, Pedro Alves wrote:
>> We could make the compiler catch these by making the macro return an rvalue.
> 
> With C++ getters and constructors-with-parameters such problem would have
> never exist.


Nor with C getters and setters with opaque types.  Nor with Java getters/setters.
It would however still exist with a field.bitpos C# property, as that'd allow
the += style.

Tested on x86_64 Fedora 16, and applied.

2012-04-17  Pedro Alves  <palves@redhat.com>

	* gdbtypes.h (FIELD_BITPOS): Rename to ...
	(FIELD_BITPOS_LVAL): ... this.
	(FIELD_BITPOS): New.
	(SET_FIELD_BITPOS): Adjust to use FIELD_BITPOS_LVAL.
	* dwarf2read.c (dwarf2_add_field): Use SET_FIELD_BITPOS.
	* gdbtypes.c (append_composite_type_field_aligned): Adjust to use
	SET_FIELD_BITPOS.
	* gnu-v3-abi.c (build_gdb_vtable_type): Adjust to use
	SET_FIELD_BITPOS.
	* stabsread.c (read_cpp_abbrev, read_one_struct_field)
	(read_baseclasses): Adjust to use SET_FIELD_BITPOS.
	* target-descriptions.c (tdesc_gdb_type): Adjust to use
	SET_FIELD_BITPOS.
---
 gdb/dwarf2read.c          |    8 +++++---
 gdb/gdbtypes.c            |    9 +++++----
 gdb/gdbtypes.h            |    5 +++--
 gdb/gnu-v3-abi.c          |    8 ++++----
 gdb/stabsread.c           |    9 +++++----
 gdb/target-descriptions.c |    4 ++--
 6 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0e211ae..eec52fb 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -7066,7 +7066,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 	         anonymous object to the MSB of the field.  We don't
 	         have to do anything special since we don't need to
 	         know the size of the anonymous object.  */
-	      FIELD_BITPOS (*fp) += DW_UNSND (attr);
+	      SET_FIELD_BITPOS (*fp, FIELD_BITPOS (*fp) + DW_UNSND (attr));
 	    }
 	  else
 	    {
@@ -7095,8 +7095,10 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 		     bit field.  */
 		  anonymous_size = TYPE_LENGTH (fp->type);
 		}
-	      FIELD_BITPOS (*fp) += anonymous_size * bits_per_byte
-		- bit_offset - FIELD_BITSIZE (*fp);
+	      SET_FIELD_BITPOS (*fp,
+				(FIELD_BITPOS (*fp)
+				 + anonymous_size * bits_per_byte
+				 - bit_offset - FIELD_BITSIZE (*fp)));
 	    }
 	}

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 90e33a5..d772d9a 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -3666,9 +3666,10 @@ append_composite_type_field_aligned (struct type *t, char *name,
       TYPE_LENGTH (t) = TYPE_LENGTH (t) + TYPE_LENGTH (field);
       if (TYPE_NFIELDS (t) > 1)
 	{
-	  FIELD_BITPOS (f[0]) = (FIELD_BITPOS (f[-1])
-				 + (TYPE_LENGTH (FIELD_TYPE (f[-1]))
-				    * TARGET_CHAR_BIT));
+	  SET_FIELD_BITPOS (f[0],
+			    (FIELD_BITPOS (f[-1])
+			     + (TYPE_LENGTH (FIELD_TYPE (f[-1]))
+				* TARGET_CHAR_BIT)));

 	  if (alignment)
 	    {
@@ -3679,7 +3680,7 @@ append_composite_type_field_aligned (struct type *t, char *name,

 	      if (left)
 		{
-		  FIELD_BITPOS (f[0]) += (alignment - left);
+		  SET_FIELD_BITPOS (f[0], FIELD_BITPOS (f[0]) + (alignment - left));
 		  TYPE_LENGTH (t) += (alignment - left) / TARGET_CHAR_BIT;
 		}
 	    }
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 07c3a86..a2b2432 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1087,13 +1087,14 @@ extern void allocate_gnat_aux_type (struct type *);
 #define FIELD_TYPE(thisfld) ((thisfld).type)
 #define FIELD_NAME(thisfld) ((thisfld).name)
 #define FIELD_LOC_KIND(thisfld) ((thisfld).loc_kind)
-#define FIELD_BITPOS(thisfld) ((thisfld).loc.bitpos)
+#define FIELD_BITPOS_LVAL(thisfld) ((thisfld).loc.bitpos)
+#define FIELD_BITPOS(thisfld) (FIELD_BITPOS_LVAL (thisfld) + 0)
 #define FIELD_STATIC_PHYSNAME(thisfld) ((thisfld).loc.physname)
 #define FIELD_STATIC_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
 #define FIELD_DWARF_BLOCK(thisfld) ((thisfld).loc.dwarf_block)
 #define SET_FIELD_BITPOS(thisfld, bitpos)			\
   (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_BITPOS,		\
-   FIELD_BITPOS (thisfld) = (bitpos))
+   FIELD_BITPOS_LVAL (thisfld) = (bitpos))
 #define SET_FIELD_PHYSNAME(thisfld, name)			\
   (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_PHYSNAME,		\
    FIELD_STATIC_PHYSNAME (thisfld) = (name))
diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
index 1095c60..ed94b84 100644
--- a/gdb/gnu-v3-abi.c
+++ b/gdb/gnu-v3-abi.c
@@ -130,28 +130,28 @@ build_gdb_vtable_type (struct gdbarch *arch)
   /* ptrdiff_t vcall_and_vbase_offsets[0]; */
   FIELD_NAME (*field) = "vcall_and_vbase_offsets";
   FIELD_TYPE (*field) = lookup_array_range_type (ptrdiff_type, 0, -1);
-  FIELD_BITPOS (*field) = offset * TARGET_CHAR_BIT;
+  SET_FIELD_BITPOS (*field, offset * TARGET_CHAR_BIT);
   offset += TYPE_LENGTH (FIELD_TYPE (*field));
   field++;

   /* ptrdiff_t offset_to_top; */
   FIELD_NAME (*field) = "offset_to_top";
   FIELD_TYPE (*field) = ptrdiff_type;
-  FIELD_BITPOS (*field) = offset * TARGET_CHAR_BIT;
+  SET_FIELD_BITPOS (*field, offset * TARGET_CHAR_BIT);
   offset += TYPE_LENGTH (FIELD_TYPE (*field));
   field++;

   /* void *type_info; */
   FIELD_NAME (*field) = "type_info";
   FIELD_TYPE (*field) = void_ptr_type;
-  FIELD_BITPOS (*field) = offset * TARGET_CHAR_BIT;
+  SET_FIELD_BITPOS (*field, offset * TARGET_CHAR_BIT);
   offset += TYPE_LENGTH (FIELD_TYPE (*field));
   field++;

   /* void (*virtual_functions[0]) (); */
   FIELD_NAME (*field) = "virtual_functions";
   FIELD_TYPE (*field) = lookup_array_range_type (ptr_to_void_fn_type, 0, -1);
-  FIELD_BITPOS (*field) = offset * TARGET_CHAR_BIT;
+  SET_FIELD_BITPOS (*field, offset * TARGET_CHAR_BIT);
   offset += TYPE_LENGTH (FIELD_TYPE (*field));
   field++;

diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 39e0d7b..40b2465 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -2830,8 +2830,8 @@ read_cpp_abbrev (struct field_info *fip, char **pp, struct type *type,
       {
 	int nbits;

-	FIELD_BITPOS (fip->list->field) = read_huge_number (pp, ';', &nbits,
-                                                            0);
+	SET_FIELD_BITPOS (fip->list->field,
+			  read_huge_number (pp, ';', &nbits, 0));
 	if (nbits != 0)
 	  return 0;
       }
@@ -2907,7 +2907,8 @@ read_one_struct_field (struct field_info *fip, char **pp, char *p,
   {
     int nbits;

-    FIELD_BITPOS (fip->list->field) = read_huge_number (pp, ',', &nbits, 0);
+    SET_FIELD_BITPOS (fip->list->field,
+		      read_huge_number (pp, ',', &nbits, 0));
     if (nbits != 0)
       {
 	stabs_general_complaint ("bad structure-type format");
@@ -3187,7 +3188,7 @@ read_baseclasses (struct field_info *fip, char **pp, struct type *type,
 	   corresponding to this baseclass.  Always zero in the absence of
 	   multiple inheritance.  */

-	FIELD_BITPOS (new->field) = read_huge_number (pp, ',', &nbits, 0);
+	SET_FIELD_BITPOS (new->field, read_huge_number (pp, ',', &nbits, 0));
 	if (nbits != 0)
 	  return 0;
       }
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index cbcca76..0b12e3e 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -657,9 +657,9 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
 		bitsize = f->end - f->start + 1;
 		total_size = tdesc_type->u.u.size * TARGET_CHAR_BIT;
 		if (gdbarch_bits_big_endian (gdbarch))
-		  FIELD_BITPOS (fld[0]) = total_size - f->start - bitsize;
+		  SET_FIELD_BITPOS (fld[0], total_size - f->start - bitsize);
 		else
-		  FIELD_BITPOS (fld[0]) = f->start;
+		  SET_FIELD_BITPOS (fld[0], f->start);
 		FIELD_BITSIZE (fld[0]) = bitsize;
 	      }
 	    else

Re: [commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Jan Kratochvil]

On Tue, 17 Apr 2012 15:57:11 +0200, Pedro Alves wrote:
> Nor with C getters and setters with opaque types.

It is just too much coding, like here _LVAL, it is just borind this C++
reimplementation in C, the standard is C++, why GDB has to call all the
standard C++ constructs differently.


Jan

Re: [commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Pedro Alves]

On 04/17/2012 03:01 PM, Jan Kratochvil wrote:

> On Tue, 17 Apr 2012 15:57:11 +0200, Pedro Alves wrote:
>> > Nor with C getters and setters with opaque types.
> It is just too much coding, like here _LVAL, it is just borind this C++
> reimplementation in C, the standard is C++, why GDB has to call all the
> standard C++ constructs differently.


You're deeply confused if you think C++ invented getters/setters.

-- 
Pedro Alves

Re: [commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Jan Kratochvil]

On Tue, 17 Apr 2012 16:03:40 +0200, Pedro Alves wrote:
> You're deeply confused if you think C++ invented getters/setters.

I never thought so and I never said so.  But C++ is the most easily applicable
getters/setters-featuring language to GDB.


Jan

Re: [commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Pedro Alves]

On 04/17/2012 03:05 PM, Jan Kratochvil wrote:

> On Tue, 17 Apr 2012 16:03:40 +0200, Pedro Alves wrote:
>> You're deeply confused if you think C++ invented getters/setters.
> 
> I never thought so and I never said so.  But C++ is the most easily applicable
> getters/setters-featuring language to GDB.


Yet, the patch was so simple.  There could be other reasons for a C++ switch,
but you just took an easy opportunity to jump on me.  As I'm sure I've said before
more than once, I prefer C++ over C, but I don't think C++ is currently a good idea
for GDB.  Let's not turn every suggestion to improve the sources into a C->C++
discussion, please?

-- 
Pedro Alves

Re: [commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Jan Kratochvil]

On Tue, 17 Apr 2012 15:57:11 +0200, Pedro Alves wrote:
> -	      FIELD_BITPOS (*fp) += DW_UNSND (attr);
> +	      SET_FIELD_BITPOS (*fp, FIELD_BITPOS (*fp) + DW_UNSND (attr));

I do not like this apprach as it misleads the reader that FIELD_BITPOS is
newly set here.  This is why I did not want to implement such change and just
fixed the existing cases.

FIELD_BITPOS is being only added to, which the original code said.  Now the
code is not so clear.

I understand it was coded to prevent new code using
	FIELD_BITPOS (*fp) = newval;
but I guess it can happen again but now the contributor will just use:
	FIELD_BITPOS_LVAL (*fp) = newval;

That it gets caught by review is true but particularly with GDB many patches
are even never submitted.


Regards,
Jan

Re: [commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Pedro> We could make the compiler catch these by making the macro return
Pedro> an rvalue.

Yes, please do it.  If there is a setter we should make the getter an
rvalue when possible.

Jan> With C++ getters and constructors-with-parameters such problem would have
Jan> never exist.

Also true :)

Tom

Re: [commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> I do not like this apprach as it misleads the reader that
Jan> FIELD_BITPOS is newly set here.  This is why I did not want to
Jan> implement such change and just fixed the existing cases.

Jan> FIELD_BITPOS is being only added to, which the original code said.
Jan> Now the code is not so clear.

We could add a comment to clarify it.

This technique is already used elsewhere, so the reader presumably will
be familiar with the idiom.

Jan> That it gets caught by review is true but particularly with GDB
Jan> many patches are even never submitted.

Let's pretend that patches that aren't submitted just don't exist.
That's what I do :)

The reason I like the rvalue accessor approach is that it helps prevent
future bugs of this sort.  No approach is perfect, especially because C
has relatively weak access protection mechanisms, but I still see this
as an improvement overall.

I agree that this is yet another thing that C++ does better.
But that is being discussed under a different thread and, until some
decision is reached, I think we should continue to deal with the patches
we see as they are, and not link them to C++.  After all, even if C++ is
approved the migration will not be instantaneous.

Tom

Re: [commit] Do not rely on FIELD_LOC_KIND_BITPOS being zero

[Joel Brobecker]

> >> We could make the compiler catch these by making the macro return
> >> an rvalue.
[...]
> 2012-04-17  Pedro Alves  <palves@redhat.com>
> 
> 	* gdbtypes.h (FIELD_BITPOS): Rename to ...
> 	(FIELD_BITPOS_LVAL): ... this.
> 	(FIELD_BITPOS): New.
> 	(SET_FIELD_BITPOS): Adjust to use FIELD_BITPOS_LVAL.
> 	* dwarf2read.c (dwarf2_add_field): Use SET_FIELD_BITPOS.
> 	* gdbtypes.c (append_composite_type_field_aligned): Adjust to use
> 	SET_FIELD_BITPOS.
> 	* gnu-v3-abi.c (build_gdb_vtable_type): Adjust to use
> 	SET_FIELD_BITPOS.
> 	* stabsread.c (read_cpp_abbrev, read_one_struct_field)
> 	(read_baseclasses): Adjust to use SET_FIELD_BITPOS.
> 	* target-descriptions.c (tdesc_gdb_type): Adjust to use
> 	SET_FIELD_BITPOS.

Thanks for doing that, Pedro - I was going to suggest the same thing.
The more the compiler catches errors for us, the better!

-- 
Joel