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

[Brendan Shanks (＠bshanks)]

--
  v2: include/msvcrt: Add __WINE_(ALLOC_SIZE|DEALLOC|MALLOC) attributes to _aligned_malloc functions.
      include/msvcrt: Add __WINE_(DEALLOC|MALLOC) attributes to _strdup and _wcsdup.

https://gitlab.winehq.org/wine/wine/-/merge_requests/1686

[PATCH v2 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 v2 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 v2 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=127229

Your paranoid android.

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

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

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

[Alex Henrie (＠alexhenrie)]

Because stdlib.h is now indirectly included in dlls/msvcp140_atomic_wait/main.c, you will have to change __std_create_threadpool_work to not use the reserved variable name `environ`. (This is why the tests are failing on GitLab.) In CreateThreadpoolWork that variable is called `environment`, which I think would be a good name in __std_create_threadpool_work too.

In my opinion, the variable rename would be best done in a separate commit before annotating the CRT functions.

[PATCH v2 0/3] MR1686: include/msvcrt: Add __WINE_(ALLOC_SIZE|DEALLOC|MALLOC) attributes to _strdup, _wcsdup, _aligned_malloc

[Brendan Shanks (＠bshanks)]

On Wed Dec  7 16:18:44 2022 +0000, Alex Henrie wrote:
> Because stdlib.h is now indirectly included in
> dlls/msvcp140_atomic_wait/main.c, you will have to change
> __std_create_threadpool_work to not use the reserved variable name
> `environ`. (This is why the tests are failing on GitLab.) In
> CreateThreadpoolWork that variable is called `environment`, which I
> think would be a good name in __std_create_threadpool_work too.
> In my opinion, the variable rename would be best done in a separate
> commit before annotating the CRT functions.

Ah thanks for figuring that out, I'll push a new version.
Maybe we should look into whether plain `environ` should be defined by default, MS doesn't document it (just `_environ`) and I see it gated behind an ifdef in the Windows SDK.