Thread: pgsql-server: Fix TAS assembly stuff for Solaris/386.
Log Message:
-----------
Fix TAS assembly stuff for Solaris/386. (I'm not in a position to
actually test this, but it couldn't be broken any worse than it was...)
Modified Files:
--------------
pgsql-server/src/include/storage:
s_lock.h (r1.129 -> r1.130)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/include/storage/s_lock.h.diff?r1=1.129&r2=1.130)
pgsql-server/src/template:
solaris (r1.19 -> r1.20)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/template/solaris.diff?r1=1.19&r2=1.20)
On Fri, 24 Sep 2004, Tom Lane wrote: > Log Message: > ----------- > Fix TAS assembly stuff for Solaris/386. (I'm not in a position to > actually test this, but it couldn't be broken any worse than it was...) > It passes regression tests, but adds a warning: "../../../../src/include/storage/s_lock.h", line 661: warning: /* encountered inside a comment There are a number of other warnings in the compile as well. Is our goal a warning free compile on just gcc or all compilers? Kris Jurka
Kris Jurka <books@ejurka.com> writes:
> It passes regression tests,
Regression tests on what exactly --- which platform, which compiler?
(The gcc and non-gcc paths are different on Solaris, so if you can
test both it'd be worth doing. Also someone should verify that I
didn't break Solaris/Sparc, same two cases again.)
> but adds a warning:
> "../../../../src/include/storage/s_lock.h", line 661: warning: /*
> encountered inside a comment
Oops, my mistake.
> There are a number of other warnings in the compile as well. Is our goal
> a warning free compile on just gcc or all compilers?
Ideally I'd like it warning-free on everything, but I'm not sure how
practical that is. The main thing that non-gcc compilers tend to warn
about in my experience is "char *" vs "unsigned char *", of which there
are a lot of occurrences in and around the multibyte code. This does
not really seem worth cleaning up at the moment. If you see anything
that looks interesting, or readily fixable, send it in.
regards, tom lane
On Fri, 2004-09-24 at 11:53, Tom Lane wrote: > Ideally I'd like it warning-free on everything, but I'm not sure how > practical that is. The main thing that non-gcc compilers tend to warn > about in my experience is "char *" vs "unsigned char *", of which there > are a lot of occurrences in and around the multibyte code. This does > not really seem worth cleaning up at the moment. Per the recent GCC 4.0 report on -hackers, it seems GCC4 will now warn about char * vs. unsigned char *, so perhaps it is time to clean that up... -Neil
On Fri, Sep 24, 2004 at 12:15:23PM +1000, Neil Conway wrote:
> On Fri, 2004-09-24 at 11:53, Tom Lane wrote:
> > Ideally I'd like it warning-free on everything, but I'm not sure how
> > practical that is. The main thing that non-gcc compilers tend to warn
> > about in my experience is "char *" vs "unsigned char *", of which there
> > are a lot of occurrences in and around the multibyte code. This does
> > not really seem worth cleaning up at the moment.
>
> Per the recent GCC 4.0 report on -hackers, it seems GCC4 will now warn
> about char * vs. unsigned char *, so perhaps it is time to clean that
> up...
I've read on some changelog that starting from 3.4 it also gives more
(and more correct) warnings about type punning and strict aliasing
violations. Could we also look at fixing those?
IMHO that's 8.1 material anyway ...
--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"Porque Kim no hacia nada, pero, eso sí,
con extraordinario éxito" ("Kim", Kipling)
On Thu, 23 Sep 2004, Tom Lane wrote:
> Kris Jurka <books@ejurka.com> writes:
> > It passes regression tests,
>
> Regression tests on what exactly --- which platform, which compiler?
> (The gcc and non-gcc paths are different on Solaris, so if you can
> test both it'd be worth doing. Also someone should verify that I
> didn't break Solaris/Sparc, same two cases again.)
Sorry:
$ uname -a
SunOS albert 5.9 Generic_112234-03 i86pc i386 i86pc
$ cc -V
cc: Sun WorkShop 6 update 2 C 5.3 Patch 111680-09 2003/05/18
$ gcc -v
Reading specs from /usr/local/lib/gcc-lib/i386-pc-solaris2.9/3.2.3/specs
Configured with: ../configure --disable-nls --with-as=/usr/ccs/bin/as
--with-ld=/usr/ccs/bin/ld
Thread model: posix
gcc version 3.2.3
I don't have access to sparc hardware at the moment.
> Ideally I'd like it warning-free on everything, but I'm not sure how
> practical that is. The main thing that non-gcc compilers tend to warn
> about in my experience is "char *" vs "unsigned char *", of which there
> are a lot of occurrences in and around the multibyte code. This does
> not really seem worth cleaning up at the moment. If you see anything
> that looks interesting, or readily fixable, send it in.
>
char * vs unsigned char * are a good number of them, but I also see:
**********************************************************
"path.c", line 35: warning: storage class after type is obsolescent
**********************************************************
UINT64CONST produces these in a number of places:
"xlog.c", line 552: warning: constant promoted to unsigned long long
**********************************************************
----------------------------------------------------------
"dynloader.c", line 4: warning: empty translation unit
**********************************************************
"pg_shmem.c", line 415: warning: argument #1 is incompatible with
prototype:
prototype: pointer to char : "/usr/include/sys/shm.h", line 241
argument : pointer to struct PGShmemHeader {signed int magic, long
creatorPID, unsigned int totalsize, unsigned int freeoffset}
**********************************************************
I see this in both cc and gcc builds:
cc -Xa -O -v -g -I../../../src/interfaces/libpq -I../../../src/include
-I/usr/local/include -DFRONTEND -c -o psqlscan.o psqlscan.c
"../../../src/include/pg_config.h", line 656: warning: macro redefined:
_FILE_OFFSET_BITS
gcc -O2 -fno-strict-aliasing -g -Wall -Wmissing-prototypes
-Wmissing-declarations -I../../../src/interfaces/libpq -I../../../src/include
-I/usr/local/include -DFRONTEND -c -o psqlscan.o psqlscan.c -MMD
In file included from ../../../src/include/c.h:53,
from ../../../src/include/postgres_fe.h:21,
from psqlscan.l:38:
../../../src/include/pg_config.h:656:1: warning: "_FILE_OFFSET_BITS"
redefined
In file included from /usr/include/iso/stdio_iso.h:35,
from
/usr/local/lib/gcc-lib/i386-pc-solaris2.9/3.2.3/include/st
dio.h:36,
from psqlscan.c:12:
/usr/include/sys/feature_tests.h:96:1: warning: this is the location of
the previous definition
**********************************************************
"float.c", line 168: warning: division by 0
**********************************************************
Applications that have a "int main" prototype don't return values
"main.c", line 322: warning: Function has no return statement : main
**********************************************************
Then there are a whole lot of code reachability warnings in these classes
- statement not reached
- end-of-loop code not reached
- loop not entered at top
I've attached these as a separate file because they are numerous.
Kris Jurka
Attachment
Kris Jurka <books@ejurka.com> writes:
> char * vs unsigned char * are a good number of them, but I also see:
> "path.c", line 35: warning: storage class after type is obsolescent
Okay, that's easily fixed.
> UINT64CONST produces these in a number of places:
> "xlog.c", line 552: warning: constant promoted to unsigned long long
This is pretty annoying, considering that the entire point of the
UINT64CONST macro is to suppress such complaints. Can you suggest an
incantation that will shut this compiler up?
> "dynloader.c", line 4: warning: empty translation unit
Yup, so it is.
> "pg_shmem.c", line 415: warning: argument #1 is incompatible with
> prototype:
Fixed.
> I see this in both cc and gcc builds:
> cc -Xa -O -v -g -I../../../src/interfaces/libpq -I../../../src/include
> -I/usr/local/include -DFRONTEND -c -o psqlscan.o psqlscan.c
> "../../../src/include/pg_config.h", line 656: warning: macro redefined:
> _FILE_OFFSET_BITS
Hmmm ... that happens only in psqlscan.c? My first guess about the
reason would apply to all our flex-generated files ...
> "float.c", line 168: warning: division by 0
Probably can't avoid this, unless you have another way to generate
NaN on that compiler.
> Applications that have a "int main" prototype don't return values
> "main.c", line 322: warning: Function has no return statement : main
Compilers that don't want to special-case "exit()" have no business
making this complaint :-(. We could possibly add return statements
but I'm worried about introducing "unreachable statement" warnings
from compilers with a slightly larger clue quotient.
> Then there are a whole lot of code reachability warnings in these classes
> - statement not reached
> - end-of-loop code not reached
> - loop not entered at top
I think most of these come from flex and/or bison code that we don't
have a lot of control over.
regards, tom lane
On Fri, 24 Sep 2004, Tom Lane wrote:
> > UINT64CONST produces these in a number of places:
> > "xlog.c", line 552: warning: constant promoted to unsigned long long
>
> This is pretty annoying, considering that the entire point of the
> UINT64CONST macro is to suppress such complaints. Can you suggest an
> incantation that will shut this compiler up?
it likes either ##ULL or unadorned. The problem is we're taking a
constant larger than long long and explicitly saying it's a long long.
> > Then there are a whole lot of code reachability warnings in these classes
> > - statement not reached
> > - end-of-loop code not reached
> > - loop not entered at top
>
> I think most of these come from flex and/or bison code that we don't
> have a lot of control over.
>
Another significant amount is from switch statements written like this:
switch(i) {
case 1:
return 1;
break;
}
Kris Jurka
Kris Jurka <books@ejurka.com> writes:
> UINT64CONST produces these in a number of places:
> "xlog.c", line 552: warning: constant promoted to unsigned long long
> it likes either ##ULL or unadorned. The problem is we're taking a
> constant larger than long long and explicitly saying it's a long long.
No other machine we use thinks it's larger than long long --- are you
sure about that? If that is the problem, why does the message use the
word "promoted" and not, say, "truncated"?
>> I think most of these come from flex and/or bison code that we don't
>> have a lot of control over.
> Another significant amount is from switch statements written like this:
> switch(i) {
> case 1:
> return 1;
> break;
> }
Yeah, there are some of those. Do you think it's worth cleaning up,
given that we can't do anything about the ones induced by flex/bison?
regards, tom lane
On Fri, 24 Sep 2004, Tom Lane wrote:
> Kris Jurka <books@ejurka.com> writes:
> > UINT64CONST produces these in a number of places:
> > "xlog.c", line 552: warning: constant promoted to unsigned long long
>
> > it likes either ##ULL or unadorned. The problem is we're taking a
> > constant larger than long long and explicitly saying it's a long long.
>
> No other machine we use thinks it's larger than long long --- are you
> sure about that? If that is the problem, why does the message use the
> word "promoted" and not, say, "truncated"?
Well, it's not really truncated it just overflows long long.
Looking at the following code, the warning is only produced for the c2
constant.
#define ULL(x) (x##ULL)
#define LL(x) (x##LL)
void f() {
unsigned long long c1 = ULL(0xFFFFFFFFFFFFFFFF);
unsigned long long c2 = LL(0xFFFFFFFFFFFFFFFF);
unsigned long long c3 = 0xFFFFFFFFFFFFFFFF;
unsigned long long c4 = ULL(0x1111111111111111);
unsigned long long c5 = LL(0x1111111111111111);
unsigned long long c6 = 0x1111111111111111;
}
Kris Jurka <books@ejurka.com> writes:
> Well, it's not really truncated it just overflows long long.
Oh, okay, I finally get the point --- it's the difference between signed
and unsigned that's at issue.
I suppose that the UINT64CONST macro really ought to use x##ULL not
x##LL. Does anyone think that we need a separate configure test for
this, or can we assume any compiler that eats LL will eat ULL?
regards, tom lane