# New Ticket Created by Andy Dougherty
# Please include the string: [perl #51652]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=51652 >
There might be a possible alignment issue lurking in the new streamlined
PMC and STRING structures.
First, consider the Buffer structure:
typedef struct Buffer {
UnionVal cache;
Parrot_UInt flags;
} Buffer;
Next, consider the old and new parrot_string_t structures.
OLD:
struct parrot_string_t {
Buffer obj;
UINTVAL bufused;
char *strstart;
/* ... remaining elements omitted ... */
};
NEW: (Preliminary version)
struct parrot_string_t {
UnionVal cache;
Parrot_UInt flags;
UINTVAL bufused;
char *strstart;
/* ... remaining elements omitted ... */
};
NEW: (Current version)
struct parrot_string_t {
UnionVal cache;
Parrot_UInt flags;
char *strstart; /* Note flipped order */
UINTVAL bufused;
/* ... remaining elements omitted ... */
};
The flattened "Preliminary" version didn't work on SPARC. The
simplest test case I was able to find was
$ ./parrot t/op/string_100.pasm | cat -v
[EMAIL PROTECTED]@[EMAIL PROTECTED]@[EMAIL PROTECTED]@[EMAIL
PROTECTED]@[EMAIL PROTECTED]@[EMAIL PROTECTED]@%ll46368
[EMAIL PROTECTED]@[EMAIL PROTECTED]@[EMAIL PROTECTED]@[EMAIL
PROTECTED]@[EMAIL PROTECTED]@[EMAIL PROTECTED]@-1
Of course, turning off garbage collection makes it work:
$ ./parrot -G t/op/string_100.pasm | cat -v
46368
-1
And running with the -R gcdebug core makes it even weirder:
$ ./parrot -R gcdebug t/op/string_100.pasm | cat -v
[EMAIL PROTECTED]@[EMAIL PROTECTED]@[EMAIL PROTECTED]@[EMAIL
PROTECTED]@[EMAIL PROTECTED]@[EMAIL PROTECTED]@[EMAIL PROTECTED]
[EMAIL PROTECTED]@[EMAIL PROTECTED]@[EMAIL PROTECTED]@[EMAIL
PROTECTED]@[EMAIL PROTECTED]@[EMAIL PROTECTED]@[EMAIL PROTECTED]@[EMAIL
PROTECTED]@-1-1
The problem can be avoided by adding in an 'int dummy' field before
'bufused', or switching the order of the 'bufused' and 'strstart'
elements (as is done in the current version). I say "avoided" rather
than "fixed" because the order of the elements in the structure
shouldn't matter, in principle.
I haven't been able to track down the problem. I vaguely suspect it's
an alignment/structure padding issue. Specifically, in the old
version,
offsetof(struct parrot_string_t, bufused) == sizeof(Buffer),
but that wasn't necessarily true in the Preliminary flattened version
because the padding on the end of Buffer is no longer part of the string
structure.
This might matter because strings are stored in "bufferlike" pools, but
I am not confident they are "bufferlike" anymore, since sizeof(Buffer)
doesn't necessarily mean anything inside the new string structure.
Unfortunately, I have not been able to find the root of the problem.
Building with --gc=libc hides the problem a little more, but it still
fails with the -R gcdebug runcore. I hope someone who understands
the memory pools better than I will be able to look and make sure the
changes in string and PMC structures haven't violated any assumptions
of that code.
Meanwhile, this patch is probably an appropriate reminder:
--- parrot-svn/include/parrot/pobj.h Tue Mar 11 15:51:23 2008
+++ parrot-gcc/include/parrot/pobj.h Wed Mar 12 08:38:43 2008
@@ -117,6 +117,23 @@
enum_stringrep_four = 4
} parrot_string_representation_t;
+/* XXX Possible alignment issue:
+ If the 'strstart' and 'bufused' elements are interchanged, parrot
+ no longer builds correctly on SPARC -- something goes amiss during
+ garbage collection. If a dummy element is inserted into the
+ structure just after 'flags', all is well again.
+
+ I vaguely suspect that the storage of strings in a
+ "bufferlike_pool" might need revising. In the pre March-2008
+ version, where string contained a nested pobj_t, the offset of
+ 'strstart' was the same as sizeof(Buffer). Now, that is not
+ necessarily true, because sizeof(Buffer) includes padding on the
+ end. (Specifically, on 32-bit SPARC, sizeof(Buffer) == 16, but
+ offsetof(struct parrot_string_t, strstart) == 12.)
+
+ --Andy D. 12 March 2008
+ */
+
struct parrot_string_t {
UnionVal cache;
Parrot_UInt flags;
--
Andy Dougherty [EMAIL PROTECTED]
