Prev: [HACKERS] LLVM / clang
Next: Git: Unable to get pack file
From: Tom Lane on 11 Jun 2010 00:24
Peter Eisentraut <peter_e(a)gmx.net> writes:
> [ assorted LLVM warnings ]

> dt_common.c:818:75: warning: more data arguments than '%' conversions
> [-Wformat-extra-args]
> sprintf(str + strlen(str), (min != 0) ?
> "%+03d:%02d" : "%+03d", hour, min);
> ~~~~~~~ ^

> [and a few more like that]

> These are instances where a format string is an expression that results
> in a variable number of format arguments. Not sure if that is actually
> legal in C.

I believe it's legal, but I'd be in favor of making a project policy
against it, simply because you aren't going to get any static checking
from gcc about whether the arguments match the format string. There
isn't any good excuse not to code the above like

if (min != 0)
sprintf(str + strlen(str), "%+03d:%02d", hour, min);
else
sprintf(str + strlen(str), "%+03d", hour);

which would produce warnings if you managed to mess up the format match.

> print.c:778:22: warning: field width should have type 'int', but
> argument has type 'unsigned int' [-Wformat]
> fprintf(fout, "%-*s%s\n", (width_total -
> width) / 2, "",

> Not sure about that.

That one, on the other hand, is pretty silly ...

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Takahiro Itagaki on 11 Jun 2010 01:42

Peter Eisentraut <peter_e(a)gmx.net> wrote:

> Some new warnings, however:
>
> xlog.c:7759:22: warning: self-comparison always results in a constant
> value
> max_locks_per_xact != max_locks_per_xact)
> ^
>
> Looks like a bug.

Ah, it should be compared with the same name field in ControlFile.

Index: src/backend/access/transam/xlog.c
===================================================================
--- src/backend/access/transam/xlog.c (HEAD)
+++ src/backend/access/transam/xlog.c (fixed)
@@ -7756,7 +7756,7 @@
if (wal_level != ControlFile->wal_level ||
MaxConnections != ControlFile->MaxConnections ||
max_prepared_xacts != ControlFile->max_prepared_xacts ||
- max_locks_per_xact != max_locks_per_xact)
+ max_locks_per_xact != ControlFile->max_locks_per_xact)
{
/*
* The change in number of backend slots doesn't need to be



Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Peter Eisentraut on 11 Jun 2010 02:03
On tor, 2010-06-10 at 09:52 -0400, Tom Lane wrote:
> Peter Eisentraut <peter_e(a)gmx.net> writes:
> > Quick testing shows that clang doesn't get through the configure stage
> > on this Debian system -- it looks like some amount of better integration
> > with glibc might be needed. Building with llvm-gcc works fine, but I
> > understand that using llvm-gcc with native code generation isn't all
> > that different from using gcc itself, so that's not a surprising result.
> > The only issue is that the float8 regression test fails, so it is
> > apparently not *exactly* the same.
>
> There's a buildfarm animal using llvm-gcc, and it passes just fine ...
> so the float8 failure sounds to me like another integration problem.

The diff in this case is

*** src/test/regress/expected/float8.out
--- src/test/regress/results/float8.out
***************
*** 384,390 ****
SELECT '' AS bad, f.f1 * '1e200' from FLOAT8_TBL f;
ERROR: value out of range: overflow
SELECT '' AS bad, f.f1 ^ '1e200' from FLOAT8_TBL f;
! ERROR: value out of range: overflow
SELECT 0 ^ 0 + 0 ^ 1 + 0 ^ 0.0 + 0 ^ 0.5;
?column?
----------
--- 384,398 ----
SELECT '' AS bad, f.f1 * '1e200' from FLOAT8_TBL f;
ERROR: value out of range: overflow
SELECT '' AS bad, f.f1 ^ '1e200' from FLOAT8_TBL f;
! bad | ?column?
! -----+----------
! | 0
! | NaN
! | NaN
! | NaN
! | NaN
! (5 rows)
!
SELECT 0 ^ 0 + 0 ^ 1 + 0 ^ 0.0 + 0 ^ 0.5;
?column?
----------

which means that this combo signals an overflow in pow() by returning
NaN and not setting errno.

Curiously enough, the problem goes away when you insert elog()
statements after the pow() call. Could be a code generation/pipelining
issue.

Btw., this is

llvm-gcc (GCC) 4.2.1 (Based on Apple Inc. build 5649) (LLVM build)

which sounds somewhat old.



--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Tom Lane on 11 Jun 2010 11:23
Takahiro Itagaki <itagaki.takahiro(a)oss.ntt.co.jp> writes:
> Peter Eisentraut <peter_e(a)gmx.net> wrote:
>> max_locks_per_xact != max_locks_per_xact)
>>
>> Looks like a bug.

> Ah, it should be compared with the same name field in ControlFile.

Yeah, obvious typo, please commit.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Peter Eisentraut on 25 Jun 2010 15:49
On fre, 2010-06-11 at 07:00 +0300, Peter Eisentraut wrote:
> The second problem is that the prototype check for accept() fails.
> This
> is because glibc defines the second argument to be a "transparent
> union", apparently to make it look like a lot of things at once.
> clang
> apparently doesn't understand that. One could address this by
> checking
> for the typedef that glibc uses explicitly in the configure check, but
> that would appear to defeat the point of the *transparent* union. A
> workaround is to remove -D_GNU_SOURCE from src/template/linux.
>
> Predictably, this will make PL/Perl fail to build.
>
> Also, it will make src/backend/libpq/auth.c fail to build, because
> struct ucred is only defined when _GNU_SOURCE is used. This would
> actually fail to work on GCC as well, so I think we should add an
> explicit configure check for struct ucred.

For the record, here is a patch that would address these issues.

At the moment, I'm waiting to get my hands on the new version 2.7 of
clang to see if some of these issues have gone away.

Considering that clang already helped us find one bug in the code, I
think it's worth trying to make this work.
First  |  Prev  |  Next  |  Last
Pages: 1 2 3
Prev: [HACKERS] LLVM / clang
Next: Git: Unable to get pack file