Re: [PATCH] Fix PR preprocessor/15167

[Zack Weinberg <zack at codesourcery dot com>]

Eric Botcazou <ebotcazou@libertysurf.fr> writes:

>> The bug you've found is just the tip of the iceberg, though.  Calling
>> read_file from this loop, for a file which is already on the stack,
>> clobbers the buffer for that file - which must not happen, because
>> that buffer is still being used by the lexer.  I suspect that we get
>> away with it because the block of memory holding the file contents is
>> not freed, and the lexer doesn't ever refer to file->buffer.  So it's
>> "just" a memory leak.  I think.
>
> Could you elaborate a little on the "clobber"?  Do you mean that, because 
> buffer_valid is cleared in _cpp_stack_file, read_file allocates a new buffer 
> even though the previous one is still alive?

Yes.

> If so, I agree.  However, I think the bug I'm trying to fix is orthogonal to 
> this and I can't think of any reason why clearing buffer_valid in 
> _cpp_pop_file_buffer can be wrong.

Because it is a data structure invariant that buffer_valid becomes
false on _cpp_stack_file and stays that way.  Clearing it in
_cpp_pop_file_buffer papers over bugs.

>> You can fix _this_ bug properly, I think, by splitting out the body of
>> that loop to a separate function that reads the file into a private
>> block of memory (*not* using read_file_guts - copy the inner loop) and
>> does the comparison from there.  I suspect we are gonna be in just as
>> much trouble from plain old recursive #include of a header with no
>> multiple-include guards whatsoever - but that's a very rare case, so
>> let's not worry about it now.
>
> OK, thanks for the tip.  But re-reading the pragma-onced each time could be 
> unnecessarily costly though.  Should I add another buffer+flag to _cpp_file 
> and use them in the new comparison function?

I really don't care how expensive #pragma once is.

zw