Hi,

In a memory manager i wrote, i get corruption somewhere from
newly malloced (gnu malloc) memory overwriting existing heap variables.

I have:

----------------------------------------------------------------------------
#include<stddef.h>

#define DATA_PTR_FROM_BLOB(blob)    ((void*)(blob) + offsetof(Blob, data))

#define BLOB_FROM_DATA_PTR(ptr)    ((void*)(ptr) - offsetof(Blob, data))

typedef struct Blob {
int magic;
size_t payloadsize;
unsigned char data[0];
struct Blob *next, *prev; // overwritten by payload starting at data[0]
} Blob;
----------------------------------------------------------------------------

Macro offsetof() comes from stddef.h (gcc 4.0.3 on debian linux).

I've been doing:

struct Blob ablob;
..
void *dataptr = DATA_PTR_FROM_BLOB(ablob);
..
Blob *blobheader = BLOB_FROM_DATA_PTR(dataptr);
..

Will this have alignment problems with gaps in the Blob struct?

[ Post a follow-up to this message ]

Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:

> In a memory manager i wrote, i get corruption somewhere from
> newly malloced (gnu malloc) memory overwriting existing heap variables.

If the corruption is reproducible, the easiest way to catch it is
with data breakpoint. On P3 and above, gdb implements data
breakpoints via (hardware) debug registers, and they are very fast.

> #define DATA_PTR_FROM_BLOB(blob)    ((void*)(blob) + offsetof(Blob, data))
> #define BLOB_FROM_DATA_PTR(ptr)    ((void*)(ptr) - offsetof(Blob, data))

Pointer arithmetic on 'void *' is non-portable.
Quick, which of the two expressions below is true?

((void *)p + 1) == ((char *)p + 1)
((void *)p + 1) == ((int *)p + 1)

>    Blob *blobheader = BLOB_FROM_DATA_PTR(dataptr);
>
> Will this have alignment problems with gaps in the Blob struct?

No. The offsetof() takes care of accounting for them.

Cheers,
--
In order to understand recursion you must first understand recursion.
Remove /-nsp/ for email.

[ Post a follow-up to this message ]

Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:

> In a memory manager i wrote, i get corruption somewhere from
> newly malloced (gnu malloc) memory overwriting existing heap variables.
>
> typedef struct Blob {
>     int magic;
>     size_t payloadsize;
>     unsigned char data[0];
>     struct Blob *next, *prev; // overwritten by payload starting at data&#
91;0]
> } Blob;

You declared data[] as having 0 elements.  Thus, it overlaps with
next and prev.  (Is this really a surprise?)

ANSI C doesn't allow 0-element arrays.  You must be using some
compiler extension.

I recommend moving data[] to the end of the struct.
--
Ben Pfaff
email: blp@cs.stanford.edu
web: http://benpfaff.org

[ Post a follow-up to this message ]

Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
> In a memory manager i wrote, i get corruption somewhere from
> newly malloced (gnu malloc) memory overwriting existing heap variables.
>
> I have:
>
> --------------------------------------------------------------------------
--
> #include<stddef.h>
>
> #define DATA_PTR_FROM_BLOB(blob)    ((void*)(blob) + offsetof(Blob, data))
>
> #define BLOB_FROM_DATA_PTR(ptr)    ((void*)(ptr) - offsetof(Blob, data))
>
> typedef struct Blob {
>      int magic;
>      size_t payloadsize;
>      unsigned char data[0];
>      struct Blob *next, *prev; // overwritten by payload starting at data&
#91;0]
> } Blob;
> --------------------------------------------------------------------------
--
>
> Macro offsetof() comes from stddef.h (gcc 4.0.3 on debian linux).
>
> I've been doing:
>
>    struct Blob ablob;
>    ...
>    void *dataptr = DATA_PTR_FROM_BLOB(ablob);
>    ...
>    Blob *blobheader = BLOB_FROM_DATA_PTR(dataptr);
>    ...
>
> Will this have alignment problems with gaps in the Blob struct?

The use of a zero-element array is not allowed in standard C.  There's
a more nearly conforming version declares a 1-element array, but
allocates more memory so elements past the end of the array can be
accessed; this is known as the "struct hack".  See
<http://www.eskimo.com/~scs/C-faq/q2.6.html> for more information.

But the array must be the last declared member of the struct;
otherwise it will write over any following members.

C99 adds, and I'm fairly sure gcc supports, a conforming version of
the struct hack known as the "flexible array member".  The declaration
uses empty brackets rather than the fictitious [0] or [1]:
unsigned char data[];
Search the gcc documentation, or a recent C reference, for "flexible
array member".

--
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
San Diego Supercomputer Center             <*>  <http://users.sdsc.edu/~kst>
We must do something.  This is something.  Therefore, we must do this.

[ Post a follow-up to this message ]

