Re: [Devel] 64-bit compatability

Bogdan-Andrei Iancu Thu, 04 Jan 2007 10:01:40 -0800

Hi Eliot,

Eliot Gable wrote:

For some reason, this E-mail seemed to not go out last time I sent it, so if 
you get two, please ignore.

because you sent it privately, only to me :D

I'm definitely not setting CFLAGS globally anywhere before running make. I'll check through things and see if I can figure out what's causing the missing make flags.

if you find something that may show up to other users, please have itposted on the list.

static int w_sl_send_reply(struct sip_msg* msg, char* str, char* str2)
{
   return sl_send_reply( msg, (unsigned int)(unsigned long)str, str2);
}

The way I am reading this is:

Take the memory address stored in the pointer variable call str (which, 
incidentally, that memory address refers to a single byte in memory) and 
convert that memory address into an unsigned long (64-bit) number. Then, 
convert that unsigned long memory address into an unsigned int (32-bit) number. 
Now, if that line were written like this:

return sl_send_reply( msg, (unsigned int)(unsigned long)(*str), str2);
Then, I would agree it is safe, as the (*str) refers to a value that is only 8-bits long. Therefore, converting it to 64-bits and then back to 32-bits looses no information.
But, that is not the case. The str variable is a pointer variable. Pointer 
variables always store 32-bit values on 32-bit architectures and 64-bit values 
on 64-bit architectures. The char keyword simply tells the compiler the size, 
in bits, of the value stored at the particular address that the pointer 
variable stores. On 32-bit architectures, it is essentially equivalent to 
saying:

typedef unsigned int ptr;
char ptr str;

Where this would mean that str is a 32-bit memory location that stores an 8-bit 
value.

On 64-bit architectures, it would be like saying:

typedef unsigned long ptr;
char ptr str;
Where this would mean that str is a 64-bit memory location that stores an 8-bit value.Therefore, the cast of str from a pointer variable to an unsigned long will be safe on a 64-bit architecture as you do not loose any information. But, the cast of that 64-bit number to a 32-bit number looses information if the number is larger than 2^32.
And, as I stated before, I understand that OpenSER is stable for you when you run it on a 64-bit architecture. In fact, I am running it on 64-bit, as well. The problem is that unless you have >4GB of memory in the box AND >4GB of memory in use before you start OpenSER, it will be stable because you will not run into a case where the memory location is greater than 2^32.

Eliot, the str variable (as content) is *never* used as pointer in thiscase. It is just used to transport an integer value (which any how isless than 700 as it is a reply code). It is like transporting a box in atruck and not like sticking a truck in a box :).


the str var (on 32/64 bits) is used to pass an int (32 bits).

regards,
bogdan

Eliot Gable
Operations Engineer
CCNA, CWNA, CWSP, Network+, Security+
Broadvox, LLC
1228 Euclid Avenue
Suite 390
Cleveland, OH 44115-1800
216-373-4808

