[cfe-commits] r56818 - in /cfe/trunk/lib/Lex: LiteralSupport.cpp PPMacroExpansion.cpp

[Chris Lattner]

On Sep 29, 2008, at 8:00 PM, Eli Friedman wrote:

> On Mon, Sep 29, 2008 at 4:12 PM, Chris Lattner <sabre at nondot.org>  
> wrote:
>> Author: lattner
>> Date: Mon Sep 29 18:12:31 2008
>> New Revision: 56818
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=56818&view=rev
>> Log:
>> Fix the root cause of PR2750 instead of the side effect.
>>
>> NumericLiteral parser is not careful about overrun because
>> it should never be possible.
>
> I'm not very confident in the whole thing, but if you think it works,
> that's fine.
>
> It looks like the tokens in question always include some sort of
> terminator; the natural terminator when the token points to the
> source, a space for artificial numeric macros (with this patch), and
> null for tokens created through token pasting (which looks kind of
> suspicious...).  Is there some sort of guarantee about the token
> including some sort of separator after the token?

Yep, that's the intention.  Just to make it very clear, I added an  
assertion to NumericLiteralParser.  It seems useful to document and  
verify assumptions like this :).

> Wait, so if I'm reading this correctly, it doesn't stop the overrun at
> all; is the idea that it's supposed to be legal to overrun the buffer
> by one?

Yep, the character one past the end of the lexed number must be valid  
(this is used elsewhere in the lexer) and must not be a continuation  
of a pp-number.

>  This seems like a bad way to fix this... it seems really
> fragile.

Why?  The lexer as a whole depends on this in many cases, this is why  
it require all file buffers to end with a NUL character (to allow and  
fastpath overrun).

> There's some code which appears to be working completely by accident
> in Sema::ActOnNumericConstant; if PP.getSpelling actually uses the
> buffer (e.g. there's an escaped newline in the number), an overrun
> reads random data from the stack.  Or rather, it would, except that
> SmallVector initializes the relevant part of the buffer to zero.

Wow, great catch!  Fixed (twice):
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20080929/007860.html
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20080929/007861.html

-Chris