public inbox for gcc-help@gcc.gnu.org
 help / color / mirror / Atom feed
* is this a valid approach to aliasing?
@ 2008-02-12  9:35 Robert William Fuller
  2008-02-12  9:43 ` Andrew Haley
  0 siblings, 1 reply; 6+ messages in thread
From: Robert William Fuller @ 2008-02-12  9:35 UTC (permalink / raw)
  To: gcc-help

After twenty years of programming in C, I thought I understood the 
language pretty well until some new code I was writing started emitting 
"type-punned pointer" aliasing warnings.  So I read up on the problem so 
I would understand it.  I understand that is necessary to restrict 
aliasing in order to optimize/load stores.

My question is if my solution to the problem is valid.  Basically, I had 
some functionality I wanted to abstract which became the following function:

int reallocItems(void **items, int itemSize, int *itemAlloc, int 
numItems, int newItems, int itemHint)
{
     void *rcAlloc;
     int emptyItems, allocItems;

     // for idempotence
     allocItems = *itemAlloc;

     // will the new entries fit?
     //
     emptyItems = allocItems - numItems;
     if (newItems > emptyItems) {

         // allocate the number of entries needed or the hint, whichever 
is more
         //
         allocItems = numItems + newItems;
         if (itemHint > allocItems) {
             allocItems = itemHint;
         }

         rcAlloc = (void *) realloc(*items, allocItems * itemSize);
         if (rcAlloc) {

             *items = rcAlloc;
             *itemAlloc = allocItems;

         } else {
             return -1;
         }
     }

     return 0;
}

Naturally, when I started trying to call this function, I started 
getting the type-punned pointer aliasing warnings.  The code in question 
looked roughly like this:

static opt_param_t *shrtOpts, *longOpts;

rc = reallocItems((void **) &shrtOpts, sizeof(opt_param_t), 
&shrtOptAlloc, numShrtOpts, newShrtOpts, shrtOptHint);

Obviously, this breaks the aliasing rules.  I read that I could work 
around this by casting through a union.  I settled on this approach, but 
I'm not sure if it is valid, or if I'm merely masking the problem:

typedef union _opt_param_alias_t {

     opt_param_t *o;
     void *v;

} opt_param_alias_t;

rc = reallocItems(&((opt_param_alias_t *) &shrtOpts)->v, 
sizeof(opt_param_t), &shrtOptAlloc, numShrtOpts, newShrtOpts, shrtOptHint)

Will casting through this union force the compiler to reload the address 
of shrtOpts when I try to reference it a few lines later?  Is this 
solution correct?  Is there a better approach that does NOT involve 
changing reallocItems to return a void pointer?

I tried several different approaches, and I'm trying to find an approach 
that requires little extra code and does not ugly things up too much.

Please advise.  Thank you.

Regards,

Rob

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: is this a valid approach to aliasing?
  2008-02-12  9:35 is this a valid approach to aliasing? Robert William Fuller
@ 2008-02-12  9:43 ` Andrew Haley
  2008-02-12 10:11   ` Robert William Fuller
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Haley @ 2008-02-12  9:43 UTC (permalink / raw)
  To: Robert William Fuller; +Cc: gcc-help

Robert William Fuller wrote:
>
> Naturally, when I started trying to call this function, I started 
> getting the type-punned pointer aliasing warnings.  The code in question 
> looked roughly like this:
> 
> static opt_param_t *shrtOpts, *longOpts;
> 
> rc = reallocItems((void **) &shrtOpts, sizeof(opt_param_t), 
> &shrtOptAlloc, numShrtOpts, newShrtOpts, shrtOptHint);
> 
> Obviously, this breaks the aliasing rules.  I read that I could work 
> around this by casting through a union.  I settled on this approach, but 
> I'm not sure if it is valid, or if I'm merely masking the problem:
> 
> typedef union _opt_param_alias_t {
> 
>     opt_param_t *o;
>     void *v;
> 
> } opt_param_alias_t;
> 
> rc = reallocItems(&((opt_param_alias_t *) &shrtOpts)->v, 
> sizeof(opt_param_t), &shrtOptAlloc, numShrtOpts, newShrtOpts, shrtOptHint)

It really isn't necessary to do this.  The rule is that you mustn't use
a pointer cast to create an lvalue of incompatible type.  So, do this:

static opt_param_t *shrtOpts, *longOpts;

void *p1;

rc = reallocItems(&p1, sizeof(opt_param_t),
    &shrtOptAlloc, numShrtOpts, newShrtOpts, shrtOptHint);
shrtOpts = p1;

Andrew.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: is this a valid approach to aliasing?
  2008-02-12  9:43 ` Andrew Haley
