Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
Quoting Joe Schaefer <[EMAIL PROTECTED]>: At least now it's a bit clearer why the no-strict-aliasing optimization is getting confused ;-) Hey, speak for yourself ;-) -- Bojan
Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
Bojan Smojver <[EMAIL PROTECTED]> writes:
> Quoting Joe Schaefer <[EMAIL PROTECTED]>:
>
>> Are you sure the "intermediate code" is correct?
>> It's calling free and destroy, which makes me think
>> that's some other "delete buckets" loop, not this one.>
>
> Nice catch! The correct snippet is:
>
> ---
> do {
> apr_bucket *f = (&(in)->list)->next;
> do { (f->link.prev)->link.next =
> f->link.next;
> (f->link.next)->link.prev = f->link.prev; } while (0);
> do { apr_bucket *ap__b = (f); do { ap__b->link.next =
> ((struct apr_bucket *)((char *)(((&(out)->list))) - ((long) (((char *)
> (&(((struct apr_bucket*)((void *)0))->link))) - ((char *) ((void *)0));
> ap__b->link.prev = (((struct apr_bucket *)((char *)(((&(out)->list)))
> - ((long) (((char *) (&(((struct apr_bucket*)((void *)0))->link))) - ((char
> *)
> ((void *)0)))->link.prev; struct apr_bucket *)((char
> *)(((&(out)->list))) - ((long) (((char *) (&(((struct apr_bucket*)((void
> *)0))->link))) - ((char *) ((void *)0)))->link.prev)->link.next =
> (((ap__b))); (((struct apr_bucket *)((char *)(((&(out)->list))) - ((long)
> (((char *) (&(((struct apr_bucket*)((void *)0))->link))) - ((char *) ((void
> *)0)))->link.prev = (((ap__b))); } while (0); ; } while (0);
> } while (e != (&(in)->list)->next);
> ---
At least now it's a bit clearer why the no-strict-aliasing
optimization is getting confused ;-)
--
Joe Schaefer
Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
Quoting Joe Schaefer <[EMAIL PROTECTED]>:
Are you sure the "intermediate code" is correct?
It's calling free and destroy, which makes me think
that's some other "delete buckets" loop, not this one.>
Nice catch! The correct snippet is:
---
do {
apr_bucket *f = (&(in)->list)->next;
do { (f->link.prev)->link.next =
f->link.next; (f->link.next)->link.prev =
f->link.prev; } while (0);
do { apr_bucket *ap__b = (f); do {
ap__b->link.next = ((struct apr_bucket *)((char
*)(((&(out)->list))) - ((long) (((char *) (&(((struct
apr_bucket*)((void *)0))->link))) - ((char *) ((void *)0));
ap__b->link.prev = (((struct apr_bucket *)((char
*)(((&(out)->list))) - ((long) (((char *) (&(((struct
apr_bucket*)((void *)0))->link))) - ((char *) ((void
*)0)))->link.prev; struct apr_bucket *)((char
*)(((&(out)->list))) - ((long) (((char *) (&(((struct
apr_bucket*)((void *)0))->link))) - ((char *) ((void
*)0)))->link.prev)->link.next = (((ap__b))); (((struct apr_bucket
*)((char *)(((&(out)->list))) - ((long) (((char *) (&(((struct
apr_bucket*)((void *)0))->link))) - ((char *) ((void
*)0)))->link.prev = (((ap__b))); } while (0); ; } while (0);
} while (e != (&(in)->list)->next);
---
Thanks Joe!
PS. Fixed Bugzilla too.
--
Bojan
Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
Bojan Smojver <[EMAIL PROTECTED]> writes: > Quoting Bojan Smojver <[EMAIL PROTECTED]>: > >> I will report this in Red Hat Bugzilla and we'll see what the problem >> really is (i.e. is it something in gcc or maybe even APR). > > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=193740 Are you sure the "intermediate code" is correct? It's calling free and destroy, which makes me think that's some other "delete buckets" loop, not this one. -- Joe Schaefer
Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
Quoting Bojan Smojver <[EMAIL PROTECTED]>: I will report this in Red Hat Bugzilla and we'll see what the problem really is (i.e. is it something in gcc or maybe even APR). https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=193740 -- Bojan
Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
Quoting "Philip M. Gollucci" <[EMAIL PROTECTED]>: gcc version 3.4.4 [FreeBSD] 20050518 That's probably why. Optimization was reworked in 4.1 completely. -- Bojan
Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
Bojan Smojver wrote:
Quoting "Philip M. Gollucci" <[EMAIL PROTECTED]>:
my CFLAGS on FreeBSD are
-g -O2 -Werror -Wall -Wmissing-prototypes -Wstrict-prototypes
-Wmissing-declarations -Wwrite-strings -Wcast-qual -Wfloat-equal
-Wshadow -Wpointer-arith -Wbad-function-cast -Wsign-compare
-Waggregate-return -Wmissing-noreturn -Wmissing-format-attribute
-Wpacked -Wredundant-decls -Wnested-externs -Wdisabled-optimization
-Wno-long-long -Wendif-labels -Wcast-align
gcc 4.1.1?
[EMAIL PROTECTED] /home/pgollucci/tm/webcore/lib/perl/TMCS/Util 82 0>gcc -v
Using built-in specs.
Configured with: FreeBSD/i386 system compiler
Thread model: posix
gcc version 3.4.4 [FreeBSD] 20050518
I'll try some combination of the following tonight.
gcc40/Makefile:PORTREVISION=20060525
gcc40/Makefile:VERSIONSTRING= 4.0-${PORTREVISION}
gcc41/Makefile:PORTREVISION=20060526
gcc41/Makefile:VERSIONSTRING= 4.1-${PORTREVISION}
gcc42/Makefile:PORTREVISION=20060527
gcc42/Makefile:VERSIONSTRING= 4.2-${PORTREVISION}
--
Philip M. Gollucci ([EMAIL PROTECTED]) 323.219.4708
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Software Engineer - TicketMaster - http://ticketmaster.com
1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F
"It takes a minute to have a crush on someone, an hour to like someone,
and a day to love someone, but it takes a lifetime to forget someone..."
Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
Quoting "Philip M. Gollucci" <[EMAIL PROTECTED]>: my CFLAGS on FreeBSD are -g -O2 -Werror -Wall -Wmissing-prototypes -Wstrict-prototypes -Wmissing-declarations -Wwrite-strings -Wcast-qual -Wfloat-equal -Wshadow -Wpointer-arith -Wbad-function-cast -Wsign-compare -Waggregate-return -Wmissing-noreturn -Wmissing-format-attribute -Wpacked -Wredundant-decls -Wnested-externs -Wdisabled-optimization -Wno-long-long -Wendif-labels -Wcast-align gcc 4.1.1? BTW, I'm not 100% sure that -fno-strict-aliasing is the right flag to use. I just know that by trying them in order (as described in gcc man page) and above -O1, this was the one that started the problems. Then I went the other way and did -O2 -fno-strict-aliasing (i.e. removed only that one from the lot above -O1) and the assembly was still good. The manual page of gcc seem to suggest that "type-punning" may cause problems like this. I will report this in Red Hat Bugzilla and we'll see what the problem really is (i.e. is it something in gcc or maybe even APR). -- Bojan
Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
With -fno-strict-aliasing, I get: my CFLAGS on FreeBSD are -g -O2 -Werror -Wall -Wmissing-prototypes -Wstrict-prototypes -Wmissing-declarations -Wwrite-strings -Wcast-qual -Wfloat-equal -Wshadow -Wpointer-arith -Wbad-function-cast -Wsign-compare -Waggregate-return -Wmissing-noreturn -Wmissing-format-attribute -Wpacked -Wredundant-decls -Wnested-externs -Wdisabled-optimization -Wno-long-long -Wendif-labels -Wcast-align Which seem to work and come from this: ./buildconf --with-perl=/usr/local/software/perl/5.8.8/bin/perl setenv PERL5LIB /usr/local/software/mod_perl/trunk/lib ./configure \ --prefix=/usr/local/software/apreq/trunk \ --with-perl=/usr/local/software/perl/5.8.8/bin/perl \ --with-apache2-apxs=/usr/local/software/httpd/2.2.2/prefork/bin/apxs \ --with-expat=/usr/local \ --enable-maintainer-mode \ --enable-perl-glue gmake all gmake test -- Philip M. Gollucci ([EMAIL PROTECTED]) 323.219.4708 Consultant / http://p6m7g8.net/Resume/resume.shtml Senior Software Engineer - TicketMaster - http://ticketmaster.com 1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F "It takes a minute to have a crush on someone, an hour to like someone, and a day to love someone, but it takes a lifetime to forget someone..."
Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
On Wed, 2006-05-31 at 10:49 -0700, Philip M. Gollucci wrote: > So, does library/t/parsers fail for you without the -fno-strict-aliasing flag > under -02 ? Yes. The output is: versionok cookie.ok params.ok parsersok 1/542 With process lt-parsers consuming all CPU and having no strace output (i.e. it's stuck in an infinite loop). Backtrace from gdb is: (gdb) bt #0 0x0019c514 in split_on_bdry (out=0x8a569e8, in=0x8a56a08, pattern=0x8a563e0, bdry=0x8a563d1 "\r\n--AaB03x") at parser_multipart.c:167 #1 0x0019cb29 in apreq_parse_multipart (parser=0x8a56330, t=0x8a561b8, bb=0x8a56350) at parser_multipart.c:580 #2 0x0804ba87 in parse_multipart (AT=0x890e138) at ../../include/apreq_parser.h:126 #3 0x0804c3da in at_run (AT=0x890e138, test=0xbff45a30) at at.c:337 #4 0x08049426 in main () at parsers.c:546 With -fno-strict-aliasing, I get: versionok cookie.ok params.ok parsersok error..ok util...ok All tests successful. -- Bojan
Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
Now, the same code compiled with -fno-strict-aliasing (and -O2) gives: EGAD! I'll look later... I don't think I'll do much better then you. Maybe we should forward to the gcc-dev list ? Also, is -fno-strict-aliasing support on all gcc versions ? So, does library/t/parsers fail for you without the -fno-strict-aliasing flag under -02 ? -- Philip M. Gollucci ([EMAIL PROTECTED]) 323.219.4708 Consultant / http://p6m7g8.net/Resume/resume.shtml Senior Software Engineer - TicketMaster - http://ticketmaster.com 1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F "It takes a minute to have a crush on someone, an hour to like someone, and a day to love someone, but it takes a lifetime to forget someone..."
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)
movl4(%edx), %ecx
.LVL27:
cmpl%ecx, -44(%ebp)
.p2align 4,,7
.L30:
.loc 1 166 0
movl4(%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
movl4(%edi), %eax
movl%eax, 4(%ecx)
movl4(%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
movl4(%edx), %eax
.LVL25:
.loc 1 166 0
movl4(%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
movl4(%esi), %edx
movl%edx, 4(%eax)
movl4(%esi), %edx
movl%eax, 4(%esi)
movl%eax, (%edx)
.LBE6:
.LBE4:
.loc 1 168 0
movl-56(%ebp), %ecx
movl-44(%ebp), %eax
.LVL26:
cmpl4(%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
Re: Endless loop in split_on_bdry() of library/parser_multipart.c?
Thanks for your help. I'll see I go with GDB first. If I get nowhwere,
I'll try the .t.
Looking at library/parser_multipart.c and associated tests t/parsers.c ->
we definetely exercise that code block sucessfully.
If you copy httpd's .gdbinit file you can make use of
dump_brigade in
dump_brigade out
dump_bucket e
dump_bucket f
from within gdb, but I'm betting you are already using them since you're
using gdb.
static apr_status_t split_on_bdry(apr_bucket_brigade *out,
apr_bucket_brigade *in,
const apr_strmatch_pattern *pattern,
const char *bdry)
{
apr_bucket *e = APR_BRIGADE_FIRST(in);
apr_size_t blen = strlen(bdry), off = 0;
--> line ~121 while ( e != APR_BRIGADE_SENTINEL(in) ) {
or better yet, at your problem source:
do {
apr_bucket *f = APR_BRIGADE_FIRST(in);
APR_BUCKET_REMOVE(f);
APR_BRIGADE_INSERT_TAIL(out, f);
-> line ~171} while (e != APR_BRIGADE_FIRST(in));
and send a
bt full
with the dump_* commands from above after it gets stuck.
maybe something will jump out.
I looked at your mod_spin module (1.0) in src/mod_spin.c where you call
ap_discard_request_body. Seems sane to me.
In one of the tests that hits this do {} while, I see this
(gdb) bt full
#0 split_on_bdry (out=0x81f9438, in=0x81f9458, pattern=0x81f8e38,
bdry=0x81f8e29 "\r\n--AaB03x") at parser_multipart.c:171
idx = 136287324
len = 181
buf = 0x804d7d0 "\n\r\n--AaB03x\r\ncontent-disposition:
form-data; name=\"\"\r\ncontent-type: text/plain;\r\n
charset=windows-1250\r\ncontent-transfer-encoding:
quoted-printable\r\n\r\nJoe owes =80100.\r\n--AaB03x--\r\n"
s = 136146624
e = (apr_bucket *) 0x81d6e68
blen = 10
off = 1
#1 0x2807f6eb in apreq_parse_multipart (parser=0x81f8d88, t=0x81f8c10,
bb=0x81f8da8) at parser_multipart.c:583
param = (apreq_param_t *) 0x81f9998
pool = (apr_pool_t *) 0x8063018
ba = (apr_bucket_alloc_t *) 0x8075018
ctx = (struct mfd_ctx *) 0x81f8dc8
s = 5
This part confuses me why they are error buckets, let alone why that
symbol isn't defined for me... but thats a goof on my part, that I'll
have to figure out somehow... Interestly, I don't think any of them
are none error buckets from tests 9 -> 494. (attached -verbose output)
(gdb) dump_bucket e
bucket=IMMORTAL (0x081d6e68) length=181data=0x0804d6a0
No symbol "ap_bucket_type_error" in current context.
(gdb) dump_brigade in
dump of brigade 0x81f9458
| type (address)| length | data addr | contents
| rc
0 | IMMORTAL (0x081d6e68) | 181| 0x0804d6a0No symbol
"ap_bucket_type_error" in current context.
(gdb) dump_brigade out
dump of brigade 0x81f9438
| type (address)| length | data addr | contents
| rc
0 | IMMORTAL (0x081d6ec0) | 29 | 0x0804d6a0No symbol
"ap_bucket_type_error" in current context.
Time for sleep.
--
Philip M. Gollucci ([EMAIL PROTECTED]) 323.219.4708
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Software Engineer - TicketMaster - http://ticketmaster.com
1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F
"It takes a minute to have a crush on someone, an hour to like someone,
and a day to love someone, but it takes a lifetime to forget someone..."
1..542
# testing: f == (apreq_parser_function_t)apreq_parse_urlencoded (parsers.c:124)
# format: %pp
#left: 8048da4
# right: 8048da4
ok 1 - f == (apreq_parser_function_t)apreq_parse_urlencoded # at
(parsers.c:124) test 1 in locate_default_parsers
# testing: f == (apreq_parser_function_t)apreq_parse_multipart (parsers.c:127)
# format: %pp
#left: 8048e94
# right: 8048e94
ok 2 - f == (apreq_parser_function_t)apreq_parse_multipart # at (parsers.c:127)
test 2 in locate_default_parsers
# testing: f == (apreq_parser_function_t)apreq_parse_multipart (parsers.c:130)
# format: %pp
#left: 8048e94
# right: 8048e94
ok 3 - f == (apreq_parser_function_t)apreq_parse_multipart # at (parsers.c:130)
test 3 in locate_default_parsers
ok 4 - rv == APR_INCOMPLETE, as integers
ok 5 - rv == APR_SUCCESS, as integers
ok 6 - apr_table_get(body,"alpha") == "one", as strings
ok 7 - apr_table_get(body,"beta") == "two", as strings
ok 8 - apr_table_get(body,"omega") == "last+last", as strings
ok 9 - collected 5832 passing tests
ok 10 - collected 5832 passing tests
ok 11 - collected 5832 passing tests
ok 12 - collected 5832 passing tests
ok 13 - collected 5832 passing tests
ok 14 - collected 5832 passing tests
ok 15 - collected 5832 passing tests
ok 16 - collected 5832 passing tests
ok 17 - collected 5832 pas
