Skip to content

SDL_malloc uses simpler api if available to initialize #7374

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

SDL_malloc uses simpler api if available to initialize #7374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc1750a
SDL_malloc uses simpler api if available to initialize
devnexen Feb 26, 2023
9635eb9
disable Linux
devnexen Feb 26, 2023
b9e710d
disable Linux
devnexen Feb 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,7 @@ if(SDL_LIBC)
check_symbol_exists(getauxval "sys/auxv.h" HAVE_GETAUXVAL)
check_symbol_exists(elf_aux_info "sys/auxv.h" HAVE_ELF_AUX_INFO)
check_symbol_exists(poll "poll.h" HAVE_POLL)
check_symbol_exists(arc4random_buf, "stdlib.h" HAVE_ARC4RANDOM)

check_library_exists(m pow "" HAVE_LIBM)
if(HAVE_LIBM)
Expand Down
6 changes: 6 additions & 0 deletions include/build_config/SDL_build_config.h.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@
#cmakedefine HAVE_POLL 1
#cmakedefine HAVE__EXIT 1

#if !defined(__linux__)
/* Disabling for Linux at least for now until the feature matures
a bit on this platform. */
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look good. I know @madebr expressed concerns about compatibility
of library built on newer systems would be inoperable on older ones, but that
is a well known story anyway. So, if this patch is going to go in, and if we
really want to address that, I suggest making this a compile time option in
cmake rather than explicitly disabling it for linux.

Besides, this part of the code isn't even enabled by default, so I don't know
why is there sof much effont on it...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, in general we do build-time checks for system functionality and we don't guarantee that something built on one version of Linux will work on an earlier version.

To @sezero's point, is this code that will be run in SDL?

Copy link
Contributor

Choose a reason for hiding this comment

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

To @sezero's point, is this code that will be run in SDL?

Unless SDL_malloc.c is explicitly patched, no: The patch changes code guarded
by FOOTERS which isn't defined by default (neither is USE_DEV_RANDOM)

#cmakedefine HAVE_ARC4RANDOM 1
#endif

#else
#cmakedefine HAVE_STDARG_H 1
#cmakedefine HAVE_STDDEF_H 1
Expand Down
4 changes: 4 additions & 0 deletions src/stdlib/SDL_malloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2527,6 +2527,9 @@ static int init_mparams(void) {

#if (FOOTERS && !INSECURE)
{
#if HAVE_ARC4RANDOM
arc4random_buf(&s, sizeof(s));
#else
#if USE_DEV_RANDOM
int fd;
unsigned char buf[sizeof(size_t)];
Expand All @@ -2542,6 +2545,7 @@ static int init_mparams(void) {
else
#endif /* USE_DEV_RANDOM */
s = (size_t)(time(0) ^ (size_t)0x55555555U);
#endif

s |= (size_t)8U; /* ensure nonzero */
s &= ~(size_t)7U; /* improve chances of fault for bad values */
Expand Down