compiler warnings on the buildfarm

Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:

What is the official stance on handling compiler warnings?

The compilers I use give me 1 or 2 warnings on HEAD, coming from flex's
sloppiness about not generating unused code. I wouldn't care to work
with a compiler that generated more than a few. At the same time,
I'm not prepared to contort the source code unduly to suppress what
you referred to as "spam" warnings from over-chatty compilers.

What would probably be useful if you want to pursue this is to filter
out the obvious spam like statement-not-reached, and see what's left.

regards, tom lane

Am Donnerstag, 12. Juli 2007 15:25 schrieb Stefan Kaltenbrunner:

a lot of those are simply noise (like the LOOP VECTORIZED stuff from the
icc boxes or the "statement not reached" spam from the sun compilers)
but others might indicate real issues.
To find warnings that might be a real problem we might want to look into
�suppressing those - if possible - �using compiler switches.

It would be good to determine an appropriate set of compiler switches to
reduce the warnings to a reasonable level.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Tom Lane wrote:

Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:

What is the official stance on handling compiler warnings?

The compilers I use give me 1 or 2 warnings on HEAD, coming from flex's
sloppiness about not generating unused code. I wouldn't care to work
with a compiler that generated more than a few. At the same time,
I'm not prepared to contort the source code unduly to suppress what
you referred to as "spam" warnings from over-chatty compilers.

What would probably be useful if you want to pursue this is to filter
out the obvious spam like statement-not-reached, and see what's left.

ok I did that for a few members (removing all the statement not reached
ones as well as some purely informal notices and all the flex related
warnings) and came up with something similiar to:

