Mysterious pointer mixed with array indices bug

[David Mathog]

This one is driving me slightly batty. The code in question is
buried deep in somebody else's massive package but it boils down to
this, two pointers are declared, the first is:

char **resname

which is part of the "atoms" structure that is passed into the function
from the outside. I have not yet found where it is allocated but I'm
reasonably sure from other chunks of this code that it was by:

atoms->resname=malloc(sizeof(char**)*SOMENUMBER);

Within my function these are found:

char **pnew_res_names;

pnew_res_names=malloc(sizeof(char*)*10);
/* char ***, pointers to pointers to pointers to the name strings */
(void) fprintf(stderr,"0e: %8x\n",(void *)atoms->resname);
/* char **, pointers to pointers to the name strings */
(void) fprintf(stderr,"0f: %8x\n",(void *)(atoms->resname[0]));
(void) fprintf(stderr,"1f: %8x\n",(void *)(atoms->resname[1]));
(void) fprintf(stderr,"2f: %8x\n",(void *)(atoms->resname[2]));
/* char *, pointers to the first characters in the name strings */
(void) fprintf(stderr,"0g: %8x\n",(void *) *(atoms->resname[0]));
(void) fprintf(stderr,"1g: %8x\n",(void *) *(atoms->resname[1]));
(void) fprintf(stderr,"2g: %8x\n",(void *) *(atoms->resname[2]));

atoms->resname= (char ***) &pnew_res_names;
/* the explict cast doesn't seem to make any difference */

/* char **, pointers to pointers to the name strings */
(void) fprintf(stderr,"-1a: %8x\n",(void *) pnew_res_names);
/* char *, pointers to the first characters in the name strings */
(void) fprintf(stderr,"0a: %8x\n",(void *)pnew_res_names[0]);
(void) fprintf(stderr,"1a: %8x\n",(void *)pnew_res_names[1]);
(void) fprintf(stderr,"2a: %8x\n",(void *)pnew_res_names[2]);
/* char **, pointers to pointers to the name strings */
(void) fprintf(stderr,"0b: %8x\n",(void *)(atoms->resname[0]));
(void) fprintf(stderr,"1b: %8x\n",(void *)(atoms->resname[1]));
(void) fprintf(stderr,"2b: %8x\n",(void *)(atoms->resname[2]));
/* char *, pointers to the first characters in the name strings */
(void) fprintf(stderr,"0c: %8x\n",(void *) *(atoms->resname[0]));
(void) fprintf(stderr,"1c: %8x\n",(void *) *(atoms->resname[1]));
(void) fprintf(stderr,"2c: %8x\n",(void *) *(atoms->resname[2]));

When this code runs it emits this:

0e: 804c110 #so the original atoms->resname can be accessed by
0f: 804a6e8 #indices
1f: 804a750
2f: 804a75c
0g: 804b6e8
1g: 804b888
2g: 804b8b8

-1a: 80e98e0 #location of pnew_res_names
0a: 804b6e8 #first points to first string
1a: 804b6f8 #second points to second string
2a: 804b708 #third points to third string
0b: 80e98e0 #atoms->resname[0] points to first spot in pnew_res_names
1b: 0 #second is broken, no pointer to 2nd string, but see 1a
2b: 0 #third is broken, no pointer to 3rd string, but see 2a
0c: 804b6e8 #correctly points to first string
segfaults when it tries to reference address 0.

In other words, the chunk of memory that was originally allocated
and passed in as atoms->resname can be accessed by indices
properly. The newly allocated region can also be accessed
when referenced from pnew_res_names. However, it cannot be
accessed via indices through atoms->resname after that is set
to point to the memory block. The first value works
properly, but the second and subsequent ones do not.

All (reasonable) accesses to atoms->resname work before the memory
block is reassigned. All (reasonable) accesses to pnew_res_names
work before and after the memory block is reassigned. But mixed
pointer/index accesses to the data in pnew_res_names does not work
once it is linked to atoms->resname.