@ 2008-02-12 10:11   ` Robert William Fuller
  2008-02-12 10:24     ` Andrew Haley
  0 siblings, 1 reply; 6+ messages in thread
From: Robert William Fuller @ 2008-02-12 10:11 UTC (permalink / raw)
  To: gcc-help; +Cc: Andrew Haley

Andrew Haley wrote:
> Robert William Fuller wrote:
>>
>> Naturally, when I started trying to call this function, I started 
>> getting the type-punned pointer aliasing warnings.  The code in 
>> question looked roughly like this:
>>
>> static opt_param_t *shrtOpts, *longOpts;
>>
>> rc = reallocItems((void **) &shrtOpts, sizeof(opt_param_t), 
>> &shrtOptAlloc, numShrtOpts, newShrtOpts, shrtOptHint);
>>
>> Obviously, this breaks the aliasing rules.  I read that I could work 
>> around this by casting through a union.  I settled on this approach, 
>> but I'm not sure if it is valid, or if I'm merely masking the problem:
>>
>> typedef union _opt_param_alias_t {
>>
>>     opt_param_t *o;
>>     void *v;
>>
>> } opt_param_alias_t;
>>
>> rc = reallocItems(&((opt_param_alias_t *) &shrtOpts)->v, 
>> sizeof(opt_param_t), &shrtOptAlloc, numShrtOpts, newShrtOpts, 
>> shrtOptHint)
> 
> It really isn't necessary to do this.  The rule is that you mustn't use
> a pointer cast to create an lvalue of incompatible type.  So, do this:
> 
> static opt_param_t *shrtOpts, *longOpts;
> 
> void *p1;
> 
> rc = reallocItems(&p1, sizeof(opt_param_t),
>    &shrtOptAlloc, numShrtOpts, newShrtOpts, shrtOptHint);
> shrtOpts = p1;

I suspected that might work.  I asked so I would not have to look at 
gcc's output.  I have not looked at (dis)assembly since I worked on 
Windows NT in its early days when things were even more closed than they 
are now.  I am now a totally reformed free software user and programmer. 
  (Ask yourself if there is a causal relation there.)

I want to make sure I am very clear on this.  Are you saying the 
following is okay?

p1 = shrtOpts;
rc = reallocItems(&p1, ...)
shrtOpts = p1;

Thank you.

Regards,

Rob

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: is this a valid approach to aliasing?
  2008-02-12 10:11   ` Robert William Fuller
@ 2008-02-12 10:24     ` Andrew Haley
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Haley @ 2008-02-12 10:24 UTC (permalink / raw)
  To: Robert William Fuller; +Cc: gcc-help

Robert William Fuller wrote:
> Andrew Haley wrote:
>> Robert William Fuller wrote:
>>>
>>> Naturally, when I started trying to call this function, I started 
>>> getting the type-punned pointer aliasing warnings.  The code in 
>>> question looked roughly like this:
>>>
>>> static opt_param_t *shrtOpts, *longOpts;
>>>
>>> rc = reallocItems((void **) &shrtOpts, sizeof(opt_param_t), 
>>> &shrtOptAlloc, numShrtOpts, newShrtOpts, shrtOptHint);
>>>
>>> Obviously, this breaks the aliasing rules.  I read that I could work 
>>> around this by casting through a union.  I settled on this approach, 
>>> but I'm not sure if it is valid, or if I'm merely masking the problem:
>>>
>>> typedef union _opt_param_alias_t {
>>>
>>>     opt_param_t *o;
>>>     void *v;
>>>
>>> } opt_param_alias_t;
>>>
>>> rc = reallocItems(&((opt_param_alias_t *) &shrtOpts)->v, 
>>> sizeof(opt_param_t), &shrtOptAlloc, numShrtOpts, newShrtOpts, 
>>> shrtOptHint)
>>
>> It really isn't necessary to do this.  The rule is that you mustn't use
>> a pointer cast to create an lvalue of incompatible type.  So, do this:
>>
>> static opt_param_t *shrtOpts, *longOpts;
>>
>> void *p1;
>>
>> rc = reallocItems(&p1, sizeof(opt_param_t),
>>    &shrtOptAlloc, numShrtOpts, newShrtOpts, shrtOptHint);
>> shrtOpts = p1;
> 
> I suspected that might work.  I asked so I would not have to look at 
> gcc's output.  I have not looked at (dis)assembly since I worked on 
> Windows NT in its early days when things were even more closed than they 
> are now.  I am now a totally reformed free software user and programmer. 
>  (Ask yourself if there is a causal relation there.)
> 
> I want to make sure I am very clear on this.  Are you saying the 
> following is okay?
> 
> p1 = shrtOpts;
> rc = reallocItems(&p1, ...)
> shrtOpts = p1;

