[PATCH v2 0/1] MR2046: server: Annotate allocation functions with __WINE_(ALLOC_SIZE|DEALLOC|MALLOC).
-- v2: server: Annotate allocation functions with __WINE_(ALLOC_SIZE|DEALLOC|MALLOC). https://gitlab.winehq.org/wine/wine/-/merge_requests/2046
From: Alex Henrie <alexhenrie24(a)gmail.com> --- server/file.h | 2 +- server/object.h | 6 +++--- server/unicode.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/server/file.h b/server/file.h index 0ffe0e2c8dc..210fcec5e78 100644 --- a/server/file.h +++ b/server/file.h @@ -102,7 +102,7 @@ extern obj_handle_t lock_fd( struct fd *fd, file_pos_t offset, file_pos_t count, extern void unlock_fd( struct fd *fd, file_pos_t offset, file_pos_t count ); extern void allow_fd_caching( struct fd *fd ); extern void set_fd_signaled( struct fd *fd, int signaled ); -extern char *dup_fd_name( struct fd *root, const char *name ); +extern char *dup_fd_name( struct fd *root, const char *name ) __WINE_DEALLOC(free) __WINE_MALLOC; extern void get_nt_name( struct fd *fd, struct unicode_str *name ); extern int default_fd_signaled( struct object *obj, struct wait_queue_entry *entry ); diff --git a/server/object.h b/server/object.h index f156f1d2f13..727972b9278 100644 --- a/server/object.h +++ b/server/object.h @@ -139,12 +139,12 @@ struct wait_queue_entry struct thread_wait *wait; }; -extern void *mem_alloc( size_t size ); /* malloc wrapper */ -extern void *memdup( const void *data, size_t len ); +extern void *mem_alloc( size_t size ) __WINE_ALLOC_SIZE(1) __WINE_DEALLOC(free) __WINE_MALLOC; +extern void *memdup( const void *data, size_t len ) __WINE_ALLOC_SIZE(2) __WINE_DEALLOC(free) __WINE_MALLOC; extern void *alloc_object( const struct object_ops *ops ); extern void namespace_add( struct namespace *namespace, struct object_name *ptr ); extern const WCHAR *get_object_name( struct object *obj, data_size_t *len ); -extern WCHAR *default_get_full_name( struct object *obj, data_size_t *ret_len ); +extern WCHAR *default_get_full_name( struct object *obj, data_size_t *ret_len ) __WINE_DEALLOC(free) __WINE_MALLOC; extern void dump_object_name( struct object *obj ); extern struct object *lookup_named_object( struct object *root, const struct unicode_str *name, unsigned int attr, struct unicode_str *name_left ); diff --git a/server/unicode.h b/server/unicode.h index e031d6ef0cf..951100564cd 100644 --- a/server/unicode.h +++ b/server/unicode.h @@ -28,7 +28,7 @@ extern int memicmp_strW( const WCHAR *str1, const WCHAR *str2, data_size_t len ); extern unsigned int hash_strW( const WCHAR *str, data_size_t len, unsigned int hash_size ); -extern WCHAR *ascii_to_unicode_str( const char *str, struct unicode_str *ret ); +extern WCHAR *ascii_to_unicode_str( const char *str, struct unicode_str *ret ) __WINE_DEALLOC(free) __WINE_MALLOC; extern int parse_strW( WCHAR *buffer, data_size_t *len, const char *src, char endchar ); extern int dump_strW( const WCHAR *str, data_size_t len, FILE *f, const char escape[2] ); extern struct fd *load_intl_file(void); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2046
\__WINE_MALLOC is not appropriate for functions that return initialized objects.
I was confused about this assertion, so I went and looked at the documentation, which includes the restriction "no pointers to valid objects occur in any storage addressed by [the return value]". I don't know why this is a restriction, but it also means that we can't put that attribute on realloc or anything like it, which we already have incorrectly done. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2046#note_22172
On Fri Jan 27 18:23:47 2023 +0000, Zebediah Figura wrote:
\__WINE_MALLOC is not appropriate for functions that return initialized objects. I was confused about this assertion, so I went and looked at the documentation, which includes the restriction "no pointers to valid objects occur in any storage addressed by [the return value]". I don't know why this is a restriction, but it also means that we can't put that attribute on realloc or anything like it, which we already have incorrectly done. Thank you for pointing me to the GCC documentation. You're right: The documentation says that the returned memory cannot contain valid pointers, and it explicitly says that realloc cannot have `__attribute__((malloc))`. I'm sure I read that at one point but forgot about it :-( I will send patches to remove the annotations that I've added in error.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2046#note_22174
Brendan Shanks (@bshanks) commented about server/object.h:
struct thread_wait *wait; };
-extern void *mem_alloc( size_t size ); /* malloc wrapper */ -extern void *memdup( const void *data, size_t len ); +extern void *mem_alloc( size_t size ) __WINE_ALLOC_SIZE(1) __WINE_DEALLOC(free) __WINE_MALLOC; +extern void *memdup( const void *data, size_t len ) __WINE_ALLOC_SIZE(2) __WINE_DEALLOC(free) __WINE_MALLOC;
`memdup` returns initialized data which could contain pointers, I don't think `__WINE_MALLOC` should be used. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2046#note_22175
`memdup` returns initialized data which could contain pointers, I don't think `__WINE_MALLOC` should be used.
Note that extends to __WINE_DEALLOC, which is also underlyingly __attribute__((malloc)). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2046#note_22176
On Fri Jan 27 19:24:25 2023 +0000, Zebediah Figura wrote:
`memdup` returns initialized data which could contain pointers, I don't think `__WINE_MALLOC` should be used. Note that extends to __WINE_DEALLOC, which is also underlyingly __attribute__((malloc)). It's gross, but I'm pretty sure the plain form of `attribute((malloc))` is independent from the deallocator forms.
For example, [in the GCC docs' example](https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html) both plain `malloc` and `malloc(deallocator)` are specified. Also, `glibc` [only specifies a deallocator](https://github.com/bminor/glibc/blob/ae612c45efb5e34713859a5facf92368307efb6...) for `reallocarray`. (Aside from this kind of ambiguity, overloading `attribute((malloc))` also prevents using `has_attribute` to detect whether the deallocator form is supported. A bizarre decision.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2046#note_22179
It's gross, but I'm pretty sure the plain form of `attribute((malloc))` is independent from the deallocator forms.
Ah, on rereading I believe you're right—the relevant wording being "Independently, the form of the attribute with one or two arguments..." -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2046#note_22180
participants (4)
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Brendan Shanks (＠bshanks)
-
Zebediah Figura (＠zfigura)