Fix -Wcast-function-type warnings

Three groups of issues needed to be addressed:

load_external_function() and related functions returned PGFunction,
even though not necessarily all callers are looking for a function of
type PGFunction.  Since these functions are really just wrappers
around dlsym(), change to return void * just like dlsym().

In dynahash.c, we are using strlcpy() where a function with a
signature like memcpy() is expected.  This should be safe, as the new
comment there explains, but the cast needs to be augmented to avoid
the warning.

In PL/Python, methods all need to be cast to PyCFunction, per Python
API, but this now runs afoul of these warnings.  (This issue also
exists in core CPython.)

To fix the second and third case, we add a new type pg_funcptr_t that
is defined specifically so that gcc accepts it as a special function
pointer that can be cast to any other function pointer without the
warning.

Also add -Wcast-function-type to the standard warning flags, subject
to configure check.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/1e97628e-6447-b4fd-e230-d109cec2d584%402ndquadrant.com
This commit is contained in:
Peter Eisentraut 2020-07-14 19:36:30 +02:00
parent 101f903e51
commit de8feb1f3a
7 changed files with 127 additions and 18 deletions

91
configure vendored
View File

@ -5643,6 +5643,97 @@ if test x"$pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough_3" = x"yes"; then
fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wcast-function-type, for CFLAGS" >&5
$as_echo_n "checking whether ${CC} supports -Wcast-function-type, for CFLAGS... " >&6; }
if ${pgac_cv_prog_CC_cflags__Wcast_function_type+:} false; then :
$as_echo_n "(cached) " >&6
else
pgac_save_CFLAGS=$CFLAGS
pgac_save_CC=$CC
CC=${CC}
CFLAGS="${CFLAGS} -Wcast-function-type"
ac_save_c_werror_flag=$ac_c_werror_flag
ac_c_werror_flag=yes
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
int
main ()
{
;
return 0;
}
_ACEOF
if ac_fn_c_try_compile "$LINENO"; then :
pgac_cv_prog_CC_cflags__Wcast_function_type=yes
else
pgac_cv_prog_CC_cflags__Wcast_function_type=no
fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
ac_c_werror_flag=$ac_save_c_werror_flag
CFLAGS="$pgac_save_CFLAGS"
CC="$pgac_save_CC"
fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__Wcast_function_type" >&5
$as_echo "$pgac_cv_prog_CC_cflags__Wcast_function_type" >&6; }
if test x"$pgac_cv_prog_CC_cflags__Wcast_function_type" = x"yes"; then
CFLAGS="${CFLAGS} -Wcast-function-type"
fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -Wcast-function-type, for CXXFLAGS" >&5
$as_echo_n "checking whether ${CXX} supports -Wcast-function-type, for CXXFLAGS... " >&6; }
if ${pgac_cv_prog_CXX_cxxflags__Wcast_function_type+:} false; then :
$as_echo_n "(cached) " >&6
else
pgac_save_CXXFLAGS=$CXXFLAGS
pgac_save_CXX=$CXX
CXX=${CXX}
CXXFLAGS="${CXXFLAGS} -Wcast-function-type"
ac_save_cxx_werror_flag=$ac_cxx_werror_flag
ac_cxx_werror_flag=yes
ac_ext=cpp
ac_cpp='$CXXCPP $CPPFLAGS'
ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
int
main ()
{
;
return 0;
}
_ACEOF
if ac_fn_cxx_try_compile "$LINENO"; then :
pgac_cv_prog_CXX_cxxflags__Wcast_function_type=yes
else
pgac_cv_prog_CXX_cxxflags__Wcast_function_type=no
fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
ac_ext=c
ac_cpp='$CPP $CPPFLAGS'
ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
ac_compiler_gnu=$ac_cv_c_compiler_gnu
ac_cxx_werror_flag=$ac_save_cxx_werror_flag
CXXFLAGS="$pgac_save_CXXFLAGS"
CXX="$pgac_save_CXX"
fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CXX_cxxflags__Wcast_function_type" >&5
$as_echo "$pgac_cv_prog_CXX_cxxflags__Wcast_function_type" >&6; }
if test x"$pgac_cv_prog_CXX_cxxflags__Wcast_function_type" = x"yes"; then
CXXFLAGS="${CXXFLAGS} -Wcast-function-type"
fi
# This was included in -Wall/-Wformat in older GCC versions
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wformat-security, for CFLAGS" >&5

View File

@ -498,6 +498,8 @@ if test "$GCC" = yes -a "$ICC" = no; then
PGAC_PROG_CXX_CFLAGS_OPT([-Wmissing-format-attribute])
PGAC_PROG_CC_CFLAGS_OPT([-Wimplicit-fallthrough=3])
PGAC_PROG_CXX_CFLAGS_OPT([-Wimplicit-fallthrough=3])
PGAC_PROG_CC_CFLAGS_OPT([-Wcast-function-type])
PGAC_PROG_CXX_CFLAGS_OPT([-Wcast-function-type])
# This was included in -Wall/-Wformat in older GCC versions
PGAC_PROG_CC_CFLAGS_OPT([-Wformat-security])
PGAC_PROG_CXX_CFLAGS_OPT([-Wformat-security])

View File