animal: eel warnings: 4
dirmod.c:206: warning: no previous prototype for 'pgsymlink'
dirmod.c:206: warning: no previous prototype for 'pgsymlink'
pg_ctl.c: In function `pgwin32_doRunAsService':
pg_ctl.c:1240: warning: initialization discards qualifiers from pointer
target type

animal: clownfish warnings: 12
"dynloader.c", line 4: warning: empty translation unit
"postgres.c", line 3758: warning: loop not entered at top
"xml.c", line 2810: warning: integer overflow detected: op "<<"
"xml.c", line 2810: warning: integer overflow detected: op "UNARY -"
"xml.c", line 2810: warning: integer overflow detected: op "<<"
"dfmgr.c", line 117: warning: assignment type mismatch:
pointer to function(pointer to struct FunctionCallInfoData
{pointer to struct FmgrInfo {..} flinfo, pointer to struct Node {..}
context, pointer to struc
t Node {..} resultinfo, char isnull, short nargs, array[100] of unsigned
long arg, array[100] of char argnull}) returning unsigned long "="
pointer to void
"dfmgr.c", line 165: warning: return value type mismatch
"wchar.c", line 283: warning: integer overflow detected: op "<<"
"wchar.c", line 283: warning: integer overflow detected: op "<<"
"pg_resetxlog.c", line 76: warning: initializer does not fit or is out
of range: -1
"pg_resetxlog.c", line 80: warning: initializer does not fit or is out
of range: -1

animal: jackal warnings: 2
postmaster.c: In function 'PostmasterMain':
postmaster.c:796: warning: 'DNSServiceRegistrationCreate' is deprecated
(declared at /usr/include/DNSServiceDiscovery/DNSServiceDiscovery.h:114)

animal: impala warnings: 6
auth.c: In function 'pg_GSS_recvauth':
auth.c:435: warning: format '%u' expects type 'unsigned int', but
argument 3 has type 'size_t'
auth.c:456: warning: format '%u' expects type 'unsigned int', but
argument 5 has type 'size_t'
auth.c:464: warning: format '%u' expects type 'unsigned int', but
argument 3 has type 'size_t'
auth.c: In function 'sendAuthRequest':
auth.c:787: warning: format '%u' expects type 'unsigned int', but
argument 3 has type 'size_t'

animal: grebe warnings: 45
xlog.c: In function 'XLogInsert':
xlog.c:651: warning: implicit declaration of function '_check_lock'
xlog.c:654: warning: implicit declaration of function '_clear_lock'
hba.c: In function 'ident_unix':
hba.c:1449: warning: implicit declaration of function 'getpeereid'
ip.c: In function 'getaddrinfo_unix':
ip.c:254: warning: large integer implicitly truncated to unsigned type
bgwriter.c: In function 'BackgroundWriterMain':
bgwriter.c:305: warning: implicit declaration of function '_check_lock'
bgwriter.c:308: warning: implicit declaration of function '_clear_lock'
buf_init.c: In function 'InitBufferPool':
buf_init.c:118: warning: implicit declaration of function '_clear_lock'
bufmgr.c: In function 'ReadBuffer_common':
bufmgr.c:249: warning: implicit declaration of function '_check_lock'
bufmgr.c:252: warning: implicit declaration of function '_clear_lock'
freelist.c: In function 'StrategyGetBuffer':
freelist.c:143: warning: implicit declaration of function '_check_lock'
freelist.c:150: warning: implicit declaration of function '_clear_lock'
shmem.c: In function 'InitShmemAllocation':
shmem.c:127: warning: implicit declaration of function '_clear_lock'
shmem.c: In function 'ShmemAlloc':
shmem.c:168: warning: implicit declaration of function '_check_lock'
proc.c: In function 'InitProcGlobal':
proc.c:216: warning: implicit declaration of function '_clear_lock'
proc.c: In function 'InitProcess':
proc.c:247: warning: implicit declaration of function '_check_lock'
lwlock.c: In function 'CreateLWLocks':
lwlock.c:256: warning: implicit declaration of function '_clear_lock'
lwlock.c: In function 'LWLockAssign':
lwlock.c:291: warning: implicit declaration of function '_check_lock'
s_lock.c: In function 's_lock':
s_lock.c:99: warning: implicit declaration of function '_check_lock'
dynahash.c: In function 'init_htab':
dynahash.c:526: warning: implicit declaration of function '_clear_lock'
dynahash.c: In function 'hash_search_with_hash_value':
dynahash.c:876: warning: implicit declaration of function '_check_lock'
guc.c:2866: warning: 'guc_get_index' defined but not used
Extra instructions are being generated for each reference to a TOC
symbol if the symbol is in the TOC overflow area.
ip.c: In function 'getaddrinfo_unix':
ip.c:254: warning: large integer implicitly truncated to unsigned type
connect.c:23: warning: missing braces around initializer
connect.c:23: warning: (near initialization for
'actual_connection_key_once.__on_word')
misc.c:67: warning: missing braces around initializer
misc.c:67: warning: (near initialization for 'sqlca_key_once.__on_word')

is that enough reduction in noise to make it useful ?

Stefan

Peter Eisentraut wrote:

Am Donnerstag, 12. Juli 2007 15:25 schrieb Stefan Kaltenbrunner:

a lot of those are simply noise (like the LOOP VECTORIZED stuff from the
icc boxes or the "statement not reached" spam from the sun compilers)
but others might indicate real issues.
To find warnings that might be a real problem we might want to look into
suppressing those - if possible - using compiler switches.

It would be good to determine an appropriate set of compiler switches to
reduce the warnings to a reasonable level.

yeah once we have determined that this whole experiment is useful it
should be pretty easy to tweak the compiler switches for the non-gcc
compilers (mostly icc and sun studio seem to be the ones that generate
excessive output).

Stefan

Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:

ok I did that for a few members (removing all the statement not reached
ones as well as some purely informal notices and all the flex related
warnings) and came up with something similiar to:
[snip]

Yeah, this looks like a good list. I can't readily check the ones from
"eel" as they appear to be in Windows-specific code; anyone else want to
fix those?

animal: jackal warnings: 2
postmaster.c: In function 'PostmasterMain':
postmaster.c:796: warning: 'DNSServiceRegistrationCreate' is deprecated
(declared at /usr/include/DNSServiceDiscovery/DNSServiceDiscovery.h:114)

This one we knew about; there's been previous discussion of rewriting
the Bonjour support to use a more portable API, but I don't think anyone
feels like doing it right now.

I'll take a look at the rest.

regards, tom lane

Tom Lane wrote:

Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:

ok I did that for a few members (removing all the statement not reached
ones as well as some purely informal notices and all the flex related
warnings) and came up with something similiar to:
[snip]

Yeah, this looks like a good list. I can't readily check the ones from
"eel" as they appear to be in Windows-specific code; anyone else want to
fix those?

The pg_ctl one is a windows one, I'll deal with that one.

The dirmod one doesn't appear on win32, only cygwin. I don't have a
cygwin to check that against, so I'll have to pass on that one.

//Magnus

Magnus Hagander <magnus@hagander.net> writes:

Tom Lane wrote:

Yeah, this looks like a good list. I can't readily check the ones from
"eel" as they appear to be in Windows-specific code; anyone else want to
fix those?

The pg_ctl one is a windows one, I'll deal with that one.

The dirmod one doesn't appear on win32, only cygwin. I don't have a
cygwin to check that against, so I'll have to pass on that one.

Eyeing the code, it looks like the issue is that port.h declares
pgsymlink if
#if defined(WIN32) && !defined(__CYGWIN__)
while dirmod.c defines it if
#ifdef WIN32

So this seems like an actual bug, at least to the extent that pgsymlink
is being compiled into code but not used on Cygwin. But more to the
point, maybe port.h is wrong and we should be using pgsymlink on Cygwin?
In any case these two files need to be put into sync.

regards, tom lane

Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

Tom Lane wrote:

Yeah, this looks like a good list. I can't readily check the ones from
"eel" as they appear to be in Windows-specific code; anyone else want to
fix those?

The pg_ctl one is a windows one, I'll deal with that one.

The dirmod one doesn't appear on win32, only cygwin. I don't have a
cygwin to check that against, so I'll have to pass on that one.

Eyeing the code, it looks like the issue is that port.h declares
pgsymlink if
#if defined(WIN32) && !defined(__CYGWIN__)
while dirmod.c defines it if
#ifdef WIN32

So this seems like an actual bug, at least to the extent that pgsymlink
is being compiled into code but not used on Cygwin. But more to the
point, maybe port.h is wrong and we should be using pgsymlink on Cygwin?
In any case these two files need to be put into sync.

Agreed, but I don't know which should be the master :(

Well, actually. Since it's not #defined in port.h, that should mean it's
not being used anyway? Since the symbol in dirmod.c is pgsymlink, which
shouldn't be referenced directly anywhere. If that's so, then changing
dirmod.c to just exclude the whole thing on CYGWIN should fix the issue.

I don't have a way to test it, but perhaps we should just throw it in
and see if eel breaks on the buildfarm? Unless some poor soul actually
has a cygwin dev box to test on?

//Magnus

Tom Lane wrote:

Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:

ok I did that for a few members (removing all the statement not reached
ones as well as some purely informal notices and all the flex related
warnings) and came up with something similiar to:
[snip]

Yeah, this looks like a good list. I can't readily check the ones from
"eel" as they appear to be in Windows-specific code; anyone else want to
fix those?

animal: jackal warnings: 2
postmaster.c: In function 'PostmasterMain':
postmaster.c:796: warning: 'DNSServiceRegistrationCreate' is deprecated
(declared at /usr/include/DNSServiceDiscovery/DNSServiceDiscovery.h:114)

This one we knew about; there's been previous discussion of rewriting
the Bonjour support to use a more portable API, but I don't think anyone
feels like doing it right now.

I'll take a look at the rest.

some more(I have removed duplicates and ones that should be fixed by
your latest commits though):

animal: salamander warnings: 27
cash.c: In function `cash_in':
cash.c:244: warning: subscript has type `char'
pg_lzcompress.c: In function `pglz_compress':
pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'
pg_lzcompress.c:578: warning: called from here

