Debian 12 gcc warning
On Debian 12, gcc version 12.2.0 (Debian 12.2.0-14) generates a warning
on PG 13 to current, but only with -O1 optimization level, and not at
-O0/-O2/-O3:
clauses.c: In function ‘recheck_cast_function_args’:
clauses.c:4293:19: warning: ‘actual_arg_types’ may be used uninitialized [-Wmaybe-uninitialized]
4293 | rettype = enforce_generic_type_consistency(actual_arg_types,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4294 | declared_arg_types,
| ~~~~~~~~~~~~~~~~~~~
4295 | nargs,
| ~~~~~~
4296 | funcform->prorettype,
| ~~~~~~~~~~~~~~~~~~~~~
4297 | false);
| ~~~~~~
In file included from clauses.c:45:
../../../../src/include/parser/parse_coerce.h:82:17: note: by argument 1 of type ‘const Oid *’ {aka ‘const unsigned int *’} to ‘enforce_generic_type_consistency’ declared here
82 | extern Oid enforce_generic_type_consistency(const Oid *actual_arg_types,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
clauses.c:4279:33: note: ‘actual_arg_types’ declared here
4279 | Oid actual_arg_types[FUNC_MAX_ARGS];
| ^~~~~~~~~~~~~~~~
The code is:
static void
recheck_cast_function_args(List *args, Oid result_type,
Oid *proargtypes, int pronargs,
HeapTuple func_tuple)
{
Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
int nargs;
Oid actual_arg_types[FUNC_MAX_ARGS];
Oid declared_arg_types[FUNC_MAX_ARGS];
Oid rettype;
ListCell *lc;
if (list_length(args) > FUNC_MAX_ARGS)
elog(ERROR, "too many function arguments");
nargs = 0;
foreach(lc, args)
{
actual_arg_types[nargs++] = exprType((Node *) lfirst(lc));
}
Assert(nargs == pronargs);
memcpy(declared_arg_types, proargtypes, pronargs * sizeof(Oid));
--> rettype = enforce_generic_type_consistency(actual_arg_types,
declared_arg_types,
nargs,
funcform->prorettype,
false);
/* let's just check we got the same answer as the parser did ... */
I don't see a clean way of avoiding the warning except by initializing
the array, which seems wasteful.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On Mon, Aug 28, 2023 at 03:37:20PM -0400, Bruce Momjian wrote:
I don't see a clean way of avoiding the warning except by initializing
the array, which seems wasteful.
Or just initialize the array with a {0}?
--
Michael
On Tue, Aug 29, 2023 at 07:30:15AM +0900, Michael Paquier wrote:
On Mon, Aug 28, 2023 at 03:37:20PM -0400, Bruce Momjian wrote:
I don't see a clean way of avoiding the warning except by initializing
the array, which seems wasteful.Or just initialize the array with a {0}?
Uh, doesn't that set all elements to zero? See:
https://stackoverflow.com/questions/2589749/how-to-initialize-array-to-0-in-c
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On Tue, 29 Aug 2023 at 07:37, Bruce Momjian <bruce@momjian.us> wrote:
nargs = 0;
foreach(lc, args)
{
actual_arg_types[nargs++] = exprType((Node *) lfirst(lc));
}
Does it still produce the warning if you form the above more like?
nargs = list_length(args);
for (int i = 0; i < nargs; i++)
actual_arg_types[i] = exprType((Node *) list_nth(args, i));
I'm just not sure if it's unable to figure out if at least nargs
elements is set or if it won't be happy until all 100 elements are
set.
David
On Tue, Aug 29, 2023 at 11:55:48AM +1200, David Rowley wrote:
On Tue, 29 Aug 2023 at 07:37, Bruce Momjian <bruce@momjian.us> wrote:
nargs = 0;
foreach(lc, args)
{
actual_arg_types[nargs++] = exprType((Node *) lfirst(lc));
}Does it still produce the warning if you form the above more like?
nargs = list_length(args);
for (int i = 0; i < nargs; i++)
actual_arg_types[i] = exprType((Node *) list_nth(args, i));I'm just not sure if it's unable to figure out if at least nargs
elements is set or if it won't be happy until all 100 elements are
set.
I applied the attached patch but got the same warning:
clauses.c: In function ‘recheck_cast_function_args’:
clauses.c:4297:19: warning: ‘actual_arg_types’ may be used uninitialized [-Wmaybe-uninitialized]
4297 | rettype = enforce_generic_type_consistency(actual_arg_types,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4298 | declared_arg_types,
| ~~~~~~~~~~~~~~~~~~~
4299 | nargs,
| ~~~~~~
4300 | funcform->prorettype,
| ~~~~~~~~~~~~~~~~~~~~~
4301 | false);
| ~~~~~~
In file included from clauses.c:45:
../../../../src/include/parser/parse_coerce.h:82:17: note: by argument 1 of type ‘const Oid *’ {aka ‘const unsigned int *’} to ‘enforce_generic_type_consistency’ declared here
82 | extern Oid enforce_generic_type_consistency(const Oid *actual_arg_types,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
clauses.c:4279:33: note: ‘actual_arg_types’ declared here
4279 | Oid actual_arg_types[FUNC_MAX_ARGS];
| ^~~~~~~~~~~~~~~~
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
Attachments:
warning.difftext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index da258968b8..6cf020acef 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -4279,15 +4279,19 @@ recheck_cast_function_args(List *args, Oid result_type,
Oid actual_arg_types[FUNC_MAX_ARGS];
Oid declared_arg_types[FUNC_MAX_ARGS];
Oid rettype;
- ListCell *lc;
+// ListCell *lc;
if (list_length(args) > FUNC_MAX_ARGS)
elog(ERROR, "too many function arguments");
- nargs = 0;
- foreach(lc, args)
- {
- actual_arg_types[nargs++] = exprType((Node *) lfirst(lc));
- }
+// nargs = 0;
+// foreach(lc, args)
+// {
+// actual_arg_types[nargs++] = exprType((Node *) lfirst(lc));
+// }
+ nargs = list_length(args);
+ for (int i = 0; i < nargs; i++)
+ actual_arg_types[i] = exprType((Node *) list_nth(args, i));
+
Assert(nargs == pronargs);
memcpy(declared_arg_types, proargtypes, pronargs * sizeof(Oid));
rettype = enforce_generic_type_consistency(actual_arg_types,
On Mon, Aug 28, 2023 at 07:10:38PM -0400, Bruce Momjian wrote:
On Tue, Aug 29, 2023 at 07:30:15AM +0900, Michael Paquier wrote:
On Mon, Aug 28, 2023 at 03:37:20PM -0400, Bruce Momjian wrote:
I don't see a clean way of avoiding the warning except by initializing
the array, which seems wasteful.Or just initialize the array with a {0}?
Uh, doesn't that set all elements to zero? See:
https://stackoverflow.com/questions/2589749/how-to-initialize-array-to-0-in-c
FYI, that does stop the warning.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On Tue, Aug 29, 2023 at 6:56 AM David Rowley <dgrowleyml@gmail.com> wrote:
I'm just not sure if it's unable to figure out if at least nargs
elements is set or if it won't be happy until all 100 elements are
set.
It looks like the former, since I can silence it on gcc 13 / -O1 by doing:
/* keep compiler quiet */
actual_arg_types[0] = InvalidOid;
--
John Naylor
EDB: http://www.enterprisedb.com
On Mon Aug 28, 2023 at 2:37 PM CDT, Bruce Momjian wrote:
I don't see a clean way of avoiding the warning except by initializing
the array, which seems wasteful.
For what it's worth, we recently committed a patch[0]https://github.com/postgres/postgres/commit/4a8fef0d733965c1a1836022f8a42ab1e83a721f that initialized
an array due to a similar warning being generated on Fedora 38 (gcc
(GCC) 13.2.1 20230728 (Red Hat 13.2.1-1)).
[0]: https://github.com/postgres/postgres/commit/4a8fef0d733965c1a1836022f8a42ab1e83a721f
--
Tristan Partin
Neon (https://neon.tech)
On Tue, Aug 29, 2023 at 10:26:27AM +0700, John Naylor wrote:
On Tue, Aug 29, 2023 at 6:56 AM David Rowley <dgrowleyml@gmail.com> wrote:
I'm just not sure if it's unable to figure out if at least nargs
elements is set or if it won't be happy until all 100 elements are
set.It looks like the former, since I can silence it on gcc 13 / -O1 by doing:
/* keep compiler quiet */
actual_arg_types[0] = InvalidOid;
Agreed, that fixes it for me too. In fact, assigning to only element 99 or
200 also prevents the warning, and considering the array is defined for
100 elements, the fact is accepts 200 isn't a good thing. Patch attached.
I think the question is whether we add this to silence a common compiler
but non-default optimization level. It is the only such case in our
source code right now.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
Attachments:
warning.difftext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index da258968b8..f4a1d1049c 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -4284,6 +4284,10 @@ recheck_cast_function_args(List *args, Oid result_type,
if (list_length(args) > FUNC_MAX_ARGS)
elog(ERROR, "too many function arguments");
nargs = 0;
+
+ /* Silence gcc 12 compiler at -O1. */
+ actual_arg_types[0] = InvalidOid;
+
foreach(lc, args)
{
actual_arg_types[nargs++] = exprType((Node *) lfirst(lc));
Bruce Momjian <bruce@momjian.us> writes:
On Tue, Aug 29, 2023 at 10:26:27AM +0700, John Naylor wrote:
It looks like the former, since I can silence it on gcc 13 / -O1 by doing:
/* keep compiler quiet */
actual_arg_types[0] = InvalidOid;
Agreed, that fixes it for me too. In fact, assigning to only element 99 or
200 also prevents the warning, and considering the array is defined for
100 elements, the fact is accepts 200 isn't a good thing. Patch attached.
That seems like a pretty clear compiler bug, particularly since it just
appears in this one version. Rather than contorting our code, I'd
suggest filing a gcc bug.
regards, tom lane
On Tue, Aug 29, 2023 at 10:18:36AM -0400, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
On Tue, Aug 29, 2023 at 10:26:27AM +0700, John Naylor wrote:
It looks like the former, since I can silence it on gcc 13 / -O1 by doing:
/* keep compiler quiet */
actual_arg_types[0] = InvalidOid;Agreed, that fixes it for me too. In fact, assigning to only element 99 or
200 also prevents the warning, and considering the array is defined for
100 elements, the fact is accepts 200 isn't a good thing. Patch attached.That seems like a pretty clear compiler bug, particularly since it just
appears in this one version. Rather than contorting our code, I'd
suggest filing a gcc bug.
I assume I have to create a test case to report this to the gcc team. I
tried to create such a test case on gcc 12 but it doesn't generate the
warning. Attached is my attempt. Any ideas? I assume we can't just
tell them to download our software and compile it.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
Attachments:
Bruce Momjian <bruce@momjian.us> writes:
On Tue, Aug 29, 2023 at 10:18:36AM -0400, Tom Lane wrote:
That seems like a pretty clear compiler bug, particularly since it just
appears in this one version. Rather than contorting our code, I'd
suggest filing a gcc bug.
I assume I have to create a test case to report this to the gcc team. I
tried to create such a test case on gcc 12 but it doesn't generate the
warning. Attached is my attempt. Any ideas? I assume we can't just
tell them to download our software and compile it.
IIRC, they'll accept preprocessed compiler input for the specific file;
you don't need to provide a complete source tree. Per
https://gcc.gnu.org/bugs/
Please include all of the following items, the first three of which can be obtained from the output of gcc -v:
the exact version of GCC;
the system type;
the options given when GCC was configured/built;
the complete command line that triggers the bug;
the compiler output (error messages, warnings, etc.); and
the preprocessed file (*.i*) that triggers the bug, generated by adding -save-temps to the complete compilation command, or, in the case of a bug report for the GNAT front end, a complete set of source files (see below).
Obviously, if you can trim the input it's good, but it doesn't
have to be a minimal reproducer.
regards, tom lane
On Tue, Aug 29, 2023 at 11:30:06PM -0400, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
On Tue, Aug 29, 2023 at 10:18:36AM -0400, Tom Lane wrote:
That seems like a pretty clear compiler bug, particularly since it just
appears in this one version. Rather than contorting our code, I'd
suggest filing a gcc bug.I assume I have to create a test case to report this to the gcc team. I
tried to create such a test case on gcc 12 but it doesn't generate the
warning. Attached is my attempt. Any ideas? I assume we can't just
tell them to download our software and compile it.IIRC, they'll accept preprocessed compiler input for the specific file;
you don't need to provide a complete source tree. Per
https://gcc.gnu.org/bugs/Please include all of the following items, the first three of which can be obtained from the output of gcc -v:
the exact version of GCC;
the system type;
the options given when GCC was configured/built;
the complete command line that triggers the bug;
the compiler output (error messages, warnings, etc.); and
the preprocessed file (*.i*) that triggers the bug, generated by adding -save-temps to the complete compilation command, or, in the case of a bug report for the GNAT front end, a complete set of source files (see below).Obviously, if you can trim the input it's good, but it doesn't
have to be a minimal reproducer.
Bug submitted, thanks for th preprocessed file tip.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On Wed, Aug 30, 2023 at 11:16:48AM -0400, Bruce Momjian wrote:
On Tue, Aug 29, 2023 at 11:30:06PM -0400, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
On Tue, Aug 29, 2023 at 10:18:36AM -0400, Tom Lane wrote:
That seems like a pretty clear compiler bug, particularly since it just
appears in this one version. Rather than contorting our code, I'd
suggest filing a gcc bug.I assume I have to create a test case to report this to the gcc team. I
tried to create such a test case on gcc 12 but it doesn't generate the
warning. Attached is my attempt. Any ideas? I assume we can't just
tell them to download our software and compile it.IIRC, they'll accept preprocessed compiler input for the specific file;
you don't need to provide a complete source tree. Per
https://gcc.gnu.org/bugs/Please include all of the following items, the first three of which can be obtained from the output of gcc -v:
the exact version of GCC;
the system type;
the options given when GCC was configured/built;
the complete command line that triggers the bug;
the compiler output (error messages, warnings, etc.); and
the preprocessed file (*.i*) that triggers the bug, generated by adding -save-temps to the complete compilation command, or, in the case of a bug report for the GNAT front end, a complete set of source files (see below).Obviously, if you can trim the input it's good, but it doesn't
have to be a minimal reproducer.Bug submitted, thanks for th preprocessed file tip.
Good news, I was able to prevent the bug by causing compiling of
clauses.c to use -O2 by adding this to src/Makefile.custom:
clauses.o : CFLAGS+=-O2
Here is my submitted bug report:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111240
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On Wed, Aug 30, 2023 at 11:34:22AM -0400, Bruce Momjian wrote:
Good news, I was able to prevent the bug by causing compiling of
clauses.c to use -O2 by adding this to src/Makefile.custom:clauses.o : CFLAGS+=-O2
Here is my submitted bug report:
I got this reply on the bug report:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111240#c5
Richard Biener 2023-08-31 09:46:44 UTC
Confirmed.
rettype_58 = enforce_generic_type_consistency (&actual_arg_types, &declared_arg_types, 0, _56, 0);
and we reach this on the args == 0 path where indeed actual_arg_types
is uninitialized and our heuristic says that a const qualified pointer
is an input and thus might be read. So you get a maybe-uninitialized
diagnostic at the call.
GCC doesn't know that the 'nargs' argument relates to the array and
that at most 'nargs' (zero here) arguments are inspected.
So I think it works as designed, we have some duplicate bugreports
complaining about this "heuristic".
We are exposing this to ourselves by optimizing the args == 0 case
(skipping the initialization loop and constant propagating the
nargs argument). Aka jump-threading.
I think we just have to assume this incorrect warning will be around for
a while.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.