Skip to content

Fix undefined behavior defined by ASAN#1451

Merged
madolson merged 7 commits into
valkey-io:unstablefrom
madolson:Undefined_void
Dec 18, 2024
Merged

Fix undefined behavior defined by ASAN#1451
madolson merged 7 commits into
valkey-io:unstablefrom
madolson:Undefined_void

Conversation

@madolson

@madolson madolson commented Dec 17, 2024

Copy link
Copy Markdown
Member

Asan now supports making sure you are passing in the correct pointer type, which seems useful but we can't support it since we pass in an incorrect pointer in several places. This is most commonly done with generic free functions, where we simply cast it to the correct type.

Example: https://github.com/valkey-io/valkey/actions/runs/12334182647/job/34424037283#step:6:318.

It's not a lot of code to clean up, so it seems appropriate to cleanup instead of disabling the check.

Validating run: https://github.com/valkey-io/valkey/actions/runs/12382893783/job/34564554891

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
@codecov

codecov Bot commented Dec 17, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 90.90909% with 3 lines in your changes missing coverage. Please review.

Project coverage is 70.78%. Comparing base (7892bf8) to head (c2f1ab3).
Report is 3 commits behind head on unstable.

Files with missing lines Patch % Lines
src/call_reply.c 0.00% 1 Missing ⚠️
src/module.c 0.00% 1 Missing ⚠️
src/networking.c 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1451      +/-   ##
============================================
- Coverage     70.81%   70.78%   -0.04%     
============================================
  Files           119      119              
  Lines         64691    64703      +12     
============================================
- Hits          45812    45797      -15     
- Misses        18879    18906      +27     
Files with missing lines Coverage Δ
src/acl.c 88.77% <100.00%> (ø)
src/adlist.c 71.79% <100.00%> (+0.44%) ⬆️
src/db.c 89.55% <100.00%> (ø)
src/defrag.c 88.85% <100.00%> (-0.17%) ⬇️
src/eval.c 57.20% <100.00%> (ø)
src/functions.c 95.54% <100.00%> (ø)
src/listpack.c 91.63% <100.00%> (+1.43%) ⬆️
src/replication.c 87.38% <100.00%> (-0.13%) ⬇️
src/t_stream.c 93.27% <100.00%> (+0.02%) ⬆️
src/call_reply.c 0.00% <0.00%> (ø)
... and 2 more

... and 9 files with indirect coverage changes

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
@madolson madolson requested a review from zuiderkwast December 17, 2024 23:28

@zuiderkwast zuiderkwast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

Comment thread src/adlist.c Outdated
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
@madolson madolson merged commit e203ca3 into valkey-io:unstable Dec 18, 2024
kronwerk pushed a commit to kronwerk/valkey that referenced this pull request Jan 27, 2025
Asan now supports making sure you are passing in the correct pointer
type, which seems useful but we can't support it since we pass in an
incorrect pointer in several places. This is most commonly done with
generic free functions, where we simply cast it to the correct type.

It's not a lot of code to clean up, so it seems appropriate to cleanup
instead of disabling the check.

---------

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
vitarb pushed a commit to vitarb/valkey that referenced this pull request Jun 24, 2025
Asan now supports making sure you are passing in the correct pointer
type, which seems useful but we can't support it since we pass in an
incorrect pointer in several places. This is most commonly done with
generic free functions, where we simply cast it to the correct type.

It's not a lot of code to clean up, so it seems appropriate to cleanup
instead of disabling the check.

---------

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
vitarb pushed a commit to vitarb/valkey that referenced this pull request Jun 24, 2025
Asan now supports making sure you are passing in the correct pointer
type, which seems useful but we can't support it since we pass in an
incorrect pointer in several places. This is most commonly done with
generic free functions, where we simply cast it to the correct type.

It's not a lot of code to clean up, so it seems appropriate to cleanup
instead of disabling the check.

---------

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
zuiderkwast added a commit to vitarb/valkey that referenced this pull request Aug 15, 2025
Asan now supports making sure you are passing in the correct pointer
type, which seems useful but we can't support it since we pass in an
incorrect pointer in several places. This is most commonly done with
generic free functions, where we simply cast it to the correct type.