@ -95,7 +95,7 @@ static const Pg_magic_struct magic_data = PG_MODULE_MAGIC_DATA;
* named funcname in it.
*
* If the function is not found, we raise an error if signalNotFound is true,
* else return (PGFunction) NULL. Note that errors in loading the library
* else return NULL. Note that errors in loading the library
* will provoke ereport() regardless of signalNotFound.
*
* If filehandle is not NULL, then *filehandle will be set to a handle
@ -103,13 +103,13 @@ static const Pg_magic_struct magic_data = PG_MODULE_MAGIC_DATA;
* lookup_external_function to lookup additional functions in the same file
* at less cost than repeating load_external_function.
*/
PGFunction
void *
load_external_function(const char *filename, const char *funcname,
bool signalNotFound, void **filehandle)
{
char *fullname;
void *lib_handle;
PGFunction retval;
void *retval;
/* Expand the possibly-abbreviated filename to an exact path name */
fullname = expand_dynamic_library_name(filename);
@ -122,7 +122,7 @@ load_external_function(const char *filename, const char *funcname,
*filehandle = lib_handle;
/* Look up the function within the library. */
retval = (PGFunction) dlsym(lib_handle, funcname);
retval = dlsym(lib_handle, funcname);
if (retval == NULL && signalNotFound)
ereport(ERROR,
@ -165,12 +165,12 @@ load_file(const char *filename, bool restricted)
/*
* Lookup a function whose library file is already loaded.
* Return (PGFunction) NULL if not found.
* Return NULL if not found.
*/
PGFunction
void *
lookup_external_function(void *filehandle, const char *funcname)
{
return (PGFunction) dlsym(filehandle, funcname);
return dlsym(filehandle, funcname);
}

View File

@ -398,7 +398,16 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
if (flags & HASH_KEYCOPY)
hashp->keycopy = info->keycopy;
else if (hashp->hash == string_hash)
hashp->keycopy = (HashCopyFunc) strlcpy;
{
/*
* The signature of keycopy is meant for memcpy(), which returns
* void*, but strlcpy() returns size_t. Since we never use the return
* value of keycopy, and size_t is pretty much always the same size as
* void *, this should be safe. The extra cast in the middle is to
* avoid warnings from -Wcast-function-type.
*/
hashp->keycopy = (HashCopyFunc) (pg_funcptr_t) strlcpy;
}
else
hashp->keycopy = memcpy;

View File

@ -265,6 +265,13 @@
#define dummyret char
#endif
/*
* Generic function pointer. This can be used in the rare cases where it's
* necessary to cast a function pointer to a seemingly incompatible function
* pointer type while avoiding gcc's -Wcast-function-type warnings.
*/
typedef void (*pg_funcptr_t) (void);
/*
* We require C99, hence the compiler should understand flexible array
* members. However, for documentation purposes we still consider it to be

View File

@ -716,9 +716,9 @@ extern bool CheckFunctionValidatorAccess(Oid validatorOid, Oid functionOid);
*/
extern char *Dynamic_library_path;
extern PGFunction load_external_function(const char *filename, const char *funcname,
bool signalNotFound, void **filehandle);
extern PGFunction lookup_external_function(void *filehandle, const char *funcname);
extern void *load_external_function(const char *filename, const char *funcname,
bool signalNotFound, void **filehandle);
extern void *lookup_external_function(void *filehandle, const char *funcname);
extern void load_file(const char *filename, bool restricted);
extern void **find_rendezvous_variable(const char *varName);
extern Size EstimateLibraryStateSpace(void);

View File

@ -61,13 +61,13 @@ static PyMethodDef PLy_methods[] = {
/*
* logging methods
*/
{"debug", (PyCFunction) PLy_debug, METH_VARARGS | METH_KEYWORDS, NULL},
{"log", (PyCFunction) PLy_log, METH_VARARGS | METH_KEYWORDS, NULL},
{"info", (PyCFunction) PLy_info, METH_VARARGS | METH_KEYWORDS, NULL},
{"notice", (PyCFunction) PLy_notice, METH_VARARGS | METH_KEYWORDS, NULL},
{"warning", (PyCFunction) PLy_warning, METH_VARARGS | METH_KEYWORDS, NULL},
{"error", (PyCFunction) PLy_error, METH_VARARGS | METH_KEYWORDS, NULL},
{"fatal", (PyCFunction) PLy_fatal, METH_VARARGS | METH_KEYWORDS, NULL},
{"debug", (PyCFunction) (pg_funcptr_t) PLy_debug, METH_VARARGS | METH_KEYWORDS, NULL},
{"log", (PyCFunction) (pg_funcptr_t) PLy_log, METH_VARARGS | METH_KEYWORDS, NULL},
{"info", (PyCFunction) (pg_funcptr_t) PLy_info, METH_VARARGS | METH_KEYWORDS, NULL},
{"notice", (PyCFunction) (pg_funcptr_t) PLy_notice, METH_VARARGS | METH_KEYWORDS, NULL},
{"warning", (PyCFunction) (pg_funcptr_t) PLy_warning, METH_VARARGS | METH_KEYWORDS, NULL},
{"error", (PyCFunction) (pg_funcptr_t) PLy_error, METH_VARARGS | METH_KEYWORDS, NULL},
{"fatal", (PyCFunction) (pg_funcptr_t) PLy_fatal, METH_VARARGS | METH_KEYWORDS, NULL},
/*
* create a stored plan