Skip to content

Commit

Permalink
improve debugging of padlist API
Browse files Browse the repository at this point in the history
xpadl_alloc should really be pointer to a struct with a flexible array
member, but flexible array members aren't portable enough among CCs. While
debugging the padlist API for memory corruption (caused by an unrelated
XS module), I saw that the pointer in the first slice of xpadl_alloc
pointed to an AV head of gibberish but 2nd slice was fine. This was
confusing and led me to belive the memory corruption was a bad write to
the array in xpadl_alloc. PadlistARRAY's POD a couple pages down mentions
that index 0 is not an AV *, but the struct comments just said
"pointer to beginning of array of AVs " and didnt mention index 0.

Fix the comments to make it clear what xpadl_alloc is. Add a union so it
is easier to analyze a crash dump/breakpoint with a C debugger, without
writing new code "PADNAMELIST * pnl =  PadlistNAMES(pl);" in many places
and recompiling the interp with -O0, just to be able to inspect the
padnamelist struct.
  • Loading branch information
bulk88 authored and tonycoz committed Jul 9, 2015
1 parent 779c45c commit 30dc90f
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions pad.h
Expand Up @@ -33,7 +33,15 @@ typedef U64TYPE PADOFFSET;

struct padlist {
SSize_t xpadl_max; /* max index for which array has space */
PAD ** xpadl_alloc; /* pointer to beginning of array of AVs */
union {
PAD ** xpadlarr_alloc; /* Pointer to beginning of array of AVs.
index 0 is a padnamelist * */
struct {
PADNAMELIST * padnl;
PAD * pad_1; /* this slice of PAD * array always alloced */
PAD * pad_2; /* maybe unalloced */
} * xpadlarr_dbg; /* for use with a C debugger only */
} xpadl_arr;
U32 xpadl_id; /* Semi-unique ID, shared between clones */
U32 xpadl_outid; /* ID of outer pad */
};
Expand Down Expand Up @@ -293,7 +301,7 @@ Restore the old pad saved into the local variable opad by PAD_SAVE_LOCAL()
=cut
*/

#define PadlistARRAY(pl) (pl)->xpadl_alloc
#define PadlistARRAY(pl) (pl)->xpadl_arr.xpadlarr_alloc
#define PadlistMAX(pl) (pl)->xpadl_max
#define PadlistNAMES(pl) *((PADNAMELIST **)PadlistARRAY(pl))
#define PadlistNAMESARRAY(pl) PadnamelistARRAY(PadlistNAMES(pl))
Expand Down

0 comments on commit 30dc90f

Please sign in to comment.