[Libguestfs] [libnbd PATCH] RFC: generator: Mark APIs with allocator/deallocator semantics
Modern GCC has two related attributes for functions returning a
pointer:
__attribute__((__malloc__)) - this function returns a new pointer, not
aliased to any existing pointer
__attribute__((__malloc__(fn,1))) - call fn(return_value) to avoid
leaking memory allocated by this function
With those attributes, static analyzers can better detect when we pass
the resulting pointer to the wrong deallocator, deallocate more than
once, have a use after free, or otherwise leak the memory. (Sadly, as
of gcc 12.2.1, -fanalyzer still has a LOT of false positives, such as:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107100; since our code
base triggers some of these, we can't quite rely on it yet).
---
lib/internal.h | 4 +++-
generator/C.ml | 24 +++++++++++++++++++++---
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 0e17fbea..22f500b4 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -508,8 +508,10 @@ extern void nbd_internal_fork_safe_perror (const char *s)
extern char *nbd_internal_printable_buffer (const void *buf, size_t count)
LIBNBD_ATTRIBUTE_NONNULL(1);
extern char *nbd_internal_printable_string (const char *str)
+ LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(free)
LIBNBD_ATTRIBUTE_NONNULL(1);
-extern char *nbd_internal_printable_string_list (char **list);
+extern char *nbd_internal_printable_string_list (char **list)
+ LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(free);
/* These are wrappers around socket(2) and socketpair(2). They
* always set SOCK_CLOEXEC. nbd_internal_socket can set SOCK_NONBLOCK
diff --git a/generator/C.ml b/generator/C.ml
index c74fae77..cfa2964c 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -244,6 +244,16 @@ let
pr "extern ";
print_call ?wrap ?closure_style name args optargs ret;
+ (* For RString, note that the caller must free() the argument.
+ * Since this is used in a public header, we must use gcc's spelling
+ * of __builtin_free in case bare free is defined as a macro.
+ *)
+ (match ret with
+ | RString ->
+ pr "\n LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(__builtin_free)";
+ | _ -> ()
+ );
+
(* Output __attribute__((nonnull)) for the function parameters:
* eg. struct nbd_handle *, int, char *
* => [ true, false, true ]
@@ -403,6 +413,13 @@ let
pr "#endif\n";
pr "#endif /* ! defined LIBNBD_ATTRIBUTE_NONNULL */\n";
pr "\n";
+ pr "#if defined(__GNUC__) && LIBNBD_GCC_VERSION >= 120000 /* gcc >=
12.0 */\n";
+ pr "#define LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(fn) \\\n";
+ pr " __attribute__((__malloc__, __malloc__(fn, 1)))\n";
+ pr "#else\n";
+ pr "#define LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(fn)\n";
+ pr "#endif\n";
+ pr "\n";
pr "struct nbd_handle;\n";
pr "\n";
List.iter (
@@ -433,12 +450,13 @@ let
pr "#define %-40s %d\n" n i
) constants;
pr "\n";
- pr "extern struct nbd_handle *nbd_create (void);\n";
- pr "#define LIBNBD_HAVE_NBD_CREATE 1\n";
- pr "\n";
pr "extern void nbd_close (struct nbd_handle *h); /* h can be NULL */\n";
pr "#define LIBNBD_HAVE_NBD_CLOSE 1\n";
pr "\n";
+ pr "extern struct nbd_handle *nbd_create (void)\n";
+ pr " LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(nbd_close);\n";
+ pr "#define LIBNBD_HAVE_NBD_CREATE 1\n";
+ pr "\n";
pr "extern const char *nbd_get_error (void);\n";
pr "#define LIBNBD_HAVE_NBD_GET_ERROR 1\n";
pr "\n";
--
2.37.3