You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Pavel Novy <no...@feld.cvut.cz> on 2001/08/27 16:08:25 UTC

[PATCH] Apache 1.3.21-dev for NetWare builds (5)

Hi all,
here is the biggest issue I am experiencing with my builds of the Apache 
for NetWare using the GNU stuff:

5. CodeWarrior bitfields alignment (compatibility) issue

The Apache binaries produced by the CodeWarrior (currently the only one 
officially supported tool for NetWare builds of the Apache server) are 
affected by an alignment issue (bitfields). It causes incompatibility 
between the core and external modules built with other tools (GNU stuff, 
Watcom, ...).

There is a structure called "uri_components" defined in src/include/util_uri.h:

typedef struct {
//   ...
    unsigned short port;
    unsigned is_initialized:1;
    unsigned dns_looked_up:1;
    unsigned dns_resolved:1;
} uri_components;

... and it is used within a structure called "request_rec" (widely used
by the modules and the core) defined in src/include/httpd.h:

struct request_rec {
//...
    uri_components parsed_uri; /* components of uri, dismantled */
    void *per_dir_config; /* Options set in config files, etc. */
    void *request_config; /* Notes on *this* request */
//...
};*

*An "unsigned short port" element is incorrectly aligned to 4 bytes (should be 2 bytes) and the following set of bitfields is incorrectly aligned to 4 bytes (should by 1 byte) by CW compiler => the size of "uri_components" is different (+5 bytes than usual, e.g. if compiling with gcc), offset to bitfield (the same for all its components) also differ.

Most problems (abends) occur if accessing something below a "parsed_uri" structure element (typically a "per_dir_config" used by a "ap_get_module_config" macro in some modules).

For more understanding: Here is the latest posting from Guenter Knauf 
taken from the "support.apachebuilds" forum on nntp.wiserlabz.com news 
server, where this issue was discussed:

>>>>>
Hi all,
the following (unofficial) answer I received from Metrowerks support;
I hope that this is NOT the final word......
=========
Hi,

I just got this in, and it looks to be the final word I am going to get.  So
why don't you look this over, (I've left it verbatim) and tell me if this is
acceptable to you.  It looks just like and implementation dependent item
where you should not count on it working the same.

Ron

=========

I agree this is an undesirable way for bitfields to be aligned, but this
agrees with MSVC, upon which we based the bitfield layout.  (In MSVC the
smallest we can make the struct is 4 bytes!)

I think the way to work around this is to change the bitfield base type to
"unsigned char".  The type of the bitfield is used in determining how it is
aligned; thus, using a smaller type will require a smaller alignment.

P.S.  Remind the developers that this style of bitfield alignment is neither
"correct" or "incorrect" (unless something really bad is happening such as
bitfields overlapping each other or other members of the struct) since
neither ANSI nor C99 define how compilers implement bitfields.  This is
strictly the vendor's decision.
<<<<<

I suggest to fix the problem as described (patch attached), even if it will be considered as CodeWarrior's bug (and fixed, then) by Metrowerks or not. I've detailedly analyzed the issue and I see no way how to stay backwardly compatible with this issue, so all binaries for NetWare platform (Apache core+modules from CVS sources and all "3rd party" external modules produced with CW) should be re-built after the fix (if it will be commited) - it will not be possible to mix new NLMs with older Apache core or modules.

I have tested the fix with CW 5.3 and gcc 2.95.3 and it seems that alignment of problematic structures is the same (it differs from current status quo, of course). The change in the source file shared on all platforms is requested, so verification on these ones will be needed. I don't expect problems (no change in alignment) on platforms where the GNU stuff is used (gcc). Thanks.

Regards,
Pavel


Re: [PATCH] Apache 1.3.21-dev for NetWare builds (5)

Posted by Pavel Novy <no...@feld.cvut.cz>.
Hi,

Greg Stein wrote:

>On Mon, Aug 27, 2001 at 04:08:25PM +0200, Pavel Novy wrote:
>
>>...
>>I agree this is an undesirable way for bitfields to be aligned, but this
>>agrees with MSVC, upon which we based the bitfield layout.  (In MSVC the
>>smallest we can make the struct is 4 bytes!)
>>
>>I think the way to work around this is to change the bitfield base type to
>>"unsigned char".  The type of the bitfield is used in determining how it is
>>aligned; thus, using a smaller type will require a smaller alignment.
>>...
>>--- util_uri.h.orig	Mon Feb 26 16:49:32 2001
>>+++ util_uri.h	Mon Aug 27 15:44:41 2001
>>@@ -106,10 +106,10 @@
>> 
>>     unsigned short port;	/* The port number, numeric, valid only if port_str != NULL */
>>     
>>-    unsigned is_initialized:1;
>>+    unsigned char is_initialized:1;
>> 
>>-    unsigned dns_looked_up:1;
>>-    unsigned dns_resolved:1;
>>+    unsigned char dns_looked_up:1;
>>+    unsigned char dns_resolved:1;
>> 
>> } uri_components;
>>
>
>
>Screw the bitfields. Just change all of them to plain old unsigned chars.
>(and apr_byte_t in APR). There is no reason to be miserly with bits here.
>
>Cheers,
>-g
>
I agree, but the proposed change was designed to not affect 
sizeof(uri_components) on other platforms then NetWare. It's not a good 
idea to invoke unwanted compatibility issues on those platforms. After 
change, the bitfield still occupies one byte (on Linux aligned to 2 
bytes, on NetWare to 1 byte).

