[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#42701: 28.0.50; Duplicate Edebug instrumentation of lambda form in s

From: Philipp Stephani
Subject: bug#42701: 28.0.50; Duplicate Edebug instrumentation of lambda form in some cases
Date: Sun, 9 Aug 2020 16:59:57 +0200


Am Di., 4. Aug. 2020 um 22:12 Uhr schrieb Alan Mackenzie <acm@muc.de>:
> Hello, Philipp.
> In article <mailman.680.1596473284.2739.bug-gnu-emacs@gnu.org> you wrote:
> > Create a file /tmp/edebug.el:
> > $ cat /tmp/edebug.el
> > (defun f () (if-let (x (funcall (lambda (y) 1) 2)) 3 4))
> > Visit the file:
> > $ emacs -Q -l subr-x -l edebug /tmp/edebug.el
> > Now instrument the function `f' using C-u C-M-x.  The *Messages* buffer
> > will now contain
> > Edebug: edebug-anon0 [2 times]
> > Edebug: f
> > The [2 times] indicates the problem: Edebug has instrumented two
> > definitions with the same (generated) symbol.
> I don't think you're correct, here.  I think it's instrumented the
> lambda form twice, once for each arm of the edebug spec.  It discards
> the first attempt, then succeeds at the second.
> The pertinent edebug spec (from the if-let definition in subr-x.el)
> looks like:
>     ([&or (&rest [&or symbolp (symbolp form) (form)])
>           (symbolp form)]
>      form body)
> .  In the outer &or sub-spec, the (&rest ....) bit doesn't match, but
> the failure to match only happens after the "Edebug: edebug-anon0"
> message has been output the first time.  Edebug then tries the (symbolp
> form) alternative, which does match and outputs the "...-anon0" message
> a second time.

That is correct, but when the match fails, it's already too late:
Edebug (in `edebug-make-form-wrapper') has already performed some
global mutations such as modifying `edebug-form-data' of the `edebug'
property of the newly-generated symbol. These mutations aren't unwound
after the first failed match.

> > This is a problem when using Edebug for e.g. coverage instrumentation,
> > as the coverage information is attached to the symbol itself (as a
> > symbol property), and duplicate symbols when instrumenting code lead
> > to subtle errors such as mismatching vector lengths for the position
> > of the breakpoints and the hit counts.
> I don't think this is happening (see above), but I admit not having
> looked into it all that closely.  Do you have any further evidence of
> two distinct functions being mapped onto the same generated symbol?

I've tried to provide some context in
The most common symptom of such duplicate definitions (as defined by
"calling `edebug-make-form-wrapper' twice with the same
`edebug-def-name'") is

> > I'd speculate that this issue is similar to Bug#41988 in that Edebug
> > defines instrumented symbols even when backtracking later.  In this
> > case, Edebug backtracks to the legacy (SYM VAL) form, but has already
> > partially matched the ((SYM VAL)) form, including instrumenting the
> > lambda form therein.  I guess Edebug should perform some kind of
> > two-phase instrumentation and instrument subforms only when a form has
> > been chosen after backtracking.  Since this is somewhat difficult to
> > implement without rewriting larger parts of Edebug, it might be more
> > feasible to regenerate anonymous symbols after a failed match.
> I don't know why Edebug re-uses the symbol.
My guess is that `edebug-old-def-name' becomes non-nil because the
form has already been instrumented
It's then reused in

>  But beyond the misleading
> double message in *Messages*, is there any harm done?  Would it be less
> confusing if two distinct messages "Edebug: edebug-anon0" and "Edebug:
> edebug-anon1" were to be output?

The duplicate message is just a symptom of calling
`edebug-make-form-wrapper' twice with the same `edebug-def-name'
symbol, and attaching various instrumentation data to the symbol.
Depending on whether the two definitions are compatible, this can be
harmless or lead to subtle issues like
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=41853. Therefore in my
coverage instrumentation driver I'm rejecting such duplicate
since it doesn't seem to be possible to predict whether the
redefinition would indeed be harmless or trigger

reply via email to

[Prev in Thread] Current Thread [Next in Thread]