Yes.  But you shouldn't believe me!  Read the C Standard.
The section you need to understand is 6.3.2.3.

http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf

Andrew.


http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: is this a valid approach to aliasing?
  2008-02-12 23:22 ` Robert William Fuller
@ 2008-02-13 11:32   ` Andrew Haley
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Haley @ 2008-02-13 11:32 UTC (permalink / raw)
  To: Robert William Fuller; +Cc: John Love-Jensen, gcc-help

Robert William Fuller wrote:
> John Love-Jensen wrote:
>> Hi Robert,
>>
>> Can you just get rid of the void** for the items parameter (see below)?
>>
>> --Eljay
>>
>> #include <stdlib.h>
>>
>> int reallocItems(
>>   void* items,
>>   int itemSize,
>>   int* itemAlloc,
>>   int numItems,
>>   int newItems,
>>   int itemHint)
>> {
>>   void* rcAlloc;
>>   int emptyItems;
>>   int allocItems;
>>
>>   // for idempotence
>>   allocItems = *itemAlloc;
>>
>>   // will the new entries fit?
>>   emptyItems = allocItems - numItems;
>>
>>   if (newItems > emptyItems)
>>   {
>>     // allocate the number of entries needed or the hint, whichever is 
>> more
>>     allocItems = numItems + newItems;
>>
>>     if (itemHint > allocItems)
>>     {
>>       allocItems = itemHint;
>>     }
>>
>>     rcAlloc = realloc(items, allocItems * itemSize);
>>
>>     if (rcAlloc)
>>     {
>>       *(void**)items = rcAlloc;

This code is probably wrong.  Make item a void** and get rid of the cast.

Andrew.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: is this a valid approach to aliasing?
       [not found] <C3D6F1DD.2CC82%eljay@adobe.com>
@ 2008-02-12 23:22 ` Robert William Fuller
  2008-02-13 11:32   ` Andrew Haley
  0 siblings, 1 reply; 6+ messages in thread
From: Robert William Fuller @ 2008-02-12 23:22 UTC (permalink / raw)
  To: John Love-Jensen, gcc-help; +Cc: Andrew Haley

John Love-Jensen wrote:
> Hi Robert,
> 
> Can you just get rid of the void** for the items parameter (see below)?
> 
> --Eljay
> 
> #include <stdlib.h>
> 
> int reallocItems(
>   void* items,
>   int itemSize,
>   int* itemAlloc,
>   int numItems,
>   int newItems,
>   int itemHint)
> {
>   void* rcAlloc;
>   int emptyItems;
>   int allocItems;
> 
>   // for idempotence
>   allocItems = *itemAlloc;
> 
>   // will the new entries fit?
>   emptyItems = allocItems - numItems;
> 
>   if (newItems > emptyItems)
>   {
>     // allocate the number of entries needed or the hint, whichever is more
>     allocItems = numItems + newItems;
> 
>     if (itemHint > allocItems)
>     {
>       allocItems = itemHint;
>     }
> 
>     rcAlloc = realloc(items, allocItems * itemSize);
> 
>     if (rcAlloc)
>     {
>       *(void**)items = rcAlloc;
>       *itemAlloc = allocItems;
>     }
>     else
>     {
>       return -1;
>     }
>   }
> 
>   return 0;
> }

Would this circumvent the type system?  Is there still an alias problem 
with your suggestion, albeit without a warning?  I am not sure.

Rob

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-02-13 11:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-12  9:35 is this a valid approach to aliasing? Robert William Fuller
2008-02-12  9:43 ` Andrew Haley
2008-02-12 10:11   ` Robert William Fuller
2008-02-12 10:24     ` Andrew Haley
     [not found] <C3D6F1DD.2CC82%eljay@adobe.com>
2008-02-12 23:22 ` Robert William Fuller
2008-02-13 11:32   ` Andrew Haley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).