Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > uninitialised value & valgrind

Reply
Thread Tools

uninitialised value & valgrind

 
 
Mark
Guest
Posts: n/a
 
      06-26-2012
Hello

I'm working with a legacy code that I link to my application, and run it
under 'valgrind' tool to catch memory leaks, corrupted memory etc. So here
is what it looks like:

#define VLOG_MSG_MAX_DATA_SIZE 1024
struct vlog_msg_header
{
u_int32_t vmh_vr_id;
u_int16_t vmh_msg_type;
module_id_t vmh_mod_id;
u_int32_t vmh_proc_id;
s_int8_t vmh_priority;
u_int32_t vmh_data_len;
};

struct vlog_msg
{
struct vlog_msg_header vms_msg_hdr;
char vms_msg_data[VLOG_MSG_MAX_DATA_SIZE+1];
};

int vlog_client_send_message (struct vlog_msg *msg)
{
...
write (sock, (void *) msg, msg_len); /* line 103. */
}

int vlog_client_send_ctrl_msg ()
{
struct vlog_msg_header header;

/* initialise 'header' .. */

vlog_client_send_message ((struct vlog_msg *)&header); /* line 41*/

return RES_OK;
}

Valgrind reports:

==1634== Syscall param write(buf) points to uninitialised byte(s)
==1634== at 0x6D2D22E: __write_nocancel (syscall-template.S:82)
==1634== by 0x8116E68: vlog_client_send_message (vlog_client.c:103)
==1634== by 0x8116EBF: vlog_client_send_ctrl_msg (vlog_client.c:41)
==1634== by 0x8116EE6: vlog_client_connect_cb (vlog_client.c:211)
==1634== by 0x80C9A3F: message_client_start (message.c:597)
==1634== by 0x8116BA8: vlog_client_start (vlog_client.c:294)
==1634== by 0x8116DA1: vlog_client_create (vlog_client.c:255)
==1634== by 0x80C8FF1: openzlog (log.c:116)
==1634== by 0x805AF95: ldp_start (ldp_main.c:85)
==1634== by 0x8049F39: main (ldp.c:167)
==1634== Address 0xfe541aee is on thread 1's stack
==1634== Uninitialised value was created by a stack allocation
==1634== at 0x8116E7F: vlog_client_send_ctrl_msg (vlog_client.c:32)

I assume valgrind complains because vlog_client_send_message() expects to
get object of type 'struct vlog_msg', but instead we pass 'struct
vlog_msg_header' and everything beyond it (what is supposed to be occupied
by 'char vms_msg_data[VLOG_MSG_MAX_DATA_SIZE+1]' is undefined. Does it sound
correct ?

My question is -- is it valid to typecast this way or it involves undefined
behaviours?
Thanks.

Mark


 
Reply With Quote
 
 
 
 
James Kuyper
Guest
Posts: n/a
 
      06-26-2012
On 06/26/2012 12:35 PM, Mark wrote:
> Hello
>
> I'm working with a legacy code that I link to my application, and run it
> under 'valgrind' tool to catch memory leaks, corrupted memory etc. So here
> is what it looks like:
>
> #define VLOG_MSG_MAX_DATA_SIZE 1024
> struct vlog_msg_header
> {
> u_int32_t vmh_vr_id;
> u_int16_t vmh_msg_type;
> module_id_t vmh_mod_id;
> u_int32_t vmh_proc_id;
> s_int8_t vmh_priority;
> u_int32_t vmh_data_len;
> };
>
> struct vlog_msg
> {
> struct vlog_msg_header vms_msg_hdr;
> char vms_msg_data[VLOG_MSG_MAX_DATA_SIZE+1];
> };
>
> int vlog_client_send_message (struct vlog_msg *msg)
> {
> ...
> write (sock, (void *) msg, msg_len); /* line 103. */
> }
>
> int vlog_client_send_ctrl_msg ()
> {
> struct vlog_msg_header header;
>
> /* initialise 'header' .. */
>
> vlog_client_send_message ((struct vlog_msg *)&header); /* line 41*/
>
> return RES_OK;
> }
>
> Valgrind reports:
>
> ==1634== Syscall param write(buf) points to uninitialised byte(s)
> ==1634== at 0x6D2D22E: __write_nocancel (syscall-template.S:82)
> ==1634== by 0x8116E68: vlog_client_send_message (vlog_client.c:103)
> ==1634== by 0x8116EBF: vlog_client_send_ctrl_msg (vlog_client.c:41)
> ==1634== by 0x8116EE6: vlog_client_connect_cb (vlog_client.c:211)
> ==1634== by 0x80C9A3F: message_client_start (message.c:597)
> ==1634== by 0x8116BA8: vlog_client_start (vlog_client.c:294)
> ==1634== by 0x8116DA1: vlog_client_create (vlog_client.c:255)
> ==1634== by 0x80C8FF1: openzlog (log.c:116)
> ==1634== by 0x805AF95: ldp_start (ldp_main.c:85)
> ==1634== by 0x8049F39: main (ldp.c:167)
> ==1634== Address 0xfe541aee is on thread 1's stack
> ==1634== Uninitialised value was created by a stack allocation
> ==1634== at 0x8116E7F: vlog_client_send_ctrl_msg (vlog_client.c:32)
>
> I assume valgrind complains because vlog_client_send_message() expects to
> get object of type 'struct vlog_msg', but instead we pass 'struct
> vlog_msg_header' and everything beyond it (what is supposed to be occupied
> by 'char vms_msg_data[VLOG_MSG_MAX_DATA_SIZE+1]' is undefined. Does it sound
> correct ?
>
> My question is -- is it valid to typecast this way or it involves undefined
> behaviours?


