On 07/22/11 13:44, Jason Merrill wrote:
> On 07/18/2011 08:02 AM, Aldy Hernandez wrote:
>> + /* If other threads can't see this value, no need to restrict
>> stores. */
>> + if (ALLOW_STORE_DATA_RACES
>> + || !DECL_THREAD_VISIBLE_P (innerdecl))
>> + {
>> + *bitstart = *bitend = 0;
>> + return;
>> + }
>
> What if get_inner_reference returns something that isn't a DECL, such as
> an INDIRECT_REF?

I had changed this already to take into account aliasing, so if we get 
an INDIRECT_REF, ptr_deref_may_alias_global_p() returns true, and we 
proceed with the restriction:

+  /* If other threads can't see this value, no need to restrict stores.  */
+  if (ALLOW_STORE_DATA_RACES
+      || (!ptr_deref_may_alias_global_p (innerdecl)
+         && (DECL_THREAD_LOCAL_P (innerdecl)
+             || !TREE_STATIC (innerdecl))))


>> + if (fld)
>> + {
>> + /* We found the end of the bit field sequence. Include the
>> + padding up to the next field and be done. */
>> + *bitend = bitpos - 1;
>> + }
>
> bitpos is the position of "field", and it seems to me we want the
> position of "fld" here.

Notice that bitpos gets recalculated at each iteration by 
get_inner_reference, so bitpos is actually the position of fld.

>> + /* If unset, no restriction. */
>> + if (!bitregion_end)
>> + maxbits = 0;
>> + else
>> + maxbits = (bitregion_end - bitregion_start) % align;
>
> Maybe use MAX_FIXED_MODE_SIZE so you don't have to test it against 0?

Fixed everywhere.

>> + if (!bitregion_end)
>> + maxbits = 0;
>> + else if (1||bitpos + offset * BITS_PER_UNIT < bitregion_start)
>> + maxbits = bitregion_end - bitregion_start;
>> + else
>> + maxbits = bitregion_end - (bitpos + offset * BITS_PER_UNIT) + 1;
>
> I assume the 1|| was there for debugging?

Fixed, plus I adjusted the calculation of maxbits everywhere because I 
found an off-by-one error.

I have also overhauled store_bit_field() to adjust the address of the 
address to point to the beginning of the bit region.  This fixed a 
myraid of corner cases pointed out by a test Hans Boehm was kind enough 
to provide.

I have added more tests.

How does this look?  (Pending tests.)