-----Original Message-----
From: Bogdan-Andrei Iancu [mailto:[EMAIL PROTECTED]
Sent: Friday, December 22, 2006 1:49 PM
To: Eliot Gable
Cc: devel@openser.org
Subject: Re: [Devel] 64-bit compatability

Hi Eliot,

I guess there is a small problem on you system as non of the compiling
flags (generated by Makefile) show into the gcc line.

Are you sure you do not have the CFLAGS environment variable set before
doing make?! You do not need to change anything in the Makefile.defs.

Please double check that. Normally you should get something like:

gcc -g -O9 -funroll-loops -Wcast-align -Wall -minline-all-stringops
-falign-loops -ftree-vectorize -mtune=opteron    -DNAME='"openser"'
-DVERSION='"1.2.0-dev15-notls"' -DARCH='"x86_64"' -DOS='"linux"'
-DCOMPILER='"gcc 4.1.2"' -D__CPU_x86_64 -D__OS_linux -D__SMP_yes
-DCFG_DIR='"/usr/local/etc/openser/"' -DPKG_MALLOC -DSHM_MEM  -DSHM_MMAP
-DUSE_IPV6 -DUSE_MCAST -DUSE_TCP -DDISABLE_NAGLE -DHAVE_RESOLV_RES
-DSTATISTICS -DF_MALLOC  -DFAST_LOCK -DADAPTIVE_WAIT
-DADAPTIVE_WAIT_LOOPS=1024  -DHAVE_GETHOSTBYNAME2 -DHAVE_UNION_SEMUN
-DHAVE_SCHED_YIELD -DHAVE_MSG_NOSIGNAL -DHAVE_MSGHDR_MSG_CONTROL
-DHAVE_ALLOCA_H -DHAVE_TIMEGM -DHAVE_EPOLL -DHAVE_SIGIO_RT -DHAVE_SELECT
-c action.c -o action.o

regarding your alarm on sl_send_reply(), I ensure you that everything is
ok :). both on 32 and 64 bits - actually we are running openser on 64
arch for some time with no problem.

I agree with that you are saying, but is not the case as the char* first
param is used to "transport" and integer. The function must respect a
generic prototype, but from some time you need to pass an integer; in
such a case, the int is placed over the pointer by casting - this is
perfectly safe as sizeof(int)>=sizeof(char*) all the time.

Regards and Merry Christmas,
Bogdan

Eliot Gable wrote:

Using OpenSER-1.1.0-notls:

gcc     -DNAME='"openser"' -DVERSION='"1.1.0-notls"' -DARCH='"i686"' -

DOS='"linux"' -DCOMPILER='"gcc 4.1.0"' -D__CPU_i686 -D__OS_linux -
D__SMP_no -DCFG_DIR='"/usr/local/etc/openser/"' -DPKG_MALLOC -DSHM_MEM  -
DSHM_MMAP -DUSE_IPV6 -DUSE_MCAST -DUSE_TCP -DDISABLE_NAGLE -
DHAVE_RESOLV_RES -DSTATISTICS -DF_MALLOC  -DHAVE_GETHOSTBYNAME2 -
DHAVE_UNION_SEMUN -DHAVE_SCHED_YIELD -DHAVE_MSG_NOSIGNAL -
DHAVE_MSGHDR_MSG_CONTROL -DHAVE_ALLOCA_H -DHAVE_TIMEGM -DUSE_SYSV_SEM   -
DHAVE_EPOLL -DHAVE_SIGIO_RT -DHAVE_SELECT -c action.c -o action.o

Or, if I turn on extra debugging:

gcc     -DNAME='"openser"' -DVERSION='"1.1.0-notls"' -DARCH='"i686"' -

DOS='"linux"' -DCOMPILER='"gcc 4.1.0"' -D__CPU_i686 -D__OS_linux -
D__SMP_no -DCFG_DIR='"/usr/local/etc/openser/"' -DPKG_MALLOC -DSHM_MEM  -
DSHM_MMAP -DUSE_IPV6 -DUSE_MCAST -DUSE_TCP -DDISABLE_NAGLE -
DHAVE_RESOLV_RES -DSTATISTICS -DF_MALLOC -DEXTRA_DEBUG  -
DHAVE_GETHOSTBYNAME2 -DHAVE_UNION_SEMUN -DHAVE_SCHED_YIELD -
DHAVE_MSG_NOSIGNAL -DHAVE_MSGHDR_MSG_CONTROL -DHAVE_ALLOCA_H -DHAVE_TIMEGM
-DUSE_SYSV_SEM   -DHAVE_EPOLL -DHAVE_SIGIO_RT -DHAVE_SELECT -c action.c -o
action.o

If I turn on -Wall by manually adding it to the Makefile.defs here:

ifeq ($(mode),debug)
  DEFS+= -DEXTRA_DEBUG
endif

Changed to:

