[PATCH 0/1] MR2050: include: Remove __WINE_MALLOC from functions that shouldn't have it.
According to the GCC documentation,[1] `__attribute__((malloc))` may not be used on functions that return memory that contains one or more valid pointers. [1] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2050
From: Alex Henrie <alexhenrie24(a)gmail.com> According to the GCC documentation,[1] __attribute__((malloc)) may not be used on functions that return memory that contains one or more valid pointers. [1] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html --- include/adshlp.h | 2 +- include/threadpoolapiset.h | 12 ++++++------ include/winber.h | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/adshlp.h b/include/adshlp.h index 5dacbb6b821..7366c077788 100644 --- a/include/adshlp.h +++ b/include/adshlp.h @@ -25,7 +25,7 @@ extern "C" { BOOL WINAPI FreeADsMem(void*); void* WINAPI AllocADsMem(DWORD) __WINE_ALLOC_SIZE(1) __WINE_DEALLOC(FreeADsMem) __WINE_MALLOC; -void* WINAPI ReallocADsMem(void*,DWORD,DWORD) __WINE_ALLOC_SIZE(3) __WINE_DEALLOC(FreeADsMem) __WINE_MALLOC; +void* WINAPI ReallocADsMem(void*,DWORD,DWORD) __WINE_ALLOC_SIZE(3) __WINE_DEALLOC(FreeADsMem); BOOL WINAPI FreeADsStr(WCHAR*); WCHAR* WINAPI AllocADsStr(WCHAR*) __WINE_DEALLOC(FreeADsStr) __WINE_MALLOC; diff --git a/include/threadpoolapiset.h b/include/threadpoolapiset.h index 6d8ac44a058..73203f413ee 100644 --- a/include/threadpoolapiset.h +++ b/include/threadpoolapiset.h @@ -33,17 +33,17 @@ WINBASEAPI void WINAPI CloseThreadpoolIo(TP_IO*); WINBASEAPI void WINAPI CloseThreadpoolTimer(PTP_TIMER); WINBASEAPI void WINAPI CloseThreadpoolWait(PTP_WAIT); WINBASEAPI void WINAPI CloseThreadpoolWork(PTP_WORK); -WINBASEAPI TP_POOL* WINAPI CreateThreadpool(void*) __WINE_DEALLOC(CloseThreadpool) __WINE_MALLOC; +WINBASEAPI TP_POOL* WINAPI CreateThreadpool(void*) __WINE_DEALLOC(CloseThreadpool); WINBASEAPI TP_CLEANUP_GROUP* WINAPI CreateThreadpoolCleanupGroup(void) - __WINE_DEALLOC(CloseThreadpoolCleanupGroup) __WINE_MALLOC; + __WINE_DEALLOC(CloseThreadpoolCleanupGroup); WINBASEAPI TP_IO* WINAPI CreateThreadpoolIo(HANDLE,PTP_WIN32_IO_CALLBACK,void*,TP_CALLBACK_ENVIRON*) - __WINE_DEALLOC(CloseThreadpoolIo) __WINE_MALLOC; + __WINE_DEALLOC(CloseThreadpoolIo); WINBASEAPI TP_TIMER* WINAPI CreateThreadpoolTimer(PTP_TIMER_CALLBACK,void*,TP_CALLBACK_ENVIRON*) - __WINE_DEALLOC(CloseThreadpoolTimer) __WINE_MALLOC; + __WINE_DEALLOC(CloseThreadpoolTimer); WINBASEAPI TP_WAIT* WINAPI CreateThreadpoolWait(PTP_WAIT_CALLBACK,void*,TP_CALLBACK_ENVIRON*) - __WINE_DEALLOC(CloseThreadpoolWait) __WINE_MALLOC; + __WINE_DEALLOC(CloseThreadpoolWait); WINBASEAPI TP_WORK* WINAPI CreateThreadpoolWork(PTP_WORK_CALLBACK,void*,TP_CALLBACK_ENVIRON*) - __WINE_DEALLOC(CloseThreadpoolWork) __WINE_MALLOC; + __WINE_DEALLOC(CloseThreadpoolWork); WINBASEAPI void WINAPI DisassociateCurrentThreadFromCallback(PTP_CALLBACK_INSTANCE); WINBASEAPI void WINAPI FreeLibraryWhenCallbackReturns(PTP_CALLBACK_INSTANCE,HMODULE); WINBASEAPI BOOL WINAPI IsThreadpoolTimerSet(PTP_TIMER); diff --git a/include/winber.h b/include/winber.h index 83055fa8c72..cb4f8e3ff54 100644 --- a/include/winber.h +++ b/include/winber.h @@ -29,12 +29,12 @@ typedef unsigned int ber_len_t; void CDECL ber_free( BerElement *, int ); void CDECL ber_bvfree( BERVAL * ); -BerElement * CDECL ber_alloc_t( int ) __WINE_DEALLOC(ber_free) __WINE_MALLOC; -BERVAL * CDECL ber_bvdup( BERVAL * ) __WINE_DEALLOC(ber_bvfree) __WINE_MALLOC; +BerElement * CDECL ber_alloc_t( int ) __WINE_DEALLOC(ber_free); +BERVAL * CDECL ber_bvdup( BERVAL * ) __WINE_DEALLOC(ber_bvfree); void CDECL ber_bvecfree( BERVAL ** ); ULONG CDECL ber_first_element( BerElement *, ULONG *, char ** ); int CDECL ber_flatten( BerElement *, BERVAL ** ); -BerElement * CDECL ber_init( BERVAL * ) __WINE_DEALLOC(ber_free) __WINE_MALLOC; +BerElement * CDECL ber_init( BERVAL * ) __WINE_DEALLOC(ber_free); ULONG CDECL ber_next_element( BerElement *, ULONG *, char * ); ULONG CDECL ber_peek_tag( BerElement *, ULONG * ); int WINAPIV ber_printf( BerElement *, char *, ... ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2050
The [GCC document][1] states:
Using the attribute with no arguments is designed to improve optimization by relying on the aliasing property it implies. Functions like malloc and calloc have this property because they return a pointer to uninitialized or zeroed-out, newly obtained storage. However, functions like realloc do not have this property, as they may return pointers to storage containing pointers to existing objects. Additionally, since all such functions are assumed to return null only infrequently, callers can be optimized based on that assumption.
However, it goes on give an example that goes like this:
``` int fclose (FILE*); int pclose (FILE*);
__attribute__ ((malloc, malloc (fclose, 1))) FILE* fdopen (int, const char*); __attribute__ ((malloc, malloc (fclose, 1))) FILE* fopen (const char*, const char*); __attribute__ ((malloc, malloc (fclose, 1))) FILE* fmemopen(void *, size_t, const char *); __attribute__ ((malloc, malloc (pclose, 1))) FILE* popen (const char*, const char*); __attribute__ ((malloc, malloc (fclose, 1))) FILE* tmpfile (void); ```
Notice that `FILE*` is internally known to contain valid pointers. Maybe it merely meant that `realloc` returns pointers that aliases with existing pointers instead? [1]: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2050#note_22235
On Sat Jan 28 11:39:19 2023 +0000, Jinoh Kang wrote:
The [GCC document][1] states:
Using the attribute with no arguments is designed to improve optimization by relying on the aliasing property it implies. Functions like malloc and calloc have this property because they return a pointer to uninitialized or zeroed-out, newly obtained storage. However, functions like realloc do not have this property, as they may return pointers to storage containing pointers to existing objects. Additionally, since all such functions are assumed to return null only infrequently, callers can be optimized based on that assumption. However, it goes on give an example that goes like this: ``` int fclose (FILE*); int pclose (FILE*);
__attribute__ ((malloc, malloc (fclose, 1))) FILE* fdopen (int, const char*); __attribute__ ((malloc, malloc (fclose, 1))) FILE* fopen (const char*, const char*); __attribute__ ((malloc, malloc (fclose, 1))) FILE* fmemopen(void *, size_t, const char *); __attribute__ ((malloc, malloc (pclose, 1))) FILE* popen (const char*, const char*); __attribute__ ((malloc, malloc (fclose, 1))) FILE* tmpfile (void); ``` Notice that `FILE` is internally known to contain valid pointers. Maybe it merely meant that `realloc` returns pointers that aliases with existing pointers instead? [1]: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html Reading through the GCC bug report titled "documentation for attribute malloc contradicts itself",[1] which ended in a change to the documentation, it's clear that not only must the returned pointer be unique, but also it may not point to valid pointers either. There was a substantial amount of argument over whether such a strict requirement was a good idea.
My guess is that it's OK to use the malloc attribute on functions that return FILE* because you're not allowed to dereference a FILE pointer, so the compiler never even considers whether it points to valid pointers. Internally, I don't think glibc calls one of those functions and dereferences the result either. Please let me know if you find a counterexample. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56955 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2050#note_22241
On Sun Jan 29 00:44:30 2023 +0000, Alex Henrie wrote:
Reading through the GCC bug report titled "documentation for attribute malloc contradicts itself",[1] which ended in a change to the documentation, it's clear that not only must the returned pointer be unique, but also it may not point to valid pointers either. There was a substantial amount of argument over whether such a strict requirement was a good idea. My guess is that it's OK to use the malloc attribute on functions that return FILE* because you're not allowed to dereference a FILE pointer, so the compiler never even considers whether it points to valid pointers. Internally, I don't think glibc calls one of those functions and dereferences the result either. Please let me know if you find a counterexample. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56955 I stand corrected. Thank you.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2050#note_22250
participants (3)
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Jinoh Kang (＠iamahuman)