Regards,
Pavel



Re: [PATCH] Apache 1.3.21-dev for NetWare builds (5)

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Aug 27, 2001 at 04:08:25PM +0200, Pavel Novy wrote:
>...
> I agree this is an undesirable way for bitfields to be aligned, but this
> agrees with MSVC, upon which we based the bitfield layout.  (In MSVC the
> smallest we can make the struct is 4 bytes!)
> 
> I think the way to work around this is to change the bitfield base type to
> "unsigned char".  The type of the bitfield is used in determining how it is
> aligned; thus, using a smaller type will require a smaller alignment.
>...
> --- util_uri.h.orig	Mon Feb 26 16:49:32 2001
> +++ util_uri.h	Mon Aug 27 15:44:41 2001
> @@ -106,10 +106,10 @@
>  
>      unsigned short port;	/* The port number, numeric, valid only if port_str != NULL */
>      
> -    unsigned is_initialized:1;
> +    unsigned char is_initialized:1;
>  
> -    unsigned dns_looked_up:1;
> -    unsigned dns_resolved:1;
> +    unsigned char dns_looked_up:1;
> +    unsigned char dns_resolved:1;
>  
>  } uri_components;


Screw the bitfields. Just change all of them to plain old unsigned chars.
(and apr_byte_t in APR). There is no reason to be miserly with bits here.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: [PATCH] Apache 1.3.21-dev for NetWare builds (5)

Posted by Pavel Novy <no...@feld.cvut.cz>.
Hi,

William A. Rowe, Jr. wrote:

>From: "Pavel Novy" <no...@feld.cvut.cz>
>Sent: Monday, August 27, 2001 9:08 AM
>
>
>>I suggest to fix the problem as described (patch attached), even if it will be considered as CodeWarrior's bug (and fixed, then)
>>
>by Metrowerks or not. I've detailedly analyzed the issue and I see no way how to stay backwardly compatible with this issue, so all
>binaries for NetWare platform (Apache core+modules from CVS sources and all "3rd party" external modules produced with CW) should be
>re-built after the fix (if it will be commited) - it will not be possible to mix new NLMs with older Apache core or modules.
>
>>I have tested the fix with CW 5.3 and gcc 2.95.3 and it seems that alignment of problematic structures is the same (it differs
>>
>from current status quo, of course). The change in the source file shared on all platforms is requested, so verification on these
>ones will be needed. I don't expect problems (no change in alignment) on platforms where the GNU stuff is used (gcc). Thanks.
>
>Pavel,
>
>  I'm sorry, I cannot see this code change for the 1.3 generation.  I sympathize that the
>implementors on the Netware platform refuse to see eye to eye (or even attempt to bring
>some sort of compatiblity options to the table.)  And the unsigned char solution is very
>slow, compared to integer manipulation, so I'm not sure we would choose that anyways.
>
I don't know how much are you familiar with the bitfields and gcc, but 
I've spend enough hours analyzing this weird issue already... I have 
tested with gcc 2.95.2 on Linux - it is obvious that  this change in the 
base type doesn't affect speed of produced code. Please take a look on 
attached sample (especially .s files) to see differences with changed 
base type to char, short, int and long. The bitfield is very specific 
kind of C(++) type and gcc implements it the following way: the smallest 
amount of memory is used to store merged "bit elements" used in a struct 
- if it fits into char (8 bits), then one byte is used to store/access 
whole bitfield (gcc, Watcom).

There is the essential difference in implementation of the bitfields 
between gcc and CodeWarrior and it is the source of the problem. I am 
not able to decide which implementation is wrong and which is not (even 
if that used in CW is very unusual), but I am pretty sure, that proposed 
change causes no problems on Linux platform (not sure for other ones). 
The problems expected on NetWare platform I've described already.