On Tue, 15 Nov 2005 03:31:52 +1100
Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
[vbcol=seagreen]
> typedef struct Blob {
>      int magic;
>      size_t payloadsize;
>      unsigned char data[0];
>      struct Blob *next, *prev; // overwritten by payload starting at data[0][/
vbcol]

Well yes and if the payload size is greater than the size of the
two pointers something else is going to get overwritten and you have no
idea what that something else will be.

--
C:>WIN                                      |   Directable Mirror Arrays
The computer obeys and wins.                | A better way to focus the sun
You lose and Bill collects.                 |    licences available see
|    http://www.sohara.org/

[ Post a follow-up to this message ]

Russell Shaw wrote:
> Hi,
>
> In a memory manager i wrote, i get corruption somewhere from
> newly malloced (gnu malloc) memory overwriting existing heap variables.
>
> I have:
>
> --------------------------------------------------------------------------
--
> #include<stddef.h>
>
> #define DATA_PTR_FROM_BLOB(blob)    ((void*)(blob) + offsetof(Blob, data))
>
> #define BLOB_FROM_DATA_PTR(ptr)    ((void*)(ptr) - offsetof(Blob, data))
>
> typedef struct Blob {
>      int magic;
>      size_t payloadsize;
>      unsigned char data[0];
>      struct Blob *next, *prev; // overwritten by payload starting at data&
#91;0]
> } Blob;
> --------------------------------------------------------------------------
--
>
> Macro offsetof() comes from stddef.h (gcc 4.0.3 on debian linux).
>
> I've been doing:
>
>    struct Blob ablob;
>    ...
>    void *dataptr = DATA_PTR_FROM_BLOB(ablob);
>    ...
>    Blob *blobheader = BLOB_FROM_DATA_PTR(dataptr);
>    ...
>
> Will this have alignment problems with gaps in the Blob struct?

Others have already explained this, but just to make it clear I think
what you want is this:

typedef struct Blob {
int magic;
size_t payloadsize;
struct Blob *next, *prev;
unsigned char data[0];
} Blob;

Happened to me once when I had to add a member to my struct. Always
declare stuff BEFORE the data section. Otherwise the member variable
and data will corrupt each other.

[ Post a follow-up to this message ]

"slebetman@yahoo.com" <slebetman@gmail.com> writes:

> Others have already explained this, but just to make it clear I think
> what you want is this:

He probably doesn't. When writing a memory manager, it is a *very*
common technique to have a "Blob" either allocated, or on the
free-list.

Thus it makes sense to share the storage for forward/back links and
the user data, because either one or the other is used at any given
moment, but never both at the same time.

I think the OP did this *intentionally* (and provided a comment
stating this intent), and everybody who pointed out that data[]
would overwrite the links missed the point. But only the OP can
say for sure.

Cheers,
--
In order to understand recursion you must first understand recursion.
Remove /-nsp/ for email.

[ Post a follow-up to this message ]

Ben Pfaff wrote:
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
> 
>
> You declared data[] as having 0 elements.  Thus, it overlaps with
> next and prev.  (Is this really a surprise?)
>
> ANSI C doesn't allow 0-element arrays.  You must be using some
> compiler extension.

I'm using "data" as an empty marker for the first free memory location follo
wing
that point. Anything written starting at "data" will overwrite next/prev,
and extend outside the struct. It's a bit like using alloca(0), but i'm doin
g
it on the heap.

> I recommend moving data[] to the end of the struct.

[ Post a follow-up to this message ]

Keith Thompson wrote:
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
> 
>
> The use of a zero-element array is not allowed in standard C.  There's
> a more nearly conforming version declares a 1-element array, but
> allocates more memory so elements past the end of the array can be
> accessed; this is known as the "struct hack".  See
> <http://www.eskimo.com/~scs/C-faq/q2.6.html> for more information.

I'm using a gcc extension for zero length arrays.

> But the array must be the last declared member of the struct;
> otherwise it will write over any following members.

When Blob is taken out of storage for use, user data starts at "data",
overwriting next/prev pointers. Those pointers are only used when the
blob is back in the pool, so 8 bytes are gained for free during use.

> C99 adds, and I'm fairly sure gcc supports, a conforming version of
> the struct hack known as the "flexible array member".  The declaration
> uses empty brackets rather than the fictitious [0] or [1]:
>     unsigned char data[];
> Search the gcc documentation, or a recent C reference, for "flexible
> array member".

5.11 in gcc-4.0 info explains it. Unfortunately, it says:

* Flexible array members may only appear as the last member of a
`struct' that is otherwise non-empty.

[ Post a follow-up to this message ]

Ben Pfaff wrote:
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
> 
>
> You declared data[] as having 0 elements.  Thus, it overlaps with
> next and prev.  (Is this really a surprise?)
>
> ANSI C doesn't allow 0-element arrays.  You must be using some
> compiler extension.

I'm using gcc-4. I don't see why they should disallow empty array decs.,
because it's good for marking a memory location used for variable size
storage.

> I recommend moving data[] to the end of the struct.

[ Post a follow-up to this message ]