From 59dc9f7e69c399102e9ebe3670360ef52706ff23 Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Mon, 20 May 2024 14:21:05 +0200 Subject: [PATCH] build: untangle `CURLDEBUG` and `DEBUGBUILD` macros `CURLDEBUG` is meant to enable memory tracking, but in a bunch of cases, it was protecting debug features that were supposed to be guarded with `DEBUGBUILD`. Replace these uses with `DEBUGBUILD`. This leaves `CURLDEBUG` uses solely for its intended purpose: to enable the memory tracking debug feature. Also: - autotools: rely on `DEBUGBUILD` to enable `checksrc`. Instead of `CURLDEBUG`, which worked in most cases because debug builds enable `CURLDEBUG` by default, but it's not accurate. - include `lib/easyif.h` instead of keeping a copy of a declaration. - add CI test jobs for the build issues discovered. Ref: https://github.com/curl/curl/pull/13694#issuecomment-2120311894 Closes #13718 --- .github/workflows/windows.yml | 2 +- CMake/CurlSymbolHiding.cmake | 7 ++++--- appveyor.yml | 11 +++++++++++ configure.ac | 2 +- include/curl/Makefile.am | 2 +- lib/Makefile.am | 2 +- lib/cfilters.c | 2 +- lib/conncache.h | 2 +- lib/doh.c | 14 +++++++------- lib/easy.c | 6 +++--- lib/easyif.h | 2 +- lib/http.c | 4 ++-- lib/mqtt.c | 4 ++-- lib/rand.c | 2 +- lib/request.c | 2 +- lib/url.c | 2 +- lib/urldata.h | 2 +- src/Makefile.am | 2 +- src/tool_cfgable.h | 2 +- src/tool_getparam.c | 2 +- src/tool_operate.c | 16 +++++++--------- tests/Makefile.am | 2 +- tests/libtest/Makefile.am | 2 +- tests/server/Makefile.am | 2 +- 24 files changed, 53 insertions(+), 43 deletions(-) diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 2ad79f34a9..038ddd9ead 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -165,7 +165,7 @@ jobs: - { build: 'autotools', sys: 'msys' , env: 'x86_64' , tflags: '!19 !1233', config: '--enable-debug --disable-threaded-resolver --disable-proxy' } - { build: 'autotools', sys: 'msys' , env: 'x86_64' , tflags: '!19 !504 !704 !705 !1233', config: '--enable-debug --disable-threaded-resolver' } - { build: 'autotools', sys: 'msys' , env: 'x86_64' , tflags: '!19 !504 !704 !705 !1233', config: '' } - - { build: 'autotools', sys: 'mingw64', env: 'x86_64' , tflags: 'skiprun' , config: '--enable-debug --disable-threaded-resolver --enable-static=no' } + - { build: 'autotools', sys: 'mingw64', env: 'x86_64' , tflags: 'skiprun' , config: '--enable-debug --disable-threaded-resolver --disable-curldebug --enable-static=no' } # FIXME: WebSockets test results ignored due to frequent failures on native Windows: - { build: 'cmake' , sys: 'mingw64', env: 'x86_64' , tflags: '!TFTP ~2301 ~2302' , config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON', type: 'Debug' } - { build: 'cmake' , sys: 'mingw64', env: 'x86_64' , tflags: 'skiprun' , config: '-DENABLE_DEBUG=OFF -DBUILD_SHARED_LIBS=ON -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_CURLDEBUG=ON', type: 'Release' } diff --git a/CMake/CurlSymbolHiding.cmake b/CMake/CurlSymbolHiding.cmake index 8289b49246..07f4fc0b6d 100644 --- a/CMake/CurlSymbolHiding.cmake +++ b/CMake/CurlSymbolHiding.cmake @@ -26,9 +26,10 @@ include(CheckCSourceCompiles) option(CURL_HIDDEN_SYMBOLS "Set to ON to hide libcurl internal symbols (=hide all symbols that aren't officially external)." ON) mark_as_advanced(CURL_HIDDEN_SYMBOLS) -if(WIN32 AND ENABLE_CURLDEBUG) - # We need to export internal debug functions (e.g. curl_dbg_*), so disable - # symbol hiding for debug builds. +if(WIN32 AND (ENABLE_DEBUG OR ENABLE_CURLDEBUG)) + # We need to export internal debug functions, + # e.g. curl_easy_perform_ev() or curl_dbg_*(), + # so disable symbol hiding for debug builds and for memory tracking. set(CURL_HIDDEN_SYMBOLS OFF) endif() diff --git a/appveyor.yml b/appveyor.yml index 7c4e3b4cbe..381d672869 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -100,6 +100,17 @@ environment: SCHANNEL: 'ON' ENABLE_UNICODE: 'ON' HTTP_ONLY: 'OFF' + - job_name: 'CMake, VS2022, Release, x64, Schannel, Shared, Unicode, DEBUGBULID, no-CURLDEBUG, Build-only' + APPVEYOR_BUILD_WORKER_IMAGE: 'Visual Studio 2022' + BUILD_SYSTEM: CMake + PRJ_GEN: 'Visual Studio 17 2022' + TARGET: '-A x64' + PRJ_CFG: Release + SCHANNEL: 'ON' + ENABLE_UNICODE: 'ON' + HTTP_ONLY: 'OFF' + SHARED: 'ON' + CURLDEBUG: 'OFF' - job_name: 'CMake, VS2022, Debug, x64, no SSL, Static, Build-only' APPVEYOR_BUILD_WORKER_IMAGE: 'Visual Studio 2022' BUILD_SYSTEM: CMake diff --git a/configure.ac b/configure.ac index 646c255b99..85f0ab599b 100644 --- a/configure.ac +++ b/configure.ac @@ -44,6 +44,7 @@ AM_MAINTAINER_MODE m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])]) CURL_CHECK_OPTION_DEBUG +AM_CONDITIONAL(DEBUGBUILD, test x$want_debug = xyes) CURL_CHECK_OPTION_OPTIMIZE CURL_CHECK_OPTION_WARNINGS CURL_CHECK_OPTION_WERROR @@ -549,7 +550,6 @@ CURL_CHECK_COMPILER_PROTOTYPE_MISMATCH CURL_CHECK_COMPILER_SYMBOL_HIDING CURL_CHECK_CURLDEBUG -AM_CONDITIONAL(CURLDEBUG, test x$want_curldebug = xyes) supports_unittests=yes # cross-compilation of unit tests static library/programs fails when diff --git a/include/curl/Makefile.am b/include/curl/Makefile.am index a655aff1c8..8864d60e1f 100644 --- a/include/curl/Makefile.am +++ b/include/curl/Makefile.am @@ -35,7 +35,7 @@ CS_ = $(CS_0) checksrc: $(CHECKSRC)@PERL@ $(top_srcdir)/scripts/checksrc.pl -D$(top_srcdir)/include/curl $(pkginclude_HEADERS) -if CURLDEBUG +if DEBUGBUILD # for debug builds, we scan the sources on all regular make invokes all-local: checksrc endif diff --git a/lib/Makefile.am b/lib/Makefile.am index 96afc59ac1..413fcd4d55 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -126,7 +126,7 @@ checksrc: -W$(srcdir)/curl_config.h $(srcdir)/*.[ch] $(srcdir)/vauth/*.[ch] \ $(srcdir)/vtls/*.[ch] $(srcdir)/vquic/*.[ch] $(srcdir)/vssh/*.[ch]) -if CURLDEBUG +if DEBUGBUILD # for debug builds, we scan the sources on all regular make invokes all-local: checksrc endif diff --git a/lib/cfilters.c b/lib/cfilters.c index 87b9ba4d43..a1e405bc5e 100644 --- a/lib/cfilters.c +++ b/lib/cfilters.c @@ -718,7 +718,7 @@ CURLcode Curl_conn_send(struct Curl_easy *data, int sockindex, DEBUGASSERT(data); DEBUGASSERT(data->conn); conn = data->conn; -#ifdef CURLDEBUG +#ifdef DEBUGBUILD { /* Allow debug builds to override this logic to force short sends */ diff --git a/lib/conncache.h b/lib/conncache.h index e685123948..295057e42f 100644 --- a/lib/conncache.h +++ b/lib/conncache.h @@ -50,7 +50,7 @@ struct conncache { #define BUNDLE_UNKNOWN 0 /* initial value */ #define BUNDLE_MULTIPLEX 2 -#ifdef CURLDEBUG +#ifdef DEBUGBUILD /* the debug versions of these macros make extra certain that the lock is never doubly locked or unlocked */ #define CONNCACHE_LOCK(x) \ diff --git a/lib/doh.c b/lib/doh.c index 03317ae27e..fda28a0916 100644 --- a/lib/doh.c +++ b/lib/doh.c @@ -191,7 +191,7 @@ doh_write_cb(const void *contents, size_t size, size_t nmemb, void *userp) return realsize; } -#if defined(USE_HTTPSRR) && defined(CURLDEBUG) +#if defined(USE_HTTPSRR) && defined(DEBUGBUILD) static void local_print_buf(struct Curl_easy *data, const char *prefix, unsigned char *buf, size_t len) @@ -285,7 +285,7 @@ static CURLcode dohprobe(struct Curl_easy *data, ERROR_CHECK_SETOPT(CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2TLS); ERROR_CHECK_SETOPT(CURLOPT_PIPEWAIT, 1L); #endif -#ifndef CURLDEBUG +#ifndef DEBUGBUILD /* enforce HTTPS if not debug */ ERROR_CHECK_SETOPT(CURLOPT_PROTOCOLS, CURLPROTO_HTTPS); #else @@ -870,7 +870,7 @@ static void showdoh(struct Curl_easy *data, } #ifdef USE_HTTPSRR for(i = 0; i < d->numhttps_rrs; i++) { -# ifdef CURLDEBUG +# ifdef DEBUGBUILD local_print_buf(data, "DoH HTTPS", d->https_rrs[i].val, d->https_rrs[i].len); # else @@ -1143,7 +1143,7 @@ err: return CURLE_BAD_CONTENT_ENCODING; } -#ifdef CURLDEBUG +#ifdef DEBUGBUILD static CURLcode test_alpn_escapes(void) { /* we'll use an example from draft-ietf-dnsop-svcb, figure 10 */ @@ -1176,7 +1176,7 @@ static CURLcode Curl_doh_decode_httpsrr(unsigned char *rrval, size_t len, struct Curl_https_rrinfo *lhrr = NULL; char *dnsname = NULL; -#ifdef CURLDEBUG +#ifdef DEBUGBUILD /* a few tests of escaping, shouldn't be here but ok for now */ if(test_alpn_escapes() != CURLE_OK) return CURLE_OUT_OF_MEMORY; @@ -1244,7 +1244,7 @@ err: return CURLE_OUT_OF_MEMORY; } -# ifdef CURLDEBUG +# ifdef DEBUGBUILD static void local_print_httpsrr(struct Curl_easy *data, struct Curl_https_rrinfo *hrr) { @@ -1382,7 +1382,7 @@ CURLcode Curl_doh_is_resolved(struct Curl_easy *data, return result; } infof(data, "Some HTTPS RR to process"); -# ifdef CURLDEBUG +# ifdef DEBUGBUILD local_print_httpsrr(data, hrr); # endif (*dnsp)->hinfo = hrr; diff --git a/lib/easy.c b/lib/easy.c index a04dbedd87..ba8cb2ed58 100644 --- a/lib/easy.c +++ b/lib/easy.c @@ -374,7 +374,7 @@ struct Curl_easy *curl_easy_init(void) return data; } -#ifdef CURLDEBUG +#ifdef DEBUGBUILD struct socketmonitor { struct socketmonitor *next; /* the next node in the list or NULL */ @@ -655,7 +655,7 @@ static CURLcode easy_events(struct Curl_multi *multi) return wait_or_timeout(multi, &evs); } -#else /* CURLDEBUG */ +#else /* DEBUGBUILD */ /* when not built with debug, this function doesn't exist */ #define easy_events(x) CURLE_NOT_BUILT_IN #endif @@ -788,7 +788,7 @@ CURLcode curl_easy_perform(struct Curl_easy *data) return easy_perform(data, FALSE); } -#ifdef CURLDEBUG +#ifdef DEBUGBUILD /* * curl_easy_perform_ev() is the external interface that performs a blocking * transfer using the event-based API internally. diff --git a/lib/easyif.h b/lib/easyif.h index 6ce3483c64..d77bb98f92 100644 --- a/lib/easyif.h +++ b/lib/easyif.h @@ -34,7 +34,7 @@ CURLcode Curl_senddata(struct Curl_easy *data, const void *buffer, CURLcode Curl_connect_only_attach(struct Curl_easy *data); #endif -#ifdef CURLDEBUG +#ifdef DEBUGBUILD CURL_EXTERN CURLcode curl_easy_perform_ev(struct Curl_easy *easy); #endif diff --git a/lib/http.c b/lib/http.c index 0adfc63386..514e9aebea 100644 --- a/lib/http.c +++ b/lib/http.c @@ -2849,7 +2849,7 @@ CURLcode Curl_http_header(struct Curl_easy *data, #ifndef CURL_DISABLE_ALTSVC v = (data->asi && ((data->conn->handler->flags & PROTOPT_SSL) || -#ifdef CURLDEBUG +#ifdef DEBUGBUILD /* allow debug builds to circumvent the HTTPS restriction */ getenv("CURL_ALTSVC_HTTP") #else @@ -3116,7 +3116,7 @@ CURLcode Curl_http_header(struct Curl_easy *data, /* If enabled, the header is incoming and this is over HTTPS */ v = (data->hsts && ((conn->handler->flags & PROTOPT_SSL) || -#ifdef CURLDEBUG +#ifdef DEBUGBUILD /* allow debug builds to circumvent the HTTPS restriction */ getenv("CURL_HSTS_HTTP") #else diff --git a/lib/mqtt.c b/lib/mqtt.c index 4ca24eb41c..17a8d2a3d7 100644 --- a/lib/mqtt.c +++ b/lib/mqtt.c @@ -585,7 +585,7 @@ static size_t mqtt_decode_len(unsigned char *buf, return len; } -#ifdef CURLDEBUG +#ifdef DEBUGBUILD static const char *statenames[]={ "MQTT_FIRST", "MQTT_REMAINING_LENGTH", @@ -606,7 +606,7 @@ static void mqstate(struct Curl_easy *data, { struct connectdata *conn = data->conn; struct mqtt_conn *mqtt = &conn->proto.mqtt; -#ifdef CURLDEBUG +#ifdef DEBUGBUILD infof(data, "%s (from %s) (next is %s)", statenames[state], statenames[mqtt->state], diff --git a/lib/rand.c b/lib/rand.c index c62b1a4032..65e05e57ab 100644 --- a/lib/rand.c +++ b/lib/rand.c @@ -105,7 +105,7 @@ static CURLcode randit(struct Curl_easy *data, unsigned int *rnd) static unsigned int randseed; static bool seeded = FALSE; -#ifdef CURLDEBUG +#ifdef DEBUGBUILD char *force_entropy = getenv("CURL_ENTROPY"); if(force_entropy) { if(!seeded) { diff --git a/lib/request.c b/lib/request.c index 26741fa6cb..794bdb7245 100644 --- a/lib/request.c +++ b/lib/request.c @@ -187,7 +187,7 @@ static CURLcode xfer_send(struct Curl_easy *data, CURLcode result = CURLE_OK; *pnwritten = 0; -#ifdef CURLDEBUG +#ifdef DEBUGBUILD { /* Allow debug builds to override this logic to force short initial sends diff --git a/lib/url.c b/lib/url.c index 2814d31ad9..7287d2d893 100644 --- a/lib/url.c +++ b/lib/url.c @@ -3028,7 +3028,7 @@ static CURLcode parse_connect_to_slist(struct Curl_easy *data, #ifndef CURL_DISABLE_ALTSVC if(data->asi && !host && (port == -1) && ((conn->handler->protocol == CURLPROTO_HTTPS) || -#ifdef CURLDEBUG +#ifdef DEBUGBUILD /* allow debug builds to circumvent the HTTPS restriction */ getenv("CURL_ALTSVC_HTTP") #else diff --git a/lib/urldata.h b/lib/urldata.h index 8b1bd65d61..48844d2707 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -1375,7 +1375,7 @@ struct UrlState { unsigned char select_bits; /* != 0 -> bitmask of socket events for this transfer overriding anything the socket may report */ -#ifdef CURLDEBUG +#ifdef DEBUGBUILD BIT(conncache_lock); #endif /* when curl_easy_perform() is called, the multi handle is "owned" by diff --git a/src/Makefile.am b/src/Makefile.am index 3bf57d36ce..fe53644e39 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -145,7 +145,7 @@ checksrc: $(CHECKSRC)(@PERL@ $(top_srcdir)/scripts/checksrc.pl -D$(srcdir) \ -W$(srcdir)/tool_hugehelp.c $(srcdir)/*.[ch]) -if CURLDEBUG +if DEBUGBUILD # for debug builds, we scan the sources on all regular make invokes all-local: checksrc endif diff --git a/src/tool_cfgable.h b/src/tool_cfgable.h index 74d0c45f2e..5d99d605f3 100644 --- a/src/tool_cfgable.h +++ b/src/tool_cfgable.h @@ -323,7 +323,7 @@ struct GlobalConfig { bool styled_output; /* enable fancy output style detection */ long ms_per_transfer; /* start next transfer after (at least) this many milliseconds */ -#ifdef CURLDEBUG +#ifdef DEBUGBUILD bool test_event_based; #endif bool parallel; diff --git a/src/tool_getparam.c b/src/tool_getparam.c index 508e56797a..e3eb12fa0b 100644 --- a/src/tool_getparam.c +++ b/src/tool_getparam.c @@ -1825,7 +1825,7 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */ config->sasl_ir = toggle; break; case C_TEST_EVENT: /* --test-event */ -#ifdef CURLDEBUG +#ifdef DEBUGBUILD global->test_event_based = toggle; #else warnf(global, "--test-event is ignored unless a debug build"); diff --git a/src/tool_operate.c b/src/tool_operate.c index 535712d123..b5e7d4ccff 100644 --- a/src/tool_operate.c +++ b/src/tool_operate.c @@ -83,14 +83,12 @@ #include "tool_progress.h" #include "tool_ipfs.h" #include "dynbuf.h" +#ifdef DEBUGBUILD +#include "easyif.h" /* for libcurl's debug-only curl_easy_perform_ev() */ +#endif #include "memdebug.h" /* keep this as LAST include */ -#ifdef CURLDEBUG -/* libcurl's debug builds provide an extra function */ -CURLcode curl_easy_perform_ev(CURL *easy); -#endif - #ifndef O_BINARY /* since O_BINARY as used in bitmasks, setting it to zero makes it usable in source code but yet it doesn't ruin anything */ @@ -1315,7 +1313,7 @@ static CURLcode single_transfer(struct GlobalConfig *global, my_setopt(curl, CURLOPT_SEEKFUNCTION, tool_seek_cb); { -#ifdef CURLDEBUG +#ifdef DEBUGBUILD char *env = getenv("CURL_BUFFERSIZE"); if(env) { long size = strtol(env, NULL, 10); @@ -1647,7 +1645,7 @@ static CURLcode single_transfer(struct GlobalConfig *global, * must do the same thing as classic: * --cert : --cert-type p12 * but is designed to test blob */ -#if defined(CURLDEBUG) || defined(DEBUGBUILD) +#ifdef DEBUGBUILD if(config->cert && (strlen(config->cert) > 8) && (memcmp(config->cert, "loadmem=",8) == 0)) { FILE *fInCert = fopen(config->cert + 8, "rb"); @@ -1690,7 +1688,7 @@ static CURLcode single_transfer(struct GlobalConfig *global, config->proxy_cert_type); -#if defined(CURLDEBUG) || defined(DEBUGBUILD) +#ifdef DEBUGBUILD if(config->key && (strlen(config->key) > 8) && (memcmp(config->key, "loadmem=",8) == 0)) { FILE *fInCert = fopen(config->key + 8, "rb"); @@ -2484,7 +2482,7 @@ static CURLcode serial_transfers(struct GlobalConfig *global, break; } start = tvnow(); -#ifdef CURLDEBUG +#ifdef DEBUGBUILD if(global->test_event_based) result = curl_easy_perform_ev(per->curl); else diff --git a/tests/Makefile.am b/tests/Makefile.am index 7e6697f977..726d37b52c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -162,7 +162,7 @@ checksrc: (cd server && $(MAKE) checksrc) (cd http && $(MAKE) checksrc) -if CURLDEBUG +if DEBUGBUILD # for debug builds, we scan the sources on all regular make invokes all-local: checksrc endif diff --git a/tests/libtest/Makefile.am b/tests/libtest/Makefile.am index 8ae972a248..eed916eebb 100644 --- a/tests/libtest/Makefile.am +++ b/tests/libtest/Makefile.am @@ -134,7 +134,7 @@ CS_ = $(CS_0) checksrc: $(CHECKSRC)@PERL@ $(top_srcdir)/scripts/checksrc.pl -D$(srcdir) $(srcdir)/*.[ch] -if CURLDEBUG +if DEBUGBUILD # for debug builds, we scan the sources on all regular make invokes all-local: checksrc endif diff --git a/tests/server/Makefile.am b/tests/server/Makefile.am index b08942263c..dda121377a 100644 --- a/tests/server/Makefile.am +++ b/tests/server/Makefile.am @@ -59,7 +59,7 @@ CS_ = $(CS_0) checksrc: $(CHECKSRC)@PERL@ $(top_srcdir)/scripts/checksrc.pl $(srcdir)/*.[ch] -if CURLDEBUG +if DEBUGBUILD # for debug builds, we scan the sources on all regular make invokes all-local: checksrc endif -- 2.47.3