Your code has undefined behavior. vlog_client_send_msg() expects to
receive a pointer to an actual vlog_msg struct, and is entitled to
access msg->vms_msg_data, which in your case doesn't exist. This is
called "lying to the compiler", and behavior such as you describe is a
normal consequence of lying about such things.

The messages you're getting from valgrind suggest that something is
attempting to access the non-existent message data inside of __write().
You don't show us how msg_len is calculated. However, if it's greater
than sizeof(struct vlog_msg_header), that would explain the messages
from valgrind.

I'd recommend figuring out why msg_len Is greater than that limit - it
suggests a serious disconnect between what you think you're doing and
what you're actually doing. The most plausible kind of mistake would be
failing to set header.vmh_data_len to 0.
 
Reply With Quote
 
 
 
 
Mark
Guest
Posts: n/a
 
      06-26-2012

"James Kuyper" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
[skip]
> The messages you're getting from valgrind suggest that something is
> attempting to access the non-existent message data inside of __write().
> You don't show us how msg_len is calculated. However, if it's greater
> than sizeof(struct vlog_msg_header), that would explain the messages
> from valgrind.
>
> I'd recommend figuring out why msg_len Is greater than that limit - it
> suggests a serious disconnect between what you think you're doing and
> what you're actually doing. The most plausible kind of mistake would be
> failing to set header.vmh_data_len to 0.


#define VLOG_MSG_HDR_SIZE sizeof (struct vlog_msg_header)
int vlog_client_send_message (struct vlog_msg *msg)
{
u_int16_t msg_len = VLOG_MSG_HDR_SIZE + msg->vms_msg_hdr.vmh_data_len;
...
}

and this is how 'header' is initialised:

struct vlog_msg_header header;
...
header.vmh_msg_type = VLOG_MSG_TYPE_CTRL;
header.vmh_vr_id = 0;
header.vmh_mod_id = zg->protocol;
header.vmh_proc_id = pal_get_process_id();
header.vmh_priority = 0;
header.vmh_data_len = 0;
vlog_client_send_message (zg->vlog_clt, (struct vlog_msg *)&header);
...

So msg_len should be of valid length.

Mark


 
Reply With Quote
 
James Kuyper
Guest
Posts: n/a
 
      06-26-2012
On 06/26/2012 02:07 PM, Mark wrote:
> "James Kuyper" <(E-Mail Removed)> wrote in message
> news:(E-Mail Removed)...
> [skip]
>> The messages you're getting from valgrind suggest that something is
>> attempting to access the non-existent message data inside of __write().
>> You don't show us how msg_len is calculated. However, if it's greater
>> than sizeof(struct vlog_msg_header), that would explain the messages
>> from valgrind.
>>
>> I'd recommend figuring out why msg_len Is greater than that limit - it
>> suggests a serious disconnect between what you think you're doing and
>> what you're actually doing. The most plausible kind of mistake would be
>> failing to set header.vmh_data_len to 0.

>
> #define VLOG_MSG_HDR_SIZE sizeof (struct vlog_msg_header)
> int vlog_client_send_message (struct vlog_msg *msg)
> {
> u_int16_t msg_len = VLOG_MSG_HDR_SIZE + msg->vms_msg_hdr.vmh_data_len;
> ...
> }
>
> and this is how 'header' is initialised:
>
> struct vlog_msg_header header;
> ...
> header.vmh_msg_type = VLOG_MSG_TYPE_CTRL;
> header.vmh_vr_id = 0;
> header.vmh_mod_id = zg->protocol;
> header.vmh_proc_id = pal_get_process_id();
> header.vmh_priority = 0;
> header.vmh_data_len = 0;
> vlog_client_send_message (zg->vlog_clt, (struct vlog_msg *)&header);
> ...
>
> So msg_len should be of valid length.


I'd recommend checking to make sure that msg_len is still <= sizeof
(struct vlog_msg_header) at the point of the call to write(). It might
have changed, and if so, you need to find out why.

Another possibility is that struct vlog_msg_header may have padding
bytes. The fact that there's an s_int8_t member between two u_int32_t
members would generally result in padding on any system where u_int32_t
has a non-trivial alignment requirement. If that's the case, then those
padding bytes would be uninitialized by your code, but the call to
write() would attempt to write them anyway.

