Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UB when calling cctype functions #92

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

glebm
Copy link

@glebm glebm commented Dec 21, 2022

Calling cctype functions, such as isspace, with negative values is undefined behaviour.

While glibc allows it, it crashes on uClibc compiled without UCLIBC_HAS_CTYPE_SIGNED.

Fixes the undefined behaviour by casting all arguments to cctype functions to unsigned char.

Calling cctype functions, such as isspace, with negative values
is undefined behaviour.

While glibc allows it, it crashes on uClibc compiled without
`UCLIBC_HAS_CTYPE_SIGNED`.

Fixes the undefined behaviour by casting all arguments to
cctype functions to `unsigned char`.
Fixes the following warnings

```
load_abc.cpp:2374:42: warning: ‘%ld’ directive writing between 1 and 20 bytes into a region of size 18 [-Wformat-overflow=]
 2374 |                         sprintf(buf,"%s=-%ld",ABC_ENV_NORANDOMPICK,retval->pickrandom+2);
      |                                          ^~~
load_abc.cpp:2374:37: note: directive argument in the range [-9223372036854775806, 9223372036854775807]
 2374 |                         sprintf(buf,"%s=-%ld",ABC_ENV_NORANDOMPICK,retval->pickrandom+2);
      |                                     ^~~~~~~~~
load_abc.cpp:2374:32: note: ‘sprintf’ output between 24 and 43 bytes into a destination of size 40
 2374 |                         sprintf(buf,"%s=-%ld",ABC_ENV_NORANDOMPICK,retval->pickrandom+2);
      |                         ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
glebm added a commit to glebm/od-buildroot that referenced this pull request Dec 23, 2022
@glebm
Copy link
Author

glebm commented Jan 8, 2023

@Konstanty Any chance you can merge this please? It's been tested by the OpenDingux community and it does resolve a few playback issues. Thanks

@AliceLR
Copy link
Contributor

AliceLR commented Jan 24, 2023

This looks OK, though I think this library still tries to target C++98. I would recommend C-style casts instead. (@Konstanty @sezero please correct me if I am wrong.)

@glebm
Copy link
Author

glebm commented Jan 25, 2023

static_cast is in C++98
https://en.cppreference.com/w/cpp/language/static_cast

@AliceLR
Copy link
Contributor

AliceLR commented Jan 25, 2023

You're right, nevermind!

glebm added a commit to glebm/od-buildroot that referenced this pull request Mar 24, 2023
These are patches for libmodplug that fix various UB issues.
They come from these PRs to the upstream repository:
1. Konstanty/libmodplug#92
2. Konstanty/libmodplug#93

The upstream maintainer has not merged any PRs in over a year.

Fixes OpenDingux#105.
glebm added a commit to glebm/od-buildroot that referenced this pull request Mar 24, 2023
These are patches for libmodplug that fix various UB issues.
They come from these PRs to the upstream repository:
1. Konstanty/libmodplug#92
2. Konstanty/libmodplug#93

The upstream maintainer has not merged any PRs in over a year.

Fixes OpenDingux#105.

Signed-off-by: Gleb Mazovetskiy <[email protected]>
pcercuei pushed a commit to OpenDingux/buildroot that referenced this pull request May 6, 2023
These are patches for libmodplug that fix various UB issues.
They come from these PRs to the upstream repository:
1. Konstanty/libmodplug#92
2. Konstanty/libmodplug#93

The upstream maintainer has not merged any PRs in over a year.

Fixes #105.

Signed-off-by: Gleb Mazovetskiy <[email protected]>
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