Why? Or more specifically, is there some magical cast that can
be used instead of (char ***) to make this work?

Thanks,

David Mathog
ude TOD hcetlac TA gohtam

[Eric Sosman]

David Mathog wrote:

This one is driving me slightly batty. The code in question is
buried deep in somebody else's massive package but it boils down to
this, two pointers are declared, the first is:

char **resname

which is part of the "atoms" structure that is passed into the function
from the outside. I have not yet found where it is allocated but I'm
reasonably sure from other chunks of this code that it was by:

atoms->resname=malloc(sizeof(char**)*SOMENUMBER);
This is just the first of several type errors -- or maybe
not, because you say this particular allocation is a matter of
hypothesis and not observation. Observe that sizeof(char**)
ought to be sizeof(char*), and the two might be different on
some machines -- word-addressed machines, for example. This
kind of confusion is one reason why the formulation

atoms->resname = malloc(SOMENUMBER * sizeof *atoms->resname)

is generally favored on c.l.c.

But this may not be a problem, since you don't actually know
that the storage is allocated in this erroneous manner.
Within my function these are found:

char **pnew_res_names;

pnew_res_names=malloc(sizeof(char*)*10);
This one is right, although again the c.l.c.-favored formula
would be less error-prone. Note that the newly-allocated storage
has indeterminate content; it is not initialized to anything in
particular. (This will become important later on.)
/* char ***, pointers to pointers to pointers to the name strings */
(void) fprintf(stderr,"0e: %8x\n",(void *)atoms->resname);
Here and below, some variant of "%p" would be preferable to
"%x". The latter handles integers, which a `void*' is not. Also,
the comment is wrong: `atoms->resname' is a `char**', not a `char***'
(or so you said at the beginning).
/* char **, pointers to pointers to the name strings */
(void) fprintf(stderr,"0f: %8x\n",(void *)(atoms->resname[0]));
(void) fprintf(stderr,"1f: %8x\n",(void *)(atoms->resname[1]));
(void) fprintf(stderr,"2f: %8x\n",(void *)(atoms->resname[2]));
/* char *, pointers to the first characters in the name strings */
(void) fprintf(stderr,"0g: %8x\n",(void *) *(atoms->resname[0]));
These three are bogus. You've started with a `char**' and
done two levels of indirection, so what you get is a plain `char'.
Converting that `char' to a `void*' is suspect, to say the least.

Then again, you may have become lost in the thicket of types.
Perhaps the comment is correct and your original statement about
the type of `atoms->resname' is wrong. If `atoms->resname' is
actually a `char***', then `*(atoms->resname[0])' is indeed a
`char*' and not a `char', and this almost makes sense. (Of course,
the dubious "%x" is still a potential problem.)
(void) fprintf(stderr,"1g: %8x\n",(void *) *(atoms->resname[1]));
(void) fprintf(stderr,"2g: %8x\n",(void *) *(atoms->resname[2]));

atoms->resname= (char ***) &pnew_res_names;
/* the explict cast doesn't seem to make any difference */
If `atoms->resname' is a `char**' as you say, the compiler
should issue a diagnostic for the type mismatch here.
/* char **, pointers to pointers to the name strings */
(void) fprintf(stderr,"-1a: %8x\n",(void *) pnew_res_names);
Semi-legitimate, the "%x" being the only problem I see.
/* char *, pointers to the first characters in the name strings */
(void) fprintf(stderr,"0a: %8x\n",(void *)pnew_res_names[0]);
Remember what I said above about indeterminate values? The
variable `pnew_res_names' points to some memory (assuming malloc()
succeeded), but the contents of that memory are "random garbage."
In other words, this call prints random garbage (if it prints
anything at all; the use of an indeterminate value produces
undefined behavior).
(void) fprintf(stderr,"1a: %8x\n",(void *)pnew_res_names[1]);
(void) fprintf(stderr,"2a: %8x\n",(void *)pnew_res_names[2]);
/* char **, pointers to pointers to the name strings */
(void) fprintf(stderr,"0b: %8x\n",(void *)(atoms->resname[0]));
Here you imitate Wile E. Coyote by running off the edge of
a cliff in hot pursuit of Roadrunner. `pnew_res_names' points to
memory filled with random garbage, so here you fetch some of that
garbage and try to use it as the address of another piece of memory
which you try to fetch and print. All Hell breaks loose.
(void) fprintf(stderr,"1b: %8x\n",(void *)(atoms->resname[1]));
(void) fprintf(stderr,"2b: %8x\n",(void *)(atoms->resname[2]));
/* char *, pointers to the first characters in the name strings */
(void) fprintf(stderr,"0c: %8x\n",(void *) *(atoms->resname[0]));
(void) fprintf(stderr,"1c: %8x\n",(void *) *(atoms->resname[1]));
(void) fprintf(stderr,"2c: %8x\n",(void *) *(atoms->resname[2]));