ifeq ($(mode),debug)
  DEFS+= -DEXTRA_DEBUG -Wall
endif

Then I get this line:

gcc     -DNAME='"openser"' -DVERSION='"1.1.0-notls"' -DARCH='"i686"' -

DOS='"linux"' -DCOMPILER='"gcc 4.1.0"' -D__CPU_i686 -D__OS_linux -
D__SMP_no -DCFG_DIR='"/usr/local/etc/openser/"' -DPKG_MALLOC -DSHM_MEM  -
DSHM_MMAP -DUSE_IPV6 -DUSE_MCAST -DUSE_TCP -DDISABLE_NAGLE -
DHAVE_RESOLV_RES -DSTATISTICS -DF_MALLOC -DEXTRA_DEBUG -Wall -
DHAVE_GETHOSTBYNAME2 -DHAVE_UNION_SEMUN -DHAVE_SCHED_YIELD -
DHAVE_MSG_NOSIGNAL -DHAVE_MSGHDR_MSG_CONTROL -DHAVE_ALLOCA_H -DHAVE_TIMEGM
-DUSE_SYSV_SEM   -DHAVE_EPOLL -DHAVE_SIGIO_RT -DHAVE_SELECT -c action.c -o
action.o

But, that doesn't complain about all possible warnings. This does
(-W -Wall):

gcc     -DNAME='"openser"' -DVERSION='"1.1.0-notls"' -DARCH='"i686"' -

DOS='"linux"' -DCOMPILER='"gcc 4.1.0"' -D__CPU_i686 -D__OS_linux -
D__SMP_no -DCFG_DIR='"/usr/local/etc/openser/"' -DPKG_MALLOC -DSHM_MEM  -
DSHM_MMAP -DUSE_IPV6 -DUSE_MCAST -DUSE_TCP -DDISABLE_NAGLE -
DHAVE_RESOLV_RES -DSTATISTICS -DF_MALLOC -DEXTRA_DEBUG -W -Wall -
DHAVE_GETHOSTBYNAME2 -DHAVE_UNION_SEMUN -DHAVE_SCHED_YIELD -
DHAVE_MSG_NOSIGNAL -DHAVE_MSGHDR_MSG_CONTROL -DHAVE_ALLOCA_H -DHAVE_TIMEGM
-DUSE_SYSV_SEM   -DHAVE_EPOLL -DHAVE_SIGIO_RT -DHAVE_SELECT -c action.c -o
action.o

In file included from mem/mem.h:54,
               from lock_alloc.h:50,
               from locking.h:66,
               from statistics.h:102,
               from sr_module.h:42,
               from action.c:55:
mem/f_malloc.h: In function âfm_get_fragsâ:
mem/f_malloc.h:162: warning: comparison between signed and unsigned



This should be size_t:

flags.h:#define MAX_FLAG  ((unsigned int)( sizeof(flag_t) * CHAR_BIT - 1

))

And these:

mem/vq_malloc.c:        unsigned int real_size;
mem/vq_malloc.c:struct vqm_block* vqm_malloc_init(char* address, unsigned

int size)

mem/vq_malloc.c:void* vqm_malloc(struct vqm_block* qm, unsigned int size,
mem/vq_malloc.c:        char* file, char* func, unsigned int line)
mem/vq_malloc.c:void* vqm_malloc(struct vqm_block* qm, unsigned int size)

mem/vq_malloc.h:struct vqm_block* vqm_malloc_init(char* address, unsigned

int size);

mem/vq_malloc.h:void* vqm_malloc(struct vqm_block*, unsigned int size,

char* file, char* func,

mem/vq_malloc.h:                                        unsigned int

line);

mem/vq_malloc.h:void* vqm_malloc(struct vqm_block*, unsigned int size);

