On Mon, May 02, 2016 at 12:01:18AM -0400, James McCoy wrote:
> Thanks for the analysis and working on a patch. I'll note that
> _Alignas, if it had worked, would likely have been too new a compiler
> feature to use.
>
> It looks like Debian's ia64 porterbox is still running, so I can try
> reproducing the problem there (using prctl to force SIGBUS) and then
> fiddling around with the code.
Here goes further analysis. Running utf-test under gdb I set a break
point at line 807 of utf-test.c and after a number of iterations of
the enclosing loop I get:
Breakpoint 1, test_utf_conversions (pool=0x20001018028)
at
/extra/mjc/debian/subversion/subversion-1.9.4/subversion/tests/libsvn_subr/utf-test.c:807
807 SVN_ERR_ASSERT(0 == strcmp(result->data, tc->result));
(gdb) print tc->source
$20 = 0x120003117 "\330\064\335\036"
(gdb) print result->data
$21 = 0x20001018218 "\360\220\204\236"
(gdb) print tc->result
$22 = 0x12000373c "\360\235\204\236"
(gdb) cont
Continuing.
svn_tests: E235000: In file
'/extra/mjc/debian/subversion/subversion-1.9.4/subversion/tests/libsvn_subr/utf-test.c'
line 807: assertion failed (0 == strcmp(result->data, tc->result))
FAIL: lt-utf-test 9: test svn_utf__utf{16,32}_to_utf8
[Inferior 1 (process 24827) exited with code 01]
On this iteration I believe it has just read line 769 of utf-test.c,
i.e., this line:
{ UTF_16_BE, "\xD8\x34" "\xDD\x1E" "\0\0", "\xf0\x9d\x84\x9e" }, /*
U+01D11E */
into variable tc, and it passes tc->source (which is at address
0x120003117, i.e., misaligned for apr_uint16_t type) to
svn_utf__utf16_to_utf8() and the result is incorrect and results in
the failed assert.
On previous iterations of the test loop it happened to pass the test
even with misaligned tc->source strings, and it is instructive to
disassemble svn_utf__utf16_to_utf8() to see why this can happen:
1040 svn_error_t *
1041 svn_utf__utf16_to_utf8(const svn_string_t **result,
1042 const apr_uint16_t *utf16str,
1043 apr_size_t utf16len,
1044 svn_boolean_t big_endian,
1045 apr_pool_t *result_pool,
1046 apr_pool_t *scratch_pool)
1047 {
0x00000200000ed990 <+0>: ldah gp,12(t12)
0x00000200000ed994 <+4>: lda gp,10976(gp)
0x00000200000ed998 <+8>: lda sp,-160(sp)
0x00000200000ed9a0 <+16>: stq s0,8(sp)
0x00000200000ed9a4 <+20>: mov a1,s0
0x00000200000ed9a8 <+24>: stq s1,16(sp)
0x00000200000ed9ac <+28>: mov a2,s1
0x00000200000ed9b0 <+32>: stq ra,0(sp)
0x00000200000ed9b4 <+36>: stq s2,24(sp)
0x00000200000ed9b8 <+40>: stq s3,32(sp)
0x00000200000ed9bc <+44>: stq s4,40(sp)
0x00000200000ed9c0 <+48>: stq s5,48(sp)
0x00000200000ed9c4 <+52>: stq fp,56(sp)
0x00000200000ed9c8 <+56>: stq a0,152(sp)
0x00000200000ed9cc <+60>: stq a3,128(sp)
0x00000200000ed9d0 <+64>: stq a4,144(sp)
1048 static const apr_uint16_t endiancheck = 0xa55a;
1049 const svn_boolean_t arch_big_endian =
1050 (((const char*)&endiancheck)[sizeof(endiancheck) - 1] == '\x5a');
1051 const svn_boolean_t swap_order = (!big_endian != !arch_big_endian);
1052
1053 apr_uint16_t lead_surrogate;
1054 apr_size_t length;
1055 apr_size_t offset;
1056 svn_membuf_t ucs4buf;
1057 svn_membuf_t resultbuf;
1058 svn_string_t *res;
1059
1060 if (utf16len == SVN_UTF__UNKNOWN_LENGTH)
0x00000200000ed99c <+12>: lda t0,1(a2)
0x00000200000ed9d4 <+68>: beq t0,0x200000edbd0
<svn_utf__utf16_to_utf8+576>
1061 {
1062 const apr_uint16_t *endp = utf16str;
1063 while (*endp++)
0x00000200000edbd0 <+576>: mov a1,s1
0x00000200000edbd4 <+580>: unop
0x00000200000edbe0 <+592>: lda s1,2(s1)
0x00000200000edbe4 <+596>: lda t1,-2(s1)
0x00000200000edbe8 <+600>: ldq_u t0,-2(s1)
0x00000200000edbec <+604>: extwl t0,t1,t0
0x00000200000edbf0 <+608>: bne t0,0x200000edbe0
<svn_utf__utf16_to_utf8+592>
1064 ;
In the loop immediately above (lines 1061--1064 of the source code)
we see the code is walking through the utf16 string (register a1 holds
argument 1, i.e., utf16str, which is first copied to register s1 and
used as the pointer incremented in the loop). The load instructions
to get the utf16 character are the two instructions:
0x00000200000edbe8 <+600>: ldq_u t0,-2(s1)
0x00000200000edbec <+604>: extwl t0,t1,t0
The ldq_u zeroes the lowest three bits of the address and loads the
quadword (64-bit word) from the resultant address, and the extwl
extracts the 16-bit word identified by the lowest three bits of the
address (which are in register t1) from the quadword in register t0,
however this is only correct if the lowest three bits are not 0x7.
If the source string address has the lowest three bits all set then,
and only then, the generated code loads incorrect utf16 chars.
This is a reasonable optimisation of the code made by gcc as the
argument utf16str is declared to be a pointer to apr_uint16_t
type, i.e., data that are 16-bit aligned, thus gcc can reason that
an address with the lowest three bits set is not possible.
Cheers
Michael.