animal: canary warnings: 14
plpython.c: In function `PLyMapping_ToTuple':
plpython.c:1717: warning: variable `i' might be clobbered by `longjmp'
or `vfork'
plpython.c:1732: warning: variable `value' might be clobbered by
`longjmp' or `vfork'
plpython.c:1733: warning: variable `so' might be clobbered by `longjmp'
or `vfork'
plpython.c: In function `PLySequence_ToTuple':
plpython.c:1797: warning: variable `i' might be clobbered by `longjmp'
or `vfork'
plpython.c:1821: warning: variable `value' might be clobbered by
`longjmp' or `vfork'
plpython.c:1822: warning: variable `so' might be clobbered by `longjmp'
or `vfork'
plpython.c: In function `PLyObject_ToTuple':
plpython.c:1879: warning: variable `i' might be clobbered by `longjmp'
or `vfork'
plpython.c:1892: warning: variable `value' might be clobbered by
`longjmp' or `vfork'
plpython.c:1893: warning: variable `so' might be clobbered by `longjmp'
or `vfork'
plpython.c: In function `PLy_spi_execute_plan':
plpython.c:2434: warning: variable `i' might be clobbered by `longjmp'
or `vfork'

animal: dragonfly warnings: 67
auth.c:61: warning: initialization from incompatible pointer type
cash.c: In function `cash_in':
cash.c:244: warning: subscript has type `char'
connect.c:23: warning: missing braces around initializer
connect.c:23: warning: (near initialization for
`actual_connection_key_once.__pthread_once_pad')
misc.c:67: warning: missing braces around initializer
misc.c:67: warning: (near initialization for
`sqlca_key_once.__pthread_once_pad')

animal: emperor_moth warnings: 10
auth.c:61: warning: initialization from incompatible pointer type

animal: osprey warnings: 22
s_lock.c:222: warning: `tas_dummy' defined but not used
pg_lzcompress.c: In function `pglz_compress':
pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'
pg_lzcompress.c:578: warning: called from here
fmgr.c: In function `fmgr_oldstyle':
fmgr.c:629: warning: assignment makes pointer from integer without a cast
fmgr.c:638: warning: assignment makes pointer from integer without a cast
fmgr.c:641: warning: assignment makes pointer from integer without a cast
fmgr.c:645: warning: assignment makes pointer from integer without a cast
fmgr.c:649: warning: assignment makes pointer from integer without a cast
fmgr.c:654: warning: assignment makes pointer from integer without a cast
fmgr.c:659: warning: assignment makes pointer from integer without a cast
fmgr.c:665: warning: assignment makes pointer from integer without a cast
fmgr.c:671: warning: assignment makes pointer from integer without a cast
fmgr.c:678: warning: assignment makes pointer from integer without a cast
fmgr.c:685: warning: assignment makes pointer from integer without a cast
fmgr.c:693: warning: assignment makes pointer from integer without a cast
fmgr.c:701: warning: assignment makes pointer from integer without a cast
fmgr.c:710: warning: assignment makes pointer from integer without a cast
fmgr.c:719: warning: assignment makes pointer from integer without a cast
fmgr.c:729: warning: assignment makes pointer from integer without a cast
fmgr.c:739: warning: assignment makes pointer from integer without a cast

animal: lionfish warnings: 16
/tmp/cclwN8N9.s: Assembler messages:
/tmp/cclwN8N9.s:109082: Warning: Macro instruction expanded into
multiple instructions
/tmp/cclwN8N9.s:109246: Warning: Macro instruction expanded into
multiple instructions
pg_lzcompress.c: In function `pglz_compress':
pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'
pg_lzcompress.c:578: warning: called from here
/tmp/ccnsL6Et.s: Assembler messages:
/tmp/ccnsL6Et.s:160502: Warning: Macro instruction expanded into
multiple instructions
/tmp/ccnsL6Et.s:160661: Warning: Macro instruction expanded into
multiple instructions
/tmp/ccnsL6Et.s:160702: Warning: Macro instruction expanded into
multiple instructions
/tmp/ccnsL6Et.s:160805: Warning: Macro instruction expanded into
multiple instructions
/tmp/ccnsL6Et.s:190739: Warning: Macro instruction expanded into
multiple instructions
/tmp/ccnsL6Et.s:192442: Warning: Macro instruction expanded into
multiple instructions
scan.l:180: warning, the character range [<80>-<FF>] is ambiguous in a
case-insensitive scanner
scan.l:180: warning, the character range [<80>-<FF>] is ambiguous in a
case-insensitive scanner
scan.l:302: warning, the character range [<80>-<FF>] is ambiguous in a
case-insensitive scanner

Stefan showed me via Jabber this warning:

/tmp/ccM7MfqX.s: Assembler messages:
/tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to 00000000fffffffc
/tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to 00000000fffffffc

He says that this comes from trgm_op.c file. I don't get the warning
myself obviously, so I started guessing.

3FFFFFFFC = 1111111111111111111111111111111100
FFFFFFFC = 11111111111111111111111111111100

So the upper 2 bits are being lost (the second number is 32 bits wide).

The only thing that I think is related is the usage of VARSIZE(). It
looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
bits left. I can see no such operation though.

--
Alvaro Herrera http://www.flickr.com/photos/alvherre/
Dios hizo a Ad�n, pero fue Eva quien lo hizo hombre.

Alvaro Herrera wrote:

Stefan showed me via Jabber this warning:

/tmp/ccM7MfqX.s: Assembler messages:
/tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to 00000000fffffffc
/tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to 00000000fffffffc

He says that this comes from trgm_op.c file. I don't get the warning
myself obviously, so I started guessing.

3FFFFFFFC = 1111111111111111111111111111111100
FFFFFFFC = 11111111111111111111111111111100

So the upper 2 bits are being lost (the second number is 32 bits wide).

The only thing that I think is related is the usage of VARSIZE(). It
looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
bits left. I can see no such operation though.

this one is from
http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=caracara&amp;dt=2007-07-12%20200001&amp;stg=make-contrib

as I'm going through the warnings in contrib now.

Stefan

Alvaro Herrera wrote:

Stefan showed me via Jabber this warning:

/tmp/ccM7MfqX.s: Assembler messages:
/tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to 00000000fffffffc
/tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to 00000000fffffffc

He says that this comes from trgm_op.c file. I don't get the warning
myself obviously, so I started guessing.

3FFFFFFFC = 1111111111111111111111111111111100
FFFFFFFC = 11111111111111111111111111111100

So the upper 2 bits are being lost (the second number is 32 bits wide).

The only thing that I think is related is the usage of VARSIZE(). It
looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
bits left. I can see no such operation though.

The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that
warning is coming from, though. What's the real source behind "ccM7MfqX.s"?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas wrote:

Alvaro Herrera wrote:

Stefan showed me via Jabber this warning:
/tmp/ccM7MfqX.s: Assembler messages:
/tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to
00000000fffffffc
/tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to
00000000fffffffc
He says that this comes from trgm_op.c file. I don't get the warning
myself obviously, so I started guessing.
3FFFFFFFC = 1111111111111111111111111111111100
FFFFFFFC = 11111111111111111111111111111100
So the upper 2 bits are being lost (the second number is 32 bits wide).
The only thing that I think is related is the usage of VARSIZE(). It
looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
bits left. I can see no such operation though.

The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that
warning is coming from, though. What's the real source behind "ccM7MfqX.s"?

trgm_op.c

I'm not sure that the shift in SET_VARSIZE_4B is applicable here,
because it would have to be passed a len of FFFFFFFF.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

"Heikki Linnakangas" <heikki@enterprisedb.com> writes:

Alvaro Herrera wrote:

Stefan showed me via Jabber this warning:

/tmp/ccM7MfqX.s: Assembler messages:
/tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to 00000000fffffffc
/tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to 00000000fffffffc

He says that this comes from trgm_op.c file. I don't get the warning
myself obviously, so I started guessing.

I've occasionally seen this warning too. It seems to be pretty random though.
Nor have I been able to find any actual problems caused by it.

But istm this has to be a compiler bug since any overflow in the C code should
be handled (and potentially warned) by the compiler long before the assembler
gets to see it.

So the upper 2 bits are being lost (the second number is 32 bits wide).

The only thing that I think is related is the usage of VARSIZE(). It
looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
bits left. I can see no such operation though.

The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that warning is
coming from, though. What's the real source behind "ccM7MfqX.s"?

Huh. I wonder.

Could the compiler be incorrectly optimizing cases of

SET_VARSIZE(new_datum VARSIZE(olddatum))

which amounts to:

((olddatum >> 2) & 0x3FFFFFFF) << 2

If it does constant propagation without handling overflow it could end up
with:

(olddatum >> 2 << 2) & 0x3FFFFFFFC

note that in fact truncating the high two bits as the assembler did would in
fact be the correct thing to do here which would explain why it doesn't cause
any actual problems.

Also note that it doesn't require an actual single statement of the form
above. The compiler could be doing constant propagation from an earlier
statement. Code which calls VARSIZE(olddatum) and then passes the result to
SET_VARSIZE(newdatum,len) is quite common.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Alvaro Herrera wrote:

Heikki Linnakangas wrote:

Alvaro Herrera wrote:

Stefan showed me via Jabber this warning:
/tmp/ccM7MfqX.s: Assembler messages:
/tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to
00000000fffffffc
/tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to
00000000fffffffc
He says that this comes from trgm_op.c file. I don't get the warning
myself obviously, so I started guessing.
3FFFFFFFC = 1111111111111111111111111111111100
FFFFFFFC = 11111111111111111111111111111100
So the upper 2 bits are being lost (the second number is 32 bits wide).
The only thing that I think is related is the usage of VARSIZE(). It
looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
bits left. I can see no such operation though.

The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that
warning is coming from, though. What's the real source behind "ccM7MfqX.s"?

trgm_op.c

I'm not sure that the shift in SET_VARSIZE_4B is applicable here,
because it would have to be passed a len of FFFFFFFF.

Hmm. It looks like I get that warning on my laptop as well. I tracked it
down to these two places:

Line 209:

while (ptr - GETARR(trg) < ARRNELEM(trg))
{
text *item = (text *) palloc(VARHDRSZ + 3);

SET_VARSIZE(item, VARHDRSZ + 3);
CPTRGM(VARDATA(item), ptr);

d[ptr - GETARR(trg)] = PointerGetDatum(item);

ptr++;
}

Line 224:

ptr = GETARR(trg);
while (ptr - GETARR(trg) < ARRNELEM(trg))
{

pfree(DatumGetPointer(d[ptr - GETARR(trg)]));

ptr++;
}

The warning seems to be in related array indexing. If you replace ptr -
GETARR(trg) with a constant, the warning goes away. But having "i = ptr
- GETARR(trg)" in there doesn't give a warning.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas <heikki@enterprisedb.com> writes:

Hmm. It looks like I get that warning on my laptop as well. I tracked it
down to these two places:

Line 209:

while (ptr - GETARR(trg) < ARRNELEM(trg))
{
text *item = (text *) palloc(VARHDRSZ + 3);

SET_VARSIZE(item, VARHDRSZ + 3);
CPTRGM(VARDATA(item), ptr);

d[ptr - GETARR(trg)] = PointerGetDatum(item);

ptr++;
}

I'll betcha the compiler is trying to optimize the repeated calculations
of "ptr - GETARR(trg)" into a separate variable that it increments along
with ptr. Maybe it is getting it wrong, or maybe the assembler is just
confused. Does the warning go away if you dial down the -O level?

regards, tom lane

"Heikki Linnakangas" <heikki@enterprisedb.com> writes:

The warning seems to be in related array indexing. If you replace ptr -
GETARR(trg) with a constant, the warning goes away. But having "i = ptr -
GETARR(trg)" in there doesn't give a warning.

Can you compile with -save-temps and send the corresponding assembly file?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Gregory Stark <stark@enterprisedb.com> writes:

If it does constant propagation without handling overflow it could end up
with:

(olddatum >> 2 << 2) & 0x3FFFFFFFC

note that in fact truncating the high two bits as the assembler did would in
fact be the correct thing to do here which would explain why it doesn't cause
any actual problems.

Good point, but I also note that the places Heikki saw were inside
loops. I think it might be some combination of the above and a loop
strength reduction optimization.

regards, tom lane

Tom Lane wrote:

Heikki Linnakangas <heikki@enterprisedb.com> writes:

Hmm. It looks like I get that warning on my laptop as well. I tracked it
down to these two places:

Line 209:

while (ptr - GETARR(trg) < ARRNELEM(trg))
{
text *item = (text *) palloc(VARHDRSZ + 3);

SET_VARSIZE(item, VARHDRSZ + 3);
CPTRGM(VARDATA(item), ptr);

d[ptr - GETARR(trg)] = PointerGetDatum(item);

ptr++;
}

I'll betcha the compiler is trying to optimize the repeated calculations
of "ptr - GETARR(trg)" into a separate variable that it increments along
with ptr. Maybe it is getting it wrong, or maybe the assembler is just
confused. Does the warning go away if you dial down the -O level?

Yes, I don't get it with -O1 or -O0.

$ gcc --version
gcc (GCC) 4.1.2 20061028 (prerelease) (Debian 4.1.1-19)

Hmm. Prerelease? This version came from debian/testing.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com