Skip to content

Commit e6bc697

Browse files
committed
nestlevel: Fix user data alignment
We need to align the user data properly not to trigger undefined behavior, which even apparently crashes on SPARC. As `NestingLevels::levels` is actually a single allocation for all levels and their user data mapped as `[NL0|UD0|NL1|UD1|...]` (where NL is a NestingLevel, and UD a user data), we need to align twice, as we need every `NL*` and every `UD*` to align properly. Here we align everything to `2*sizeof(size_t)`, which is a logic borrowed from GLib, which seems to have borrowed the value from glibc. This is pretty conservative in our case, because actually `NL*`s only need aligning to `int`'s requirements currently, which on some architectures is 4, not 16; but it's trickier to implement (and actually impossible with the current API) as we'd need to compute the actual alignment for each level taking into account it's position in the overall memory region to still align `UD*`s to a conservative value. Also, having all NL+UD group at the same size makes things a bit simpler for debugging, I guess. We make sure to only add alignment padding manually for cases where there's actually some user data, not to waste memory needlessly for the common case where `sizeof(UD)` is 0, and thus where we can merely align to `sizeof(NL)` -- which C does for us already. Note that currently only the Ruby parser is affected, as it's the only current consumer of nesting level user data. Fixes #3881.
1 parent a4d751b commit e6bc697

File tree

2 files changed

+14
-5
lines changed

2 files changed

+14
-5
lines changed

main/nestlevel.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,16 @@
2020

2121
#include <string.h>
2222

23-
/* TODO: Alignment */
24-
#define NL_SIZE(nls) (sizeof(NestingLevel) + (nls)->userDataSize)
23+
/* struct alignment trick, copied from GObject's gtype.c, which borrows
24+
* 2*szieof(size_t) from glibc */
25+
#define STRUCT_ALIGNMENT (2 * sizeof (size_t))
26+
#define ALIGN_STRUCT(offset) ((offset + (STRUCT_ALIGNMENT - 1)) & -STRUCT_ALIGNMENT)
27+
28+
/* account for the user data alignment if we have user data, otherwise allocate
29+
* exactly what's needed not to waste memory for unneeded alignment */
30+
#define NL_SIZE(nls) ((nls)->userDataSize ? (ALIGN_STRUCT (sizeof (NestingLevel)) + ALIGN_STRUCT ((nls)->userDataSize)) : sizeof (NestingLevel))
31+
#define NL_USER_DATA(nl) ((void *)(((char *) nl) + ALIGN_STRUCT (sizeof (NestingLevel))))
32+
2533
#define NL_NTH(nls,n) (NestingLevel *)(((char *)((nls)->levels)) + ((n) * NL_SIZE (nls)))
2634

2735
/*
@@ -73,7 +81,7 @@ extern NestingLevel * nestingLevelsPush(NestingLevels *nls, int corkIndex)
7381

7482
nl->corkIndex = corkIndex;
7583
if (nls->userDataSize > 0)
76-
memset (nl->userData, 0, nls->userDataSize);
84+
memset (NL_USER_DATA (nl), 0, ALIGN_STRUCT (nls->userDataSize));
7785

7886
return nl;
7987
}
@@ -117,5 +125,5 @@ extern NestingLevel *nestingLevelsGetNthParent (const NestingLevels *nls, int n)
117125

118126
extern void *nestingLevelGetUserData (const NestingLevel *nl)
119127
{
120-
return (void *)nl->userData;
128+
return NL_USER_DATA (nl);
121129
}

main/nestlevel.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ typedef struct NestingLevels NestingLevels;
2626
struct NestingLevel
2727
{
2828
int corkIndex;
29-
char userData [];
29+
/* user data is allocated at the end of this struct (possibly with some
30+
* offset for alignment), get it with nestingLevelGetUserData() */
3031
};
3132

3233
struct NestingLevels

0 commit comments

Comments
 (0)