On Tue, Mar 19, 2019 at 07:30:09PM -0500, William A Rowe Jr wrote:
> According to my observations, apr_time_t should match the APR_TIME_T_FMT
> token in every case. Please inspect that line of httpd code to see how some
> non-apr_time_t value was passed in APR_TIME_T_FMT formatting.
Indeed, this value is not a time_t, it's an apr_int64_t, i.e. long.
The problematic format string is in this bit code from proxy_util.c
starting at line 3176:
ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(00959)
"ap_proxy_connect_backend disabling worker for (%s) for %"
APR_TIME_T_FMT "s",
worker->s->hostname_ex, apr_time_sec(worker->s->retry));
This assumes apr_time_sec returns apr_time_t, but in fact apr_time_sec is
a macro. So the expression returns the type of the variable passed in,
which in this case is apr_interval_time_t.
from mod_proxy.h:
apr_interval_time_t retry; /* retry interval */
and from apr_time.h:
typedef apr_int64_t apr_interval_time_t;
So we need this patch:
--- modules/proxy/proxy_util.c (revision 1855812)
+++ modules/proxy/proxy_util.c (working copy)
@@ -3175,7 +3175,7 @@ PROXY_DECLARE(int) ap_proxy_connect_backend(const
worker->s->status |= PROXY_WORKER_IN_ERROR;
ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(00959)
"ap_proxy_connect_backend disabling worker for (%s) for %"
- APR_TIME_T_FMT "s",
+ APR_INT64_T_FMT "s",
worker->s->hostname_ex, apr_time_sec(worker->s->retry));
}
}
But even with this patch, the error remains:
[[[
cc1: warnings being treated as errors
proxy_util.c: In function 'ap_proxy_connect_backend':
proxy_util.c:3179: warning: format '%ld' expects type 'long int', but argument
9 has type 'long long int'
]]]
This was confusing me at first: apr_int64_t is a long, so why does the
compiler see a 'long long' type?
I then realized that httpd's configure script was picking up the ancient
GPLv2 version of gcc which is still installed even though it's rarely used.
I forced my httpd turnk build to use clang and got a more informative warning:
[[[
proxy_util.c:3179:45: warning: format specifies type 'long' but the argument has
type 'long long' [-Wformat]
worker->s->hostname_ex, apr_time_sec(worker->s->retry));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/apr/include/apr-2/apr_time.h:63:28: note: expanded from macro
'apr_time_sec'
#define apr_time_sec(time) ((time) / APR_USEC_PER_SEC)
]]]
Turns out the remaining problem lies in the definition of APR_USEC_PER_SEC:
/** number of microseconds per second */
#define APR_USEC_PER_SEC APR_TIME_C(1000000)
Following the chain of definitions, we arrive at a constant suffixed with LL:
/* Mechanisms to properly type numeric literals */
#define APR_INT64_C(val) INT64_C(val)
/usr/include/sys/stdint.h:#define INT64_C(_c) __CONCAT(_c, LL)
/usr/include/sys/stdint.h:#define UINT64_C(_c) __CONCAT(_c,
ULL)
Yet APR blindly assumes that INT64_C results in constants suffixed with
just one L (long)!
A similar problem exists for time_t, by the way.
OpenBSD defines time_t as 'long long', but apr_time_t is defined as 'long'
in a 64 bit environment. This means that apr_time_t won't have the same type
as time_t on 64 bit OpenBSD architectures. On 32 bit platforms, apr_time_t
and time_t are both defined as 'long long'.
OpenBSD's convention is to use '%lld' for printing time_t, regardless
of the CPU platform -- all platforms use 64-bit time_t since OpenBSD 5.5:
https://www.openbsd.org/lyrics.html#55