It's not a lot of code to clean up, so it seems appropriate to cleanup
instead of disabling the check.

---------

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
zuiderkwast added a commit to vitarb/valkey that referenced this pull request Aug 15, 2025
Asan now supports making sure you are passing in the correct pointer
type, which seems useful but we can't support it since we pass in an
incorrect pointer in several places. This is most commonly done with
generic free functions, where we simply cast it to the correct type.

It's not a lot of code to clean up, so it seems appropriate to cleanup
instead of disabling the check.

---------

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
zuiderkwast added a commit to vitarb/valkey that referenced this pull request Aug 21, 2025
Asan now supports making sure you are passing in the correct pointer
type, which seems useful but we can't support it since we pass in an
incorrect pointer in several places. This is most commonly done with
generic free functions, where we simply cast it to the correct type.

It's not a lot of code to clean up, so it seems appropriate to cleanup
instead of disabling the check.

---------

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
zuiderkwast added a commit that referenced this pull request Aug 22, 2025
Asan now supports making sure you are passing in the correct pointer
type, which seems useful but we can't support it since we pass in an
incorrect pointer in several places. This is most commonly done with
generic free functions, where we simply cast it to the correct type.

It's not a lot of code to clean up, so it seems appropriate to cleanup
instead of disabling the check.

---------

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request Sep 16, 2025
Asan now supports making sure you are passing in the correct pointer
type, which seems useful but we can't support it since we pass in an
incorrect pointer in several places. This is most commonly done with
generic free functions, where we simply cast it to the correct type.

It's not a lot of code to clean up, so it seems appropriate to cleanup
instead of disabling the check.

---------

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@zuiderkwast zuiderkwast mentioned this pull request Sep 30, 2025
sarthakaggarwal97 added a commit to sarthakaggarwal97/valkey that referenced this pull request Apr 16, 2026
Replace (void (*)(void*))sdsfree casts with sdsfreeVoid (which already
exists on 7.2 in sds.c). Add engineLibraryFreeVoid wrapper in
functions.c. Remove unnecessary cast on zfree (already void*).

Fixes UBSan error: 'call to function sdsfree through pointer to
incorrect function type void (*)(void *)' at adlist.c:185.

Minimal backport of valkey-io#1451 from unstable.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
sarthakaggarwal97 added a commit to sarthakaggarwal97/valkey that referenced this pull request Apr 27, 2026
Replace (void (*)(void*))sdsfree casts with sdsfreeVoid (which already
exists on 7.2 in sds.c). Add engineLibraryFreeVoid wrapper in
functions.c. Remove unnecessary cast on zfree (already void*).

Fixes UBSan error: 'call to function sdsfree through pointer to
incorrect function type void (*)(void *)' at adlist.c:185.

Minimal backport of valkey-io#1451 from unstable.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
(cherry picked from commit 5eeedb1)
sarthakaggarwal97 added a commit to sarthakaggarwal97/valkey that referenced this pull request May 4, 2026
Replace (void (*)(void*))sdsfree casts with sdsfreeVoid (which already
exists on 7.2 in sds.c). Add engineLibraryFreeVoid wrapper in
functions.c. Remove unnecessary cast on zfree (already void*).

Fixes UBSan error: 'call to function sdsfree through pointer to
incorrect function type void (*)(void *)' at adlist.c:185.

Minimal backport of valkey-io#1451 from unstable.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
(cherry picked from commit 5eeedb1)
madolson pushed a commit that referenced this pull request May 6, 2026
Replace (void (*)(void*))sdsfree casts with sdsfreeVoid (which already
exists on 7.2 in sds.c). Add engineLibraryFreeVoid wrapper in
functions.c. Remove unnecessary cast on zfree (already void*).

Fixes UBSan error: 'call to function sdsfree through pointer to
incorrect function type void (*)(void *)' at adlist.c:185.

Minimal backport of #1451 from unstable.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
(cherry picked from commit 5eeedb1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants