Turns out that the original way we did things was quite broken, as it
skipped a _lot_ of code. This was because we just threw everything into
one pile and tried to spatch it, but this made Coccinelle sad, like when
man page examples redefined some of our macros, causing typedef
conflicts.
For example, with a minimal reproducer that defines a cleanup macro in
two source files, Coccinelle has no issues when spatch-ing each one
separately:
$ spatch --verbose-parsing --sp-file zz-drop-braces.cocci main.c
init_defs_builtins: /usr/lib64/coccinelle/standard.h
HANDLING: main.c
SPECIAL NAMES: adding _cleanup_ as a attribute with arguments
SPECIAL NAMES: adding _cleanup_free_ as a attribute
$ spatch --verbose-parsing --sp-file zz-drop-braces.cocci
logcontrol-example.c
init_defs_builtins: /usr/lib64/coccinelle/standard.h
HANDLING: logcontrol-example.c
SPECIAL NAMES: adding _cleanup_ as a attribute with arguments
But when you try to spatch both of them at once, Coccinelle starts
complaining and skipping the "bad" code:
$ spatch --verbose-parsing --sp-file zz-drop-braces.cocci main.c logcontrol-example.c
init_defs_builtins: /usr/lib64/coccinelle/standard.h
HANDLING: main.c logcontrol-example.c
SPECIAL NAMES: adding _cleanup_ as a attribute with arguments
SPECIAL NAMES: adding _cleanup_free_ as a attribute
remapping: _cleanup_ to an ident in macro name
ERROR-RECOV: found sync end of #define, line 44
parsing pass2: try again
ERROR-RECOV: found sync end of #define, line 44
parse error
= File "logcontrol-example.c", line 44, column 21, charpos = 1719
around = '__attribute__',
whole content = #define _cleanup_(f) __attribute__((cleanup(f)))
badcount: 2
bad: #include <systemd/sd-journal.h>
bad:
BAD:!!!!! #define _cleanup_(f) __attribute__((cleanup(f)))
This was, unfortunately, hidden as it is visible only with
--verbose-parsing (or --parse-error-msg).
Another issue was how we handled includes. The original way of throwing
them into the pile of source files doesn't really work, leading up to
similar issues as above. The better way is to let Coccinelle properly
resolve all includes by telling it where to find our own include files
(basically the same thing we already do during compilation).
After fixing all this, Coccinelle now has a chance to process much more
of our code (there are still some issues in more complex macros, but
that requires further investigation). However, there's a huge downside
from all of this - doing a _proper_ code analysis is surprisingly time
and resource heavy; meaning that processing just one Coccinelle rule now
takes 15 - 30 minutes.
To make this slightly less painful, Coccinelle supports caching the
generated ASTs, which actually helps a lot - it gets the runtime of one
rule from 15 - 30 minutes down to ~1 minute. It, of course, has its own
downside - the cache is _really_ big (ATTOW the cache takes ~15 GiB).
However, even with the aggressive AST caching you're still looking at
~1 hour for one full Coccinelle run, which is a bit annoying, but I
guess that's the price of doing things _properly_ (but I'll definitely
look into ways of further optimizing this).
Avoid passing a NULL message to sd_bus_message_is_signal(), to not trip
over an assertion:
[ 132.869436] H testsuite-82.sh[614]: + systemctl --no-block --check-inhibitors=yes soft-reboot
[ 132.967386] H systemd[1]: Created slice system-systemd\x2dcoredump.slice.
[ 133.018292] H systemd[1]: Starting inhibit.service...
[ 133.122610] H systemd[1]: Started systemd-coredump@0-665-0.service.
[ 133.163643] H systemd[1]: Started inhibit.service.
[ 133.206836] H testsuite-82.sh[614]: + exec sleep infinity
[ 133.236762] H systemd-logind[611]: The system will reboot now!
[ 135.891607] H systemd-coredump[667]: [🡕] Process 663 (busctl) of user 0 dumped core.
Stack trace of thread 663:
#0 0x00007f2ec45e6acf raise (libc.so.6 + 0x4eacf)
#1 0x00007f2ec45b9ea5 abort (libc.so.6 + 0x21ea5)
#2 0x00007f2ec4b5c9a6 log_assert_failed (libsystemd-shared-255.so + 0x1ff9a6)
#3 0x00007f2ec4b5dca5 log_assert_failed_return (libsystemd-shared-255.so + 0x200ca5)
#4 0x00007f2ec4bb3df6 sd_bus_message_is_signal (libsystemd-shared-255.so + 0x256df6)
#5 0x000000000040e478 monitor (busctl + 0xe478)
#6 0x000000000040e82f verb_monitor (busctl + 0xe82f)
#7 0x00007f2ec4b202cb dispatch_verb (libsystemd-shared-255.so + 0x1c32cb)
#8 0x00000000004074fa busctl_main (busctl + 0x74fa)
#9 0x0000000000407525 run (busctl + 0x7525)
#10 0x000000000040ff67 main (busctl + 0xff67)
#11 0x00007f2ec45d2d85 __libc_start_main (libc.so.6 + 0x3ad85)
#12 0x00000000004044be _start (busctl + 0x44be)
ELF object binary architecture: AMD x86-64
[ 136.141152] H dbus-daemon[634]: [system] Monitoring connection :1.2 closed.
[ 136.152233] H systemd[1]: busctl.service: Main process exited, code=dumped, status=6/ABRT
[ 136.153996] H systemd[1]: busctl.service: Failed with result 'core-dump'.
The asertion in question:
Assertion 'm' failed at src/libsystemd/sd-bus/bus-message.c:1015, function sd_bus_message_is_signal(). Aborting.
We can get a NULL message here through sd_bus_process() ->
bus_process_internal() -> process_running(), so let's handle this case
appropriately.
Since the triggered unit intentionally fails without consuming any data
from the socket, we'd try to trigger it again and again, and we might
try to check the unit state in one of the "in-between" states, failing
the test:
[ 165.271698] H testsuite-07.sh[1032]: + systemctl start badbin_assert.socket
[ 165.977637] H testsuite-07.sh[1032]: + socat - ABSTRACT-CONNECT:badbin_assert.socket
[ 165.983787] H systemd[1]: Cannot find unit for notify message of PID 1039, ignoring.
[ 166.817187] H testsuite-07.sh[1032]: + timeout 10 sh -c 'while systemctl is-active badbin_assert.service; do sleep .5; done'
[ 167.049218] H testsuite-07.sh[1065]: active
[ 167.146854] H systemd[1]: Listening on badbin_assert.socket.
[ 167.163473] H systemd[1]: badbin_assert.socket: Incoming traffic
[ 167.542626] H systemd[1]: Cannot find unit for notify message of PID 1065, ignoring.
[ 167.543437] H (badbin)[1062]: badbin_assert.service: Failed to execute /tmp/badbin: Exec format error
[ 167.548346] H systemd[1]: badbin_assert.service: Main process exited, code=exited, status=203/EXEC
[ 167.549482] H systemd[1]: badbin_assert.service: Failed with result 'exit-code'.
[ 167.561537] H systemd[1]: badbin_assert.socket: Incoming traffic
[ 167.933390] H systemd[1]: Started badbin_assert.service.
[ 167.950489] H (badbin)[1070]: badbin_assert.service: Failed to execute /tmp/badbin: Exec format error
[ 167.956318] H systemd[1]: badbin_assert.service: Main process exited, code=exited, status=203/EXEC
[ 167.957173] H systemd[1]: badbin_assert.service: Failed with result 'exit-code'.
[ 167.974609] H systemd[1]: badbin_assert.socket: Incoming traffic
[ 168.042838] H testsuite-07.sh[1072]: failed
[ 168.094431] H testsuite-07.sh[1075]: ++ systemctl show -P ExecMainStatus badbin_assert.service
[ 168.704022] H systemd[1]: Started badbin_assert.service.
[ 168.778680] H (badbin)[1074]: badbin_assert.service: Failed to execute /tmp/badbin: Exec format error
[ 168.826881] H systemd[1]: badbin_assert.service: Main process exited, code=exited, status=203/EXEC
[ 168.833825] H systemd[1]: badbin_assert.service: Failed with result 'exit-code'.
[ 168.923931] H testsuite-07.sh[1032]: + [[ 0 == 203 ]]
[ 168.951492] H systemd[1]: Cannot find unit for notify message of PID 1075, ignoring.
[ 168.999862] H testsuite-07.sh[615]: + echo 'Subtest /usr/lib/systemd/tests/testdata/units/testsuite-07.issue-30412.sh failed'
[ 168.999862] H testsuite-07.sh[615]: Subtest /usr/lib/systemd/tests/testdata/units/testsuite-07.issue-30412.sh failed
Follow-up for 1eeaa93de3 and 28a2d27650.
ukify (and all the tests, including the autogenerated check-version-ukify)
does not work unless pefile is available, so track it as a dependency
in meson to avoid unit test failures later
Triggering assert_return() should be a bug in general, and we should
really fix that. But, previously, it is hard to notice such bug, as
it was not critical.
This is for making CI or our testing environment fail if we unexpectedly
trigger assert_return(). So, hopefully we can easily find such bugs.
Several test cases intentionally trigger assert_return(). So, to avoid
the entire test fails, this introduces several macros that tentatively
make assert_return() not critical.