You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@attglobal.net> on 2002/03/01 17:14:05 UTC
[PATCH] fix alignment on shared memory
Jeff Trawick <tr...@attglobal.net> writes:
> Aaron Bannert <aa...@clove.org> writes:
>
> > Could this be what's causing the SIGBUS? My current theory is that
> > there's a size mismatch betwen ws->conn_bytes (an apr_off_t) and
> > the (unsigned long) that only shows up on word-alignment-picky
> > chips like sparcs.
>
> I'm bombing today on Solaris/sparc trying to do
>
> ap_scoreboard_image->global->restart_time = apr_time_now()
>
> That field lives at 0x14 from the start of the page. I would suppose
> that there are alignment requirements that we aren't maintaining.
> ap_scoreboard_image->global is at 0x4 from the start of the page.
> Does anybody know about any documentation on alignment requirements?
> Should the shared memory code return storage on 8-byte alignment?
This patch gets me running again:
Index: srclib/apr/shmem/unix/shm.c
===================================================================
RCS file: /home/cvs/apr/shmem/unix/shm.c,v
retrieving revision 1.14
diff -u -r1.14 shm.c
--- srclib/apr/shmem/unix/shm.c 5 Feb 2002 04:32:52 -0000 1.14
+++ srclib/apr/shmem/unix/shm.c 1 Mar 2002 16:03:33 -0000
@@ -59,6 +59,12 @@
#include "apr_user.h"
#include "apr_strings.h"
+/* APR_ALIGN() is only to be used to align on a power of 2 boundary */
+#define APR_ALIGN(size, boundary) \
+ (((size) + ((boundary) - 1)) & ~((boundary) - 1))
+
+#define APR_ALIGN_DEFAULT(size) APR_ALIGN(size, 8)
+
static apr_status_t shm_cleanup_owner(void *m_)
{
apr_shm_t *m = (apr_shm_t *)m_;
@@ -189,7 +195,7 @@
/* store the real size in the metadata */
*(apr_size_t*)(new_m->base) = new_m->realsize;
/* metadata isn't usable */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
apr_pool_cleanup_null);
@@ -206,7 +212,7 @@
/* store the real size in the metadata */
*(apr_size_t*)(new_m->base) = new_m->realsize;
/* metadata isn't usable */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
apr_pool_cleanup_null);
@@ -345,7 +351,7 @@
/* store the real size in the metadata */
*(apr_size_t*)(new_m->base) = new_m->realsize;
/* metadata isn't usable */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
apr_pool_cleanup_null);
@@ -508,7 +514,7 @@
}
/* metadata isn't part of the usable segment */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_attach,
apr_pool_cleanup_null);
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
"Sander Striker" <st...@apache.org> writes:
> > [mailto:trawick@rdu163-40-092.nc.rr.com]On Behalf Of Jeff Trawick
> > Sent: 01 March 2002 20:08
>
> > Cliff Woolley <jw...@virginia.edu> writes:
> >
> > > On Fri, 1 Mar 2002, Aaron Bannert wrote:
> > >
> > > > Can we put that alignment macro in a common place in APR, since it
> > > > is not useful to apps and internals?
> >
> > but where :) apr_lib.h? (duck)
>
> apr_general.h would be my first guess.
+1
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
RE: [PATCH] fix alignment on shared memory
Posted by Sander Striker <st...@apache.org>.
> [mailto:trawick@rdu163-40-092.nc.rr.com]On Behalf Of Jeff Trawick
> Sent: 01 March 2002 20:08
> Cliff Woolley <jw...@virginia.edu> writes:
>
> > On Fri, 1 Mar 2002, Aaron Bannert wrote:
> >
> > > Can we put that alignment macro in a common place in APR, since it
> > > is not useful to apps and internals?
>
> but where :) apr_lib.h? (duck)
apr_general.h would be my first guess.
Sander
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Cliff Woolley <jw...@virginia.edu> writes:
> On Fri, 1 Mar 2002, Aaron Bannert wrote:
>
> > Can we put that alignment macro in a common place in APR, since it
> > is not useful to apps and internals?
but where :) apr_lib.h? (duck)
> Does this look right? (attached to avoid line wrapping)
that is how I would have done it
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Cliff Woolley <jw...@virginia.edu> writes:
> On Fri, 1 Mar 2002, Aaron Bannert wrote:
>
> > Can we put that alignment macro in a common place in APR, since it
> > is not useful to apps and internals?
but where :) apr_lib.h? (duck)
> Does this look right? (attached to avoid line wrapping)
that is how I would have done it
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Cliff Woolley <jw...@virginia.edu> writes:
> On Fri, 1 Mar 2002, Cliff Woolley wrote:
>
> > Does this look right? (attached to avoid line wrapping)
>
> Hmm, no, it doesn't. :-/ Scratch that. Back to the drawing board.
What was wrong with it?
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Cliff Woolley <jw...@virginia.edu> writes:
> On Fri, 1 Mar 2002, Cliff Woolley wrote:
>
> > Does this look right? (attached to avoid line wrapping)
>
> Hmm, no, it doesn't. :-/ Scratch that. Back to the drawing board.
What was wrong with it?
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Cliff Woolley <jw...@virginia.edu>.
On Fri, 1 Mar 2002, Cliff Woolley wrote:
> Does this look right? (attached to avoid line wrapping)
Hmm, no, it doesn't. :-/ Scratch that. Back to the drawing board.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: [PATCH] fix alignment on shared memory
Posted by Cliff Woolley <jw...@virginia.edu>.
On Fri, 1 Mar 2002, Cliff Woolley wrote:
> Does this look right? (attached to avoid line wrapping)
Hmm, no, it doesn't. :-/ Scratch that. Back to the drawing board.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: [PATCH] fix alignment on shared memory
Posted by Cliff Woolley <jw...@virginia.edu>.
On Fri, 1 Mar 2002, Aaron Bannert wrote:
> Can we put that alignment macro in a common place in APR, since it
> is not useful to apps and internals?
s/not/now/, right? :)
Does this look right? (attached to avoid line wrapping)
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: [PATCH] fix alignment on shared memory
Posted by Cliff Woolley <jw...@virginia.edu>.
On Fri, 1 Mar 2002, Aaron Bannert wrote:
> Can we put that alignment macro in a common place in APR, since it
> is not useful to apps and internals?
s/not/now/, right? :)
Does this look right? (attached to avoid line wrapping)
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: [PATCH] fix alignment on shared memory
Posted by Aaron Bannert <aa...@clove.org>.
On Fri, Mar 01, 2002 at 01:37:27PM -0500, Jeff Trawick wrote:
> > Yes, I completely agree that each structure needs to be 64-bit aligned.
> > I don't see how the shared memory code itself is incorrect.
>
> It is incorrect because it is returning addresses that aren't 64-bit
> aligned.
>
> This code is in error:
>
> /* metadata isn't usable */
> new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
>
> given that new_m->usable is returned to the application as storage
> that it can use.
>
> My test program was intended to show a SIGBUS where it does the same
> thing that Apache is doing when it gets a SIGBUS. In my test
> program's case, I added 4 to the result of malloc() to get storage
> which wasn't aligned. In Apache's case, the code above to set
> new_m->usable is doing essentially the same thing (adding 4 to a
> properly-aligned value resulting in a storage address which is not
> properly aligned).
Ah yes, I see what you mean now. Yes this is also a problem and
should be aligned just like anything else.
Can we put that alignment macro in a common place in APR, since it
is not useful to apps and internals?
-aaron
Re: [PATCH] fix alignment on shared memory
Posted by Aaron Bannert <aa...@clove.org>.
On Fri, Mar 01, 2002 at 01:37:27PM -0500, Jeff Trawick wrote:
> > Yes, I completely agree that each structure needs to be 64-bit aligned.
> > I don't see how the shared memory code itself is incorrect.
>
> It is incorrect because it is returning addresses that aren't 64-bit
> aligned.
>
> This code is in error:
>
> /* metadata isn't usable */
> new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
>
> given that new_m->usable is returned to the application as storage
> that it can use.
>
> My test program was intended to show a SIGBUS where it does the same
> thing that Apache is doing when it gets a SIGBUS. In my test
> program's case, I added 4 to the result of malloc() to get storage
> which wasn't aligned. In Apache's case, the code above to set
> new_m->usable is doing essentially the same thing (adding 4 to a
> properly-aligned value resulting in a storage address which is not
> properly aligned).
Ah yes, I see what you mean now. Yes this is also a problem and
should be aligned just like anything else.
Can we put that alignment macro in a common place in APR, since it
is not useful to apps and internals?
-aaron
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Aaron Bannert <aa...@clove.org> writes:
> On Fri, Mar 01, 2002 at 12:34:49PM -0500, Jeff Trawick wrote:
> >
> > printf("%d %d %d %d\n",
> > offsetof(struct s1,b),
> > offsetof(struct s2,b),
> > offsetof(struct s3,b),
> > offsetof(struct s4,b));
> >
> > s3 = (struct s3 *)((char *)malloc(100) + 4);
>
> Of course it gives us a SIGBUS here, this is guaranteed on a sparc.
> I'm not entirely sure what you're trying to illustrate here though.
it doesn't give a SIGBUS here.... It gives a SIGBUS when we try to
store s3->b.
>
> > printf("%p\n",s3);
> > s3->a = 'A';
> > s3->b = 10241024;
>
> Shouldn't this be: s3->b = 10241024LL; ?
irrelevant
>
> > return 0;
> > }
> >
> > On my sparc machine it prints
> > 1 4 8 8
> > for the offsets of the variables of different types. Note that gcc
> > places long long and double on 64-bit boundaries.
> >
> > It then dies with SIGBUS when I trick it into trying to store a
> > 64-bit quantity into storage which s 32-bit aligned but not 64-bit
> > aligned.
>
> As expected.
>
> > Back to the code above that you posted:
> >
> > I think that it too needs to take into account alignment. It would
> > need to round the size of each structure up to 64-bit alignmet before
> > multiplying to ensure that objects of any type can live in that
> > storage. This currently isn't causing problems because the size of
> > each structure is consistent with 64-bit alignment (208 bytes for
> > worker_score, 24 bytes for global_score, 16 bytes for process_score),
> > at least in a 32-bit build for Sparc.
>
> Yes, I completely agree that each structure needs to be 64-bit aligned.
> I don't see how the shared memory code itself is incorrect.
It is incorrect because it is returning addresses that aren't 64-bit
aligned.
This code is in error:
/* metadata isn't usable */
new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
given that new_m->usable is returned to the application as storage
that it can use.
My test program was intended to show a SIGBUS where it does the same
thing that Apache is doing when it gets a SIGBUS. In my test
program's case, I added 4 to the result of malloc() to get storage
which wasn't aligned. In Apache's case, the code above to set
new_m->usable is doing essentially the same thing (adding 4 to a
properly-aligned value resulting in a storage address which is not
properly aligned).
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Aaron Bannert <aa...@clove.org> writes:
> On Fri, Mar 01, 2002 at 12:34:49PM -0500, Jeff Trawick wrote:
> >
> > printf("%d %d %d %d\n",
> > offsetof(struct s1,b),
> > offsetof(struct s2,b),
> > offsetof(struct s3,b),
> > offsetof(struct s4,b));
> >
> > s3 = (struct s3 *)((char *)malloc(100) + 4);
>
> Of course it gives us a SIGBUS here, this is guaranteed on a sparc.
> I'm not entirely sure what you're trying to illustrate here though.
it doesn't give a SIGBUS here.... It gives a SIGBUS when we try to
store s3->b.
>
> > printf("%p\n",s3);
> > s3->a = 'A';
> > s3->b = 10241024;
>
> Shouldn't this be: s3->b = 10241024LL; ?
irrelevant
>
> > return 0;
> > }
> >
> > On my sparc machine it prints
> > 1 4 8 8
> > for the offsets of the variables of different types. Note that gcc
> > places long long and double on 64-bit boundaries.
> >
> > It then dies with SIGBUS when I trick it into trying to store a
> > 64-bit quantity into storage which s 32-bit aligned but not 64-bit
> > aligned.
>
> As expected.
>
> > Back to the code above that you posted:
> >
> > I think that it too needs to take into account alignment. It would
> > need to round the size of each structure up to 64-bit alignmet before
> > multiplying to ensure that objects of any type can live in that
> > storage. This currently isn't causing problems because the size of
> > each structure is consistent with 64-bit alignment (208 bytes for
> > worker_score, 24 bytes for global_score, 16 bytes for process_score),
> > at least in a 32-bit build for Sparc.
>
> Yes, I completely agree that each structure needs to be 64-bit aligned.
> I don't see how the shared memory code itself is incorrect.
It is incorrect because it is returning addresses that aren't 64-bit
aligned.
This code is in error:
/* metadata isn't usable */
new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
given that new_m->usable is returned to the application as storage
that it can use.
My test program was intended to show a SIGBUS where it does the same
thing that Apache is doing when it gets a SIGBUS. In my test
program's case, I added 4 to the result of malloc() to get storage
which wasn't aligned. In Apache's case, the code above to set
new_m->usable is doing essentially the same thing (adding 4 to a
properly-aligned value resulting in a storage address which is not
properly aligned).
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Cliff Woolley <jw...@virginia.edu> writes:
> On 1 Mar 2002, Jeff Trawick wrote:
>
> > Jeff calling Cliff and Aaron: apr_shm_baseaddr_get() returns
> > addresses which aren't 64-bit aligned. That is broken. End of story.
>
> Yeah, your last message cleared a lot up. Thanks. But I believe we're
> all right. Both things are broken. :)
Hell, I already agreed that the calc-scoreboard-size stuff was
theoretically bad (but not bad with the current structure layouts
since we don't currently lose 64-bit alignment) :)
Here is my latest patch. Somebody that understands shm.c needs to
check it over and see if a seek() is needed before a mmap(). Also, as
you suggest the align macro needs to be in an APR header file.
Index: srclib/apr/shmem/unix/shm.c
===================================================================
RCS file: /home/cvs/apr/shmem/unix/shm.c,v
retrieving revision 1.14
diff -u -r1.14 shm.c
--- srclib/apr/shmem/unix/shm.c 5 Feb 2002 04:32:52 -0000 1.14
+++ srclib/apr/shmem/unix/shm.c 1 Mar 2002 18:55:59 -0000
@@ -59,6 +59,12 @@
#include "apr_user.h"
#include "apr_strings.h"
+/* APR_ALIGN() is only to be used to align on a power of 2 boundary */
+#define APR_ALIGN(size, boundary) \
+ (((size) + ((boundary) - 1)) & ~((boundary) - 1))
+
+#define APR_ALIGN_DEFAULT(size) APR_ALIGN(size, 8)
+
static apr_status_t shm_cleanup_owner(void *m_)
{
apr_shm_t *m = (apr_shm_t *)m_;
@@ -161,7 +167,8 @@
}
new_m->pool = pool;
new_m->reqsize = reqsize;
- new_m->realsize = reqsize + sizeof(apr_size_t); /* room for metadata */
+ new_m->realsize = reqsize +
+ APR_ALIGN_DEFAULT(sizeof(apr_size_t)); /* room for metadata */
new_m->filename = NULL;
#if APR_USE_SHMEM_MMAP_ZERO
@@ -189,7 +196,7 @@
/* store the real size in the metadata */
*(apr_size_t*)(new_m->base) = new_m->realsize;
/* metadata isn't usable */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
apr_pool_cleanup_null);
@@ -206,7 +213,7 @@
/* store the real size in the metadata */
*(apr_size_t*)(new_m->base) = new_m->realsize;
/* metadata isn't usable */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
apr_pool_cleanup_null);
@@ -274,7 +281,8 @@
new_m->filename = apr_pstrdup(pool, filename);
#if APR_USE_SHMEM_MMAP_TMP || APR_USE_SHMEM_MMAP_SHM
- new_m->realsize = reqsize + sizeof(apr_size_t); /* room for metadata */
+ new_m->realsize = reqsize +
+ APR_ALIGN_DEFAULT(sizeof(apr_size_t)); /* room for metadata */
/* FIXME: Ignore error for now. *
* status = apr_file_remove(file, pool);*/
status = APR_SUCCESS;
@@ -345,7 +353,7 @@
/* store the real size in the metadata */
*(apr_size_t*)(new_m->base) = new_m->realsize;
/* metadata isn't usable */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
apr_pool_cleanup_null);
@@ -482,6 +490,9 @@
return status;
}
+ XXX use APR_ALIGN_DEFAULT() somewhere here?
+ XXX do we need to seek() prior to the mmap()?
+
nbytes = sizeof(new_m->realsize);
status = apr_file_read(file, (void *)&(new_m->realsize),
&nbytes);
@@ -508,7 +519,7 @@
}
/* metadata isn't part of the usable segment */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_attach,
apr_pool_cleanup_null);
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Cliff Woolley <jw...@virginia.edu> writes:
> On 1 Mar 2002, Jeff Trawick wrote:
>
> > Jeff calling Cliff and Aaron: apr_shm_baseaddr_get() returns
> > addresses which aren't 64-bit aligned. That is broken. End of story.
>
> Yeah, your last message cleared a lot up. Thanks. But I believe we're
> all right. Both things are broken. :)
Hell, I already agreed that the calc-scoreboard-size stuff was
theoretically bad (but not bad with the current structure layouts
since we don't currently lose 64-bit alignment) :)
Here is my latest patch. Somebody that understands shm.c needs to
check it over and see if a seek() is needed before a mmap(). Also, as
you suggest the align macro needs to be in an APR header file.
Index: srclib/apr/shmem/unix/shm.c
===================================================================
RCS file: /home/cvs/apr/shmem/unix/shm.c,v
retrieving revision 1.14
diff -u -r1.14 shm.c
--- srclib/apr/shmem/unix/shm.c 5 Feb 2002 04:32:52 -0000 1.14
+++ srclib/apr/shmem/unix/shm.c 1 Mar 2002 18:55:59 -0000
@@ -59,6 +59,12 @@
#include "apr_user.h"
#include "apr_strings.h"
+/* APR_ALIGN() is only to be used to align on a power of 2 boundary */
+#define APR_ALIGN(size, boundary) \
+ (((size) + ((boundary) - 1)) & ~((boundary) - 1))
+
+#define APR_ALIGN_DEFAULT(size) APR_ALIGN(size, 8)
+
static apr_status_t shm_cleanup_owner(void *m_)
{
apr_shm_t *m = (apr_shm_t *)m_;
@@ -161,7 +167,8 @@
}
new_m->pool = pool;
new_m->reqsize = reqsize;
- new_m->realsize = reqsize + sizeof(apr_size_t); /* room for metadata */
+ new_m->realsize = reqsize +
+ APR_ALIGN_DEFAULT(sizeof(apr_size_t)); /* room for metadata */
new_m->filename = NULL;
#if APR_USE_SHMEM_MMAP_ZERO
@@ -189,7 +196,7 @@
/* store the real size in the metadata */
*(apr_size_t*)(new_m->base) = new_m->realsize;
/* metadata isn't usable */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
apr_pool_cleanup_null);
@@ -206,7 +213,7 @@
/* store the real size in the metadata */
*(apr_size_t*)(new_m->base) = new_m->realsize;
/* metadata isn't usable */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
apr_pool_cleanup_null);
@@ -274,7 +281,8 @@
new_m->filename = apr_pstrdup(pool, filename);
#if APR_USE_SHMEM_MMAP_TMP || APR_USE_SHMEM_MMAP_SHM
- new_m->realsize = reqsize + sizeof(apr_size_t); /* room for metadata */
+ new_m->realsize = reqsize +
+ APR_ALIGN_DEFAULT(sizeof(apr_size_t)); /* room for metadata */
/* FIXME: Ignore error for now. *
* status = apr_file_remove(file, pool);*/
status = APR_SUCCESS;
@@ -345,7 +353,7 @@
/* store the real size in the metadata */
*(apr_size_t*)(new_m->base) = new_m->realsize;
/* metadata isn't usable */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
apr_pool_cleanup_null);
@@ -482,6 +490,9 @@
return status;
}
+ XXX use APR_ALIGN_DEFAULT() somewhere here?
+ XXX do we need to seek() prior to the mmap()?
+
nbytes = sizeof(new_m->realsize);
status = apr_file_read(file, (void *)&(new_m->realsize),
&nbytes);
@@ -508,7 +519,7 @@
}
/* metadata isn't part of the usable segment */
- new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+ new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_attach,
apr_pool_cleanup_null);
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Cliff Woolley <jw...@virginia.edu>.
On 1 Mar 2002, Jeff Trawick wrote:
> Jeff calling Cliff and Aaron: apr_shm_baseaddr_get() returns
> addresses which aren't 64-bit aligned. That is broken. End of story.
Yeah, your last message cleared a lot up. Thanks. But I believe we're
all right. Both things are broken. :)
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: [PATCH] fix alignment on shared memory
Posted by Cliff Woolley <jw...@virginia.edu>.
On 1 Mar 2002, Jeff Trawick wrote:
> Jeff calling Cliff and Aaron: apr_shm_baseaddr_get() returns
> addresses which aren't 64-bit aligned. That is broken. End of story.
Yeah, your last message cleared a lot up. Thanks. But I believe we're
all right. Both things are broken. :)
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Cliff Woolley <jw...@virginia.edu> writes:
> On Fri, 1 Mar 2002, Aaron Bannert wrote:
>
> > Yes, I completely agree that each structure needs to be 64-bit aligned.
> > I don't see how the shared memory code itself is incorrect. It must be
> > mapping the segment at a properly aligned boundary, so as long as the
> > contents of that segment are accessed in an aligned manner than we're
> > safe, right?
>
> That's what I'm thinking, too. I don't see the problem there. It looks
> to me like you (Aaron) were right on when you suggested that it's
> ap_calc_scoreboard_size() and ap_init_scoreboard()'s fault. Try this, for
> example:
Jeff calling Cliff and Aaron: apr_shm_baseaddr_get() returns
addresses which aren't 64-bit aligned. That is broken. End of story.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Cliff Woolley <jw...@virginia.edu> writes:
> On Fri, 1 Mar 2002, Aaron Bannert wrote:
>
> > Yes, I completely agree that each structure needs to be 64-bit aligned.
> > I don't see how the shared memory code itself is incorrect. It must be
> > mapping the segment at a properly aligned boundary, so as long as the
> > contents of that segment are accessed in an aligned manner than we're
> > safe, right?
>
> That's what I'm thinking, too. I don't see the problem there. It looks
> to me like you (Aaron) were right on when you suggested that it's
> ap_calc_scoreboard_size() and ap_init_scoreboard()'s fault. Try this, for
> example:
Jeff calling Cliff and Aaron: apr_shm_baseaddr_get() returns
addresses which aren't 64-bit aligned. That is broken. End of story.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Cliff Woolley <jw...@virginia.edu>.
On Fri, 1 Mar 2002, Aaron Bannert wrote:
> Yes, I completely agree that each structure needs to be 64-bit aligned.
> I don't see how the shared memory code itself is incorrect. It must be
> mapping the segment at a properly aligned boundary, so as long as the
> contents of that segment are accessed in an aligned manner than we're
> safe, right?
That's what I'm thinking, too. I don't see the problem there. It looks
to me like you (Aaron) were right on when you suggested that it's
ap_calc_scoreboard_size() and ap_init_scoreboard()'s fault. Try this, for
example:
#include <stddef.h>
#include <stdio.h>
int main(void)
{
void *foo;
struct s1 {
char a;
char b;
} *s1;
struct s2 {
char a;
int b;
} *s2;
struct s3 {
char a;
long long b;
} *s3;
struct s4 {
char a;
double b;
} *s4;
printf("%d %d %d %d\n",
offsetof(struct s1,b),
offsetof(struct s2,b),
offsetof(struct s3,b),
offsetof(struct s4,b));
printf(" sizes: %d %d %d %d\n",
sizeof(struct s1),
sizeof(struct s2),
sizeof(struct s3),
sizeof(struct s4));
foo = malloc(100);
s1 = (struct s1 *)foo;
s2 = (struct s2 *)(((char *)s1)+sizeof(s1)*4);
s3 = (struct s3 *)(((char *)s2)+sizeof(s2));
printf("%p\n%p\n%p\n",s3,&s3->a,&s3->b);
s3->a = 'A';
s3->b = 10241024LL;
return 0;
}
jcw5q@cobra:/uf2/jcw5q$ ./test
1 4 8 8
sizes: 2 8 16 16
20c44
20c44
20c4c
Bus Error
Except for the times 4, this is very similar to what ap_init_scoreboard()
is giving us. Each of the sizeof()'s needs to be padded out because they
can interfere with one another when you place the structs back-to-back in
memory.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: [PATCH] fix alignment on shared memory
Posted by Cliff Woolley <jw...@virginia.edu>.
On Fri, 1 Mar 2002, Aaron Bannert wrote:
> Yes, I completely agree that each structure needs to be 64-bit aligned.
> I don't see how the shared memory code itself is incorrect. It must be
> mapping the segment at a properly aligned boundary, so as long as the
> contents of that segment are accessed in an aligned manner than we're
> safe, right?
That's what I'm thinking, too. I don't see the problem there. It looks
to me like you (Aaron) were right on when you suggested that it's
ap_calc_scoreboard_size() and ap_init_scoreboard()'s fault. Try this, for
example:
#include <stddef.h>
#include <stdio.h>
int main(void)
{
void *foo;
struct s1 {
char a;
char b;
} *s1;
struct s2 {
char a;
int b;
} *s2;
struct s3 {
char a;
long long b;
} *s3;
struct s4 {
char a;
double b;
} *s4;
printf("%d %d %d %d\n",
offsetof(struct s1,b),
offsetof(struct s2,b),
offsetof(struct s3,b),
offsetof(struct s4,b));
printf(" sizes: %d %d %d %d\n",
sizeof(struct s1),
sizeof(struct s2),
sizeof(struct s3),
sizeof(struct s4));
foo = malloc(100);
s1 = (struct s1 *)foo;
s2 = (struct s2 *)(((char *)s1)+sizeof(s1)*4);
s3 = (struct s3 *)(((char *)s2)+sizeof(s2));
printf("%p\n%p\n%p\n",s3,&s3->a,&s3->b);
s3->a = 'A';
s3->b = 10241024LL;
return 0;
}
jcw5q@cobra:/uf2/jcw5q$ ./test
1 4 8 8
sizes: 2 8 16 16
20c44
20c44
20c4c
Bus Error
Except for the times 4, this is very similar to what ap_init_scoreboard()
is giving us. Each of the sizeof()'s needs to be padded out because they
can interfere with one another when you place the structs back-to-back in
memory.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: [PATCH] fix alignment on shared memory
Posted by Aaron Bannert <aa...@clove.org>.
On Fri, Mar 01, 2002 at 12:34:49PM -0500, Jeff Trawick wrote:
> I'll punt for the moment on whether or not that code is bad. The
> shared memory code definitely needs to be fixed. Look at this simple
> test of alignment:
>
> #include <stddef.h>
> #include <stdio.h>
>
> int main(void)
> {
> struct s1 {
> char a;
> char b;
> } s1;
> struct s2 {
> char a;
> int b;
> } s2;
> struct s3 {
> char a;
> long long b;
> } *s3;
> struct s4 {
> char a;
> double b;
> } s4;
>
> printf("%d %d %d %d\n",
> offsetof(struct s1,b),
> offsetof(struct s2,b),
> offsetof(struct s3,b),
> offsetof(struct s4,b));
>
> s3 = (struct s3 *)((char *)malloc(100) + 4);
Of course it gives us a SIGBUS here, this is guaranteed on a sparc.
I'm not entirely sure what you're trying to illustrate here though.
> printf("%p\n",s3);
> s3->a = 'A';
> s3->b = 10241024;
Shouldn't this be: s3->b = 10241024LL; ?
(Probably not important.)
> return 0;
> }
>
> On my sparc machine it prints
> 1 4 8 8
> for the offsets of the variables of different types. Note that gcc
> places long long and double on 64-bit boundaries.
>
> It then dies with SIGBUS when I trick it into trying to store a
> 64-bit quantity into storage which s 32-bit aligned but not 64-bit
> aligned.
As expected.
> Back to the code above that you posted:
>
> I think that it too needs to take into account alignment. It would
> need to round the size of each structure up to 64-bit alignmet before
> multiplying to ensure that objects of any type can live in that
> storage. This currently isn't causing problems because the size of
> each structure is consistent with 64-bit alignment (208 bytes for
> worker_score, 24 bytes for global_score, 16 bytes for process_score),
> at least in a 32-bit build for Sparc.
Yes, I completely agree that each structure needs to be 64-bit aligned.
I don't see how the shared memory code itself is incorrect. It must be
mapping the segment at a properly aligned boundary, so as long as the
contents of that segment are accessed in an aligned manner than we're
safe, right?
-aaron
Re: [PATCH] fix alignment on shared memory
Posted by Aaron Bannert <aa...@clove.org>.
On Fri, Mar 01, 2002 at 12:34:49PM -0500, Jeff Trawick wrote:
> I'll punt for the moment on whether or not that code is bad. The
> shared memory code definitely needs to be fixed. Look at this simple
> test of alignment:
>
> #include <stddef.h>
> #include <stdio.h>
>
> int main(void)
> {
> struct s1 {
> char a;
> char b;
> } s1;
> struct s2 {
> char a;
> int b;
> } s2;
> struct s3 {
> char a;
> long long b;
> } *s3;
> struct s4 {
> char a;
> double b;
> } s4;
>
> printf("%d %d %d %d\n",
> offsetof(struct s1,b),
> offsetof(struct s2,b),
> offsetof(struct s3,b),
> offsetof(struct s4,b));
>
> s3 = (struct s3 *)((char *)malloc(100) + 4);
Of course it gives us a SIGBUS here, this is guaranteed on a sparc.
I'm not entirely sure what you're trying to illustrate here though.
> printf("%p\n",s3);
> s3->a = 'A';
> s3->b = 10241024;
Shouldn't this be: s3->b = 10241024LL; ?
(Probably not important.)
> return 0;
> }
>
> On my sparc machine it prints
> 1 4 8 8
> for the offsets of the variables of different types. Note that gcc
> places long long and double on 64-bit boundaries.
>
> It then dies with SIGBUS when I trick it into trying to store a
> 64-bit quantity into storage which s 32-bit aligned but not 64-bit
> aligned.
As expected.
> Back to the code above that you posted:
>
> I think that it too needs to take into account alignment. It would
> need to round the size of each structure up to 64-bit alignmet before
> multiplying to ensure that objects of any type can live in that
> storage. This currently isn't causing problems because the size of
> each structure is consistent with 64-bit alignment (208 bytes for
> worker_score, 24 bytes for global_score, 16 bytes for process_score),
> at least in a 32-bit build for Sparc.
Yes, I completely agree that each structure needs to be 64-bit aligned.
I don't see how the shared memory code itself is incorrect. It must be
mapping the segment at a properly aligned boundary, so as long as the
contents of that segment are accessed in an aligned manner than we're
safe, right?
-aaron
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Aaron Bannert <aa...@clove.org> writes:
> I've still got sleep in my eyes, so I might not be seeing this correctly
> yet, but isn't the problem not that the segment itself is misaligned,
> but that the structure that is being placed over that segment not
> properly padded to account for alignment?
>
> This looks like the problem to me:
>
> AP_DECLARE(int) ap_calc_scoreboard_size(void)
> {
> ap_mpm_query(AP_MPMQ_HARD_LIMIT_THREADS, &thread_limit);
> ap_mpm_query(AP_MPMQ_HARD_LIMIT_DAEMONS, &server_limit);
> scoreboard_size = sizeof(global_score);
> scoreboard_size += sizeof(process_score) * server_limit;
> scoreboard_size += sizeof(worker_score) * server_limit * thread_limit;
> return scoreboard_size;
> }
I'll punt for the moment on whether or not that code is bad. The
shared memory code definitely needs to be fixed. Look at this simple
test of alignment:
#include <stddef.h>
#include <stdio.h>
int main(void)
{
struct s1 {
char a;
char b;
} s1;
struct s2 {
char a;
int b;
} s2;
struct s3 {
char a;
long long b;
} *s3;
struct s4 {
char a;
double b;
} s4;
printf("%d %d %d %d\n",
offsetof(struct s1,b),
offsetof(struct s2,b),
offsetof(struct s3,b),
offsetof(struct s4,b));
s3 = (struct s3 *)((char *)malloc(100) + 4);
printf("%p\n",s3);
s3->a = 'A';
s3->b = 10241024;
return 0;
}
On my sparc machine it prints
1 4 8 8
for the offsets of the variables of different types. Note that gcc
places long long and double on 64-bit boundaries.
It then dies with SIGBUS when I trick it into trying to store a
64-bit quantity into storage which s 32-bit aligned but not 64-bit
aligned.
Back to the code above that you posted:
I think that it too needs to take into account alignment. It would
need to round the size of each structure up to 64-bit alignmet before
multiplying to ensure that objects of any type can live in that
storage. This currently isn't causing problems because the size of
each structure is consistent with 64-bit alignment (208 bytes for
worker_score, 24 bytes for global_score, 16 bytes for process_score),
at least in a 32-bit build for Sparc.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Aaron Bannert <aa...@clove.org> writes:
> I've still got sleep in my eyes, so I might not be seeing this correctly
> yet, but isn't the problem not that the segment itself is misaligned,
> but that the structure that is being placed over that segment not
> properly padded to account for alignment?
>
> This looks like the problem to me:
>
> AP_DECLARE(int) ap_calc_scoreboard_size(void)
> {
> ap_mpm_query(AP_MPMQ_HARD_LIMIT_THREADS, &thread_limit);
> ap_mpm_query(AP_MPMQ_HARD_LIMIT_DAEMONS, &server_limit);
> scoreboard_size = sizeof(global_score);
> scoreboard_size += sizeof(process_score) * server_limit;
> scoreboard_size += sizeof(worker_score) * server_limit * thread_limit;
> return scoreboard_size;
> }
I'll punt for the moment on whether or not that code is bad. The
shared memory code definitely needs to be fixed. Look at this simple
test of alignment:
#include <stddef.h>
#include <stdio.h>
int main(void)
{
struct s1 {
char a;
char b;
} s1;
struct s2 {
char a;
int b;
} s2;
struct s3 {
char a;
long long b;
} *s3;
struct s4 {
char a;
double b;
} s4;
printf("%d %d %d %d\n",
offsetof(struct s1,b),
offsetof(struct s2,b),
offsetof(struct s3,b),
offsetof(struct s4,b));
s3 = (struct s3 *)((char *)malloc(100) + 4);
printf("%p\n",s3);
s3->a = 'A';
s3->b = 10241024;
return 0;
}
On my sparc machine it prints
1 4 8 8
for the offsets of the variables of different types. Note that gcc
places long long and double on 64-bit boundaries.
It then dies with SIGBUS when I trick it into trying to store a
64-bit quantity into storage which s 32-bit aligned but not 64-bit
aligned.
Back to the code above that you posted:
I think that it too needs to take into account alignment. It would
need to round the size of each structure up to 64-bit alignmet before
multiplying to ensure that objects of any type can live in that
storage. This currently isn't causing problems because the size of
each structure is consistent with 64-bit alignment (208 bytes for
worker_score, 24 bytes for global_score, 16 bytes for process_score),
at least in a 32-bit build for Sparc.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Aaron Bannert <aa...@clove.org>.
I've still got sleep in my eyes, so I might not be seeing this correctly
yet, but isn't the problem not that the segment itself is misaligned,
but that the structure that is being placed over that segment not
properly padded to account for alignment?
This looks like the problem to me:
AP_DECLARE(int) ap_calc_scoreboard_size(void)
{
ap_mpm_query(AP_MPMQ_HARD_LIMIT_THREADS, &thread_limit);
ap_mpm_query(AP_MPMQ_HARD_LIMIT_DAEMONS, &server_limit);
scoreboard_size = sizeof(global_score);
scoreboard_size += sizeof(process_score) * server_limit;
scoreboard_size += sizeof(worker_score) * server_limit * thread_limit;
return scoreboard_size;
}
-aaron
On Fri, Mar 01, 2002 at 11:14:05AM -0500, Jeff Trawick wrote:
> Jeff Trawick <tr...@attglobal.net> writes:
>
> > Aaron Bannert <aa...@clove.org> writes:
> >
> > > Could this be what's causing the SIGBUS? My current theory is that
> > > there's a size mismatch betwen ws->conn_bytes (an apr_off_t) and
> > > the (unsigned long) that only shows up on word-alignment-picky
> > > chips like sparcs.
> >
> > I'm bombing today on Solaris/sparc trying to do
> >
> > ap_scoreboard_image->global->restart_time = apr_time_now()
> >
> > That field lives at 0x14 from the start of the page. I would suppose
> > that there are alignment requirements that we aren't maintaining.
> > ap_scoreboard_image->global is at 0x4 from the start of the page.
> > Does anybody know about any documentation on alignment requirements?
> > Should the shared memory code return storage on 8-byte alignment?
>
> This patch gets me running again:
>
> Index: srclib/apr/shmem/unix/shm.c
> ===================================================================
> RCS file: /home/cvs/apr/shmem/unix/shm.c,v
> retrieving revision 1.14
> diff -u -r1.14 shm.c
> --- srclib/apr/shmem/unix/shm.c 5 Feb 2002 04:32:52 -0000 1.14
> +++ srclib/apr/shmem/unix/shm.c 1 Mar 2002 16:03:33 -0000
> @@ -59,6 +59,12 @@
> #include "apr_user.h"
> #include "apr_strings.h"
>
> +/* APR_ALIGN() is only to be used to align on a power of 2 boundary */
> +#define APR_ALIGN(size, boundary) \
> + (((size) + ((boundary) - 1)) & ~((boundary) - 1))
> +
> +#define APR_ALIGN_DEFAULT(size) APR_ALIGN(size, 8)
> +
> static apr_status_t shm_cleanup_owner(void *m_)
> {
> apr_shm_t *m = (apr_shm_t *)m_;
> @@ -189,7 +195,7 @@
> /* store the real size in the metadata */
> *(apr_size_t*)(new_m->base) = new_m->realsize;
> /* metadata isn't usable */
> - new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
> + new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
>
> apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
> apr_pool_cleanup_null);
> @@ -206,7 +212,7 @@
> /* store the real size in the metadata */
> *(apr_size_t*)(new_m->base) = new_m->realsize;
> /* metadata isn't usable */
> - new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
> + new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
>
> apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
> apr_pool_cleanup_null);
> @@ -345,7 +351,7 @@
> /* store the real size in the metadata */
> *(apr_size_t*)(new_m->base) = new_m->realsize;
> /* metadata isn't usable */
> - new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
> + new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
>
> apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
> apr_pool_cleanup_null);
> @@ -508,7 +514,7 @@
> }
>
> /* metadata isn't part of the usable segment */
> - new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
> + new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
>
> apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_attach,
> apr_pool_cleanup_null);
>
>
> --
> Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
> http://www.geocities.com/SiliconValley/Park/9289/
> Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Aaron Bannert <aa...@clove.org>.
I've still got sleep in my eyes, so I might not be seeing this correctly
yet, but isn't the problem not that the segment itself is misaligned,
but that the structure that is being placed over that segment not
properly padded to account for alignment?
This looks like the problem to me:
AP_DECLARE(int) ap_calc_scoreboard_size(void)
{
ap_mpm_query(AP_MPMQ_HARD_LIMIT_THREADS, &thread_limit);
ap_mpm_query(AP_MPMQ_HARD_LIMIT_DAEMONS, &server_limit);
scoreboard_size = sizeof(global_score);
scoreboard_size += sizeof(process_score) * server_limit;
scoreboard_size += sizeof(worker_score) * server_limit * thread_limit;
return scoreboard_size;
}
-aaron
On Fri, Mar 01, 2002 at 11:14:05AM -0500, Jeff Trawick wrote:
> Jeff Trawick <tr...@attglobal.net> writes:
>
> > Aaron Bannert <aa...@clove.org> writes:
> >
> > > Could this be what's causing the SIGBUS? My current theory is that
> > > there's a size mismatch betwen ws->conn_bytes (an apr_off_t) and
> > > the (unsigned long) that only shows up on word-alignment-picky
> > > chips like sparcs.
> >
> > I'm bombing today on Solaris/sparc trying to do
> >
> > ap_scoreboard_image->global->restart_time = apr_time_now()
> >
> > That field lives at 0x14 from the start of the page. I would suppose
> > that there are alignment requirements that we aren't maintaining.
> > ap_scoreboard_image->global is at 0x4 from the start of the page.
> > Does anybody know about any documentation on alignment requirements?
> > Should the shared memory code return storage on 8-byte alignment?
>
> This patch gets me running again:
>
> Index: srclib/apr/shmem/unix/shm.c
> ===================================================================
> RCS file: /home/cvs/apr/shmem/unix/shm.c,v
> retrieving revision 1.14
> diff -u -r1.14 shm.c
> --- srclib/apr/shmem/unix/shm.c 5 Feb 2002 04:32:52 -0000 1.14
> +++ srclib/apr/shmem/unix/shm.c 1 Mar 2002 16:03:33 -0000
> @@ -59,6 +59,12 @@
> #include "apr_user.h"
> #include "apr_strings.h"
>
> +/* APR_ALIGN() is only to be used to align on a power of 2 boundary */
> +#define APR_ALIGN(size, boundary) \
> + (((size) + ((boundary) - 1)) & ~((boundary) - 1))
> +
> +#define APR_ALIGN_DEFAULT(size) APR_ALIGN(size, 8)
> +
> static apr_status_t shm_cleanup_owner(void *m_)
> {
> apr_shm_t *m = (apr_shm_t *)m_;
> @@ -189,7 +195,7 @@
> /* store the real size in the metadata */
> *(apr_size_t*)(new_m->base) = new_m->realsize;
> /* metadata isn't usable */
> - new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
> + new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
>
> apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
> apr_pool_cleanup_null);
> @@ -206,7 +212,7 @@
> /* store the real size in the metadata */
> *(apr_size_t*)(new_m->base) = new_m->realsize;
> /* metadata isn't usable */
> - new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
> + new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
>
> apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
> apr_pool_cleanup_null);
> @@ -345,7 +351,7 @@
> /* store the real size in the metadata */
> *(apr_size_t*)(new_m->base) = new_m->realsize;
> /* metadata isn't usable */
> - new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
> + new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
>
> apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
> apr_pool_cleanup_null);
> @@ -508,7 +514,7 @@
> }
>
> /* metadata isn't part of the usable segment */
> - new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
> + new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
>
> apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_attach,
> apr_pool_cleanup_null);
>
>
> --
> Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
> http://www.geocities.com/SiliconValley/Park/9289/
> Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Jeff Trawick <tr...@attglobal.net> writes:
> This patch gets me running again:
>
> Index: srclib/apr/shmem/unix/shm.c
> ===================================================================
> RCS file: /home/cvs/apr/shmem/unix/shm.c,v
> retrieving revision 1.14
> diff -u -r1.14 shm.c
> --- srclib/apr/shmem/unix/shm.c 5 Feb 2002 04:32:52 -0000 1.14
> +++ srclib/apr/shmem/unix/shm.c 1 Mar 2002 16:03:33 -0000
> @@ -59,6 +59,12 @@
> #include "apr_user.h"
> #include "apr_strings.h"
>
> +/* APR_ALIGN() is only to be used to align on a power of 2 boundary */
> +#define APR_ALIGN(size, boundary) \
> + (((size) + ((boundary) - 1)) & ~((boundary) - 1))
> +
> +#define APR_ALIGN_DEFAULT(size) APR_ALIGN(size, 8)
> +
> static apr_status_t shm_cleanup_owner(void *m_)
> {
> apr_shm_t *m = (apr_shm_t *)m_;
> @@ -189,7 +195,7 @@
> /* store the real size in the metadata */
> *(apr_size_t*)(new_m->base) = new_m->realsize;
> /* metadata isn't usable */
> - new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
> + new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
but something missing from the patch is taking into account the
alignment when calculating new_m->realsize (minor detail :) )
that alignment has to be reflected in other places as well
(apr_shm_attach() when we mmap the file)
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix alignment on shared memory
Posted by Jeff Trawick <tr...@attglobal.net>.
Jeff Trawick <tr...@attglobal.net> writes:
> This patch gets me running again:
>
> Index: srclib/apr/shmem/unix/shm.c
> ===================================================================
> RCS file: /home/cvs/apr/shmem/unix/shm.c,v
> retrieving revision 1.14
> diff -u -r1.14 shm.c
> --- srclib/apr/shmem/unix/shm.c 5 Feb 2002 04:32:52 -0000 1.14
> +++ srclib/apr/shmem/unix/shm.c 1 Mar 2002 16:03:33 -0000
> @@ -59,6 +59,12 @@
> #include "apr_user.h"
> #include "apr_strings.h"
>
> +/* APR_ALIGN() is only to be used to align on a power of 2 boundary */
> +#define APR_ALIGN(size, boundary) \
> + (((size) + ((boundary) - 1)) & ~((boundary) - 1))
> +
> +#define APR_ALIGN_DEFAULT(size) APR_ALIGN(size, 8)
> +
> static apr_status_t shm_cleanup_owner(void *m_)
> {
> apr_shm_t *m = (apr_shm_t *)m_;
> @@ -189,7 +195,7 @@
> /* store the real size in the metadata */
> *(apr_size_t*)(new_m->base) = new_m->realsize;
> /* metadata isn't usable */
> - new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
> + new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
but something missing from the patch is taking into account the
alignment when calculating new_m->realsize (minor detail :) )
that alignment has to be reflected in other places as well
(apr_shm_attach() when we mmap the file)
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...