Mailing List Archive: Found it! GIMME binary incompatibility is actually a -DDEBUGGING issue

Found it! GIMME binary incompatibility is actually a -DDEBUGGING issue

Nov 7, 1995, 1:34 PM

Post #1 of 5 (536 views)

This struck me in the early hours of the morning whilst trying to get some
sleep on the Swansea-Cork ferry returning from a few days break in Ireland!
(I had just about managed to spend four days without thinking about perl :-)

The GIMME macro is broken when used in an extension built with a different
-DDEBUGGING setting than the perl it is loaded into.

Here's the patch:

diff -c ./op.h.1n ./op.h
*** ./op.h.1n Tue Nov 7 17:51:24 1995
--- ./op.h Tue Nov 7 17:52:51 1995
***************
*** 36,45 ****
OP* op_sibling; \
OP* (*op_ppaddr)(); \
PADOFFSET op_targ; \
- OPCODE op_type; \
U16 op_seq; \
U8 op_flags; \
! U8 op_private;

#define GIMME (op->op_flags & OPf_KNOW ? op->op_flags & OPf_LIST : dowantarray())

--- 36,45 ----
OP* op_sibling; \
OP* (*op_ppaddr)(); \
PADOFFSET op_targ; \
U16 op_seq; \
U8 op_flags; \
! U8 op_private; \
! OPCODE op_type;

#define GIMME (op->op_flags & OPf_KNOW ? op->op_flags & OPf_LIST : dowantarray())

Basically the code generated for op->op_flags in the GIMME macro in an
extension will not refer to the op_flags field in the structure in perl
if the size of OPCODE type differs (which it does with -DDEBUGGING).

The patch simply moves the variable sized op_type field to the end of
the common BASEOP fields, thus making op_seq, op_flags and op_private
binary compatible across -DDEBUGGING settings.

Note that this will require _all_ extensions to be rebuilt but that's
a small pain compared to the grief that people will through trying to
track down the symptoms of this. I know, I've been there!

Tim.

P.s. This bounced when I sent it to perlbug@perl.com:
'perlbug@perl.com (host: perl.com) (queue: smtpns)' for the following
reason: ' <perlbug@perl.com>... User unknown'

Re: Found it! GIMME binary incompatibility is actually a -DDEBUGGING issue [ In reply to ]

gsar at engin

Nov 7, 1995, 2:56 PM

Post #2 of 5 (517 views)

Permalink

On Tue, 07 Nov 1995 20:34:08 GMT, Tim Bunce wrote:
>Note that this will require _all_ extensions to be rebuilt but that's
>a small pain compared to the grief that people will through trying to
>track down the symptoms of this. I know, I've been there!
>

Wow, the README in the distribution needs to shout it for 5.002 then.

Also, it ought to go in the manpages somewhere (so _users_, who may
be building private extensions will know). perlapi(1) would be a
natural place for it.

- Sarathy.
gsar@engin.umich.edu

Re: Found it! GIMME binary incompatibility is actually a -DDEBUGGING issue [ In reply to ]

lwall at scalpel

Nov 7, 1995, 3:34 PM

Post #3 of 5 (523 views)

Permalink

: The patch simply moves the variable sized op_type field to the end of
: the common BASEOP fields, thus making op_seq, op_flags and op_private
: binary compatible across -DDEBUGGING settings.

Uh, but it specifically *doesn't* make op_type itself binary compatible
except accidentally on little-endian machines. And maybe not even then,
if the next couple bytes don't happen to be null.

Larry

Re: Found it! GIMME binary incompatibility is actually a -DDEBUGGING issue [ In reply to ]

lwall at scalpel

Nov 7, 1995, 4:14 PM

Post #4 of 5 (516 views)

Permalink

: : The patch simply moves the variable sized op_type field to the end of
: : the common BASEOP fields, thus making op_seq, op_flags and op_private
: : binary compatible across -DDEBUGGING settings.
:
: Uh, but it specifically *doesn't* make op_type itself binary compatible
: except accidentally on little-endian machines. And maybe not even then,
: if the next couple bytes don't happen to be null.

Not to mention the fact that all the fields AFTER op_type are potentially
incompatible, since op_type is a part of BASEOP.

I think the solution is probably just to go with a U16, unless maybe
REALLY_REALLY_DEBUGGING is defined. Under gdb, the op name is usually
pretty clear anyway since it lists the name of the routine pointed to
by op_ppaddr.

Larry

Re: Found it! GIMME binary incompatibility is actually a -DDEBUGGING issue [ In reply to ]

Tim.Bunce at ig

Nov 7, 1995, 7:41 PM

Post #5 of 5 (523 views)

Permalink

> From: Larry Wall <lwall@scalpel.netlabs.com>
>
> : : The patch simply moves the variable sized op_type field to the end of
> : : the common BASEOP fields, thus making op_seq, op_flags and op_private
> : : binary compatible across -DDEBUGGING settings.
> :
> : Uh, but it specifically *doesn't* make op_type itself binary compatible
> : except accidentally on little-endian machines. And maybe not even then,
> : if the next couple bytes don't happen to be null.
>
> Not to mention the fact that all the fields AFTER op_type are potentially
> incompatible, since op_type is a part of BASEOP.

Sure, I appreciate all that (and should have covered it in my message
but I was just too happy to have nailed it to worry about fine print :-).

The patch was based on the assumptions that:

1. GIMME is likely to be the only use of op fields in nearly all extensions

2. You had good reasons for OPTYPE being variable (this had been discussed
ages ago)

and hence the aim was to simply limit the scope for problems.

> I think the solution is probably just to go with a U16, unless maybe
> REALLY_REALLY_DEBUGGING is defined. Under gdb, the op name is usually
> pretty clear anyway since it lists the name of the routine pointed to
> by op_ppaddr.

That would, of course, be the best solution (but perhaps -DDEBUGGING_OPS
would be a little more practical :-)

Tim.