Skip to content

lib: fix function pointers to please UndefinedBehaviorSanitizer - #15289

Merged
bagder merged 3 commits into
masterfrom
bagder/cleaner-func-typecast
Oct 15, 2024
Merged

lib: fix function pointers to please UndefinedBehaviorSanitizer#15289
bagder merged 3 commits into
masterfrom
bagder/cleaner-func-typecast

Conversation

@bagder

@bagder bagder commented Oct 14, 2024

Copy link
Copy Markdown
Member

Use plain typecasts instead to make it more obvious what the code does.

The UndefinedBehaviorSanitizer (ubsan) now errors if a callback pointer is used to a call a function with ever so slight difference in signature. This is annoying because it means that we can no longer use our long established practice of using a different typedef for several handles when building the library vs when using it. We used to have a plain struct pointer for libcurl builds and a void * for that name when the header is used for applications.

@bagder bagder added the tidy-up label Oct 14, 2024
Comment thread lib/system_win32.c Outdated
@bagder
bagder marked this pull request as draft October 14, 2024 08:30
@bagder bagder changed the title lib: remove use of CURLX_FUNCTION_CAST macro lib: reduce typecasts for function pointers Oct 14, 2024
@bagder
bagder force-pushed the bagder/cleaner-func-typecast branch from 75b31c5 to dffb408 Compare October 14, 2024 10:41
@dfandrich

This comment was marked as outdated.

@bagder bagder changed the title lib: reduce typecasts for function pointers lib: fix function pointers to please UndefinedBehaviorSanitizer Oct 14, 2024
@bagder bagder linked an issue Oct 14, 2024 that may be closed by this pull request
@bagder
bagder force-pushed the bagder/cleaner-func-typecast branch from b979979 to cedf3c1 Compare October 14, 2024 12:39
@bagder
bagder force-pushed the bagder/cleaner-func-typecast branch from b472bb1 to fe9e69a Compare October 15, 2024 09:57
@bagder
bagder marked this pull request as ready for review October 15, 2024 10:25
Make sure we use functions with the correct prototype.

Closes #15289
It makes the callbacks get different signnatures when used from within
libcurl vs outside of it by libcurl-using applications (such as the
libtests) and this triggers UndefinedBehaviorSanitizer errors.

Closes #15289
Make test applications use the correct prototypes for callbacks.

Closes #15289
@bagder
bagder force-pushed the bagder/cleaner-func-typecast branch from fe9e69a to 8403e5a Compare October 15, 2024 12:34
@bagder bagder closed this in ad1c49b Oct 15, 2024
@bagder
bagder merged commit 8403e5a into master Oct 15, 2024
@bagder
bagder deleted the bagder/cleaner-func-typecast branch October 15, 2024 14:15
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
Make sure we use functions with the correct prototype.

Closes curl#15289
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
It makes the callbacks get different signnatures when used from within
libcurl vs outside of it by libcurl-using applications (such as the
libtests) and this triggers UndefinedBehaviorSanitizer errors.

Closes curl#15289
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
Make test applications use the correct prototypes for callbacks.

Closes curl#15289
@dreifachstein

dreifachstein commented Jun 7, 2025

Copy link
Copy Markdown

Aliasing everything to void has the downside of losing strict type checking. Also it breaks C++ code that defines CURL_STRICTER and contains references to these types. CURL& now becomes void& which is invalid.

I think it is better to keep struct typedefs in CURL_STRICTER and build everything within curl source tree with it. This will make ubsan happy and API users may still use void aliases on platforms whose ABIs permit it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

UndefinedBehaviorSanitizer: undefined-behavior hmac.c:88:3

5 participants