Broken stats on aarch64 release build

[pjungkamp]

An aarch64 release build may cause the stats to show wrong number for Recv, Send, Drop and Signals. Example log output:

11:13:10 info stats             Node           │           Recv │           Sent │           Drop │           Skip │       OWD last │       OWD mean │      Rate last │      Rate mean │       Age mean │        Age Max │     Signals
11:13:10 info stats                            │           pkts │           pkts │           pkts │           pkts │           secs │           secs │        pkt/sec │        pkt/sec │           secs │            sec │     signals
11:13:10 info stats            ────────────────┼────────────────┼────────────────┼────────────────┼────────────────┼────────────────┼────────────────┼────────────────┼────────────────┼────────────────┼────────────────┼─────────────
11:13:11 info stats             webrtc         │ 28147402498383 │ 28147402498383 │ 28147402498383 │ 28147402498383 │       0.013749 │       0.013749 │       0.013749 │       0.013749 │       0.013749 │       0.013749 │ 28147402498 
11:13:12 info stats             webrtc         │ 28147402498383 │ 28147402498383 │ 28147402498383 │ 28147402498383 │       0.014313 │       0.014313 │       0.014313 │       0.014313 │       0.014313 │       0.014313 │ 28147402498 
11:13:13 info stats             webrtc         │ 28147402498383 │ 28147402498383 │ 28147402498383 │ 28147402498383 │       0.014452 │       0.014452 │       0.014452 │       0.014452 │       0.014452 │       0.014452 │ 28147402498 
11:13:14 info stats             webrtc         │ 28147402498383 │ 28147402498383 │ 28147402498383 │ 28147402498383 │       0.013672 │       0.013672 │       0.013672 │       0.013672 │       0.013672 │       0.013672 │ 28147402498 
11:13:15 info stats             webrtc         │ 28147402498383 │ 28147402498383 │ 28147402498383 │ 28147402498383 │       0.013599 │       0.013599 │       0.013599 │       0.013599 │       0.013599 │       0.013599 │ 28147402498 
11:13:16 info stats             webrtc         │ 28147402498383 │ 28147402498383 │ 28147402498383 │ 28147402498383 │       0.014081 │       0.014081 │       0.014081 │       0.014081 │       0.014081 │       0.014081 │ 28147402498 

[IgnoreWarnings]

Description

This bug occures because "va_list" is used after beeing passed to "vasprintf", which leads to undeterministic behavior.
Functions like "vasprintf" "invoke va_arg at least once, the value of arg is indeterminate after the return." (https://en.cppreference.com/w/c/io/vfprintf : Notes).

Also, villas callers assume that "vasprintf" will take out the args of "va_list" parameter (it should advance). CPP-Insights (https://cppinsights.io/) resolves the "va_list" macro into a pointer type (pass by reference). However, the compiler output on GCC x86 and ARM GCC shows pass by reference and pass by value behavior which corresponds to the code not working as intended on ARM platforms.
To produce equivalent behavior on both platforms, references should be used, which complies with the documentation: "It is legal to pass a pointer to a va_list object to another function and then use that object after the function returns." (https://en.cppreference.com/w/c/variadic/va_list)

Trace

common/lib/table.cpp

void Table::row(int count, ...) {
 (...)
  for (unsigned i = 0; i < columns.size(); ++i) {
    char *col = vstrf(columns[i].format.c_str(), args); <--- passed here, not specified as reference explicitly

common/lib/utils.cpp

char *vstrf(const char *fmt, va_list va) {
  char *buf = nullptr;

  vstrcatf(&buf, fmt, va); <--- va_list passed (unaltered) to vstrcatf

  return buf;
}

common/lib/utils.cpp

char *vstrcatf(char **dest, const char *fmt, va_list ap) {
  (...)
  int i = vasprintf(&tmp, fmt, ap); <--- va_list passed to vasprintf, which leads to it beeing indeterminated
  (...)
  return *dest;
}

@fwege

[stv0g]

Nice analysis. I an wondering. Was AI helping here? 😁

[IgnoreWarnings]

Nice analysis. I an wondering. Was AI helping here? 😁

Just brainpower. AI was consulted during the research process but produced (obviously) incorrect and looping explanations, which had nothing to do at all with the problem.