memory: make function overrides work reliably in unity builds

Fixing:
- HTTPS-RR builds with c-ares and Linux MUSL.
- curl-for-win minimal builds with Linux MUSL.

It should fix all other kinds of entaglement between curl's redefintions
of system symbols and system (or 3rd-party) headers sensitive to that.

It also syncs memory override behavior between unity & non-unity builds,
thus reducing build variations.

The idea is to define and declare everything once in `curl_setup.h`,
without overriding any system symbols with curl ones yet. Then, like
before this patch, override them, if necessary, in each source file via
`curl_memory.h` and `memdebug.h`, after including system headers.
To ensure a clean slate with no overrides at the beginning of each
source file, reset all of them unconditionally at the end of
`curl_setup.h`, by including `curl_mem_undef.h`. (This assumes
`curl_setup.h` is always included first, which is already the case
throughout the codebase.)

`curl_mem_undef.h` can also be included explicitly wherever overrides
are causing problems. E.g. in tests which use unity-style builds and
a previously included `curl_memory.h`/`memdebug.h` can be spilling into
other source files.

The simplified role of the two override headers:
- `curl_memory.h`: overrides system memory allocator functions to
  libcurl ones, when memory tracing (aka `CURLDEBUG`) is disabled.
- `memdebug.h`: overrides system memory allocator and some other
  functions to curl debug functions, when memory tracing is enabled.

