[PATCH 0/2] MR1686: include/msvcrt: Add __WINE_(ALLOC_SIZE|DEALLOC|MALLOC) attributes to strdup, wcsdup, _aligned_malloc

[PATCH 1/2] include/msvcrt: Add __WINE_(DEALLOC|MALLOC) attributes to strdup and wcsdup.

[Brendan Shanks]

From: Brendan Shanks

---
 include/msvcrt/corecrt.h         | 12 ++++++++++++
 include/msvcrt/corecrt_wstring.h |  3 ++-
 include/msvcrt/string.h          |  3 ++-
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/include/msvcrt/corecrt.h b/include/msvcrt/corecrt.h
index aba9bc9422e..9f403c7d10e 100644
--- a/include/msvcrt/corecrt.h
+++ b/include/msvcrt/corecrt.h
@@ -342,4 +342,16 @@ typedef struct threadlocaleinfostruct {
 #define __WINE_CRT_SCANF_ATTR(fmt,args)
 #endif

+#if defined(__GNUC__) && (__GNUC__ > 10)
+#define __WINE_DEALLOC(...) __attribute__((malloc (__VA_ARGS__)))
+#else
+#define __WINE_DEALLOC(...)
+#endif
+
+#if defined(__GNUC__) && (__GNUC__ > 2)
+#define __WINE_MALLOC __attribute__((malloc))
+#else
+#define __WINE_MALLOC
+#endif
+
 #endif /* __WINE_CORECRT_H */
diff --git a/include/msvcrt/corecrt_wstring.h b/include/msvcrt/corecrt_wstring.h
index bcf9f947625..f3e859f805b 100644
--- a/include/msvcrt/corecrt_wstring.h
+++ b/include/msvcrt/corecrt_wstring.h
@@ -7,6 +7,7 @@
 #define _WSTRING_DEFINED

#include <corecrt.h>
+#include <stdlib.h>

#ifdef __cplusplus
 extern "C" {
@@ -29,7 +30,7 @@ static inline void* memccpy(void *s1, const void *s2, int c, size_t n) { return

_ACRTIMP void*   __cdecl memmove(void*,const void*,size_t);

-_ACRTIMP wchar_t* __cdecl _wcsdup(const wchar_t*);
+_ACRTIMP wchar_t* __cdecl _wcsdup(const wchar_t*) __WINE_DEALLOC(free) __WINE_MALLOC;
 _ACRTIMP int      __cdecl _wcsicmp(const wchar_t*,const wchar_t*);
 _ACRTIMP int      __cdecl _wcsicoll(const wchar_t*,const wchar_t*);
 _ACRTIMP int      __cdecl _wcsicoll_l(const wchar_t*, const wchar_t*, _locale_t);
diff --git a/include/msvcrt/string.h b/include/msvcrt/string.h
index 5e3d51076c6..3cbefd8df59 100644
--- a/include/msvcrt/string.h
+++ b/include/msvcrt/string.h
@@ -9,6 +9,7 @@
 #define __WINE_STRING_H

#include <corecrt_wstring.h>
+#include <stdlib.h>

#ifndef _NLSCMP_DEFINED
 #define _NLSCMPERROR               ((unsigned int)0x7fffffff)
@@ -22,7 +23,7 @@ extern "C" {
 _ACRTIMP size_t __cdecl __strncnt(const char*,size_t);
 _ACRTIMP int   __cdecl _strcmpi(const char*,const char*);
 _ACRTIMP int   __cdecl _strcoll_l(const char*, const char*, _locale_t);
-_ACRTIMP char* __cdecl _strdup(const char*);
+_ACRTIMP char* __cdecl _strdup(const char*) __WINE_DEALLOC(free) __WINE_MALLOC;
 _ACRTIMP char* __cdecl _strerror(const char*);
 _ACRTIMP errno_t __cdecl strerror_s(char*,size_t,int);
 _ACRTIMP int   __cdecl _stricmp(const char*,const char*);

[PATCH 2/2] include/msvcrt: Add __WINE_(ALLOC_SIZE|DEALLOC|MALLOC) attributes to _aligned_malloc functions.

[Brendan Shanks]

From: Brendan Shanks

---
 include/msvcrt/corecrt.h | 6 ++++++
 include/msvcrt/malloc.h  | 8 ++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/msvcrt/corecrt.h b/include/msvcrt/corecrt.h
index 9f403c7d10e..4dc2ca53208 100644
--- a/include/msvcrt/corecrt.h
+++ b/include/msvcrt/corecrt.h
@@ -342,6 +342,12 @@ typedef struct threadlocaleinfostruct {
 #define __WINE_CRT_SCANF_ATTR(fmt,args)
 #endif

+#if defined(__GNUC__) && ((__GNUC__ > 4) || ((__GNUC__ == 4) && (__GNUC_MINOR__ >= 3)))
+#define __WINE_ALLOC_SIZE(...) __attribute__((__alloc_size__(__VA_ARGS__)))
+#else
+#define __WINE_ALLOC_SIZE(...)
+#endif
+
 #if defined(__GNUC__) && (__GNUC__ > 10)
 #define __WINE_DEALLOC(...) __attribute__((malloc (__VA_ARGS__)))
 #else
diff --git a/include/msvcrt/malloc.h b/include/msvcrt/malloc.h
index fcbadbeb4e0..c6a26491249 100644
--- a/include/msvcrt/malloc.h
+++ b/include/msvcrt/malloc.h
@@ -69,10 +69,10 @@ _ACRTIMP void*  __cdecl malloc(size_t);
 _ACRTIMP void*  __cdecl realloc(void*,size_t);

_ACRTIMP void   __cdecl _aligned_free(void*);
-_ACRTIMP void*  __cdecl _aligned_malloc(size_t,size_t);
-_ACRTIMP void*  __cdecl _aligned_offset_malloc(size_t,size_t,size_t);
-_ACRTIMP void*  __cdecl _aligned_realloc(void*,size_t,size_t);
-_ACRTIMP void*  __cdecl _aligned_offset_realloc(void*,size_t,size_t,size_t);
+_ACRTIMP void*  __cdecl _aligned_malloc(size_t,size_t) __WINE_ALLOC_SIZE(1) __WINE_DEALLOC(_aligned_free) __WINE_MALLOC;
+_ACRTIMP void*  __cdecl _aligned_offset_malloc(size_t,size_t,size_t) __WINE_ALLOC_SIZE(1) __WINE_DEALLOC(_aligned_free) __WINE_MALLOC;
+_ACRTIMP void*  __cdecl _aligned_realloc(void*,size_t,size_t) __WINE_ALLOC_SIZE(2) __WINE_DEALLOC(_aligned_free);
+_ACRTIMP void*  __cdecl _aligned_offset_realloc(void*,size_t,size_t,size_t) __WINE_ALLOC_SIZE(2) __WINE_DEALLOC(_aligned_free);

_ACRTIMP size_t __cdecl _get_sbh_threshold(void);
 _ACRTIMP int    __cdecl _set_sbh_threshold(size_t size);

[PATCH 2/2] include/msvcrt: Add __WINE_(ALLOC_SIZE|DEALLOC|MALLOC) attributes to _aligned_malloc functions.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=127206

Your paranoid android.

=== debian11 (32 bit report) ===

msvcp140_atomic_wait:
Unhandled exception: page fault on execute access to 0x00000000 in 32-bit code (0x00000000).

[Alex Henrie (＠alexhenrie)]

I like the idea here, but I don't think strdup and wcsdup need to be annotated. Since they are standard library functions, the compiler already knows that they work like malloc.

[Brendan Shanks (＠bshanks)]

On Tue Dec  6 04:44:57 2022 +0000, Alex Henrie wrote:
> I like the idea here, but I don't think strdup and wcsdup need to be
> annotated. Since they are standard library functions, the compiler
> already knows that they work like malloc.

I did some simple tests and it seems like they actually are needed, maybe because of the underscore prefix on the actual functions (`_strdup()` and `_wcsdup()`)

[Alex Henrie (＠alexhenrie)]

On Tue Dec  6 04:44:57 2022 +0000, Brendan Shanks wrote:
> I did some simple tests and it seems like they actually are needed,
> maybe because of the underscore prefix on the actual functions
> (`_strdup()` and `_wcsdup()`)

Sorry, I overlooked the underscore. You are absolutely right, they should be annotated. Could you make that more clear by changing the commit message and merge request title to include the underscores?