[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.
From: Vibhav Pant vibhavp@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;
From: Vibhav Pant vibhavp@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);
From: Vibhav Pant vibhavp@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"])])
From: Vibhav Pant vibhavp@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;
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?
-
Marvin -
Vibhav Pant -
Vibhav Pant (＠vibhavp)