Changed made in this patch, step-by-step:
- curl_memory.h: move allocator typedefs and protos to `curl_setup.h`.
- memdebug.h: move `ALLOC_*` macros to `curl_setup.h`.
- memdebug.h: move allocator protos to `curl_setup.h`.
- memdebug.h: move `Curl_safefree()` macro to `curl_setup.h`.
  (it's a regular macro, with a one-time, global, definition.)
- curl_memory.h: move system symbol undefs to a new, separate header:
  `curl_mem_undef.h`.
- curl_setup.h: include `curl_mem_undef.h` at the end, unconditionally,
  to reset system symbol macros after each inclusion.
- handle `sclose()` and `fake_sclose()` in `curl_setup.h`. They are not
  system symbols, a one-time definition does the job.

Also:
- GHA/linux: enable unity mode for the HTTP-RR c-ares MUSL job.
  Follow-up to 17ab4d62e62bd3bfb6dbf6cb51460629f21c03e7 #16413

That said, I'd still find it better to avoid redefining system macros.
To communicate clearly the fact that they are not the original system
calls and they do behave differently. And, it would allow dropping the
undef/redef dance in each source file, and maintaining the logic with
it. The "last #include files should be in this order" comments in each
source would also become unnecessary. Also the trick of using
`(func)` (or interim macros) to call the non-overridden function where
required. This method works for printf and most everything else already.
For `_tcsdup`, socket and fopen functions this could work without
disturbing the codebase much.

Ref: #16428 (clean reboot of)

Closes #17827

diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml
index 2044060ce29011ba9e82f10dafea232a2b938f58..06f7da51c4199af78b04417630a9661542b76657 100644 (file)
--- a/.github/workflows/linux.yml
+++ b/.github/workflows/linux.yml
@@ -275,11 +275,11 @@ jobs:
             container: 'andy5995/slackware-build-essential:15.0'
 
           - name: 'Alpine MUSL https-rr'
-            configure: --enable-debug --with-ssl --with-libssh2 --with-libidn2 --with-gssapi --enable-ldap --with-libpsl --enable-threaded-resolver --enable-ares --enable-httpsrr
+            configure: --enable-debug --with-ssl --with-libssh2 --with-libidn2 --with-gssapi --enable-ldap --with-libpsl --enable-httpsrr --enable-ares --enable-threaded-resolver
             container: 'alpine:3.20'
 
-          - name: 'Alpine MUSL c-ares https-rr'
-            configure: --enable-debug --with-ssl --with-libssh2 --with-libidn2 --with-gssapi --enable-ldap --with-libpsl --disable-threaded-resolver --enable-ares --enable-httpsrr --disable-unity
+          - name: 'Alpine MUSL https-rr c-ares'
+            configure: --enable-debug --with-ssl --with-libssh2 --with-libidn2 --with-gssapi --enable-ldap --with-libpsl --enable-httpsrr --enable-ares --disable-threaded-resolver
             container: 'alpine:3.20'
 
     steps:

diff --git a/lib/Makefile.inc b/lib/Makefile.inc
index b4a27bfdcf273e2f8ec545c3cd4c0c20b484971a..a22b1e1b00196116bbac8b974886882973615921 100644 (file)
--- a/lib/Makefile.inc
+++ b/lib/Makefile.inc
@@ -294,6 +294,7 @@ LIB_HFILES =         \
   curl_ldap.h        \
   curl_md4.h         \
   curl_md5.h         \
+  curl_mem_undef.h   \
   curl_memory.h      \
   curl_memrchr.h     \
   curl_ntlm_core.h   \

diff --git a/lib/curl_mem_undef.h b/lib/curl_mem_undef.h
new file mode 100644 (file)
index 0000000..2b0bef1
--- /dev/null
+++ b/lib/curl_mem_undef.h
@@ -0,0 +1,56 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ * SPDX-License-Identifier: curl
+ *
+ ***************************************************************************/
+
+/* Unset redefined system symbols. */
+
+#undef strdup
+#undef malloc
+#undef calloc
+#undef realloc
+#undef free
+#undef send
+#undef recv
+
+#ifdef _WIN32
+#undef _tcsdup
+#endif
+
+#undef socket
+#undef accept
+#ifdef HAVE_ACCEPT4
+#undef accept4
+#endif
+#ifdef HAVE_SOCKETPAIR
+#undef socketpair
+#endif
+
+#undef fopen
+#ifdef CURL_FOPEN
+#define fopen(fname, mode) CURL_FOPEN(fname, mode)
+#endif
+#undef fdopen
+#undef fclose
+
+#undef HEADER_CURL_MEMORY_H
+#undef HEADER_CURL_MEMDEBUG_H

diff --git a/lib/curl_memory.h b/lib/curl_memory.h
index b4536c1b3f815b10d90805fc6b209a2798981812..07ef111ce78e2a8791baab00a5642a1d688072ab 100644 (file)
--- a/lib/curl_memory.h
+++ b/lib/curl_memory.h
@@ -54,74 +54,6 @@
  *
  */
 
-#ifdef HEADER_CURL_MEMDEBUG_H
-/* cleanup after memdebug.h */
-
-#ifdef CURLDEBUG
-
-#undef strdup
-#undef malloc
-#undef calloc
-#undef realloc
-#undef free
-#undef send
-#undef recv
-
-#ifdef _WIN32
-#undef _tcsdup
-#endif
-
-#undef socket
-#undef accept
-#ifdef HAVE_ACCEPT4
-#undef accept4
-#endif
-#ifdef HAVE_SOCKETPAIR
-#undef socketpair
-#endif
-
-/* sclose is probably already defined, redefine it! */
-#undef sclose
-#define sclose(x)  CURL_SCLOSE(x)
-#undef fopen
-#ifdef CURL_FOPEN
-#define fopen(fname, mode)  CURL_FOPEN(fname, mode)
-#endif
-#undef fdopen
-#undef fclose
-
-#endif /* CURLDEBUG */
-
-#undef HEADER_CURL_MEMDEBUG_H
-#endif /* HEADER_CURL_MEMDEBUG_H */
-
-/*
-** Following section applies even when CURLDEBUG is not defined.
-*/
-
-#undef fake_sclose
-
-#ifndef CURL_DID_MEMORY_FUNC_TYPEDEFS /* only if not already done */
-/*
- * The following memory function replacement typedef's are COPIED from
- * curl/curl.h and MUST match the originals. We copy them to avoid having to
- * include curl/curl.h here. We avoid that include since it includes stdio.h
- * and other headers that may get messed up with defines done here.
- */
-typedef void *(*curl_malloc_callback)(size_t size);
-typedef void (*curl_free_callback)(void *ptr);
-typedef void *(*curl_realloc_callback)(void *ptr, size_t size);
-typedef char *(*curl_strdup_callback)(const char *str);
-typedef void *(*curl_calloc_callback)(size_t nmemb, size_t size);
-#define CURL_DID_MEMORY_FUNC_TYPEDEFS
-#endif
-
-extern curl_malloc_callback Curl_cmalloc;
-extern curl_free_callback Curl_cfree;
-extern curl_realloc_callback Curl_crealloc;
-extern curl_strdup_callback Curl_cstrdup;
-extern curl_calloc_callback Curl_ccalloc;
-
 #ifndef CURLDEBUG
 
 /*

diff --git a/lib/curl_setup.h b/lib/curl_setup.h
index 36da5c6cbfdf2946f3bcb4883e141ae146d7b139..b4205cf6f420931bf89d72ba6047ba7b30df7bb6 100644 (file)
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -961,15 +961,140 @@ endings either CRLF or LF so 't' is appropriate.
 
 #define CURL_ARRAYSIZE(A) (sizeof(A)/sizeof((A)[0]))
 
+#ifndef CURL_DID_MEMORY_FUNC_TYPEDEFS /* only if not already done */
+/*
+ * The following memory function replacement typedef's are COPIED from
+ * curl/curl.h and MUST match the originals. We copy them to avoid having to
+ * include curl/curl.h here. We avoid that include since it includes stdio.h
+ * and other headers that may get messed up with defines done here.
+ */
+typedef void *(*curl_malloc_callback)(size_t size);
+typedef void (*curl_free_callback)(void *ptr);
+typedef void *(*curl_realloc_callback)(void *ptr, size_t size);
+typedef char *(*curl_strdup_callback)(const char *str);
+typedef void *(*curl_calloc_callback)(size_t nmemb, size_t size);
+#define CURL_DID_MEMORY_FUNC_TYPEDEFS
+#endif
+
+extern curl_malloc_callback Curl_cmalloc;
+extern curl_free_callback Curl_cfree;
+extern curl_realloc_callback Curl_crealloc;
+extern curl_strdup_callback Curl_cstrdup;
+extern curl_calloc_callback Curl_ccalloc;
+
+/*
+ * Curl_safefree defined as a macro to allow MemoryTracking feature
+ * to log free() calls at same location where Curl_safefree is used.
+ * This macro also assigns NULL to given pointer when free'd.
+ */
+#define Curl_safefree(ptr) \
+  do { free((ptr)); (ptr) = NULL;} while(0)
+
 #ifdef CURLDEBUG
+#ifdef __clang__
+#  define ALLOC_FUNC         __attribute__((__malloc__))
+#  if __clang_major__ >= 4
+#  define ALLOC_SIZE(s)      __attribute__((__alloc_size__(s)))
+#  define ALLOC_SIZE2(n, s)  __attribute__((__alloc_size__(n, s)))
+#  else
+#  define ALLOC_SIZE(s)
+#  define ALLOC_SIZE2(n, s)
+#  endif
+#elif defined(__GNUC__) && __GNUC__ >= 3
+#  define ALLOC_FUNC         __attribute__((__malloc__))
+#  define ALLOC_SIZE(s)      __attribute__((__alloc_size__(s)))
+#  define ALLOC_SIZE2(n, s)  __attribute__((__alloc_size__(n, s)))
+#elif defined(_MSC_VER)
+#  define ALLOC_FUNC         __declspec(restrict)
+#  define ALLOC_SIZE(s)
+#  define ALLOC_SIZE2(n, s)
+#else
+#  define ALLOC_FUNC
+#  define ALLOC_SIZE(s)
+#  define ALLOC_SIZE2(n, s)
+#endif
+
+#include <curl/curl.h> /* for CURL_EXTERN */
+
+extern FILE *curl_dbg_logfile;
+
+/* memory functions */
+CURL_EXTERN void curl_dbg_free(void *ptr, int line, const char *source);
+CURL_EXTERN ALLOC_FUNC ALLOC_SIZE(1)
+  void *curl_dbg_malloc(size_t size, int line, const char *source);
+CURL_EXTERN ALLOC_FUNC ALLOC_SIZE2(1, 2)
+  void *curl_dbg_calloc(size_t n, size_t size, int line, const char *source);
+CURL_EXTERN ALLOC_SIZE(2)
+  void *curl_dbg_realloc(void *ptr, size_t size, int line, const char *source);
+CURL_EXTERN ALLOC_FUNC
+  char *curl_dbg_strdup(const char *str, int line, const char *src);
+#if defined(_WIN32) && defined(UNICODE)
+CURL_EXTERN ALLOC_FUNC
+  wchar_t *curl_dbg_wcsdup(const wchar_t *str, int line, const char *source);
+#endif
+
+CURL_EXTERN void curl_dbg_memdebug(const char *logname);
+CURL_EXTERN void curl_dbg_memlimit(long limit);
+CURL_EXTERN void curl_dbg_log(const char *format, ...) CURL_PRINTF(1, 2);
+
+/* file descriptor manipulators */
+CURL_EXTERN curl_socket_t curl_dbg_socket(int domain, int type, int protocol,
+                                          int line, const char *source);
+CURL_EXTERN void curl_dbg_mark_sclose(curl_socket_t sockfd,
+                                      int line, const char *source);
+CURL_EXTERN int curl_dbg_sclose(curl_socket_t sockfd,
+                                int line, const char *source);
+CURL_EXTERN curl_socket_t curl_dbg_accept(curl_socket_t s, void *a, void *alen,
+                                          int line, const char *source);
+#ifdef HAVE_ACCEPT4
+CURL_EXTERN curl_socket_t curl_dbg_accept4(curl_socket_t s, void *saddr,
+                                           void *saddrlen, int flags,
+                                           int line, const char *source);
+#endif
+#ifdef HAVE_SOCKETPAIR
+CURL_EXTERN int curl_dbg_socketpair(int domain, int type, int protocol,
+                                    curl_socket_t socket_vector[2],
+                                    int line, const char *source);
+#endif
+
+/* send/receive sockets */
+CURL_EXTERN SEND_TYPE_RETV curl_dbg_send(SEND_TYPE_ARG1 sockfd,
+                                         SEND_QUAL_ARG2 SEND_TYPE_ARG2 buf,
+                                         SEND_TYPE_ARG3 len,
+                                         SEND_TYPE_ARG4 flags, int line,
+                                         const char *source);
+CURL_EXTERN RECV_TYPE_RETV curl_dbg_recv(RECV_TYPE_ARG1 sockfd,
+                                         RECV_TYPE_ARG2 buf,
+                                         RECV_TYPE_ARG3 len,
+                                         RECV_TYPE_ARG4 flags, int line,
+                                         const char *source);
+
+/* FILE functions */
+CURL_EXTERN int curl_dbg_fclose(FILE *file, int line, const char *source);
+CURL_EXTERN ALLOC_FUNC
+  FILE *curl_dbg_fopen(const char *file, const char *mode,
+                       int line, const char *source);
+CURL_EXTERN ALLOC_FUNC
+  FILE *curl_dbg_fdopen(int filedes, const char *mode,
+                        int line, const char *source);
+
+#define sclose(sockfd) curl_dbg_sclose(sockfd,__LINE__,__FILE__)
+#define fake_sclose(sockfd) curl_dbg_mark_sclose(sockfd,__LINE__,__FILE__)
+
 #define CURL_GETADDRINFO(host,serv,hint,res) \
   curl_dbg_getaddrinfo(host, serv, hint, res, __LINE__, __FILE__)
 #define CURL_FREEADDRINFO(data) \
   curl_dbg_freeaddrinfo(data, __LINE__, __FILE__)
-#else
+
+#else /* !CURLDEBUG */
+
+#define sclose(x) CURL_SCLOSE(x)
+#define fake_sclose(x) Curl_nop_stmt
+
 #define CURL_GETADDRINFO getaddrinfo
 #define CURL_FREEADDRINFO freeaddrinfo
-#endif
+
+#endif /* CURLDEBUG */
 
 /* Some versions of the Android NDK is missing the declaration */
 #if defined(HAVE_GETPWUID_R) && \
@@ -1056,3 +1181,5 @@ int getpwuid_r(uid_t uid, struct passwd *pwd, char *buf,
 #endif
 
 #endif /* HEADER_CURL_SETUP_H */
+
+#include "curl_mem_undef.h"

diff --git a/lib/curl_setup_once.h b/lib/curl_setup_once.h
index bec45de88880bf1358eb6d876a2a6a81f3149386..642e1487c8ae1a54e469ae55bdb6f12742ca8341 100644 (file)
--- a/lib/curl_setup_once.h
+++ b/lib/curl_setup_once.h
@@ -206,8 +206,6 @@ struct timeval {
 #  define CURL_SCLOSE(x)  close((x))
 #endif
 
-#define sclose(x)  CURL_SCLOSE(x)
-
 /*
  * Stack-independent version of fcntl() on sockets:
  */

diff --git a/lib/memdebug.h b/lib/memdebug.h
index 03b04b12dc2f6d3304e359100d91944a60c2196d..d8d5f807d7f9654e8885cc123fc1cccb4d876b57 100644 (file)
--- a/lib/memdebug.h
+++ b/lib/memdebug.h
@@ -1,6 +1,5 @@
 #ifndef HEADER_CURL_MEMDEBUG_H
 #define HEADER_CURL_MEMDEBUG_H
-#ifdef CURLDEBUG
 /***************************************************************************
  *                                  _   _ ____  _
  *  Project                     ___| | | |  _ \| |
@@ -30,99 +29,7 @@
  * as well as the library. Do not mix with library internals!
  */
 
-#include <curl/curl.h>
-#include "functypes.h"
-
-#ifdef __clang__
-#  define ALLOC_FUNC         __attribute__((__malloc__))
-#  if __clang_major__ >= 4
-#  define ALLOC_SIZE(s)      __attribute__((__alloc_size__(s)))
-#  define ALLOC_SIZE2(n, s)  __attribute__((__alloc_size__(n, s)))
-#  else
-#  define ALLOC_SIZE(s)
-#  define ALLOC_SIZE2(n, s)
-#  endif
-#elif defined(__GNUC__) && __GNUC__ >= 3
-#  define ALLOC_FUNC         __attribute__((__malloc__))
-#  define ALLOC_SIZE(s)      __attribute__((__alloc_size__(s)))
-#  define ALLOC_SIZE2(n, s)  __attribute__((__alloc_size__(n, s)))
-#elif defined(_MSC_VER)
-#  define ALLOC_FUNC         __declspec(restrict)
-#  define ALLOC_SIZE(s)
-#  define ALLOC_SIZE2(n, s)
-#else
-#  define ALLOC_FUNC
-#  define ALLOC_SIZE(s)
-#  define ALLOC_SIZE2(n, s)
-#endif
-
-/* Avoid redundant redeclaration warnings with modern compilers, when including
-   this header multiple times. */
-#ifndef HEADER_CURL_MEMDEBUG_H_EXTERNS
-#define HEADER_CURL_MEMDEBUG_H_EXTERNS
-extern FILE *curl_dbg_logfile;
-
-/* memory functions */
-CURL_EXTERN void curl_dbg_free(void *ptr, int line, const char *source);
-CURL_EXTERN ALLOC_FUNC ALLOC_SIZE(1)
-  void *curl_dbg_malloc(size_t size, int line, const char *source);
-CURL_EXTERN ALLOC_FUNC ALLOC_SIZE2(1, 2)
-  void *curl_dbg_calloc(size_t n, size_t size, int line, const char *source);
-CURL_EXTERN ALLOC_SIZE(2)
-  void *curl_dbg_realloc(void *ptr, size_t size, int line, const char *source);
-CURL_EXTERN ALLOC_FUNC
-  char *curl_dbg_strdup(const char *str, int line, const char *src);
-#if defined(_WIN32) && defined(UNICODE)
-CURL_EXTERN ALLOC_FUNC
-  wchar_t *curl_dbg_wcsdup(const wchar_t *str, int line, const char *source);
-#endif
-
-CURL_EXTERN void curl_dbg_memdebug(const char *logname);
-CURL_EXTERN void curl_dbg_memlimit(long limit);
-CURL_EXTERN void curl_dbg_log(const char *format, ...) CURL_PRINTF(1, 2);
-
-/* file descriptor manipulators */
-CURL_EXTERN curl_socket_t curl_dbg_socket(int domain, int type, int protocol,
-                                          int line, const char *source);
-CURL_EXTERN void curl_dbg_mark_sclose(curl_socket_t sockfd,
-                                      int line, const char *source);
-CURL_EXTERN int curl_dbg_sclose(curl_socket_t sockfd,
-                                int line, const char *source);
-CURL_EXTERN curl_socket_t curl_dbg_accept(curl_socket_t s, void *a, void *alen,
-                                          int line, const char *source);
-#ifdef HAVE_ACCEPT4
-CURL_EXTERN curl_socket_t curl_dbg_accept4(curl_socket_t s, void *saddr,
-                                           void *saddrlen, int flags,
-                                           int line, const char *source);
-#endif
-#ifdef HAVE_SOCKETPAIR
-CURL_EXTERN int curl_dbg_socketpair(int domain, int type, int protocol,
-                                    curl_socket_t socket_vector[2],
-                                    int line, const char *source);
-#endif
-
-/* send/receive sockets */
-CURL_EXTERN SEND_TYPE_RETV curl_dbg_send(SEND_TYPE_ARG1 sockfd,
-                                         SEND_QUAL_ARG2 SEND_TYPE_ARG2 buf,
-                                         SEND_TYPE_ARG3 len,
-                                         SEND_TYPE_ARG4 flags, int line,
-                                         const char *source);
-CURL_EXTERN RECV_TYPE_RETV curl_dbg_recv(RECV_TYPE_ARG1 sockfd,
-                                         RECV_TYPE_ARG2 buf,
-                                         RECV_TYPE_ARG3 len,
-                                         RECV_TYPE_ARG4 flags, int line,
-                                         const char *source);
-
-/* FILE functions */
-CURL_EXTERN int curl_dbg_fclose(FILE *file, int line, const char *source);
-CURL_EXTERN ALLOC_FUNC
-  FILE *curl_dbg_fopen(const char *file, const char *mode,
-                       int line, const char *source);
-CURL_EXTERN ALLOC_FUNC
-  FILE *curl_dbg_fdopen(int filedes, const char *mode,
-                        int line, const char *source);
-
-#endif /* HEADER_CURL_MEMDEBUG_H_EXTERNS */
+#ifdef CURLDEBUG
 
 /* Set this symbol on the command-line, recompile all lib-sources */
 #undef strdup
@@ -166,12 +73,6 @@ CURL_EXTERN ALLOC_FUNC
                       __LINE__, __FILE__)
 #endif
 
-/* sclose is probably already defined, redefine it! */
-#undef sclose
-#define sclose(sockfd) curl_dbg_sclose(sockfd,__LINE__,__FILE__)
-
-#define fake_sclose(sockfd) curl_dbg_mark_sclose(sockfd,__LINE__,__FILE__)
-
 #undef fopen
 #define fopen(file,mode) curl_dbg_fopen(file,mode,__LINE__,__FILE__)
 #undef fdopen
@@ -180,22 +81,4 @@ CURL_EXTERN ALLOC_FUNC
 #define fclose(file) curl_dbg_fclose(file,__LINE__,__FILE__)
 
 #endif /* CURLDEBUG */
-
-/*
-** Following section applies even when CURLDEBUG is not defined.
-*/
-
-#ifndef fake_sclose
-#define fake_sclose(x)  Curl_nop_stmt
-#endif
-
-/*
- * Curl_safefree defined as a macro to allow MemoryTracking feature
- * to log free() calls at same location where Curl_safefree is used.
- * This macro also assigns NULL to given pointer when free'd.
- */
-
-#define Curl_safefree(ptr) \
-  do { free((ptr)); (ptr) = NULL;} while(0)
-
 #endif /* HEADER_CURL_MEMDEBUG_H */