>
>  Please bring this discussion to Apache 2.0.  I know Brad Nicholes and company at Novell
>are working hard to support Apache 2.0.  This implementation will be a far more secure and
>stable platform than the 1.3 hybrid/threaded implementations (win32/os2/netware etc.)
>
I believe that they are doing their job well (and it is hard) and hoping 
that version 2.0 of the Apache server will be the best one ever. 
However, even if version 1.3 is ugly in some cases, it is the only one 
version currently available for NetWare platform. My target was build 
the Apache server for NetWare from the CVS sources with GNU stuff and I 
have succeeded with this, so I hope it will be supported way by default 
in 2.0. In other case I will be in the same situation like I was with 
version 1.3, when some new sources for NetWare platform will be out.

>
>  If we direct our energies at solving the problem there, and we can agree on a mechansim,
>I see this issue being resolved in Apache 2.0.
>
>Bill
>
There are no sources for NetWare platform available yet, so I can't do 
anything with this. I don't really want to waste developers' time, but 
version 1.3 is still the current one...

Regards,
Pavel

P.S.: BTW, another way how to solve the problem without changes in the 
sources is to throw CodeWarrior away and use gcc as a tool for all 
target platforms... (But it is impossible to do it just now.)


Re: [PATCH] Apache 1.3.21-dev for NetWare builds (5)

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Pavel Novy" <no...@feld.cvut.cz>
Sent: Monday, August 27, 2001 9:08 AM


> I suggest to fix the problem as described (patch attached), even if it will be considered as CodeWarrior's bug (and fixed, then)
by Metrowerks or not. I've detailedly analyzed the issue and I see no way how to stay backwardly compatible with this issue, so all
binaries for NetWare platform (Apache core+modules from CVS sources and all "3rd party" external modules produced with CW) should be
re-built after the fix (if it will be commited) - it will not be possible to mix new NLMs with older Apache core or modules.
>
> I have tested the fix with CW 5.3 and gcc 2.95.3 and it seems that alignment of problematic structures is the same (it differs
from current status quo, of course). The change in the source file shared on all platforms is requested, so verification on these
ones will be needed. I don't expect problems (no change in alignment) on platforms where the GNU stuff is used (gcc). Thanks.

Pavel,

  I'm sorry, I cannot see this code change for the 1.3 generation.  I sympathize that the
implementors on the Netware platform refuse to see eye to eye (or even attempt to bring
some sort of compatiblity options to the table.)  And the unsigned char solution is very
slow, compared to integer manipulation, so I'm not sure we would choose that anyways.

  Please bring this discussion to Apache 2.0.  I know Brad Nicholes and company at Novell
are working hard to support Apache 2.0.  This implementation will be a far more secure and
stable platform than the 1.3 hybrid/threaded implementations (win32/os2/netware etc.)

  If we direct our energies at solving the problem there, and we can agree on a mechansim,
I see this issue being resolved in Apache 2.0.

Bill





--------------------------------------------------------------------------------


> --- util_uri.h.orig Mon Feb 26 16:49:32 2001
> +++ util_uri.h Mon Aug 27 15:44:41 2001
> @@ -106,10 +106,10 @@
>
>      unsigned short port; /* The port number, numeric, valid only if port_str != NULL */
>
> -    unsigned is_initialized:1;
> +    unsigned char is_initialized:1;
>
> -    unsigned dns_looked_up:1;
> -    unsigned dns_resolved:1;
> +    unsigned char dns_looked_up:1;
> +    unsigned char dns_resolved:1;
>
>  } uri_components;
>
>


Re: [PATCH] Apache 1.3.21-dev for NetWare builds (5)

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Aug 27, 2001 at 04:08:25PM +0200, Pavel Novy wrote:
>...
> I agree this is an undesirable way for bitfields to be aligned, but this
> agrees with MSVC, upon which we based the bitfield layout.  (In MSVC the
> smallest we can make the struct is 4 bytes!)
> 
> I think the way to work around this is to change the bitfield base type to
> "unsigned char".  The type of the bitfield is used in determining how it is
> aligned; thus, using a smaller type will require a smaller alignment.
>...
> --- util_uri.h.orig	Mon Feb 26 16:49:32 2001
> +++ util_uri.h	Mon Aug 27 15:44:41 2001
> @@ -106,10 +106,10 @@
>  
>      unsigned short port;	/* The port number, numeric, valid only if port_str != NULL */
>      
> -    unsigned is_initialized:1;
> +    unsigned char is_initialized:1;
>  
> -    unsigned dns_looked_up:1;
> -    unsigned dns_resolved:1;
> +    unsigned char dns_looked_up:1;
> +    unsigned char dns_resolved:1;
>  
>  } uri_components;


Screw the bitfields. Just change all of them to plain old unsigned chars.
(and apr_byte_t in APR). There is no reason to be miserly with bits here.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/