Re: Endless loop in split_on_bdry() of library/parser_multipart.c?

On Wed, 2006-05-31 at 02:18 -0700, Philip M. Gollucci wrote:

> Looking at library/parser_multipart.c and associated tests t/parsers.c ->
> we definetely exercise that code block sucessfully.

Here is the good news: I don't think this is a problem in libapreq2 (or
at least not on all platforms/compilers).

And the bad news is: it does appear that the problem has something to do
with optimisation flags on Fedora Core 5 with gcc 4.1.1 (and I think
4.1.0) on i686, as I first suspected after the code worked when compiled
with -O0.

WARNING: Please take my attempt to explain with a grain of salt - I
really don't understand compilers and x86 assembly language all that
well.

As it turns out, when these line of code (164 - 168 in
library/parse_multipart.c) are compiled with -O2:

---------------------------
do {
    apr_bucket *f = APR_BRIGADE_FIRST(in);
    APR_BUCKET_REMOVE(f);
    APR_BRIGADE_INSERT_TAIL(out, f);
} while (e != APR_BRIGADE_FIRST(in));
---------------------------

the assembly (on an i686 machine) looks like this:

---------------------------
.LBB4:
        .loc 1 165 0
        movl    -56(%ebp), %edx
        movl    $0, -36(%ebp)
        movl    4(%edx), %ecx
.LVL27:
        cmpl    %ecx, -44(%ebp)
        .p2align 4,,7
.L30:
        .loc 1 166 0
        movl    4(%ecx), %edx
        movl    (%ecx), %eax
        movl    %eax, (%edx)
        movl    (%ecx), %eax
.LBB5:
        .loc 1 167 0
        movl    %edi, (%ecx)
.LBE5:
        .loc 1 166 0
        movl    %edx, 4(%eax)
.LBB6:
        .loc 1 167 0
        movl    4(%edi), %eax
        movl    %eax, 4(%ecx)
        movl    4(%edi), %eax
        movl    %ecx, 4(%edi)
        movl    %ecx, (%eax)
.LBE6:
.LBE4:
        .loc 1 168 0
        jne     .L30
        jmp     .L19
---------------------------

.L19 is the goto label (look_for_boundary_up_front). Note how the
comparison (i.e. cmpl) is done *before* .L30, which is where we jump
from jne. In other words, the loop is endless because the condition is
never evaluated again.

Now, the same code compiled with -fno-strict-aliasing (and -O2) gives:

---------------------------
.L28:
.LBB4:
        .loc 1 165 0
        movl    -56(%ebp), %edx
        movl    4(%edx), %eax
.LVL25:
        .loc 1 166 0
        movl    4(%eax), %ecx
        movl    (%eax), %edx
        movl    %edx, (%ecx)
        movl    (%eax), %edx
.LBB5:
        .loc 1 167 0
        movl    %esi, (%eax)
.LBE5:
        .loc 1 166 0
        movl    %ecx, 4(%edx)
.LBB6:
        .loc 1 167 0
        movl    4(%esi), %edx
        movl    %edx, 4(%eax)
        movl    4(%esi), %edx
        movl    %eax, 4(%esi)
        movl    %eax, (%edx)
.LBE6:
.LBE4:
        .loc 1 168 0
        movl    -56(%ebp), %ecx
        movl    -44(%ebp), %eax
.LVL26:
        cmpl    4(%ecx), %eax
        jne     .L28
        jmp     .L43
---------------------------

.L43 is the goto label (look_for_boundary_up_front). .L28, as you can
see, is the beginning of the loop. This code works because it does cmpl
on every pass.

It would sure be useful to have compiler/assembly folks take a look at
this, as I'm not entirely sure that my findings are correct (never
studied compilers nor x86 assembly, I'm afraid).

Anyhow, I can avoid the infinite loop by compiling with
-fno-strict-aliasing.

PS. Intermediate C code is here (same in both cases):
---------------------------
do {
    apr_bucket *f = (&(in)->list)->next;
    do { do { (((((f))))->link.prev)->link.next = ((((f))))->link.next;
(((((f))))->link.next)->link.prev = ((((f))))->link.prev; } while (0);
do { (f)->type->destroy((f)->data); (f)->free(f); } while (0); } while
(0);
} while ((&(in)->list)->next != e);
---------------------------

-- 
Bojan