Fix obscure obstack bug
Neil Booth
neil@daikokuya.demon.co.uk
Fri Dec 14 16:35:00 GMT 2001
Ulrich Drepper wrote:-
> I finally looked at this
Thanks.
> and don't see anthing wrong with the current code. Instead, your test
> program is faulty:
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> void *
> my_alloc (size_t size)
> {
> /* Grow the chunks up to a point. */
> #if SHOW_BUG
> if (obstack_chunk_size (&ob) < 100 * 1024)
> obstack_chunk_size (&ob) += 4 * 1024;
> #endif
> return malloc (size);
> }
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> You increase the chunk size but don't allocate enough memory.
Huh? I was asked to allocate "size" bytes and I allocated it.
> You want to compensate for this by changing the obstack code to not
> look at the current chunk size and instead use what was previously
> computed. That's wrong and can break existing code.
Would you care to give an example of this "existing code" that you
claim? Any such code must have encountered the obstack implementation
bug I described, and thus either a) implement a work around, or b)
itself be buggy.
> If you want to fuzz around with the chunk size change the last line of
> your function to
>
> return malloc (obstack_chunk_size (&ob));
>
> This way you can control the chunk size and the changes are
> automatically picked up in the obstack functions. With your patch
> applied this wouldn't be the case.
LOL! You are suggesting that
1) An allocation function that is requested to allocate SIZE bytes
allocates something other than SIZE bytes. Why have it take an
SIZE argument at all?
2) That user code have intimate knowledge of the obstack implementation
and its bugs, and take necessary evasive action.
3) That changes to the chunk size must be monotonically increasing,
since decreasing them would result in a segfault because,
surprise!, the function is expected to allocate SIZE bytes after
all, as evidenced by e.g. lines 257-263 that read
chunk = h->chunk = CALL_CHUNKFUN (h, h -> chunk_size);
... ^^^^^^^^^^^^^^^
h->chunk_limit = chunk->limit
= (char *) chunk + h->chunk_size;
^^^^^^^^^^^^^
4) That the contrast between lines
295: new_chunk = CALL_CHUNKFUN (h, new_size);
... ^^^^^^^^
300: new_chunk->limit = h->chunk_limit = (char *) new_chunk + new_size;
^^^^^^^^
and
197: chunk = h->chunk = CALL_CHUNKFUN (h, h -> chunk_size);
.... ^^^^^^^^^^^^^^^
202: h->chunk_limit = chunk->limit
= (char *) chunk + h->chunk_size;
^^^^^^^^^^^^^
is merely a figment of my imagination. Note that one or the other
(depending upon the relative size of the new h->chunk_size to the
old) of these lines will always buffer overflow your suggested
implementation when the allocated chunk is filled up.
5) That the fact that none of the above is documented in the obstack
documentation is an unfortunate oversight.
If I'd posted your drivel to the glibc lists you'd have justifiably
flamed me. In fact, I did a double take that it was really you
who sent the mail.
I trust you will do the right thing and apply my simple patch to fix
the obstack implementation bug that I have clearly demonstrated.
Neil.
More information about the Libc-alpha
mailing list