ecpg compile error on AIX

On Mon, Jan 07, 2002 at 02:27:43PM +0900, Tatsuo Ishii wrote:

I got followings with current on AIX 5L using xlc (native AIX
compiler):
...
:
"data.c", line 357.81: 1506-068 (S) Operation between types "void*" and "long" is not allowed.
"data.c", line 362.90: 1506-068 (S) Operation between types "void*" and "long" is not allowed.

Argh, I was afraid something like this would happen.

*((long long int *) (ind + ind_offset*act_tuple)) = variable->len;

Does it complain about all variable types?

So the code tries to add a long value to a void *pointer, which is not
correct since the storage unit size is unknown for void * I think. If
the code try to do a pointer calculation, "ind" should be casted to an
appropreate type such as char * or long * etc, depending on the logic
of the code.

Does it work with this?

*((long long int *) ((long long int *)ind + ind_offset*act_tuple)) = variable->len;

If it does we have to check whether that does what I expect it to.

The idea was to skip the rest of a struct. Let's assume you have a
struct
{
int foo;
float bar;
}

If you now read a set of tuples into an array of structs of this type you
have to calculate the address of the struct[1].foo, struct[2].foo etc. and
struct[1].bar, struct[2].bar, etc.

Michael

--
Michael Meskes
Michael@Fam-Meskes.De
Go SF 49ers! Go Rhein Fire!
Use Debian GNU/Linux! Use PostgreSQL!

Michael Meskes <meskes@postgresql.org> writes:

Does it work with this?

*((long long int *) ((long long int *)ind + ind_offset*act_tuple)) = variable->len;

If it does we have to check whether that does what I expect it to.

ind_offset is already a sizeof() measure, isn't it?
I would guess that what you want is

*((long long int *) ((char *)ind + ind_offset*act_tuple)) = variable->len;

since ind_offset*act_tuple is a number expressed in bytes, and should
not be scaled up by sizeof(long long int).

Also, if the code works for you at all, it's because GCC is (in
violation of the ANSI C standard) interpreting the construct as
addition to char* rather than addition to void*. Casting to anything
other than char* will change the behavior.

(Might be a lot easier just to declare ind as char* instead of void*
in the first place...)

regards, tom lane

Tatsuo Ishii <t-ishii@sra.co.jp> writes:

I got followings with current on AIX 5L using xlc (native AIX
compiler):

"data.c", line 357.81: 1506-068 (S) Operation between types "void*" and "long" is not allowed.
"data.c", line 362.90: 1506-068 (S) Operation between types "void*" and "long" is not allowed.

With HP's compiler I get lots of

cc: "data.c", line 57: error 1539: Cannot do arithmetic with pointers to objects of unknown size.

which is perhaps more to the point. The GCC manual explains why Michael
was able to get away with this (I assume he used gcc):

In GNU C, addition and subtraction operations are supported on
pointers to `void' and on pointers to functions. This is done by
treating the size of a `void' or of a function as 1.

A consequence of this is that `sizeof' is also allowed on `void' and
on function types, and returns 1.

The option `-Wpointer-arith' requests a warning if these extensions
are used.

It occurs to me that we ought to add -Wpointer-arith to our standard
gcc options, so that this sort of mistake will be caught sooner in
future.

I consider this compile failure a "must fix" before we can go to RC1 ...

regards, tom lane

In GNU C, addition and subtraction operations are supported on
pointers to `void' and on pointers to functions. This is done by
treating the size of a `void' or of a function as 1.

A consequence of this is that `sizeof' is also allowed on `void' and
on function types, and returns 1.

