ptr_alignment 8 not honored, sparc 64bit broken

[rurban]

Sparc 64bit with strict ptr_alignment=8 breaks when reading frozen pmc's because of misalignment.

Any 64bit big-endian cpu with strict alignment is broken without -xmemalign=4s (the immediate workaround) The default for all v9 architectures is -xmemalign=8s.

However, our goal should be to allow fast aligned code and not to skip this with a compiler relaxement.

t@1 (l@1) stopped in PF_fetch_integer at line 1076 in file "pf_items.c"
 1076       ASSERT_ARGS(PF_fetch_integer)
(dbx) step
t@1 (l@1) stopped in PF_fetch_integer at line 1078 in file "pf_items.c"
 1078       if (!pf || pf->fetch_iv == NULL)
(dbx) print *(*stream)
**stream = 4
(dbx) step
t@1 (l@1) stopped in PF_fetch_integer at line 1079 in file "pf_items.c"
 1079           return *(*stream)++;
(dbx) print *(*stream) 
**stream = 4
(dbx) step
t@1 (l@1) signal BUS (invalid address alignment) in PF_fetch_integer at line 1079 in file "pf_items.c"
 1079           return *(*stream)++;
(dbx) print *(*stream) 
**stream = 35
(dbx) print *stream
*stream = 0x100000dec

0x100000dec is not properly aligned. It must be 0x100000df0.

Sparc cc manpage:
-xmemalign[=<a><b>] Controls memory alignment, <a>={1|2|4|8|16}, b={f|i|s}.
Accepted values for b are:
i Interpret access and continue execution.
s Raise signal SIGBUS.
f For variants of -xarch=v9 only. [reduced i]

Thanks to Rolf Grossmann for coming up with all this info and debugging, and to Andy Dougherty for correcting my wrong first analysis.

The fix for 1.0 will be a hints file update to add -xmemalign=4s to cc_flags, the goal for 2.6 will be a --64bitcompat argument to parrot to create 8-byte aligned values on 32-bit, not 4-byte as now.

[doughera]

Replying to rurban:

A strict 64bit cpu with a ptr_alignment=8 will break when reading pbc's or just frozen pmc's because our alignment when writing our bytecode is 16/ptrsize and not 16.

No. You are mixing up units here. The alignment is supposed to be on a 16-byte boundary. However, the code is supposed to step through the pbc in steps of size sizeof(opcode_t). Therefore, it is also true that the alignment is 16/sizeof(opcode_t), when measured in units of sizeof(opcode_t).

As I have explained before, the ALIGN_16 macro assumes you are stepping through the bytecode in steps of sizeof(opcode_t). If there are alignment problems, it seems likely to me that those problems are due to breaking that assumption earlier. The problem is not the ALIGN_16 macro (though it could, of course, be replaced by a more defensive, but slower function), but in the steps preceeding the call to ALIGN_16 which are somehow violating the assumption that the reader is stepping through the file in units of sizeof(opcode_t). For this problem here, a backtrace showing where the stream pointer came from would be helpful.

The decision to step through the file in units of sizeof(opcode_t) was a deliberate choice for at least two reasons: efficiency, and avoiding compiler warnings. gcc on SPARC in particular will issue warnings when casting from (char *) to things like (opcode_t *): "warning: cast increases required alignment of target type". Please don't change that.

For reading foreign files with a different sizeof(opcode_t), you are correct that you will have to step through the file in steps of the foreign stepsize. But that's not the problem here -- everything is native.

[rurban]

I analyzed this further, but cannot test this. See  http://use.perl.org/~rurban/journal/38522

I believe 8-byte ptr_alignment cannot be supported as now, because we do not enforce strict pairs of 4-byte data, as integer and opcodes. There can be an uneven number, and then we get a SIGBUS.

We enforce 16-byte aligment at the start and end of segments and at end of strings, but not with integer and opcodes.

So we should update the Sparc64 hints to use -xmemalign=4i, this is the best we can guarantee (untested).

For the future we should discuss enforcing writing pairs of 4-byte data or padding uneven ending 4-byte data with a pad of one byte.

It should also be tested if the pf_items (unsigned char*)stream++ are valid with Sparc64.

[rg]

Sorry you patch doesn't apply. It seems the debug output part has already been committed but in a different way. Can you check?

I've just testet -xmemalign=4s (we want a fault if we're not correct, not emulation) and it works and passes the tests (as far as solaris passes the tests at all ;)). The native_pbc tests are still TODO, however (and they don't pass ... but then they don't pass on amd64 either).

If you could be more specific what I should test, I'd be happy to. On the other hand, it might not be worth all the effort and we'd rather just update the hints file and fix some problems that don't have a known work-around.

[rurban]

rg: We will stay with -xmemalign=4s until pbc will get fully portable.

The current goal is version 2.6 but I hope we will get it sooner. I already know why it failed. See  http://use.perl.org/~rurban/journal/38522

I also saw that my latest patch was wrong. I will update it soon. But first I will try the hints patch and would like you to test that first, so that

'perl Configure.pl --m=64'

works without any hazzle.

[rurban]

warn also at alignment check