Silence -fsanitize=function where we cast function pointers on purpose

Started by Peter Eisentraut24 days ago4 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

This thread has been committed, so CI has stopped here. Anything below is the last result it produced.

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253984
psql -h localhost -U postgres

Built from patchset v1 (message #1), October 04, 2026 at 06:16 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t253984_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t253984_1 && git checkout t253984_1

Patchset v1 (message #1) is on t253984_1

Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

In clang, -fsanitize=undefined includes -fsanitize=function, which
reports every call made through a function pointer whose type does not
exactly match the called function, so it fires all over the place on
expression tree walkers and mutators, as well as a few other places. So
-fsanitize=undefined hasn't been working cleanly under clang for a
while. (Before clang 17, it only applied to C++.)

This is the same issue that caused us to use
-Wno-cast-function-type-strict with clang. That warning applies at the
place where the mismatching function pointer is passed, so there are
potentially hundreds of sites. Therefore, a global disabling is
appropriate. The sanitizer, on the other hand, triggers where the
function is called, which are only about two dozen places, so it seems
possible to silence these checks individually and still main the check
for accidental violations elsewhere.

I propose to add pg_attribute_no_sanitize_function() and place it on the
functions that make such calls. This is similar to some existing
pg_attribute_no_sanitize_xxx attributes.

Attachments:

t253984_1
0001-Silence-fsanitize-function-where-we-cast-function-po.patchtext/plain; charset=UTF-8; name=0001-Silence-fsanitize-function-where-we-cast-function-po.patchDownload+37-2
#2Chao Li
li.evan.chao@gmail.com
In reply to: Peter Eisentraut (#1)
Re: Silence -fsanitize=function where we cast function pointers on purpose

On Sep 29, 2026, at 13:47, Peter Eisentraut <peter@eisentraut.org> wrote:

In clang, -fsanitize=undefined includes -fsanitize=function, which reports every call made through a function pointer whose type does not exactly match the called function, so it fires all over the place on expression tree walkers and mutators, as well as a few other places. So -fsanitize=undefined hasn't been working cleanly under clang for a while. (Before clang 17, it only applied to C++.)

This is the same issue that caused us to use -Wno-cast-function-type-strict with clang. That warning applies at the place where the mismatching function pointer is passed, so there are potentially hundreds of sites. Therefore, a global disabling is appropriate. The sanitizer, on the other hand, triggers where the function is called, which are only about two dozen places, so it seems possible to silence these checks individually and still main the check for accidental violations elsewhere.

I propose to add pg_attribute_no_sanitize_function() and place it on the functions that make such calls. This is similar to some existing pg_attribute_no_sanitize_xxx attributes.
<0001-Silence-fsanitize-function-where-we-cast-function-po.patch>

Overall looks good to me.

Just one comment, in dynahash.c, hash_search_with_hash_value() is marked with the new annotation, feels like hash_update_hash_key() also needs to be annotated, because it also invokes match and keycopy etc callbacks.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Chao Li (#2)
Re: Silence -fsanitize=function where we cast function pointers on purpose

On 29.09.26 11:18, Chao Li wrote:

On Sep 29, 2026, at 13:47, Peter Eisentraut <peter@eisentraut.org> wrote:

In clang, -fsanitize=undefined includes -fsanitize=function, which reports every call made through a function pointer whose type does not exactly match the called function, so it fires all over the place on expression tree walkers and mutators, as well as a few other places. So -fsanitize=undefined hasn't been working cleanly under clang for a while. (Before clang 17, it only applied to C++.)

This is the same issue that caused us to use -Wno-cast-function-type-strict with clang. That warning applies at the place where the mismatching function pointer is passed, so there are potentially hundreds of sites. Therefore, a global disabling is appropriate. The sanitizer, on the other hand, triggers where the function is called, which are only about two dozen places, so it seems possible to silence these checks individually and still main the check for accidental violations elsewhere.

I propose to add pg_attribute_no_sanitize_function() and place it on the functions that make such calls. This is similar to some existing pg_attribute_no_sanitize_xxx attributes.
<0001-Silence-fsanitize-function-where-we-cast-function-po.patch>

Overall looks good to me.

Just one comment, in dynahash.c, hash_search_with_hash_value() is marked with the new annotation, feels like hash_update_hash_key() also needs to be annotated, because it also invokes match and keycopy etc callbacks.

Yes, I think you are right. My patch was based on what is required to
get the test suite to pass. It appears that there are currently no
callers of hash_update_hash_key() with a string-based hash table, so
this case isn't triggered, but we should add the annotation there as
well for completeness.

#4Peter Eisentraut
peter_e@gmx.net
In reply to: Peter Eisentraut (#3)
Re: Silence -fsanitize=function where we cast function pointers on purpose

On 29.09.26 12:01, Peter Eisentraut wrote:

On 29.09.26 11:18, Chao Li wrote:

On Sep 29, 2026, at 13:47, Peter Eisentraut <peter@eisentraut.org>
wrote:

In clang, -fsanitize=undefined includes -fsanitize=function, which
reports every call made through a function pointer whose type does
not exactly match the called function, so it fires all over the place
on expression tree walkers and mutators, as well as a few other
places.  So -fsanitize=undefined hasn't been working cleanly under
clang for a while.  (Before clang 17, it only applied to C++.)

This is the same issue that caused us to use -Wno-cast-function-type-
strict with clang.  That warning applies at the place where the
mismatching function pointer is passed, so there are potentially
hundreds of sites.  Therefore, a global disabling is appropriate.
The sanitizer, on the other hand, triggers where the function is
called, which are only about two dozen places, so it seems possible
to silence these checks individually and still main the check for
accidental violations elsewhere.

I propose to add pg_attribute_no_sanitize_function() and place it on
the functions that make such calls.  This is similar to some existing
pg_attribute_no_sanitize_xxx attributes.
<0001-Silence-fsanitize-function-where-we-cast-function-po.patch>

Overall looks good to me.

Just one comment, in dynahash.c, hash_search_with_hash_value() is
marked with the new annotation, feels like hash_update_hash_key() also
needs to be annotated, because it also invokes match and keycopy etc
callbacks.

Yes, I think you are right.  My patch was based on what is required to
get the test suite to pass.  It appears that there are currently no
callers of hash_update_hash_key() with a string-based hash table, so
this case isn't triggered, but we should add the annotation there as
well for completeness.

Committed with that change.