When this code runs it emits this:

0e: 804c110 #so the original atoms->resname can be accessed by
0f: 804a6e8 #indices
1f: 804a750
2f: 804a75c
0g: 804b6e8
1g: 804b888
2g: 804b8b8

-1a: 80e98e0 #location of pnew_res_names
0a: 804b6e8 #first points to first string
1a: 804b6f8 #second points to second string
2a: 804b708 #third points to third string
0b: 80e98e0 #atoms->resname[0] points to first spot in pnew_res_names
1b: 0 #second is broken, no pointer to 2nd string, but see 1a
2b: 0 #third is broken, no pointer to 3rd string, but see 2a
0c: 804b6e8 #correctly points to first string
segfaults when it tries to reference address 0.

In other words, the chunk of memory that was originally allocated
and passed in as atoms->resname can be accessed by indices
properly. The newly allocated region can also be accessed
when referenced from pnew_res_names. However, it cannot be
accessed via indices through atoms->resname after that is set
to point to the memory block. The first value works
properly, but the second and subsequent ones do not.
Presumably because no values have been stored in that block
of memory. Garbage in, garbage out.
All (reasonable) accesses to atoms->resname work before the memory
block is reassigned. All (reasonable) accesses to pnew_res_names
work before and after the memory block is reassigned. But mixed
pointer/index accesses to the data in pnew_res_names does not work
once it is linked to atoms->resname.
The memory block is not "reassigned," nor has any data been
placed in the memory addressed by `pnew_res_names', nor is that
memory somehow "linked" to the memory formerly addressed by
`atoms->resname'. You start with `atoms->resnames' pointing to
a chunk of memory we'll call Area A, filled with sensible data
(or so we imagine). You allocate another chunk of memory we'll
call Area 51, and cause `pnew_res_names' to point to it; this
memory is filled with random garbage. Then you change the
`atoms->resnames' pointer so it now points to Area 51 -- nothing
has happened to either memory block, and no data has been magically
Why? Or more specifically, is there some magical cast that can
be used instead of (char ***) to make this work?

As far as I can see, magic is your only hope ;-) You've
got some code that (1) gets itself all snarled in a tangle of
types and can't keep track of what's what, and (2) mixes up the
pointer and the pointee and thus winds up reading uninitialized
memory. To fix (1) you might introducing a few typedefs to hide
some of the levels of pointer indirection -- I am usually averse
to typedef'fing aliases for pointer types, but when you go deeper
than two asterisks (and show evidence of confusion) it may be
justifiable. To fix (2) you need to step back and consider what
this newly-allocated memory block is supposed to do, what values
it is supposed to hold, and where it's supposed to get them.

--
Eric Sosman
es*****@acm-dot-org.invalid

[Robin Haigh]

"David Mathog" <ma****@caltech.edu> wrote in message
news:dv**********@naig.caltech.edu...

This one is driving me slightly batty. The code in question is
buried deep in somebody else's massive package but it boils down to
this, two pointers are declared, the first is:

char **resname

