Skip to content

Parsing::SkipFloatNumber rejects exponents without an explicit sign (1e10), corrupting HLSL tokenization #804

Description

@killerdevildog

Summary

Parsing::SkipFloatNumber() in Common/interface/ParsingTools.hpp requires the exponent of a float literal to carry an explicit +/- sign. Literals such as 1e10, 6.02e23 and 2.0E8 — valid in C, C++, HLSL and GLSL — are therefore only partially consumed.

Because Parsing::Tokenize() (documented as tokenizing "using the C-language syntax") relies on this function, the HLSL tokenizer splits such a literal into a numeric constant plus a spurious identifier. This affects HLSL2GLSLConverterImpl and HLSLParsingTools, both of which tokenize user shader source.

The public helper CountFloatNumberChars() in Common/interface/StringTools.hpp returns wrong results for the same inputs.

Affected code

https://github.com/DiligentGraphics/DiligentCore/blob/master/Common/interface/ParsingTools.hpp#L364-L376

++c;
if (c == End || (*c != '+' && *c != '-'))
{
    // 10e&
    return Pos;
}

++c;
if (c == End || !IsNum(*c))
{
    // 10e+x
    return Pos;
}

The first check treats a missing sign the same as a malformed exponent and bails out, returning the position saved before the e. Per the C grammar the sign is optional (exponent-part: e sign_opt digit-sequence).

Reproduction 1 — SkipFloatNumber / CountFloatNumberChars

#include <cstring>
#include "Common/interface/ParsingTools.hpp"
using namespace Diligent;

size_t Skip(const char* s)
{
    const char* End = s + strlen(s);
    return static_cast<size_t>(Parsing::SkipFloatNumber(s, End) - s);
}
input expected actual consumed
1e10 4 1 1
1E10 4 1 1
1.5e10 6 3 1.5
2.0E8 5 3 2.0
1e10f 5 1 1
6.02e23 7 4 6.02
1e0 3 1 1
1e+10 5 5 1e+10 (OK)
1e-10 5 5 1e-10 (OK)

Reproduction 2 — HLSL tokenizer

Using the engine's own Parsing::HLSLTokenizer (Graphics/ShaderTools/src/HLSLTokenizer.cpp):

source: float k = 6.02e23;
  other            "float"
  Identifier       "k"
  Assignment       "="
  NumericConstant  "6.02"     <-- truncated
  Identifier       "e23"      <-- spurious identifier
  Semicolon        ";"

source: float x = 1e+10;      (signed exponent, for contrast)
  NumericConstant  "1e+10"    <-- correct

Impact

The token stream handed to the HLSL→GLSL converter is corrupted for any shader using unsigned exponent notation. One concrete consequence: HLSL2GLSLConverterImpl.cpp strips the f/F suffix from numeric constants specifically to avoid GLSL compiler warnings —

https://github.com/DiligentGraphics/DiligentCore/blob/master/Graphics/HLSL2GLSLConverterLib/src/HLSL2GLSLConverterImpl.cpp#L4590-L4597

case TokenType::NumericConstant:
    // This all work is only required because some GLSL compilers are so stupid that
    // flood shader output with insane warnings like this:
    // WARNING: 0:259: Only GLSL version > 110 allows postfix "F" or "f" for float
    if (Token->Literal.back() == 'f' || Token->Literal.back() == 'F')
        Token->Literal.pop_back();

For 1e10f the suffix ends up on the spurious Identifier token (e10f), not on the NumericConstant (1), so the stripping is silently bypassed and the warning this code exists to prevent reappears. More generally, injecting an unexpected identifier into the stream of a source-to-source translator is a latent hazard for the identifier-driven transforms around it.

This has likely gone unnoticed because the notation is consistently avoided in-tree. Across the shaders shipped in DiligentCore, DiligentFX, DiligentSamples and DiligentTools there are 117 float literals using exponent notation and zero without an explicit sign — e.g. float ClosestDistance = 1e+10; in DiligentFX/Hydrogent/shaders/HnUpdateClosestSelectedLocation.psh, where 1e10 would be the more natural spelling. User-authored shaders have no reason to follow that convention.

The existing unit tests cover -1e+2, +1e-3, -1e, -1e+ and e5, but no unsigned-exponent case such as 1e10, which is why the gap was not caught.

Suggested fix

Make the sign optional and let the existing digit check reject genuinely malformed input:

++c;
// The exponent sign is optional: 1e10 is as valid as 1e+10.
if (c != End && (*c == '+' || *c == '-'))
    ++c;

if (c == End || !IsNum(*c))
{
    // 10e&, 10e+x
    return Pos;
}

I verified this patch locally:

  • all 9 previously failing cases above now pass, and both reproductions above produce correct output (6.02e23 tokenizes as a single NumericConstant);
  • all 94 existing CountFloatNumberChars assertions in Tests/DiligentCoreTest/src/Common/StringToolsTest.cpp still pass, with 0 regressions.

Malformed input is still rejected: 10e&, 10e+x, -1e, -1e+ and e5 behave exactly as before, since the digit check that follows is unchanged.

Worth adding test coverage for 1e10, 1E10, 1.5e10, 6.02e23, 1e0 and 1e10f alongside the existing signed-exponent cases.

Environment

Source-level parsing defect; platform and GPU independent.

  • DiligentEngine aca2285, DiligentCore bb821b7
  • Reproduced on Linux with GCC, C++17

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions