summaryrefslogtreecommitdiff
path: root/include/haproxy/bug.h
Commit message (Collapse)AuthorAgeFilesLines
* DEBUG: crash using an invalid opcode on aarch64 instead of an invalid accessWilly Tarreau2023-04-251-0/+7
| | | | | | | | | | | | | On aarch64 there's also a guaranted invalid instruction, called UDF, and which even supports an optional 16-bit immediate operand: https://developer.arm.com/documentation/ddi0596/2021-12/Base-Instructions/UDF--Permanently-Undefined-?lang=en It's conveniently encoded as 4 zeroes (when the operand is zero). It's unclear when support for it was added into GAS, if at all; even a not-so-old 2.27 doesn't know about it. Let's byte-encode it. Tested on an A72 and works as expected.
* DEBUG: crash using an invalid opcode on x86/x86_64 instead of an invalid accessWilly Tarreau2023-04-251-0/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | BUG_ON() calls currently trigger a segfault. This is more convenient than abort() as it doesn't rely on any function call nor signal handler and never causes non-unwindable stacks when opening cores. But it adds quite some confusion in bug reports which are rightfully tagged "segv" and do not instantly allow to distinguish real segv (e.g. null derefs) from code asserts. Some CPU architectures offer various crashing methods. On x86 we have INT3 (0xCC), which stops into the debugger, and UD0/UD1/UD2. INT3 looks appealing but for whatever reason (maybe signal handling somewhere) it loses the last call point in the stack, making backtraces unusable. UD2 has the merit of being only 2 bytes and causing an invalid instruction, which almost never happens normally, so it's easily distinguishable. Here it was defined as a macro so that the line number in the core matches the one where the BUG_ON() macro is called, and the debugger shows the last frame exactly at its calligg point. E.g. when calling "debug dev bug": Program terminated with signal SIGILL, Illegal instruction. #0 debug_parse_cli_bug (args=<optimized out>, payload=<optimized out>, appctx=<optimized out>, private=<optimized out>) at src/debug.c:408 408 BUG_ON(one > zero); [Current thread is 1 (Thread 0x7f7a660cc1c0 (LWP 14238))] (gdb) bt #0 debug_parse_cli_bug (args=<optimized out>, payload=<optimized out>, appctx=<optimized out>, private=<optimized out>) at src/debug.c:408 #1 debug_parse_cli_bug (args=<optimized out>, payload=<optimized out>, appctx=<optimized out>, private=<optimized out>) at src/debug.c:402 #2 0x000000000061a69f in cli_parse_request (appctx=appctx@entry=0x181c0160) at src/cli.c:832 #3 0x000000000061af86 in cli_io_handler (appctx=0x181c0160) at src/cli.c:1035 #4 0x00000000006ca2f2 in task_run_applet (t=0x181c0290, context=0x181c0160, state=<optimized out>) at src/applet.c:449
* BUILD: bug.h: add a warning in the base API when unsafe functions are usedWilly Tarreau2023-04-071-0/+37
| | | | | | | | | | | | Once in a while we introduce an sprintf() or strncat() function by accident. These ones are particularly dangerous and must never ever be used because the only way to use them safely is at least as complicated if not more, than their safe counterparts. By redefining a few of these functions with an attribute_warning() we can deliver a message to the developer who is tempted to use them. This commit does it for strcat(), strcpy(), strncat(), sprintf(), vsprintf(). More could come later if needed, such as strtok() and maybe a few others, but these are less common.
* MINOR: pools: report a replaced memory allocator instead of just malloc_trim()Willy Tarreau2023-03-221-0/+1
| | | | | | | | | | | Instead of reporting the inaccurate "malloc_trim() support" on -vv, let's report the case where the memory allocator was actively replaced from the one used at build time, as this is the corner case we want to be cautious about. We also put a tainted bit when this happens so that it's possible to detect it at run time (e.g. the user might have inherited it from an environment variable during a reload operation). The now unused is_trim_enabled() function was finally dropped.
* BUILD: debug: remove unnecessary quotes in HA_WEAK() callsWilly Tarreau2022-11-141-12/+12
| | | | | | | HA_WEAK() is supposed to take a symbol in argument, not a string, since the asm statements it produces already quote the argument. Having it quoted twice doesn't work on older compilers and was the only reason why DEBUG_MEM_STATS didn't work on older compilers.
* CLEANUP: debug: use struct ha_caller for memstatWilly Tarreau2022-09-081-22/+31
| | | | | | | | | The memstats code currently defines its own file/function/line number, type and extra pointer. We don't need to keep them separate and we can easily replace them all with just a struct ha_caller. Note that the extra pointer could be converted to a pool ID stored into arg8 or arg32 and be dropped as well, but this would first require to define IDs for pools (which we currently do not have).
* MINOR: debug: add struct ha_caller to describe a calling locationWilly Tarreau2022-09-081-0/+17
| | | | | | | | | | | The purpose of this structure is to assemble all constant parts of a generic calling point for a specific event. These ones are created by the compiler as a static const element outside of the code path, so they cost nothing in terms of CPU, and a pointer to that descriptor can be passed to the place that needs it. This is very similar to what is being done for the mem_stat stuff. This will be useful to simplify and improve DEBUG_TASK.
* BUILD: debug: make sure debug macros are never emptyWilly Tarreau2022-08-311-5/+5
| | | | | | | | | As outlined in commit f7ebe584d7 ("BUILD: debug: Add braces to if statement calling only CHECK_IF()"), the BUG_ON() family of macros is incorrectly defined to be empty when debugging is disabled, and that can lead to trouble. Make sure they always fall back to the usual "do { } while (0)". This may be backported to 2.6 if needed, though no such issue was met there to date.
* MINOR: debug/memstats: permit to pass the size to free()Willy Tarreau2022-08-091-6/+14
| | | | | | | | | | | | | | | | | | | Right now the free() call is not intercepted since all this is done using macros and that would break a lot of stuff. Instead a __free() macro was provided but never used. In addition it used to only report a zero size, which is not very convenient. With this patch comes a better solution. Instead it provides a new will_free() macro that can be prepended before a call to free(). It only keeps the counters up to date, and also supports being passed a size. The pool_free_area() command now uses it, which finally allows the stats to look correct: pool-os.h:38 MALLOC size: 5802127832 calls: 3868044 size/call: 1500 pool-os.h:47 FREE size: 5800041576 calls: 3867444 size/call: 1499 The few other places directly calling free() could now be instrumented to use this and to pass the correct sizeof() when known.
* MINOR: debug: also store the function name in struct mem_statsWilly Tarreau2022-08-091-0/+7
| | | | | | The calling function name is now stored in the structure, and it's reported when the "all" argument is passed. The first column is significantly enlarged because some names are really wide :-(
* MINOR: debug: store and report the pool's name in struct mem_statsWilly Tarreau2022-08-091-0/+1
| | | | | | | | | | | | | | | | | | | | | | Let's add a generic "extra" pointer to the struct mem_stats to store context-specific information. When tracing pool_alloc/pool_free, we can now store a pointer to the pool, which allows to report the pool name on an extra column. This significantly improves tracing capabilities. Example: proxy.c:1598 CALLOC size: 28832 calls: 4 size/call: 7208 dynbuf.c:55 P_FREE size: 32768 calls: 2 size/call: 16384 buffer quic_tls.h:385 P_FREE size: 34008 calls: 1417 size/call: 24 quic_tls_iv quic_tls.h:389 P_FREE size: 34008 calls: 1417 size/call: 24 quic_tls_iv quic_tls.h:554 P_FREE size: 34008 calls: 1417 size/call: 24 quic_tls_iv quic_tls.h:558 P_FREE size: 34008 calls: 1417 size/call: 24 quic_tls_iv quic_tls.h:562 P_FREE size: 34008 calls: 1417 size/call: 24 quic_tls_iv quic_tls.h:401 P_ALLOC size: 34080 calls: 1420 size/call: 24 quic_tls_iv quic_tls.h:403 P_ALLOC size: 34080 calls: 1420 size/call: 24 quic_tls_iv xprt_quic.c:4060 MALLOC size: 45376 calls: 5672 size/call: 8 quic_sock.c:328 P_ALLOC size: 46440 calls: 215 size/call: 216 quic_dgram
* MINOR: debug: make the mem_stats section aligned to void*Willy Tarreau2022-08-091-7/+7
| | | | | | | Not specifying the alignment will let the linker choose it, and it turns out that it will not necessarily be the same size as the one chosen for struct mem_stats, as can be seen if any new fields are added there. Let's enforce an alignment to void* both for the section and for the structure.
* BUILD: debug: silence warning on gcc-5Willy Tarreau2022-07-101-0/+2
| | | | | | | | | In 2.6, 8a0fd3a36 ("BUILD: debug: work around gcc-12 excessive -Warray-bounds warnings") disabled some warnings that were reported around the the BUG() statement. But the -Wnull-dereference warning isn't known from gcc-5, it only arrived in gcc-6, hence makes gcc-5 complain loudly that it doesn't know this directive. Let's just condition this one to gcc-6.
* MEDIUM: debug: improve DEBUG_MEM_STATS to also report pool alloc/freeWilly Tarreau2022-06-231-0/+2
| | | | | | | | | | | | Sometimes using "debug dev memstats" can be frustrating because all pool allocations are reported through pool-os.h and that's all. But in practice there's nothing wrong with also intercepting pool_alloc, pool_free and pool_zalloc and report their call counts and locations, so that's what this patch does. It only uses an alternate set of macroes for these 3 calls when DEBUG_MEM_STATS is defined. The outputs are reported as P_ALLOC (for both pool_malloc() and pool_zalloc()) and P_FREE (for pool_free()).
* MEDIUM: debug: detect redefinition of symbols upon dlopen()Willy Tarreau2022-06-191-0/+1
| | | | | | | | | | | | | | | | | In order to better detect the danger caused by extra shared libraries which replace some symbols, upon dlopen() we now compare a few critical symbols such as malloc(), free(), and some OpenSSL symbols, to see if the loaded library comes with its own version. If this happens, a warning is emitted and TAINTED_REDEFINITION is set. This is important because some external libs might be linked against different libraries than the ones haproxy was linked with, and most often this will end very badly (e.g. an OpenSSL object is allocated by haproxy and freed by such libs). Since the main source of dlopen() calls is the Lua lib, via a "require" statement, it's worth trying to show a Lua call trace when detecting a symbol redefinition during dlopen(). As such we emit a Lua backtrace if Lua is detected as being in use.
* MEDIUM: debug: add a tainted flag when a shared library is loadedWilly Tarreau2022-06-191-0/+1
| | | | | | | | | | | | | | | Several bug reports were caused by shared libraries being loaded by other libraries or some Lua code. Such libraries could define alternate symbols or include dependencies to alternate versions of a library used by haproxy, making it very hard to understand backtraces and analyze the issue. Let's intercept dlopen() and set a new TAINTED_SHARED_LIBS flag when it succeeds, so that "show info" makes it visible that some external libs were added. The redefinition is based on the definition of RTLD_DEFAULT or RTLD_NEXT that were previously used to detect that dlsym() is usable, since we need it as well. This should be sufficient to catch most situations.
* BUILD: debug: work around gcc-12 excessive -Warray-bounds warningsWilly Tarreau2022-05-091-3/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As was first reported by Ilya in issue #1513, compiling with gcc-12 adds warnings about size 0 around each BUG_ON() call due to the ABORT_NOW() macro that tries to dereference pointer value 1. The problem is known, seems to be complex inside gcc and could only be worked around for now by adjusting a pointer limit so that the warning still catches NULL derefs in the first page but not other values commonly used in kernels and boot loaders: https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=91f7d7e1b It's described in more details here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104657 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103768 And some projects had to work around it using various approaches, some of which are described in the bugs reports above, plus another one here: https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/HLK3BHP2T3FN6FZ46BIPIK3VD5FOU74Z/ In haproxy we can hide it by hiding the pointer in a DISGUISE() macro, but this forces the pointer to be loaded into a register, so that register is lost precisely where we want to get the maximum of them. In our case we purposely use a low-value non-null pointer because: - it's mandatory that this value fits within an unmapped page and only the lowest one has this property - we really want to avoid register loads for the address, as these will be lost and will complicate the bug analysis, and they tend to be used for large addresses (i.e. instruction length limit). - the compiler may decide to optimize away the null deref when it sees it (seen in the past already) As such, the current workaround merged in gcc-12 is not effective for us. Another approach consists in using pragmas to silently disable -Warray-bounds and -Wnull-dereference only for this part. The problem is that pragmas cannot be placed into macros. The resulting solution consists in defining a forced-inlined function only to trigger the crash, and surround the dereference with pragmas, themselves conditionned to gcc >= 5 since older versions don't understand them (but they don't complain on the dereference at least). This way the code remains the same even at -O0, without the stack pointer being modified nor any address register being modified on common archs (x86 at least). A variation could have been to rely on __builtin_trap() but it's not everywhere and it behaves differently on different platforms (undefined opcode or a nasty abort()) while the segv remains uniform and effective. This may need to be backported to older releases once users start to complain about gcc-12 breakage.
* BUILD: compiler: properly distinguish weak and global symbolsWilly Tarreau2022-04-261-12/+12
| | | | | | | | | | | | | | | | | | | | | While weak symbols were finally fixed with commit fb1b6f5bc ("BUILD: compiler: use a more portable set of asm(".weak") statements"), it was an error to think that initcall symbols were also weak. They must not be and they're only global. The reason is that any externally linked code loaded as a .so would drop its weak symbols when being loaded, hence its initcalls that may contain various function registration calls. The ambiguity came from the fact that we initially reused the initcall's HA_GLOBL macro for OSX then generalized it, then turned it to a choice between .globl and .weak based on the OS, while in fact we needed a macro to define weak symbols. Let's rename the macro to HA_WEAK() to make it clear it's only for weak symbols, and redefine HA_GLOBL() that initcall needs. This will need to be backported wherever the commit above is backported (at least 2.5 for now).
* BUILD: compiler: use a more portable set of asm(".weak") statementsWilly Tarreau2022-04-141-12/+12
| | | | | | | | | | | | | | | | | | | | The two recent patches b12966af1 ("BUILD: debug: mark the __start_mem_stats/__stop_mem_stats symbols as weak") and 2a06e248f ("BUILD: initcall: mark the __start_i_* symbols as weak, not global") aimed at fixing a build warning and resulted in a build breakage on MacOS which doesn't have a ".weak" asm statement. We've already had MacOS-specific asm() statements for section names, so this patch continues on this trend by moving HA_GLOBL() to compiler.h and using ".globl" on MacOS since apparently nobody complains there. It is debatable whether to expose this only when !USE_OBSOLETE_LINKER or all the time, but since these are just macroes it's no big deal to let them be available when needed and let the caller decide on the build conditions. If any of the patches above is backported, this one will need to as well.
* BUILD: debug: mark the __start_mem_stats/__stop_mem_stats symbols as weakWilly Tarreau2022-04-131-12/+12
| | | | | | | | | | | | | | Building with clang and DEBUG_MEM_STATS shows the following warnings: warning: __start_mem_stats changed binding to STB_WEAK [-Wsource-mgr] warning: __stop_mem_stats changed binding to STB_WEAK [-Wsource-mgr] The reason is that the symbols are declared using ".globl" while they are also referenced as __attribute__((weak)) elsewhere. It turns out that a weak symbol is implicitly a global one and that the two classes are exclusive, thus it may confuse the linker. Better fix this. This may be backported where the patch applies.
* DEBUG: reduce the footprint of BUG_ON() callsWilly Tarreau2022-03-021-33/+18
| | | | | | | | | | | | | | | | | | | | | | Many inline functions involve some BUG_ON() calls and because of the partial complexity of the functions, they're not inlined anymore (e.g. co_data()). The reason is that the expression instantiates the message, its size, sometimes a counter, then the atomic OR to taint the process, and the back trace. That can be a lot for an inline function and most of it is always the same. This commit modifies this by delegating the common parts to a dedicated function "complain()" that takes care of updating the counter if needed, writing the message and measuring its length, and tainting the process. This way the caller only has to check a condition, pass a pointer to the preset message, and the info about the type (bug or warn) for the tainting, then decide whether to dump or crash. Note that this part could also be moved to the function but resulted in complain() always being at the top of the stack, which didn't seem like an improvement. Thanks to these changes, the BUG_ON() calls do not result in uninlining functions anymore and the overall code size was reduced by 60 to 120 kB depending on the build options.
* BUILD: debug: fix build warning on older compilers around DEBUG_STRICT_ACTIONWilly Tarreau2022-02-281-3/+3
| | | | | | | | The new macro was introduced with commit 86bcc5308 ("DEBUG: implement 4 levels of choices between warn and crash.") but some older compilers can complain that we test the value when the macro is not defined despite having already been checked in a previous #if directive. Let's just repeat the test for the definition.
* DEBUG: add two new macros to enable debugging in hot pathsWilly Tarreau2022-02-281-0/+25
| | | | | | | | | | | Two new BUG_ON variants, BUG_ON_HOT() and CHECK_IF_HOT() are introduced to debug hot paths (such as low-level API functions). These ones must not be enabled by default as they would significantly affect performance but they may be enabled by setting DEBUG_STRICT to a value above 1. In this case, DEBUG_STRICT_ACTION is mostly respected with a small change, which is that the no_crash variant of BUG_ON() isn't turned to a regular warning but to a one-time warning so as not to spam with warnings in a hot path. It is for this reason that there is no WARN_ON_HOT().
* DEBUG: implement 4 levels of choices between warn and crash.Willy Tarreau2022-02-281-6/+27
| | | | | | | | | | | | We used to have DEBUG_STRICT_NOCRASH to disable crashes on BUG_ON(). Now we have other levels (WARN_ON(), CHECK_IF()) so we need something finer-grained. This patch introduces DEBUG_STRICT_ACTION which takes an integer value. 0 disables crashes and is the equivalent of DEBUG_STRICT_NOCRASH. 1 is the default and only enables crashes on BUG_ON(). 2 also enables crashes on WARN_ON(), and 3 also enables warnings on CHECK_IF(), and is suited to developers and CI.
* DEBUG: improve BUG_ON output message accuracyWilly Tarreau2022-02-281-9/+9
| | | | | | Now we'll explicitly mention if the test was a bug/warn/check, and "FATAL" is only displayed when the process crashes. The non-crashing BUG_ON() also suggests to report to developers.
* DEBUG: rename WARN_ON_ONCE() to CHECK_IF()Willy Tarreau2022-02-281-3/+3
| | | | | | The only reason for warning once is to check if a condition really happens. Let's use a term that better translates the intent, that's important when reading the code.
* DEBUG: report BUG_ON() and WARN_ON() in the tainted flagsWilly Tarreau2022-02-251-5/+17
| | | | | It can be useful to know from the "tainted" variable whether any WARN_ON() or BUG_ON() triggered. Both were now added.
* DEBUG: add a new WARN_ON_ONCE() macroWilly Tarreau2022-02-251-4/+31
| | | | | | This one will maintain a static counter per call place and will only emit the warning on the first call. It may be used to invite users to report an unexpected event without spamming them with messages.
* DEBUG: make the _BUG_ON() macro return the conditionWilly Tarreau2022-02-251-4/+7
| | | | | | | | By doing so it now becomes an expression and will allow for example to use WARN_ON() in tests, for example: if (WARN_ON(cond)) return NULL;
* DBEUG: add a new WARN_ON() macroWilly Tarreau2022-02-251-2/+5
| | | | | | | | This is the same as BUG_ON() except that it never crashes and only emits a warning and a backtrace, inviting users to report the problem. This will be usable for non-fatal issues that should not happen and need to be fixed. This way the BUG_ON() when using DEBUG_STRICT_NOCRASH is effectively an equivalent of WARN_ON().
* DEBUG: mark ABORT_NOW() as unreachableWilly Tarreau2022-02-251-1/+1
| | | | | | | The purpose is to make the program die at this point, so let's help the compiler optimise the code (especially in sensitive areas) by telling it that ABORT_NOW() does not return. This reduces the overall code size by ~0.5%.
* DEBUG: cleanup BUG_ON() configurationWilly Tarreau2022-02-251-17/+22
| | | | | | | | | | | | The BUG_ON() macro handling is complicated because it relies on a conditional CRASH_NOW() macro whose definition depends on DEBUG_STRICT and DEBUG_STRICT_NOCRASH. Let's rethink the whole thing differently, and instead make the underlying _BUG_ON() macro take a crash argument to decide whether to crash or not, as well as a prefix and a suffix for the message, that will allow to distinguish between variants. Now the suffix is set to a message explaining we don't crash when needed. This also allows to get rid of the CRASH_NOW() macro and to define much simpler new macros.
* DEBUG: cleanup back trace generationWilly Tarreau2022-02-251-3/+5
| | | | | | Most BUG()/ABORT() macros were duplicating the same code to call the backtrace production to stderr, better place that into a new DUMP_TRACE() macro.
* DEBUG: move the tainted stuff to bug.h for easier inclusionWilly Tarreau2022-02-251-0/+23
| | | | | | The functions needed to manipulate the "tainted" flags were located in too high a level to be callable from the lower code layers. Let's move them to bug.h.
* BUILD: bug: Fix error when compiling with -DDEBUG_STRICT_NOCRASHChristopher Faulet2021-12-031-1/+1
| | | | | | | | | | | ha_backtrace_to_stderr() must be declared in CRASH_NOW() macro whe HAProxy is compiled with DEBUG_STRICT_NOCRASH. Otherwise an error is reported during compilation: include/haproxy/bug.h:58:26: error: implicit declaration of function ‘ha_backtrace_to_stderr’ [-Werror=implicit-function-declaration] 58 | #define CRASH_NOW() do { ha_backtrace_to_stderr(); } while (0) This patch must be backported as far as 2.4.
* CLEANUP: tree-wide: fix prototypes for functions taking no arguments.Tim Duesterhus2021-09-151-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "f(void)" is the correct and preferred form for a function taking no argument, while some places use the older "f()". These were reported by clang's -Wmissing-prototypes, for example: src/cpuset.c:111:5: warning: no previous prototype for function 'ha_cpuset_size' [-Wmissing-prototypes] int ha_cpuset_size() include/haproxy/cpuset.h:42:5: note: this declaration is not a prototype; add 'void' to make it a prototype for a zero-parameter function int ha_cpuset_size(); ^ void This aggregate patch fixes this for the following functions: ha_backtrace_to_stderr(), ha_cpuset_size(), ha_panic(), ha_random64(), ha_thread_dump_all_to_trash(), get_exec_path(), check_config_validity(), mworker_child_nb(), mworker_cli_proxy_(create|stop)(), mworker_cleantasks(), mworker_cleanlisteners(), mworker_ext_launch_all(), mworker_reload(), mworker_(env|proc_list)_to_(proc_list|env)(), mworker_(un|)block_signals(), proxy_adjust_all_maxconn(), proxy_destroy_all_defaults(), get_tainted(), pool_total_(allocated|used)(), thread_isolate(_full|)(), thread(_sync|)_release(), thread_harmless_till_end(), thread_cpu_mask_forced(), dequeue_all_listeners(), next_timer_expiry(), wake_expired_tasks(), process_runnable_tasks(), init_acl(), init_buffer(), (de|)init_log_buffers(), (de|)init_pollers(), fork_poller(), pool_destroy_all(), pool_evict_from_local_caches(), pool_total_failures(), dump_pools_to_trash(), cfg_run_diagnostics(), tv_init_(process|thread)_date(), __signal_process_queue(), deinit_signals(), haproxy_unblock_signals()
* CLEANUP: atomic/tree-wide: replace single increments/decrements with inc/decWilly Tarreau2021-04-071-6/+6
| | | | | | This patch replaces roughly all occurrences of an HA_ATOMIC_ADD(&foo, 1) or HA_ATOMIC_SUB(&foo, 1) with the equivalent HA_ATOMIC_INC(&foo) and HA_ATOMIC_DEC(&foo) respectively. These are 507 changes over 45 files.
* BUILD: bug: refine HA_LINK_ERROR() to only be used on gcc and derivativesWilly Tarreau2021-03-091-1/+1
| | | | | | | TCC happens to define __OPTIMIZE__ at -O2 but doesn't proceed with dead code elimination, resulting in ha_free() to always reference the link error symbol. Let's condition this test on __GCC__ which others like Clang also define.
* BUILD: Fix build when using clang without optimizing.Olivier Houchard2021-03-051-6/+16
| | | | | | | ha_free() uses code that attempts to set a non-existant variable to provoke a link-time error, with the expectation that the compiler will not omit that if the code is unreachable. However, clang will emit it when compiling with no optimization, so only do that if __OPTIMIZE__ is defined.
* CLEANUP: tree-wide: replace free(x);x=NULL with ha_free(&x)Willy Tarreau2021-02-261-0/+31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This makes the code more readable and less prone to copy-paste errors. In addition, it allows to place some __builtin_constant_p() predicates to trigger a link-time error in case the compiler knows that the freed area is constant. It will also produce compile-time error if trying to free something that is not a regular pointer (e.g. a function). The DEBUG_MEM_STATS macro now also defines an instance for ha_free() so that all these calls can be checked. 178 occurrences were converted. The vast majority of them were handled by the following Coccinelle script, some slightly refined to better deal with "&*x" or with long lines: @ rule @ expression E; @@ - free(E); - E = NULL; + ha_free(&E); It was verified that the resulting code is the same, more or less a handful of cases where the compiler optimized slightly differently the temporary variable that holds the copy of the pointer. A non-negligible amount of {free(str);str=NULL;str_len=0;} are still present in the config part (mostly header names in proxies). These ones should also be cleaned for the same reasons, and probably be turned into ist strings.
* MEDIUM: debug: now always print a backtrace on CRASH_NOW() and friendsWilly Tarreau2021-01-221-3/+3
| | | | | | | | | | | | | | The purpose is to enable the dumping of a backtrace on BUG_ON(). While it's very useful to know that a condition was met, very often some caller context is missing to figure how the condition could happen. From now on, on systems featuring backtrace, a backtrace of the calling thread will also be dumped to stderr in addition to the unexpected condition. This will help users of DEBUG_STRICT as they'll most often find this backtrace in their logs even if they can't find their core file. A new "debug dev bug" expert-mode CLI command was added to test the feature.
* MINOR: debug: don't count free(NULL) in memstatsWilly Tarreau2020-11-031-1/+2
| | | | | | | | The mem stats are pretty convenient to spot leaks, except that they count free(NULL) as 1, and the code does actually have quite a number of free(foo) guards where foo is NULL if the object was already freed. Let's just not count these ones so that the stats remain consistent. Now it's possible to compare the strdup()/malloc() and free() and verify they are consistent.
* BUILD: introduce possibility to define ABORT_NOW() conditionallyIlya Shipitsin2020-09-121-3/+8
| | | | | code analysis tools recognize abort() better, so let us introduce such possibility
* BUILD: debug: avoid build warnings with DEBUG_MEM_STATSWilly Tarreau2020-07-021-0/+8
| | | | | Some libcs define strdup() as a macro and cause redefine warnings to be emitted, so let's first undefine all functions we redefine.
* MINOR: debug: add a new "debug dev memstats" commandWilly Tarreau2020-07-021-0/+90
| | | | | | | Now when building with -DDEBUG_MEM_STATS, some malloc/calloc/strdup/realloc stats are kept per file+line number and may be displayed and even reset on the CLI using "debug dev memstats". This allows to easily track potential leakers or abnormal usages.
* REORG: include: move the BUG_ON() code to haproxy/bug.hWilly Tarreau2020-06-111-0/+78
This one used to be stored into debug.h but the debug tools got larger and require a lot of other includes, which can't use BUG_ON() anymore because of this. It does not make sense and instead this macro should be placed into the lower includes and given its omnipresence, the best solution is to create a new bug.h with the few surrounding macros needed to trigger bugs and place assertions anywhere. Another benefit is that it won't be required to add include <debug.h> anymore to use BUG_ON, it will automatically be covered by api.h. No less than 32 occurrences were dropped. The FSM_PRINTF macro was dropped since not used at all anymore (probably since 1.6 or so).