Re: [INTERFACES] libecpg (8.0 and CVS) hits a gcc bug on powerpc

NB: The crossposting is actually suggested on the mailing list description:
"If you've found a bug in PostgreSQL, please send it both to this list
and also to the pgsql-patches list."

Amd64 and powerpc share an ABI design which enables variable passing in
registers and which makes va_list a complex datatype. Due to gcc not
implementing a part of the C9x standard
(http://mail-index.netbsd.org/tech-toolchain/2003/11/11/0011.html)
taking a pointer to a va_list will fail on these platforms. The
corresponding bug (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8262) is
closed, so the gcc developers seem not to plan to change this.

So I propose the following hack (feel free to improve it!) to go into
mainline. Since only powerpc and amd64 are affected (and the old method
does not work at all here at all) the patch should not be controversial.

Christof

PS: I don't know about other architectures. I can't believe that only
ppc and amd64 pass parameters in registers.
PPS: I could not test it on amd64, but all docs indicate the same behaviour.

Christof Petig schrieb:
> So I propose the following hack (feel free to improve it!) to go into
> mainline. Since only powerpc and amd64 are affected (and the old method
> does not work at all here at all) the patch should not be controversial.

I failed to mention that call by reference is the default for va_list on
these platforms (that's actually the cause of the bug).

Christof

Patch committed. Thanks.

Michael

--
Michael Meskes
Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: meskes(at)jabber(dot)org
Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Michael Meskes schrieb:
> Patch committed. Thanks.

I have to admit that I got the case of the preprocessor symbol on amd64
wrong. __AMD64__ is not defined, __amd64__ is.

Sorry, corrected patch attached
Christof
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCsZCmng+R+0ucfO0RAoABAKDKnokBW+tgXMKgeEMecpZsFXRFTQCeKeBu
YsdWP056cqxKtNAx5Gh/Wis=
=b2Og
-----END PGP SIGNATURE-----

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

Christof Petig wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Michael Meskes schrieb:
> > Patch committed. Thanks.
>
> I have to admit that I got the case of the preprocessor symbol on amd64
> wrong. __AMD64__ is not defined, __amd64__ is.
>
> Sorry, corrected patch attached
> Christof
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.1 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
>
> iD8DBQFCsZCmng+R+0ucfO0RAoABAKDKnokBW+tgXMKgeEMecpZsFXRFTQCeKeBu
> YsdWP056cqxKtNAx5Gh/Wis=
> =b2Og
> -----END PGP SIGNATURE-----

>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org

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

Patch applied. Thanks.

---------------------------------------------------------------------------

Christof Petig wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Michael Meskes schrieb:
> > Patch committed. Thanks.
>
> I have to admit that I got the case of the preprocessor symbol on amd64
> wrong. __AMD64__ is not defined, __amd64__ is.
>
> Sorry, corrected patch attached
> Christof
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.1 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
>
> iD8DBQFCsZCmng+R+0ucfO0RAoABAKDKnokBW+tgXMKgeEMecpZsFXRFTQCeKeBu
> YsdWP056cqxKtNAx5Gh/Wis=
> =b2Og
> -----END PGP SIGNATURE-----

[ text/x-patch is unsupported, treating like TEXT/PLAIN ]

> Index: src/interfaces/ecpg/ecpglib/execute.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/ecpglib/execute.c,v
> retrieving revision 1.40
> diff -u -u -r1.40 execute.c
> --- src/interfaces/ecpg/ecpglib/execute.c 2 Jun 2005 12:35:11 -0000 1.40
> +++ src/interfaces/ecpg/ecpglib/execute.c 16 Jun 2005 14:35:09 -0000
> @@ -70,7 +70,7 @@
> return res;
> }
>
> -#if defined(__GNUC__) && (defined (__powerpc__) || defined(__AMD64__) || defined(__x86_64__))
> +#if defined(__GNUC__) && (defined (__powerpc__) || defined(__amd64__) || defined(__x86_64__))
> #define APREF ap
> #else
> #define APREF *ap
> @@ -178,7 +178,7 @@
> if (!(var = (struct variable *) ECPGalloc(sizeof(struct variable), lineno)))
> return false;
>
> -#if defined(__GNUC__) && (defined (__powerpc__) || defined(__AMD64__) || defined(__x86_64__))
> +#if defined(__GNUC__) && (defined (__powerpc__) || defined(__amd64__) || defined(__x86_64__))
> ECPGget_variable(ap, type, var, true);
> #else
> ECPGget_variable(&ap, type, var, true);
> Index: src/interfaces/ecpg/ecpglib/extern.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/ecpglib/extern.h,v
> retrieving revision 1.13
> diff -u -u -r1.13 extern.h
> --- src/interfaces/ecpg/ecpglib/extern.h 2 Jun 2005 12:35:11 -0000 1.13
> +++ src/interfaces/ecpg/ecpglib/extern.h 16 Jun 2005 14:35:09 -0000
> @@ -125,7 +125,7 @@
> bool ECPGstore_result(const PGresult *results, int act_field,
> const struct statement * stmt, struct variable * var);
> bool ECPGstore_input(const int, const bool, const struct variable *, const char **, bool *);
> -#if defined(__GNUC__) && (defined (__powerpc__) || defined(__AMD64__) || defined(__x86_64__))
> +#if defined(__GNUC__) && (defined (__powerpc__) || defined(__amd64__) || defined(__x86_64__))
> // work around a gcc/ABI bug with va_lists on ppc+amd64
> void ECPGget_variable(va_list, enum ECPGttype, struct variable *, bool);
> #else

>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org

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