[PATCH v2 0/4] MR6623: Draft: include/winnt.h: Add support for thread safety annotation attributes.
This MR introduces support for [thread safety annotations,](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#conditional-locks) currently supported by Clang. The feature is similar to `__WINE_MALLOC` attributes, allowing functions, data members and global variables to be optionally annotated with synchronization details. As a proof of concept, I have successfully annotated `dlls/ntdll` code, and it should build without any warnings. Because the feature was designed as a C++ extension first, a bit of stubbing is required to annotate struct fields about what lock they are guarded by. The macros `WINE_DECLARE_LOCK_FIELD_STUB` and `WINE_DEFINE_LOCK_FIELD_STUB` take care of that. For instance, in `dlls/ntdll/threadpool.c`: ```c WINE_DECLARE_LOCK_FIELD_STUB(threadpool_group, CRITICAL_SECTION, cs); struct threadpool_group { LONG refcount; BOOL shutdown; CRITICAL_SECTION cs; /* list of group members, locked via .cs */ struct list members __WINE_FIELD_GUARDED_BY(threadpool_group, cs); }; WINE_DEFINE_LOCK_FIELD_STUB(threadpool_group, CRITICAL_SECTION, cs); ``` Clang will therefore warn if `members` is accessed without holding `cs`. Note that the analyzer does not support conditional locking. For that reason, functions that hold and release a lock conditionally have been marked with `__WINE_NO_THREAD_SAFETY_ANALYSIS` to pre-empt false positives from Clang. -- v2: ntdll: Add thread safety annotations. configure: Enable -Wthread-safety-analysis, -Wthread-safety-attributes warnings. include/winternl.h: Add thread safety annotations to several NT synchronization primitives. include/winnt.h: Add macros for thread safety annotations. https://gitlab.winehq.org/wine/wine/-/merge_requests/6623
From: Vibhav Pant <vibhavp(a)gmail.com> --- include/winnt.h | 100 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 1 deletion(-) diff --git a/include/winnt.h b/include/winnt.h index 55f0c396041..54969557c7f 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -227,6 +227,104 @@ extern "C" { #define __WINE_MALLOC #endif +/* Thread safety annotations, currently supported by Clang. */ +#define HAVE_THREAD_ANNOTATION_ATTR(a) (defined( __clang__) && __has_attribute(a)) + +#if HAVE_THREAD_ANNOTATION_ATTR(no_thread_safety_analysis) +# define __WINE_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) +# else +# define __WINE_NO_THREAD_SAFETY_ANALYSIS +#endif + +/* Used to annotate structs that represent a lockable resource. */ +#if HAVE_THREAD_ANNOTATION_ATTR(capability) +# define __WINE_LOCKABLE(l) __attribute__((capability(l))) +#else +# define __WINE_LOCKABLE(l) +#endif + +/* Used for functions that will acquire the given lock without releasing it + * (i.e, the lock should not be held before the call, and will be held after the function returns). */ +#if HAVE_THREAD_ANNOTATION_ATTR(acquire_capability) +# define __WINE_ACQUIRE(...) __attribute__((acquire_capability(__VA_ARGS__))) +#else +# define __WINE_ACQUIRE(...) +#endif + +/* Used for functions that may acquire the given lock. The first argument should be the value + * returned when a lock was successfully acquired. */ +#if HAVE_THREAD_ANNOTATION_ATTR(try_acquire_capability) +# define __WINE_TRY_ACQUIRE(...) __attribute__((try_acquire_capability(__VA_ARGS__))) +#else +# define __WINE_TRY_ACQUIRE(...) +#endif + +#if HAVE_THREAD_ANNOTATION_ATTR(try_acquire_shared_capability) +# define __WINE_TRY_ACQUIRE_SHARED(...) __attribute__((try_acquire_shared_capability(__VA_ARGS__))) +#else +# define __WINE_TRY_ACQUIRE_SHARED(...) +#endif + +/* Used for functions that will release the given lock. */ +#if HAVE_THREAD_ANNOTATION_ATTR(release_capability) +# define __WINE_RELEASE(...) __attribute__((release_capability(__VA_ARGS__))) +#else +# define __WINE_RELEASE(...) +#endif + +/* The given lock will be held for the duration of the function, and therefore should not be held + * by the caller. */ +#if HAVE_THREAD_ANNOTATION_ATTR(locks_excluded) +# define __WINE_EXCLUDES(...) __attribute__((locks_excluded(__VA_ARGS__))) +#else +# define __WINE_EXCLUDES(...) +#endif + +/* The function requires that the caller hold the given lock. */ +#if HAVE_THREAD_ANNOTATION_ATTR(requires_capability) +# define __WINE_REQUIRES(...) __attribute__((requires_capability(__VA_ARGS__))) +#else +# define __WINE_REQUIRES(...) +#endif + +/* THe function returns the given lock. */ +#if HAVE_THREAD_ANNOTATION_ATTR(lock_returned) +# define __WINE_LOCK_RETURNED(l) __attribute__((lock_returned(l))) +#else +# define __WINE_LOCK_RETURNED(l) +#endif + +/* Access to this variable is guarded by the given lock. */ +#if HAVE_THREAD_ANNOTATION_ATTR(guarded_by) +# define __WINE_GUARDED_BY(l) __attribute__((guarded_by((l)))) +#else +# define __WINE_GUARDED_BY(l) +#endif + +/* Access to the data this pointer points to is guarded by the given lock. + * There's no restriction on the variable itself. */ +#if HAVE_THREAD_ANNOTATION_ATTR(pt_guarded_by) +# define __WINE_PT_GUARDED_BY(l) __attribute__((pt_guarded_by((l)))) +#else +# define __WINE_PT_GUARDED_BY(l) +#endif + +/* (pt_)guarded_by attributes are only currently supported for global variables and non-static class members + * in Clang. These macros allow us to declare (and then define) a dummy stub function for Clang corresponding + * to a struct field containing a lock-able type. Struct fields guarded by that lock should use the + * __WINE_FIELD_(PT_)GUARDED_BY macros. */ +#if HAVE_THREAD_ANNOTATION_ATTR(lock_returned) && HAVE_THREAD_ANNOTATION_ATTR(guarded_by) && HAVE_THREAD_ANNOTATION_ATTR(pt_guarded_by) +# define WINE_DECLARE_LOCK_FIELD_STUB(s, t, f) t *__wine_thread_safety_analysis_stub##s_##f(void *) +# define WINE_DEFINE_LOCK_FIELD_STUB(s, t, f) t *__wine_thread_safety_analysis_stub##s_get_##f(void *__v__) __WINE_LOCK_RETURNED(&((struct s *)__v__)->f) +# define __WINE_FIELD_GUARDED_BY(s, f) __attribute__((guarded_by((__wine_thread_safety_analysis_stub##s_##f)(NULL)))) +# define __WINE_FIELD_PT_GUARDED_BY(s, f) __attribute__((pt_guarded_by((__wine_thread_safety_analysis_stub##s_##f)(NULL)))) +#else +# define WINE_DECLARE_LOCK_FIELD_STUB(s, f) +# define WINE_DEFINE_LOCK_FIELD_STUB(s, f) +# define __WINE_FIELD_GUARDED_BY(s, f) +# define __WINE_FIELD_PT_GUARDED_BY(s, f) +#endif + /* Anonymous union/struct handling */ #ifndef NONAMELESSSTRUCT @@ -6195,7 +6293,7 @@ typedef struct _RTL_CRITICAL_SECTION_DEBUG #endif } RTL_CRITICAL_SECTION_DEBUG, *PRTL_CRITICAL_SECTION_DEBUG, RTL_RESOURCE_DEBUG, *PRTL_RESOURCE_DEBUG; -typedef struct _RTL_CRITICAL_SECTION { +typedef struct __WINE_LOCKABLE("RTL_CRITICAL_SECTION") _RTL_CRITICAL_SECTION { PRTL_CRITICAL_SECTION_DEBUG DebugInfo; LONG LockCount; LONG RecursionCount; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6623
From: Vibhav Pant <vibhavp(a)gmail.com> --- include/winternl.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/include/winternl.h b/include/winternl.h index 4f24a921bb1..00d8d9c8596 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -4706,9 +4706,9 @@ NTSYSAPI NTSTATUS WINAPI NtWriteRequestData(HANDLE,PLPC_MESSAGE,ULONG,PVOID,ULO NTSYSAPI NTSTATUS WINAPI NtWriteVirtualMemory(HANDLE,void*,const void*,SIZE_T,SIZE_T*); NTSYSAPI NTSTATUS WINAPI NtYieldExecution(void); NTSYSAPI NTSTATUS WINAPI RtlAbsoluteToSelfRelativeSD(PSECURITY_DESCRIPTOR,PSECURITY_DESCRIPTOR,PULONG); -NTSYSAPI void WINAPI RtlAcquirePebLock(void); -NTSYSAPI BYTE WINAPI RtlAcquireResourceExclusive(LPRTL_RWLOCK,BYTE); -NTSYSAPI BYTE WINAPI RtlAcquireResourceShared(LPRTL_RWLOCK,BYTE); +NTSYSAPI void WINAPI RtlAcquirePebLock(void) __WINE_ACQUIRE(NtCurrentTeb()->Peb->FastPebLock); +NTSYSAPI BYTE WINAPI RtlAcquireResourceExclusive(LPRTL_RWLOCK rwl,BYTE) __WINE_TRY_ACQUIRE(1, &rwl->rtlCS); +NTSYSAPI BYTE WINAPI RtlAcquireResourceShared(LPRTL_RWLOCK rwl,BYTE) __WINE_TRY_ACQUIRE_SHARED(1, &rwl->rtlCS); NTSYSAPI void WINAPI RtlAcquireSRWLockExclusive(RTL_SRWLOCK*); NTSYSAPI void WINAPI RtlAcquireSRWLockShared(RTL_SRWLOCK*); NTSYSAPI NTSTATUS WINAPI RtlActivateActivationContext(DWORD,HANDLE,ULONG_PTR*); @@ -4819,7 +4819,7 @@ NTSYSAPI void WINAPI RtlDumpResource(LPRTL_RWLOCK); NTSYSAPI NTSTATUS WINAPI RtlDuplicateUnicodeString(int,const UNICODE_STRING*,UNICODE_STRING*); NTSYSAPI NTSTATUS WINAPI RtlEmptyAtomTable(RTL_ATOM_TABLE,BOOLEAN); NTSYSAPI PVOID WINAPI RtlEncodePointer(PVOID); -NTSYSAPI NTSTATUS WINAPI RtlEnterCriticalSection(RTL_CRITICAL_SECTION *); +NTSYSAPI NTSTATUS WINAPI RtlEnterCriticalSection(RTL_CRITICAL_SECTION *cs) __WINE_ACQUIRE(cs); NTSYSAPI void WINAPI RtlEraseUnicodeString(UNICODE_STRING*); NTSYSAPI NTSTATUS WINAPI RtlEqualComputerName(const UNICODE_STRING*,const UNICODE_STRING*); NTSYSAPI NTSTATUS WINAPI RtlEqualDomainName(const UNICODE_STRING*,const UNICODE_STRING*); @@ -4953,7 +4953,7 @@ NTSYSAPI BOOLEAN WINAPI RtlIsValidHandle(const RTL_HANDLE_TABLE *, const RTL_H NTSYSAPI BOOLEAN WINAPI RtlIsValidIndexHandle(const RTL_HANDLE_TABLE *, ULONG Index, RTL_HANDLE **); NTSYSAPI BOOLEAN WINAPI RtlIsValidLocaleName(const WCHAR*,ULONG); NTSYSAPI NTSTATUS WINAPI RtlLcidToLocaleName(LCID,UNICODE_STRING*,ULONG,BOOLEAN); -NTSYSAPI NTSTATUS WINAPI RtlLeaveCriticalSection(RTL_CRITICAL_SECTION *); +NTSYSAPI NTSTATUS WINAPI RtlLeaveCriticalSection(RTL_CRITICAL_SECTION *cs) __WINE_RELEASE(cs); NTSYSAPI DWORD WINAPI RtlLengthRequiredSid(DWORD); NTSYSAPI ULONG WINAPI RtlLengthSecurityDescriptor(PSECURITY_DESCRIPTOR); NTSYSAPI DWORD WINAPI RtlLengthSid(PSID); @@ -5011,7 +5011,7 @@ NTSYSAPI PVOID WINAPI RtlReAllocateHeap(HANDLE,ULONG,PVOID,SIZE_T) __WINE_AL NTSYSAPI NTSTATUS WINAPI RtlRegisterWait(PHANDLE,HANDLE,RTL_WAITORTIMERCALLBACKFUNC,PVOID,ULONG,ULONG); NTSYSAPI void WINAPI RtlReleaseActivationContext(HANDLE); NTSYSAPI void WINAPI RtlReleasePath(PWSTR); -NTSYSAPI void WINAPI RtlReleasePebLock(void); +NTSYSAPI void WINAPI RtlReleasePebLock(void) __WINE_RELEASE(NtCurrentTeb()->Peb->FastPebLock); NTSYSAPI void WINAPI RtlReleaseRelativeName(RTL_RELATIVE_NAME*); NTSYSAPI void WINAPI RtlReleaseResource(LPRTL_RWLOCK); NTSYSAPI void WINAPI RtlReleaseSRWLockExclusive(RTL_SRWLOCK*); @@ -5049,7 +5049,7 @@ NTSYSAPI void WINAPI RtlSetUnhandledExceptionFilter(PRTL_EXCEPTION_FILTER); NTSYSAPI BOOLEAN WINAPI RtlSetUserFlagsHeap(HANDLE,ULONG,void*,ULONG,ULONG); NTSYSAPI BOOLEAN WINAPI RtlSetUserValueHeap(HANDLE,ULONG,void*,void*); NTSYSAPI SIZE_T WINAPI RtlSizeHeap(HANDLE,ULONG,const void*); -NTSYSAPI NTSTATUS WINAPI RtlSleepConditionVariableCS(RTL_CONDITION_VARIABLE*,RTL_CRITICAL_SECTION*,const LARGE_INTEGER*); +NTSYSAPI NTSTATUS WINAPI RtlSleepConditionVariableCS(RTL_CONDITION_VARIABLE*,RTL_CRITICAL_SECTION*cs,const LARGE_INTEGER*) __WINE_REQUIRES(cs); NTSYSAPI NTSTATUS WINAPI RtlSleepConditionVariableSRW(RTL_CONDITION_VARIABLE*,RTL_SRWLOCK*,const LARGE_INTEGER*,ULONG); NTSYSAPI NTSTATUS WINAPI RtlStringFromGUID(REFGUID,PUNICODE_STRING); NTSYSAPI LPDWORD WINAPI RtlSubAuthoritySid(PSID,DWORD); @@ -5062,7 +5062,7 @@ NTSYSAPI BOOLEAN WINAPI RtlTimeToSecondsSince1980(const LARGE_INTEGER *,LPDWOR NTSYSAPI void WINAPI RtlTimeToTimeFields(const LARGE_INTEGER*,PTIME_FIELDS); NTSYSAPI BOOLEAN WINAPI RtlTryAcquireSRWLockExclusive(RTL_SRWLOCK *); NTSYSAPI BOOLEAN WINAPI RtlTryAcquireSRWLockShared(RTL_SRWLOCK *); -NTSYSAPI BOOL WINAPI RtlTryEnterCriticalSection(RTL_CRITICAL_SECTION *); +NTSYSAPI BOOL WINAPI RtlTryEnterCriticalSection(RTL_CRITICAL_SECTION *cs) __WINE_TRY_ACQUIRE(TRUE, cs); NTSYSAPI NTSTATUS WINAPI RtlUTF8ToUnicodeN(WCHAR*,DWORD,DWORD*,const char*,DWORD); NTSYSAPI DWORD WINAPI RtlUnicodeStringToAnsiSize(const UNICODE_STRING*); NTSYSAPI NTSTATUS WINAPI RtlUnicodeStringToAnsiString(PANSI_STRING,PCUNICODE_STRING,BOOLEAN); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6623
From: Vibhav Pant <vibhavp(a)gmail.com> --- configure.ac | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configure.ac b/configure.ac index ea406572a58..e2a43b841a7 100644 --- a/configure.ac +++ b/configure.ac @@ -955,6 +955,8 @@ This is an error since --enable-archs=$wine_arch was requested.])]) WINE_TRY_PE_CFLAGS([-Wlogical-op]) WINE_TRY_PE_CFLAGS([-Wabsolute-value]) WINE_TRY_PE_CFLAGS([-Wenum-enum-conversion],[:],WINE_TRY_PE_CFLAGS([-Wenum-conversion])) + WINE_TRY_PE_CFLAGS([-Wthread-safety-analysis]) + WINE_TRY_PE_CFLAGS([-Wthread-safety-attributes]) dnl GCC can't handle large files when -Wmisleading-indentation is enabled (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89549) WINE_TRY_PE_CFLAGS([-flarge-source-files -Wmisleading-indentation],[AS_VAR_APPEND(${wine_arch}_EXTRACFLAGS,[" -Wno-misleading-indentation"])]) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6623
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/ntdll/atom.c | 25 +++++++++++++++--------- dlls/ntdll/env.c | 16 ++++++++-------- dlls/ntdll/heap.c | 6 +++--- dlls/ntdll/loader.c | 9 +++++---- dlls/ntdll/sync.c | 4 ++-- dlls/ntdll/thread.c | 4 ++-- dlls/ntdll/threadpool.c | 42 +++++++++++++++++++++++++---------------- 7 files changed, 62 insertions(+), 44 deletions(-) diff --git a/dlls/ntdll/atom.c b/dlls/ntdll/atom.c index 650210cc2cc..d0e0da5f38d 100644 --- a/dlls/ntdll/atom.c +++ b/dlls/ntdll/atom.c @@ -49,7 +49,7 @@ struct atom_handle }; -static NTSTATUS lock_atom_table( RTL_ATOM_TABLE table ) +static NTSTATUS lock_atom_table( RTL_ATOM_TABLE table ) __WINE_TRY_ACQUIRE(STATUS_SUCCESS, &table->CriticalSection) { if (!table) return STATUS_INVALID_PARAMETER; if (table->Signature != TABLE_SIGNATURE) return STATUS_INVALID_PARAMETER; @@ -57,7 +57,7 @@ static NTSTATUS lock_atom_table( RTL_ATOM_TABLE table ) return STATUS_SUCCESS; } -static void unlock_atom_table( RTL_ATOM_TABLE table ) +static void unlock_atom_table( RTL_ATOM_TABLE table ) __WINE_RELEASE(&table->CriticalSection) { RtlLeaveCriticalSection( &table->CriticalSection ); } @@ -155,7 +155,7 @@ done: /****************************************************************** * RtlDeleteAtomFromAtomTable (NTDLL.@) */ -NTSTATUS WINAPI RtlDeleteAtomFromAtomTable( RTL_ATOM_TABLE table, RTL_ATOM atom ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI RtlDeleteAtomFromAtomTable( RTL_ATOM_TABLE table, RTL_ATOM atom ) { NTSTATUS status; struct atom_handle *handle; @@ -205,8 +205,10 @@ static ULONG integral_atom_name(WCHAR* buffer, ULONG len, RTL_ATOM atom) /****************************************************************** * RtlQueryAtomInAtomTable (NTDLL.@) */ -NTSTATUS WINAPI RtlQueryAtomInAtomTable( RTL_ATOM_TABLE table, RTL_ATOM atom, ULONG* ref, - ULONG* pin, WCHAR* name, ULONG* len ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI RtlQueryAtomInAtomTable( RTL_ATOM_TABLE table, + RTL_ATOM atom, ULONG *ref, + ULONG *pin, WCHAR *name, + ULONG *len ) { NTSTATUS status; struct atom_handle *handle; @@ -290,7 +292,9 @@ NTSTATUS WINAPI RtlDestroyAtomTable( RTL_ATOM_TABLE table ) /****************************************************************** * RtlAddAtomToAtomTable (NTDLL.@) */ -NTSTATUS WINAPI RtlAddAtomToAtomTable( RTL_ATOM_TABLE table, const WCHAR* name, RTL_ATOM* atom ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI RtlAddAtomToAtomTable( RTL_ATOM_TABLE table, + const WCHAR *name, + RTL_ATOM *atom ) { NTSTATUS status; size_t len; @@ -308,7 +312,9 @@ NTSTATUS WINAPI RtlAddAtomToAtomTable( RTL_ATOM_TABLE table, const WCHAR* name, /****************************************************************** * RtlLookupAtomInAtomTable (NTDLL.@) */ -NTSTATUS WINAPI RtlLookupAtomInAtomTable( RTL_ATOM_TABLE table, const WCHAR* name, RTL_ATOM* atom ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI RtlLookupAtomInAtomTable( RTL_ATOM_TABLE table, + const WCHAR *name, + RTL_ATOM *atom ) { RTL_ATOM_TABLE_ENTRY *entry; NTSTATUS status; @@ -336,7 +342,8 @@ NTSTATUS WINAPI RtlLookupAtomInAtomTable( RTL_ATOM_TABLE table, const WCHAR* nam /****************************************************************** * RtlEmptyAtomTable (NTDLL.@) */ -NTSTATUS WINAPI RtlEmptyAtomTable( RTL_ATOM_TABLE table, BOOLEAN delete_pinned ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI RtlEmptyAtomTable( RTL_ATOM_TABLE table, + BOOLEAN delete_pinned ) { struct atom_handle *handle; NTSTATUS status; @@ -368,7 +375,7 @@ NTSTATUS WINAPI RtlEmptyAtomTable( RTL_ATOM_TABLE table, BOOLEAN delete_pinned ) /****************************************************************** * RtlPinAtomInAtomTable (NTDLL.@) */ -NTSTATUS WINAPI RtlPinAtomInAtomTable( RTL_ATOM_TABLE table, RTL_ATOM atom ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI RtlPinAtomInAtomTable( RTL_ATOM_TABLE table, RTL_ATOM atom ) { struct atom_handle *handle; NTSTATUS status; diff --git a/dlls/ntdll/env.c b/dlls/ntdll/env.c index 720597dcaf9..9af6f36c7cb 100644 --- a/dlls/ntdll/env.c +++ b/dlls/ntdll/env.c @@ -181,9 +181,8 @@ static LPCWSTR ENV_FindVariable(PCWSTR var, PCWSTR name, unsigned namelen) * all chars (except the null) are written and success is returned * (behavior of Win2k at least) */ -NTSTATUS WINAPI RtlQueryEnvironmentVariable_U(PWSTR env, - PUNICODE_STRING name, - PUNICODE_STRING value) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI +RtlQueryEnvironmentVariable_U( PWSTR env, PUNICODE_STRING name, PUNICODE_STRING value ) { NTSTATUS nts = STATUS_VARIABLE_NOT_FOUND; PCWSTR var; @@ -224,8 +223,9 @@ NTSTATUS WINAPI RtlQueryEnvironmentVariable_U(PWSTR env, /****************************************************************** * RtlQueryEnvironmentVariable [NTDLL.@] */ -NTSTATUS WINAPI RtlQueryEnvironmentVariable( WCHAR *env, const WCHAR *name, SIZE_T namelen, - WCHAR *value, SIZE_T value_length, SIZE_T *return_length ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI +RtlQueryEnvironmentVariable( WCHAR *env, const WCHAR *name, SIZE_T namelen, WCHAR *value, + SIZE_T value_length, SIZE_T *return_length ) { NTSTATUS nts = STATUS_VARIABLE_NOT_FOUND; SIZE_T len = 0; @@ -291,7 +291,7 @@ void WINAPI RtlSetCurrentEnvironment(PWSTR new_env, PWSTR* old_env) * RtlSetEnvironmentVariable [NTDLL.@] */ NTSTATUS WINAPI RtlSetEnvironmentVariable(PWSTR* penv, PUNICODE_STRING name, - PUNICODE_STRING value) + PUNICODE_STRING value) __WINE_NO_THREAD_SAFETY_ANALYSIS { INT varlen, len, old_size; LPWSTR p, env; @@ -381,8 +381,8 @@ done: /****************************************************************************** * RtlExpandEnvironmentStrings (NTDLL.@) */ -NTSTATUS WINAPI RtlExpandEnvironmentStrings( const WCHAR *renv, WCHAR *src, SIZE_T src_len, - WCHAR *dst, SIZE_T count, SIZE_T *plen ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI RtlExpandEnvironmentStrings( + const WCHAR *renv, WCHAR *src, SIZE_T src_len, WCHAR *dst, SIZE_T count, SIZE_T *plen ) { SIZE_T len, total_size = 1; /* 1 for terminating '\0' */ LPCWSTR env, var; diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 164b8714459..789e67e21d8 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -580,13 +580,13 @@ static inline ULONG heap_get_flags( const struct heap *heap, ULONG flags ) return heap->flags | flags; } -static inline void heap_lock( struct heap *heap, ULONG flags ) +static inline void __WINE_NO_THREAD_SAFETY_ANALYSIS heap_lock( struct heap *heap, ULONG flags ) __WINE_ACQUIRE(&heap->cs) { if (flags & HEAP_NO_SERIALIZE) return; RtlEnterCriticalSection( &heap->cs ); } -static inline void heap_unlock( struct heap *heap, ULONG flags ) +static inline void __WINE_NO_THREAD_SAFETY_ANALYSIS heap_unlock( struct heap *heap, ULONG flags ) __WINE_RELEASE(&heap->cs) { if (flags & HEAP_NO_SERIALIZE) return; RtlLeaveCriticalSection( &heap->cs ); @@ -2313,7 +2313,7 @@ BOOLEAN WINAPI RtlLockHeap( HANDLE handle ) * Success: TRUE. The Heap is unlocked. * Failure: FALSE, if heap is invalid. */ -BOOLEAN WINAPI RtlUnlockHeap( HANDLE handle ) +BOOLEAN __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI RtlUnlockHeap( HANDLE handle ) { struct heap *heap; ULONG heap_flags; diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 2982b1e049b..53dd4bef342 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -2030,7 +2030,8 @@ NTSTATUS WINAPI LdrUnregisterDllNotification( void *cookie ) * Note: some flags are not implemented. * Flag 0x01 is used to raise exceptions on errors. */ -NTSTATUS WINAPI LdrLockLoaderLock( ULONG flags, ULONG *result, ULONG_PTR *magic ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI LdrLockLoaderLock( ULONG flags, ULONG *result, + ULONG_PTR *magic ) { if (flags & ~0x2) FIXME( "flags %lx not supported\n", flags ); @@ -2062,7 +2063,7 @@ NTSTATUS WINAPI LdrLockLoaderLock( ULONG flags, ULONG *result, ULONG_PTR *magic /****************************************************************** * LdrUnlockLoaderUnlock (NTDLL.@) */ -NTSTATUS WINAPI LdrUnlockLoaderLock( ULONG flags, ULONG_PTR magic ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI LdrUnlockLoaderLock( ULONG flags, ULONG_PTR magic ) { if (magic) { @@ -4265,7 +4266,7 @@ static void (WINAPI *pWow64LdrpInitialize)( CONTEXT *ctx ); void (WINAPI *pWow64PrepareForException)( EXCEPTION_RECORD *rec, CONTEXT *context ) = NULL; -static void init_wow64( CONTEXT *context ) +static void init_wow64( CONTEXT *context ) __WINE_RELEASE(&loader_section) { if (!imports_fixup_done) { @@ -4374,7 +4375,7 @@ static void release_address_space(void) * Attach to all the loaded dlls. * If this is the first time, perform the full process initialization. */ -void loader_init( CONTEXT *context, void **entry ) +void __WINE_NO_THREAD_SAFETY_ANALYSIS loader_init( CONTEXT *context, void **entry ) __WINE_EXCLUDES(&loader_section) { static int attach_done; NTSTATUS status; diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c index 522ce0a2142..ac9375867f2 100644 --- a/dlls/ntdll/sync.c +++ b/dlls/ntdll/sync.c @@ -349,7 +349,7 @@ NTSTATUS WINAPI RtlpUnWaitCriticalSection( RTL_CRITICAL_SECTION *crit ) /****************************************************************************** * RtlEnterCriticalSection (NTDLL.@) */ -NTSTATUS WINAPI RtlEnterCriticalSection( RTL_CRITICAL_SECTION *crit ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI RtlEnterCriticalSection( RTL_CRITICAL_SECTION *crit ) { if (crit->SpinCount) { @@ -431,7 +431,7 @@ BOOL WINAPI RtlIsCriticalSectionLockedByThread( RTL_CRITICAL_SECTION *crit ) /****************************************************************************** * RtlLeaveCriticalSection (NTDLL.@) */ -NTSTATUS WINAPI RtlLeaveCriticalSection( RTL_CRITICAL_SECTION *crit ) +NTSTATUS __WINE_NO_THREAD_SAFETY_ANALYSIS WINAPI RtlLeaveCriticalSection( RTL_CRITICAL_SECTION *crit ) { if (--crit->RecursionCount) { diff --git a/dlls/ntdll/thread.c b/dlls/ntdll/thread.c index bc6c2ddb741..962a6a95044 100644 --- a/dlls/ntdll/thread.c +++ b/dlls/ntdll/thread.c @@ -466,12 +466,12 @@ static RTL_CRITICAL_SECTION fls_section = { &fls_critsect_debug, -1, 0, 0, 0, 0 #define MAX_FLS_DATA_COUNT 0xff0 -static void lock_fls_data(void) +static void lock_fls_data(void) __WINE_ACQUIRE(&fls_section) { RtlEnterCriticalSection( &fls_section ); } -static void unlock_fls_data(void) +static void unlock_fls_data(void) __WINE_RELEASE(&fls_section) { RtlLeaveCriticalSection( &fls_section ); } diff --git a/dlls/ntdll/threadpool.c b/dlls/ntdll/threadpool.c index 81cf894d943..5a585ed08dc 100644 --- a/dlls/ntdll/threadpool.c +++ b/dlls/ntdll/threadpool.c @@ -82,15 +82,17 @@ struct queue_timer HANDLE event; /* removal event */ }; +WINE_DECLARE_LOCK_FIELD_STUB(timer_queue, RTL_CRITICAL_SECTION, cs); struct timer_queue { DWORD magic; RTL_CRITICAL_SECTION cs; - struct list timers; /* sorted by expiration time */ + struct list timers __WINE_FIELD_GUARDED_BY(timer_queue, cs); /* sorted by expiration time */ BOOL quit; /* queue should be deleted; once set, never unset */ HANDLE event; HANDLE thread; }; +WINE_DEFINE_LOCK_FIELD_STUB(timer_queue, RTL_CRITICAL_SECTION, cs); /* * Object-oriented thread pooling API @@ -99,6 +101,7 @@ struct timer_queue #define THREADPOOL_WORKER_TIMEOUT 5000 #define MAXIMUM_WAITQUEUE_OBJECTS (MAXIMUM_WAIT_OBJECTS - 1) +WINE_DECLARE_LOCK_FIELD_STUB(threadpool, CRITICAL_SECTION, cs); /* internal threadpool representation */ struct threadpool { @@ -107,7 +110,7 @@ struct threadpool BOOL shutdown; CRITICAL_SECTION cs; /* Pools of work items, locked via .cs, order matches TP_CALLBACK_PRIORITY - high, normal, low. */ - struct list pools[3]; + struct list pools[3] __WINE_FIELD_GUARDED_BY(threadpool, cs); RTL_CONDITION_VARIABLE update_event; /* information about worker threads, locked via .cs */ int max_workers; @@ -117,6 +120,7 @@ struct threadpool HANDLE compl_port; TP_POOL_STACK_INFORMATION stack_info; }; +WINE_DEFINE_LOCK_FIELD_STUB(threadpool, CRITICAL_SECTION, cs); enum threadpool_objtype { @@ -227,14 +231,16 @@ struct threadpool_instance }; /* internal threadpool group representation */ +WINE_DECLARE_LOCK_FIELD_STUB(threadpool_group, CRITICAL_SECTION, cs); struct threadpool_group { LONG refcount; BOOL shutdown; CRITICAL_SECTION cs; /* list of group members, locked via .cs */ - struct list members; + struct list members __WINE_FIELD_GUARDED_BY(threadpool_group, cs); }; +WINE_DEFINE_LOCK_FIELD_STUB(threadpool_group, CRITICAL_SECTION, cs); /* global timerqueue object */ static RTL_CRITICAL_SECTION_DEBUG timerqueue_debug; @@ -1053,7 +1059,7 @@ NTSTATUS WINAPI RtlDeleteTimer(HANDLE TimerQueue, HANDLE Timer, /*********************************************************************** * timerqueue_thread_proc (internal) */ -static void CALLBACK timerqueue_thread_proc( void *param ) +static void CALLBACK timerqueue_thread_proc( void *param ) __WINE_EXCLUDES(&timerqueue.cs) { ULONGLONG timeout_lower, timeout_upper, new_timeout; struct threadpool_object *other_timer; @@ -1170,7 +1176,7 @@ static NTSTATUS tp_new_worker_thread( struct threadpool *pool ) * Acquires a lock on the global timerqueue. When the lock is acquired * successfully, it is guaranteed that the timer thread is running. */ -static NTSTATUS tp_timerqueue_lock( struct threadpool_object *timer ) +static NTSTATUS tp_timerqueue_lock( struct threadpool_object *timer ) __WINE_EXCLUDES(&timerqueue.cs) { NTSTATUS status = STATUS_SUCCESS; assert( timer->type == TP_OBJECT_TYPE_TIMER ); @@ -1212,7 +1218,7 @@ static NTSTATUS tp_timerqueue_lock( struct threadpool_object *timer ) * * Releases a lock on the global timerqueue. */ -static void tp_timerqueue_unlock( struct threadpool_object *timer ) +static void tp_timerqueue_unlock( struct threadpool_object *timer ) __WINE_EXCLUDES(&timerqueue.cs) { assert( timer->type == TP_OBJECT_TYPE_TIMER ); @@ -1424,7 +1430,7 @@ static void CALLBACK waitqueue_thread_proc( void *param ) /*********************************************************************** * tp_waitqueue_lock (internal) */ -static NTSTATUS tp_waitqueue_lock( struct threadpool_object *wait ) +static NTSTATUS tp_waitqueue_lock( struct threadpool_object *wait ) __WINE_EXCLUDES(&waitqueue.cs) { struct waitqueue_bucket *bucket; NTSTATUS status; @@ -1502,7 +1508,7 @@ out: /*********************************************************************** * tp_waitqueue_unlock (internal) */ -static void tp_waitqueue_unlock( struct threadpool_object *wait ) +static void tp_waitqueue_unlock( struct threadpool_object *wait ) __WINE_EXCLUDES(&waitqueue.cs) { assert( wait->type == TP_OBJECT_TYPE_WAIT ); @@ -1521,7 +1527,7 @@ static void tp_waitqueue_unlock( struct threadpool_object *wait ) RtlLeaveCriticalSection( &waitqueue.cs ); } -static void CALLBACK ioqueue_thread_proc( void *param ) +static void CALLBACK ioqueue_thread_proc( void *param ) __WINE_EXCLUDES(&ioqueue.cs) { struct io_completion *completion; struct threadpool_object *io; @@ -1617,7 +1623,7 @@ static void CALLBACK ioqueue_thread_proc( void *param ) RtlExitUserThread( 0 ); } -static NTSTATUS tp_ioqueue_lock( struct threadpool_object *io, HANDLE file ) +static NTSTATUS tp_ioqueue_lock( struct threadpool_object *io, HANDLE file ) __WINE_EXCLUDES(&ioqueue.cs) { NTSTATUS status = STATUS_SUCCESS; @@ -1824,7 +1830,7 @@ static NTSTATUS tp_threadpool_lock( struct threadpool **out, TP_CALLBACK_ENVIRON * * Releases a lock on a threadpool. */ -static void tp_threadpool_unlock( struct threadpool *pool ) +static void tp_threadpool_unlock( struct threadpool *pool ) __WINE_EXCLUDES(&pool->cs) { RtlEnterCriticalSection( &pool->cs ); pool->objcount--; @@ -1990,6 +1996,7 @@ static void tp_object_prio_queue( struct threadpool_object *object ) * function has to be VOID because TpPostWork can never fail on Windows. */ static void tp_object_submit( struct threadpool_object *object, BOOL signaled ) + __WINE_EXCLUDES( &object->pool->cs ) { struct threadpool *pool = object->pool; NTSTATUS status = STATUS_UNSUCCESSFUL; @@ -2028,7 +2035,7 @@ static void tp_object_submit( struct threadpool_object *object, BOOL signaled ) * * Cancels all currently pending callbacks for a specific object. */ -static void tp_object_cancel( struct threadpool_object *object ) +static void tp_object_cancel( struct threadpool_object *object ) __WINE_EXCLUDES(&object->pool->cs) { struct threadpool *pool = object->pool; LONG pending_callbacks = 0; @@ -2073,7 +2080,7 @@ static BOOL object_is_finished( struct threadpool_object *object, BOOL group ) * Waits until all pending and running callbacks of a specific object * have been processed. */ -static void tp_object_wait( struct threadpool_object *object, BOOL group_wait ) +static void tp_object_wait( struct threadpool_object *object, BOOL group_wait ) __WINE_EXCLUDES(&object->pool->cs) { struct threadpool *pool = object->pool; @@ -2088,7 +2095,7 @@ static void tp_object_wait( struct threadpool_object *object, BOOL group_wait ) RtlLeaveCriticalSection( &pool->cs ); } -static void tp_ioqueue_unlock( struct threadpool_object *io ) +static void tp_ioqueue_unlock( struct threadpool_object *io ) __WINE_EXCLUDES(&ioqueue.cs) { assert( io->type == TP_OBJECT_TYPE_IO ); @@ -2182,7 +2189,9 @@ static struct list *threadpool_get_next_item( const struct threadpool *pool ) * Executes a threadpool object callback, object->pool->cs has to be * held. */ -static void tp_object_execute( struct threadpool_object *object, BOOL wait_thread ) +static void __WINE_NO_THREAD_SAFETY_ANALYSIS tp_object_execute( struct threadpool_object *object, + BOOL wait_thread ) + __WINE_RELEASE(&object->pool->cs) { TP_CALLBACK_INSTANCE *callback_instance; struct threadpool_instance instance; @@ -2338,7 +2347,8 @@ skip_cleanup: /*********************************************************************** * threadpool_worker_proc (internal) */ -static void CALLBACK threadpool_worker_proc( void *param ) +static void CALLBACK __WINE_NO_THREAD_SAFETY_ANALYSIS threadpool_worker_proc( void *param ) + __WINE_EXCLUDES(((struct threadpool *)param )->cs) { struct threadpool *pool = param; LARGE_INTEGER timeout; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6623
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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=148886 Your paranoid android. === debian11 (build log) === ../wine/dlls/ntdll/threadpool.c:85:67: error: macro "WINE_DECLARE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 ../wine/dlls/ntdll/threadpool.c:95:66: error: macro "WINE_DEFINE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 ../wine/dlls/ntdll/threadpool.c:104:62: error: macro "WINE_DECLARE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 ../wine/dlls/ntdll/threadpool.c:123:61: error: macro "WINE_DEFINE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 ../wine/dlls/ntdll/threadpool.c:234:68: error: macro "WINE_DECLARE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 ../wine/dlls/ntdll/threadpool.c:243:67: error: macro "WINE_DEFINE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 Task: The win32 Wine build failed === debian11b (build log) === ../wine/dlls/ntdll/threadpool.c:85:67: error: macro "WINE_DECLARE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 ../wine/dlls/ntdll/threadpool.c:95:66: error: macro "WINE_DEFINE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 ../wine/dlls/ntdll/threadpool.c:104:62: error: macro "WINE_DECLARE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 ../wine/dlls/ntdll/threadpool.c:123:61: error: macro "WINE_DEFINE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 ../wine/dlls/ntdll/threadpool.c:234:68: error: macro "WINE_DECLARE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 ../wine/dlls/ntdll/threadpool.c:243:67: error: macro "WINE_DEFINE_LOCK_FIELD_STUB" passed 3 arguments, but takes just 2 Task: The wow64 Wine build failed
Clang is reporting that `alloc_console` in `dlls/kernelbase/console.c` is improperly holding `&console_section`. It's likely referencing[ this line](https://gitlab.winehq.org/wine/wine/-/blob/7ee99608f469723bafadb28ef0ebd2063...), where the function returns FALSE on an allocation failure without leaving `console_section`. Rewriting the code to ``` if (!(console_si.lpAttributeList = HeapAlloc( GetProcessHeap(), 0, size ))) { RtlLeaveCriticalSection( &console_section ); return FALSE; } ``` fixes the warning. Is this intended, and clang is just displaying a false positive? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6623#note_84131
participants (3)
-
Marvin -
Vibhav Pant -
Vibhav Pant (＠vibhavp)