Ticket #364 (assigned bug) — at Version 6

Opened 13 years ago

Last modified 12 years ago

ptr_alignment 8 not honored, sparc 64bit broken

Reported by: rurban Owned by: rurban
Priority: blocker Milestone:
Component: core Version: trunk
Severity: release Keywords:
Cc: doughera, rg@… Language:
Patch status: Platform: other

Description (last modified by rurban) (diff)

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=2i (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.

 * First we should align to the foreign pbc ptrsize, not to the native ptrsize.
 * /sizeof (opcode_t) => /header->wordsize
 */
#define ROUND_16(val) (((val) & 0xf) ? 16 - ((val) & 0xf) : 0)
#define ALIGN_16(st, cursor) \
    (cursor) += ROUND_16((const char *)(cursor) - (const char *)(st))/sizeof (opcode_t)

This requires a check for all ptr alignment code (no macros used, bad) and to find the source of the problem, most likely a string.

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.

Change History

  Changed 13 years ago by rurban

  • status changed from new to assigned
  • owner set to rurban
  • description modified (diff)

  Changed 13 years ago by rurban

  • description modified (diff)

  Changed 13 years ago by rurban

  • description modified (diff)

  Changed 13 years ago by rurban

  • description modified (diff)

in reply to: ↑ description   Changed 13 years ago by 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.

  Changed 13 years ago by rurban

  • description modified (diff)
Note: See TracTickets for help on using tickets.