[PATCH] append_composite_type_field_aligned

["Andrew Burgess" <aburgess@broadcom.com>]

I've been trying to use the function
append_composite_type_field_aligned from gdbtypes.c and I was not
seeing the behaviour I was expecting.

Please excuse the rather long/rambling email, but I've tried to lay
out below the behaviour I was seeing and why this is not what I was
expecting then someone can jump in if I've made a mistake.

I have no reproducible code for this it's all just code inspection 
of the function append_composite_type_field_aligned in gdbtypes.c .

I'm working with TARGET_CHAR_BIT = 8 throughout, though the values I
calculate would obviously change I don't think it makes any other
difference to the point I'm making.

Consider creation of a composite type "CT" with type code TYPE_CODE_STRUCT.

I add an initial component type "T1" of size "S1" which will be put 
at the start of CT (bit position 0).

I then add another component type "T2" of size "S2".

I add T1 using something like this:

   append_composite_type_field_aligned (CT, "T1", T1, 0);

I then add T2 using different alignment values (A) like this:

   append_composite_type_field_aligned (CT, "T2", T2, A);

I vary the value of A to be 0, 8, 16, 24, 32, 64 and vary the size S1 
and calculate the FIELD_BITPOS at which T2 will be placed.

The FIELD_BITPOS for T1 will be 0 in all cases. The FIELD_BITPOS for
T2 will depend on the TYPE_LENGTH of T1 and the alignment value A.

I've included an example where S1 is 0, a little contrived maybe, but
it fills out an the table, and would be of interest if we added the
first type to the structure with a non-zero alignment (which should be
fine.)

| TYPE   |   FIELD BITPOS T2 for different alignment A   |
| LENGTH |                                               |
|   T1   | A == 0 | A == 8 | A == 16 | A == 24 | A == 32 |
|--------|--------|--------|---------|---------|---------|
| 0      | 0      | 0      | 0       | 0       | 0       |
| 8      | 8      | 8      | 16      | 16      | 16      |
| 16     | 16     | 16     | 16      | 32      | 32      |
| 24     | 24     | 24     | 32      | 24      | 48      |
| 32     | 32     | 32     | 32      | 40      | 32      |


My belief was that:

   ( FIELD_BITPOS(T2) % A ) == 0

after the alignment adjustment has taken place. This is obviously not
the case for (A == 24) and (A == 32) and the bit position has even
gone backwards in some cases.

I've included a patch below which changes the behaviour to match my
expectations, with the patch applied the table now looks like this:

| TYPE   |   FIELD BITPOS T2 for different alignment A   |
| LENGTH |                                               |
|   T1   | A == 0 | A == 8 | A == 16 | A == 24 | A == 32 |
|--------|--------|--------|---------|---------|---------|
| 0      | 0      | 0      | 0       | 0       | 0       |
| 8      | 8      | 8      | 16      | 24      | 32      |
| 16     | 16     | 16     | 16      | 24      | 32      |
| 24     | 24     | 24     | 32      | 24      | 32      |
| 32     | 32     | 32     | 32      | 48      | 32      |

These values seem to make more sense to me, hopefully you'll all
agree, though if I've misunderstood something or misread the code
in please could someone point out what I've missed.

If this looks good then am I OK to apply the patch?

Thanks,
Andrew


gdb/ChangeLog

2011-06-09  Andrew Burgess  <aburgess@broadcom.com>

	* gdbtypes.c (append_composite_type_field_aligned): Fix
          calculation of bit position based on alignment.

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 2bdb4eb..ba957f9 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -3654,12 +3654,14 @@ append_composite_type_field_aligned (struct type *t, char *name,
 
 	  if (alignment)
 	    {
-	      int left = FIELD_BITPOS (f[0]) % (alignment * TARGET_CHAR_BIT);
+	      int left;
+	      alignment *= TARGET_CHAR_BIT;
+	      left = FIELD_BITPOS (f[0]) % alignment;
 
 	      if (left)
 		{
-		  FIELD_BITPOS (f[0]) += left;
-		  TYPE_LENGTH (t) += left / TARGET_CHAR_BIT;
+		  FIELD_BITPOS (f[0]) += (alignment - left);
+		  TYPE_LENGTH (t) += (alignment - left) / TARGET_CHAR_BIT;
 		}
 	    }
 	}