* 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).