This all makes more sense if that should be char ***resname. I think you're
trying to treat the structure as an "array of strings" (speaking loosely),
when it's actually a whole array of arrays of strings. The result is, the
code goes looking for an array where you've only created the first element
of it, hence you get zeroes when you look where the 2nd and 3rd elements are
expected to be.

which is part of the "atoms" structure that is passed into the function
from the outside. I have not yet found where it is allocated but I'm
reasonably sure from other chunks of this code that it was by:

atoms->resname=malloc(sizeof(char**)*SOMENUMBER);

Within my function these are found:

char **pnew_res_names;

pnew_res_names=malloc(sizeof(char*)*10);
/* char ***, pointers to pointers to pointers to the name strings */
(void) fprintf(stderr,"0e: %8x\n",(void *)atoms->resname);
/* char **, pointers to pointers to the name strings */
(void) fprintf(stderr,"0f: %8x\n",(void *)(atoms->resname[0]));
(void) fprintf(stderr,"1f: %8x\n",(void *)(atoms->resname[1]));
(void) fprintf(stderr,"2f: %8x\n",(void *)(atoms->resname[2]));
/* char *, pointers to the first characters in the name strings */
(void) fprintf(stderr,"0g: %8x\n",(void *) *(atoms->resname[0]));
(void) fprintf(stderr,"1g: %8x\n",(void *) *(atoms->resname[1]));
(void) fprintf(stderr,"2g: %8x\n",(void *) *(atoms->resname[2]));

atoms->resname= (char ***) &pnew_res_names;
/* the explict cast doesn't seem to make any difference */

/* char **, pointers to pointers to the name strings */
(void) fprintf(stderr,"-1a: %8x\n",(void *) pnew_res_names);
/* char *, pointers to the first characters in the name strings */
(void) fprintf(stderr,"0a: %8x\n",(void *)pnew_res_names[0]);
(void) fprintf(stderr,"1a: %8x\n",(void *)pnew_res_names[1]);
(void) fprintf(stderr,"2a: %8x\n",(void *)pnew_res_names[2]);
/* char **, pointers to pointers to the name strings */
(void) fprintf(stderr,"0b: %8x\n",(void *)(atoms->resname[0]));
(void) fprintf(stderr,"1b: %8x\n",(void *)(atoms->resname[1]));
(void) fprintf(stderr,"2b: %8x\n",(void *)(atoms->resname[2]));
/* char *, pointers to the first characters in the name strings */
(void) fprintf(stderr,"0c: %8x\n",(void *) *(atoms->resname[0]));
(void) fprintf(stderr,"1c: %8x\n",(void *) *(atoms->resname[1]));
(void) fprintf(stderr,"2c: %8x\n",(void *) *(atoms->resname[2]));

[snip]

--
RSH

[David Mathog]

Robin Haigh wrote:

"David Mathog" <ma****@caltech.edu> wrote in message
news:dv**********@naig.caltech.edu...

This one is driving me slightly batty. The code in question is
buried deep in somebody else's massive package but it boils down to
this, two pointers are declared, the first is:

char **resname

This all makes more sense if that should be char ***resname. I think you're
trying to treat the structure as an "array of strings" (speaking loosely),
when it's actually a whole array of arrays of strings. The result is, the
code goes looking for an array where you've only created the first element
of it, hence you get zeroes when you look where the 2nd and 3rd elements are
expected to be.

I figured out where I was going wrong. The program works like this:

storage area 1: N1 chars (actual names as null terminated strings)
storage area 2: N2 char * pointers to strings in area 1
storage area 3: N3 char ** pointers to pointers in storage area 2

My resname variable was working as storage area 2 but then I was
attempting to assign it to storage area 3. The compiler didn't
complain (oddly) but the memory access to make it work would have been:

(*(atoms->resname))[i]

instead of

*(atoms->resname[i])

which is what is used elsewhere in the program. The thing that
threw me off the most was that there is no explicit pointer to
storage area 2 in the data structure. If that value is kept around for
a future free() or realloc() it must be in some other data structure.

Thanks for everybody's responses.

David Mathog
ude TOD hcetlac TA gohtam