The option `-Wpointer-arith' requests a warning if these extensions
are used.

It occurs to me that we ought to add -Wpointer-arith to our standard
gcc options, so that this sort of mistake will be caught sooner in
future.

I added this to my Makefile.custom.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian writes:

It occurs to me that we ought to add -Wpointer-arith to our standard
gcc options, so that this sort of mistake will be caught sooner in
future.

I agree with that. Actually, I could never imagine how one would use
sizeof(void*) in the first place. I guess one can.

I added this to my Makefile.custom.

I've had -Wpointer-arith and -Wcast-align in my Makefile.custom for a
year, but apparently just last week I didn't do any builds where I
actually paid attention. :-/

Btw., I've never seen any problems related to -Wcast-align? Is the TODO
item obsolete or is it platform-related?

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut wrote:

Bruce Momjian writes:

It occurs to me that we ought to add -Wpointer-arith to our standard
gcc options, so that this sort of mistake will be caught sooner in
future.

I agree with that. Actually, I could never imagine how one would use
sizeof(void*) in the first place. I guess one can.

I added this to my Makefile.custom.

I've had -Wpointer-arith and -Wcast-align in my Makefile.custom for a
year, but apparently just last week I didn't do any builds where I
actually paid attention. :-/

Here is what I have now for my custom:

CUSTOM_COPT=-g -Wall -Wmissing-prototypes \
-Wmissing-declarations -Wpointer-arith -Wcast-align

In Makefile.global.in, we have:

ifeq ($(GCC), yes)
CFLAGS += -Wall -Wmissing-prototypes -Wmissing-declarations
endif

Should I add -Wpointer-arith -Wcast-align for 7.3?

Btw., I've never seen any problems related to -Wcast-align? Is the TODO
item obsolete or is it platform-related?

That is a Tom item. I think there is some casting that masks the
problem. Tom?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Btw., I've never seen any problems related to -Wcast-align? Is the TODO
item obsolete or is it platform-related?

Youse guys that run on Intel hardware will never see any problems from
it, except possibly a lost cycle here or there due to unaligned fetches.

But a lot of non-Intel hardware (particularly RISC architectures) treats
an unaligned access as a segfault.

Right at the moment we can't usefully enable -Wcast-align because it
generates an unreasonable number of complaints. Someday I'm going to
try to clean those all up. My personal todo list has:

Reduce, or eliminate entirely, warnings issued by -Wcast-align. gcc will
warn about char* to foo* but not about void* to foo*, so the bulk of the
warnings might be controllable by using void* in places where we now use
char*. Be careful not to introduce arithmetic on void* pointers though;
use -Wpointer-arith to catch those. Ideally we should add both of these
(and maybe some other non-Wall flags) to standard gcc arguments.

regards, tom lane

Tom Lane writes:

Btw., I've never seen any problems related to -Wcast-align? Is the TODO
item obsolete or is it platform-related?

Youse guys that run on Intel hardware will never see any problems from
it, except possibly a lost cycle here or there due to unaligned fetches.

I should have said, I've never seen any warnings generated by
-Wcast-align. Shouldn't the set of warnings at least be the same on all
platforms (at least those with the same integer size) or does it just warn
if there would actually be a problem on that platform?

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:

I should have said, I've never seen any warnings generated by
-Wcast-align.

Interesting. On HP-PA with gcc 2.95.3 I see a ton of them. I'm too
lazy to count 'em, but it's certainly in the thousands, eg from the
first backend module to be compiled I get

make[4]: Entering directory `/home/postgres/pgsql/src/backend/access/common'
gcc -O2 -Wall -Wmissing-prototypes -Wmissing-declarations -g -Wcast-align -I../../../../src/include -c -o heaptuple.o heaptuple.c
heaptuple.c: In function `ComputeDataSize':
heaptuple.c:52: warning: cast increases required alignment of target type
heaptuple.c: In function `DataFill':
heaptuple.c:113: warning: cast increases required alignment of target type
heaptuple.c:113: warning: cast increases required alignment of target type
heaptuple.c:123: warning: cast increases required alignment of target type
heaptuple.c:134: warning: cast increases required alignment of target type
heaptuple.c: In function `nocachegetattr':
heaptuple.c:299: warning: cast increases required alignment of target type
heaptuple.c:299: warning: cast increases required alignment of target type
heaptuple.c:357: warning: cast increases required alignment of target type
heaptuple.c:360: warning: cast increases required alignment of target type
heaptuple.c:360: warning: cast increases required alignment of target type
heaptuple.c:400: warning: cast increases required alignment of target type
heaptuple.c:408: warning: cast increases required alignment of target type
heaptuple.c:408: warning: cast increases required alignment of target type
heaptuple.c: In function `heap_copytuple':
heaptuple.c:486: warning: cast increases required alignment of target type
heaptuple.c: In function `heap_formtuple':
heaptuple.c:608: warning: cast increases required alignment of target type
heaptuple.c:610: warning: cast increases required alignment of target type
heaptuple.c:610: warning: cast increases required alignment of target type
heaptuple.c: In function `heap_modifytuple':
heaptuple.c:680: warning: cast increases required alignment of target type
heaptuple.c:680: warning: cast increases required alignment of target type
heaptuple.c: In function `heap_addheader':
heaptuple.c:765: warning: cast increases required alignment of target type
heaptuple.c:767: warning: cast increases required alignment of target type
heaptuple.c:767: warning: cast increases required alignment of target type

Needless to say I'd be pretty much unable to spot any real warnings
if we were to turn this on by default today.

Shouldn't the set of warnings at least be the same on all
platforms (at least those with the same integer size) or does it just warn
if there would actually be a problem on that platform?

Apparently the latter. Curious; you'd think the former would be more
useful.

regards, tom lane

Shouldn't the set of warnings at least be the same on all
platforms (at least those with the same integer size) or does it just warn
if there would actually be a problem on that platform?

In fact, the GCC manual makes it pretty clear that the check is machine
dependent:

`-Wcast-align'
Warn whenever a pointer is cast such that the required alignment
of the target is increased. For example, warn if a `char *' is
cast to an `int *' on machines where integers can only be accessed
at two- or four-byte boundaries.

