Re: [PATCHES] fix for strict-alias warnings - Mailing list pgsql-hackers

[ Moved to hackers.]

Andrew Dunstan wrote:
> struct foobar {
>   int tag;
>   union {
>     struct foo foo;
>     struct bar bar;
>  } v;
> };
> 

OK, this is very clear.  Thanks.

> >   The proc.c cases were using MemSet, which was checking if the
> > int* as aligned for int* access.  In fact, we could change MemSet to
> > always take a void *, and do the int* casting when we access it after
> > testing for alignment.
> >
> 
> Since MemSet is generic, that is probably a good idea.

Done.

> > The big question in my mind is whether there there is other struct *
> > passing that could be masked right now by void* casting, and if so, do
> > they have different first elements?  This determined whether we do
> > -fstrict-aliasing for gcc, or fix just these few cases.
> 
> Just analysing this is a non-trivial piece of work, I think.

I understand the desire to deal with this later, but we never seem to
focus on compiler issues except during beta.

I think I know why we are hitting the warning in tablecmds.c and not in
trigger.c.  Trigger.c has:ExecCallTriggerFunc(TriggerData *trigdata,                    FmgrInfo *finfo,
   MemoryContext per_tuple_context){    ...    fcinfo.flinfo = finfo;    fcinfo.context = (Node *) trigdata;
 

This does not generate a warning, while this does in tablecmds.c:
   while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)   {       FunctionCallInfoData fcinfo;
TriggerDatatrigdata;
 
       /*        * Make a call to the trigger function        *        * No parameters are passed, but we do set a
context       */       MemSet(&fcinfo, 0, sizeof(fcinfo));
 
       /*        * We assume RI_FKey_check_ins won't look at flinfo...        */       trigdata.type = T_TriggerData;
   trigdata.tg_event = TRIGGER_EVENT_INSERT | TRIGGER_EVENT_ROW;       trigdata.tg_relation = rel;
trigdata.tg_trigtuple= tuple;       trigdata.tg_newtuple = NULL;       trigdata.tg_trigger = &trig;
 
       fcinfo.context = (Node *) &trigdata;
       RI_FKey_check_ins(&fcinfo);   }

trigger.c doesn't generate the warning because it is a TriggerData*,
while tablecmds.c has an acual structure.  trigger.c doesn't know if the
passed pointer was allocated via malloc(), and therefore properly
aligned for any data type, or if it was allocated on the stack, so it
does not complain.

In tablecmds.c, they create it as local loop structure that is passed to
RI_FKey_check_ins().  What we should be doing in this code is allocating
a proper Node structure using makeNode(TriggerData), and using that in
the loop.  (We should allocate it outside the loop.)

I see the same problem in execQual.c.  sysv_shmem probably needs the
void*, and psql/command.c should have parse_char defines as unsigned
char **, rather than char **.

Comments?

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073