If you have the power to change the structure definitions, I recommend
putting the members in decreasing order of size; this will tend to
minimize the number of padding bytes needed. Note: you can safely
convert from a pointer to a struct and a pointer to the first member of
the struct, and vice versa. If any of the code that uses this struct
takes advantage of that fact, you can't move that member.

There's no way to guarantee that a struct has no padding. The only way
to guarantee initialization of the whole struct, including padding
bytes, is to give the struct object static or thread storage duration,
or to use memset() or something equivalent to memset..
 
Reply With Quote
 
Johann Klammer
Guest
Posts: n/a
 
      06-27-2012
Mark wrote:
> Hello
>
> I'm working with a legacy code that I link to my application, and run it
> under 'valgrind' tool to catch memory leaks, corrupted memory etc. So here
> is what it looks like:
>
> #define VLOG_MSG_MAX_DATA_SIZE 1024
> struct vlog_msg_header
> {
> u_int32_t vmh_vr_id;
> u_int16_t vmh_msg_type;
> module_id_t vmh_mod_id;
> u_int32_t vmh_proc_id;
> s_int8_t vmh_priority;
> u_int32_t vmh_data_len;
> };
>
> struct vlog_msg
> {
> struct vlog_msg_header vms_msg_hdr;
> char vms_msg_data[VLOG_MSG_MAX_DATA_SIZE+1];
> };
>
> int vlog_client_send_message (struct vlog_msg *msg)
> {
> ...
> write (sock, (void *) msg, msg_len); /* line 103. */

You may have to check the return value to make sure all data was sent.
If not, then you may have to resume. If you are using GNU libc, take a
look at TEMP_FAILURE_RETRY

nbytes = TEMP_FAILURE_RETRY (write (desc, buffer, count));

man 2 write sez:
[SNIP]
write() writes up to count bytes from the buffer pointed buf to the file
referred to by the file descriptor fd.

The number of bytes written may be less than count if, for
example, there is insufficient space on the underlying physical
medium, or the RLIMIT_FSIZE resource limit is encountered (see
setrlimit(2)), or the call was interrupted by a signal handler
after having written less than count bytes. (See also pipe(7).)
[SNIP]
> }
>
> int vlog_client_send_ctrl_msg ()
> {
> struct vlog_msg_header header;

You almost certainly need a struct vlog_msg here.


>
> /* initialise 'header' .. */
>
> vlog_client_send_message ((struct vlog_msg *)&header); /* line 41*/
>
> return RES_OK;
> }
>
> Valgrind reports:
>
> ==1634== Syscall param write(buf) points to uninitialised byte(s)
> ==1634== at 0x6D2D22E: __write_nocancel (syscall-template.S:82)
> ==1634== by 0x8116E68: vlog_client_send_message (vlog_client.c:103)
> ==1634== by 0x8116EBF: vlog_client_send_ctrl_msg (vlog_client.c:41)
> ==1634== by 0x8116EE6: vlog_client_connect_cb (vlog_client.c:211)
> ==1634== by 0x80C9A3F: message_client_start (message.c:597)
> ==1634== by 0x8116BA8: vlog_client_start (vlog_client.c:294)
> ==1634== by 0x8116DA1: vlog_client_create (vlog_client.c:255)
> ==1634== by 0x80C8FF1: openzlog (log.c:116)
> ==1634== by 0x805AF95: ldp_start (ldp_main.c:85)
> ==1634== by 0x8049F39: main (ldp.c:167)
> ==1634== Address 0xfe541aee is on thread 1's stack
> ==1634== Uninitialised value was created by a stack allocation
> ==1634== at 0x8116E7F: vlog_client_send_ctrl_msg (vlog_client.c:32)
>
> I assume valgrind complains because vlog_client_send_message() expects to
> get object of type 'struct vlog_msg', but instead we pass 'struct
> vlog_msg_header' and everything beyond it (what is supposed to be occupied
> by 'char vms_msg_data[VLOG_MSG_MAX_DATA_SIZE+1]' is undefined. Does it sound
> correct ?

Yes
>
> My question is -- is it valid to typecast this way or it involves undefined
> behaviours?

Yes... undef behaviour. Of course some smart ass programmer may have
relied on a specific stack layout, so it may have worked as long as no
one put another variable declaration after the header decl.
But I do not anderstand why you would want to send
VLOG_MSG_MAX_DATA_SIZE of uninitialized stack data around.
> Thanks.
>
> Mark
>
>


 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Use of uninitialised value, how to avoid? Justin C Perl Misc 6 07-23-2010 08:41 AM
use of uninitialised value in pattern match (m//) Jess Perl Misc 2 05-17-2006 06:08 AM
Uninitialised value error stumping me.. Len Perl Misc 3 02-02-2005 09:06 PM
value of an uninitialised variable Andy Fish XML 7 01-10-2005 04:37 PM
use of uninitialised value Cognition Peon Perl Misc 5 04-15-2004 08:29 AM



Advertisments