This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix obscure obstack bug


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]