Re: [Sbcl-devel] [Sbcl-commits] master: Reimplement low-level logging

[Christophe R. <cs...@ca...>]

I think this somehow breaks non-threaded platforms, and single-threaded
builds on usually-threaded platforms: the use of pthread_self() without
a pthread.h include in scope.  (Of course nothing is simple because we
probably don't want to include pthread.h on Windows).

In file included from interrupt.c:71:
interrupt.c: In function 'can_handle_now':
genesis/events.h:66:30: error: implicit declaration of function 'pthread_self'; did you mean 'pthread_kill'? [-Wimplicit-function-declaration]
   66 |    eventdata[i_] = ((uword_t)pthread_self() << 2) | id;   eventdata[i_+1] = (uword_t)arg1; \
      |                              ^~~~~~~~~~~~
genesis/events.h:91:55: note: in expansion of macro 'EVENT3'
   91 | #define event_SigDeferred_IntDisabled(arg1,arg2,arg3) EVENT3(EventSigDeferred_IntDisabled,arg1,arg2,arg3)
      |                                                       ^~~~~~
interrupt.c:1262:9: note: in expansion of macro 'event_SigDeferred_IntDisabled'
 1262 |         event_SigDeferred_IntDisabled(handler, signal, in_leaving_without_gcing_race_p(thread));
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

is where it breaks for me on arm and x86-64.

It is not broken in github CI for non-threaded x86 / x86-64, which is
interesting: I guess something was making pthread_self() visible there
despite the missing explicit include?  My build breakage is with Debian
trixie in both cases, and locally adding <pthread.h> to the bit in
genesis/events.h allows the build to complete.

Christophe

snuglas via Sbcl-commits <sbc...@li...> writes:

> +(defun write-events (output &aux (defs sb-impl::*c-runtime-events*))
> +  ;; Only 5 bits are allocated to the ID in an event record
> +  (aver (<= (length defs) 32))
> +  (format output "enum vmevent {~%  Event~A=0~{,~%  Event~A~}~%};~2%"
> +          (caar defs) (mapcar 'car (cdr defs)))
> +  (format output "#include <stdint.h>
> +#define EVENTBUFMAX 400000
> +extern uintptr_t *eventdata;
> +extern int n_logevents;
> +#ifndef should_record_event
> +#define should_record_event(x) 0
> +#endif~%")
> +  ;; eventN = record event with N parameters
> +  ;; NOTE 1: The buffer is oversized by enough to ensure that i_+N does not
> +  ;; overrun, so we need not adjust the comparison of 'i_ <' by the number
> +  ;; of format arguments. A more sophisticated approach would have the log
> +  ;; be a ring buffer, which would work fine in most cases since the focus
> +  ;; of a crash is generally on the most recent events.
> +  ;; NOTE 2: Assume that pthread_self() can be cast to 'uword_t' - which is
> +  ;; true for or supported platforms - and that the low 3 bits are 0 (which
> +  ;; may not hold for 32-bit, but surely does for 64-bit). Hence the low 3 can
> +  ;; can be used for the ID. But in fact we would like 5 bits for that, so
> +  ;; left-shift an additional 2 bits. This is OK as long as the upper 2 bits
> +  ;; of pthread_t are 0, which they are if it's a virtual address.
> +  (flet ((count-printf-args (str &aux (count 0) (start 0))
> +           (loop (let ((p (position #\% str :start start)))
> +                   (setq start (cond ((not p) (return count))
> +                                     ((char= (char str (1+ p)) #\%) (+ p 2))
> +                                     (t (incf count) (1+ p)))))))
> +         (formals (arity)
> +           (if (> arity 0)
> +               (format nil "~{,arg~D~}" (loop for i from 1 repeat arity collect i))
> +               "")))
> +    (format output "#ifdef WANT_EVENTLOG_FORMAT_STRINGS
> +static char event_printf_nargs[32] = {~{~D~^,~}};
> +static char *event_printf_format[] = {~{~%  ~S~^,~}~%};~%#endif~2%"
> +            (mapcar (lambda (x) (count-printf-args (cadr x))) defs)
> +            (mapcar 'cadr defs))
> +    (dotimes (argc 7)
> +      (format output "#define EVENT~D(id~A) { if(should_record_event(id)) \\
> + { int i_ = __sync_fetch_and_add(&n_logevents, ~A); if (i_ < EVENTBUFMAX) { \\
> +   eventdata[i_] = ((uword_t)pthread_self() << 2) | id;~
> +~{   eventdata[i_+~D] = (uword_t)arg~:*~D;~^ \\~%~} }}}~%"
> +              argc (formals argc) (1+ argc) (loop for n from 1 to argc collect n)))
> +    (dolist (x defs)
> +      (let* ((event (car x))
> +             (arity (count-printf-args (cadr x)))
> +             (formals (formals arity)))
> +        (format output "#define event_~A(~A) EVENT~D(Event~A~A)~%"
> +                event (subseq formals (min 1 (length formals)))
> +                arity event formals)))))