mem/shm_mem.c:inline static void* sh_realloc(void* p, unsigned int size)
mem/shm_mem.c:void* _shm_resize( void* p, unsigned int s, const char*

file, const char* func,

mem/shm_mem.c:void* _shm_resize( void* p , unsigned int s)
mem/shm_mem.h:inline static void* _shm_malloc(unsigned int size,
mem/shm_mem.h:inline static void* _shm_realloc(void *ptr, unsigned int

size,

mem/shm_mem.h:void* _shm_resize(void* ptr, unsigned int size, const char*

f, const char* fn,

mem/shm_mem.h:inline static void* shm_malloc(unsigned int size)
mem/shm_mem.h:inline static void* shm_realloc(void *ptr, unsigned int

size)

mem/shm_mem.h:void* _shm_resize(void* ptr, unsigned int size);



And what's up with this?:

modules/sl/sl.c:        return sl_send_reply( msg, (unsigned

int)(unsigned long)str, str2);

Turns out, that is a cast from a char* (which, on 64-bit systems is 64-

bits or unsigned long) to a 32-bit value:

static int w_sl_send_reply(struct sip_msg* msg, char* str, char* str2)
{
  return sl_send_reply( msg, (unsigned int)(unsigned long)str, str2);
}

If you are on a 64-bit system and that char* points to a location > 4GB

into memory, then sl_send_reply is in trouble, because it is no longer
pointing to the same location in memory when inside that function. It is
now pointing to some random location inside the 4GB envelope.

While OpenSER may be compiled on a 64-bit system (which I run a couple,

as well, and it does compile), it is not 64-bit safe. If you were to run
this with 32 GB of memory in the machine and use up 8 GB of memory, then
start OpenSER, it would die spectacularly.

I'm not trying to pick a fight or anything -- I just want to make sure

these obvious errors are caught and resolved. Anything that references a
length or index to an array, a memory size, or any casts from a pointer to
some form of integer need to use size_t so that their number of bits
change with the platform you are compiling the program for.


Eliot Gable
Operations Engineer
CCNA, CWNA, CWSP, Network+, Security+
Broadvox, LLC
1228 Euclid Avenue
Suite 390
Cleveland, OH 44115-1800
216-373-4808

-----Original Message-----
From: Bogdan-Andrei Iancu [mailto:[EMAIL PROTECTED]
Sent: Friday, December 22, 2006 7:45 AM
To: Eliot Gable
Cc: devel@openser.org
Subject: Re: [Devel] 64-bit compatability

Hi Eliot,

the code it checked to get compiled on 64 arch. Actually I'm using a 64
machine for development.
If you get warning, could you specify the openser version and the list
of warnings?

and, the code is compiled with -Wall... can you post your compiling line

Regards,
bogdan

Eliot Gable wrote:

Throughout a lot of the code, I have found many instances where memory
locations are being cast to unsigned int or unsigned long (and even
sometimes just int) instead of size_t. This can cause a problem on
64-bit systems due to memory addresses being 64-bit as opposed to 32-

bit

like unsigned int or int. If you use size_t, it correctly uses 64-bit

on

64-bit systems and 32-bit on 32-bit systems. But that means that in
str.h, the len parameter must be size_t instead of int to prevent
warnings about signedness.

Also, is there any reason the code is not being compiled globally with
-Wall? It catches a lot of these issues for you.

Eliot Gable
Operations Engineer
CCNA, CWNA, CWSP, Network+, Security+
Broadvox, LLC
1228 Euclid Avenue
Suite 390
Cleveland, OH 44115-1800
216-373-4808



_______________________________________________
Devel mailing list
Devel@openser.org
http://openser.org/cgi-bin/mailman/listinfo/devel



_______________________________________________
Devel mailing list
Devel@openser.org
http://openser.org/cgi-bin/mailman/listinfo/devel



_______________________________________________
Devel mailing list
Devel@openser.org
http://openser.org/cgi-bin/mailman/listinfo/devel

Re: [Devel] 64-bit compatability

Reply via email to