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

[Eli Friedman]

On Mon, Sep 29, 2008 at 4:12 PM, Chris Lattner <sabre at nondot.org> wrote:
>   if (II == Ident__LINE__) {
> -    // __LINE__ expands to a simple numeric value.
> -    sprintf(TmpBuffer, "%u", SourceMgr.getLogicalLineNumber(Tok.getLocation()));
> -    unsigned Length = strlen(TmpBuffer);
> +    // __LINE__ expands to a simple numeric value.  Add a space after it so that
> +    // it will tokenize as a number (and not run into stuff after it in the temp
> +    // buffer).
> +    sprintf(TmpBuffer, "%u ",
> +            SourceMgr.getLogicalLineNumber(Tok.getLocation()));
> +    unsigned Length = strlen(TmpBuffer)-1;
>     Tok.setKind(tok::numeric_constant);
>     Tok.setLength(Length);
> -    Tok.setLocation(CreateString(TmpBuffer, Length, Tok.getLocation()));
> +    Tok.setLocation(CreateString(TmpBuffer, Length+1, Tok.getLocation()));
>   } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__) {
>     SourceLocation Loc = Tok.getLocation();
>     if (II == Ident__BASE_FILE__) {

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?  This seems like a bad way to fix this... it seems really
fragile.

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.

-Eli