Re: [PATCH] Expand bitpos to LONGEST to allow access to large offsets within a struct

[Tom Tromey <tromey@redhat.com>]

>>>>> "Siddhesh" == Siddhesh Poyarekar <siddhesh@redhat.com> writes:

Siddhesh> If a struct member is at an offset greater than or equal to
Siddhesh> 0x10000000, the resulting bit position within the struct
Siddhesh> overflows and causes an invalid access. The following program
Siddhesh> demonstrates this problem:

Thanks for doing this.  It is a long-needed fix.

Siddhesh> This happens because the bitpos in field_location within the struct
Siddhesh> main_type.field is declared as an int, limiting it to just 4 bytes. I
Siddhesh> have attached a patch that expands this to LONGEST and adjusted this
Siddhesh> change in the code. The testsuite does not report any regressions due
Siddhesh> to this patch and it fixes the problem.

I think this should fix http://sourceware.org/bugzilla/show_bug.cgi?id=7259.
If so, at the top of the ChangeLog, write 'PR symtab/7259:'.

If it does fix this PR then this suggests a simple test case that
doesn't require a huge allocation.


Most of the patch seems perfectly fine -- just the logical consequence
of the core change.  However there are a few issues, mostly minor.

Siddhesh>        printfi_filtered (spaces + 2,
Siddhesh> -			"[%d] bitpos %d bitsize %d type ",
Siddhesh> +			"[%d] bitpos %ld bitsize %d type ",
Siddhesh>  			idx, TYPE_FIELD_BITPOS (type, idx),

You can't really rely on the size of LONGEST.
Instead you have to use %s and 'plongest'.

There are a few instances of this in the patch.

Siddhesh> diff --git a/gdb/value.c b/gdb/value.c
Siddhesh> index 583be33..49a6f43 100644
Siddhesh> --- a/gdb/value.c
Siddhesh> +++ b/gdb/value.c
Siddhesh> @@ -308,7 +308,7 @@ struct value
Siddhesh>       `type', and `embedded_offset' is zero, so everything works
Siddhesh>       normally.  */
Siddhesh>    struct type *enclosing_type;
Siddhesh> -  int embedded_offset;
Siddhesh> +  LONGEST embedded_offset;
Siddhesh>    int pointed_to_offset;

I think you also have to widen the 'offset' field, and probably also
'pointed_to_offset'.  This will probably have other consequences, e.g.,
value_offset will have a different return type.

Tom