I suppose this means that we'd better run test compilations on Alphas
(or some other 8-byte-long environment) too, whenever we try to turn
on -Wcast-align.

regards, tom lane

Peter Eisentraut wrote:

Bruce Momjian writes:

It occurs to me that we ought to add -Wpointer-arith to our standard
gcc options, so that this sort of mistake will be caught sooner in
future.

I agree with that. Actually, I could never imagine how one would use
sizeof(void*) in the first place. I guess one can.

So I should add -Wpointer-arith to standard gcc compiles, but not
-Wcast-align because it generates too many warnings on some platforms.
Is this right?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

On Mon, Jan 07, 2002 at 12:40:07PM -0500, Tom Lane wrote:

ind_offset is already a sizeof() measure, isn't it?
I would guess that what you want is

*((long long int *) ((char *)ind + ind_offset*act_tuple)) = variable->len;

since ind_offset*act_tuple is a number expressed in bytes, and should
not be scaled up by sizeof(long long int).

Yes, you're right of course. I should have thought more before typing.

Also, if the code works for you at all, it's because GCC is (in
violation of the ANSI C standard) interpreting the construct as
addition to char* rather than addition to void*. Casting to anything
other than char* will change the behavior.

That's what I was afraid of and why I asked for some testing on other archs.
Right now I only have access to Intel based Linux.

(Might be a lot easier just to declare ind as char* instead of void*
in the first place...)

Did that. My test cases all work well. Please test on HP, AIX or whatever.

Michael
--
Michael Meskes
Michael@Fam-Meskes.De
Go SF 49ers! Go Rhein Fire!
Use Debian GNU/Linux! Use PostgreSQL!

On Mon, Jan 07, 2002 at 04:07:40PM -0500, Tom Lane wrote:

I consider this compile failure a "must fix" before we can go to RC1 ...

Commit is underways.

Michael
--
Michael Meskes
Michael@Fam-Meskes.De
Go SF 49ers! Go Rhein Fire!
Use Debian GNU/Linux! Use PostgreSQL!