llvm / llvm/llvm-project

[clang][OpenMP] getNameFromIdOrString returns a dangling StringRef for cleaned trait names

Open
#194,225 2 comments 0 reactions 0 assignees View on GitHub
clang:frontend clang:openmp code-quality confirmed
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

I reproduced a `stack-use-after-return` on clean upstream LLVM/Clang head.

Tested revision: d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5

This is similar to the HLSL `ParseHLSLSemantic` dangling-`StringRef` bug #194223, but the escaping buffer here is the local `Buffer` in `getNameFromIdOrString` and the stale read happens in `checkForDuplicates`.

### Why this is a bug

[`getNameFromIdOrString`](https://github.com/llvm/llvm-project/blob/d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5/clang/lib/Parse/ParseOpenMP.cpp#L787-L803) returns a borrowed `StringRef` for identifier tokens:

```cpp
static StringRef getNameFromIdOrString(Parser &P, Token &Tok, OMPContextLvl Lvl) {
if (Tok.is(tok::identifier) || Tok.is(tok::kw_for)) {
llvm::SmallString<16> Buffer;
StringRef Name = P.getPreprocessor().getSpelling(Tok, Buffer);
(void)P.ConsumeToken();
return Name;
}
...
}
```

The vulnerable trace is:

1. [`getNameFromIdOrString`](https://github.com/llvm/llvm-project/blob/d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5/clang/lib/Parse/ParseOpenMP.cpp#L789-L793): allocates a local `SmallString<16> Buffer`, calls `P.getPreprocessor().getSpelling(Tok, Buffer)`, and returns `Name`.
2. [`Preprocessor::getSpelling(...)` fast path](https://github.com/llvm/llvm-project/blob/d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5/clang/lib/Lex/Preprocessor.cpp#L517-L521): only returns stable identifier storage when `!Tok.hasUCN()` and `Tok.getIdentifierInfo()` succeeds. [`Preprocessor::getSpelling(...)` cleaning path](https://github.com/llvm/llvm-project/blob/d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5/clang/lib/Lex/Preprocessor.cpp#L524-L530): if `Tok.needsCleaning()` is true, it resizes the caller-provided buffer and returns `StringRef(Ptr, Len)` into that buffer.
3. [`getNameFromIdOrString` return](https://github.com/llvm/llvm-project/blob/d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5/clang/lib/Parse/ParseOpenMP.cpp#L792-L793): returns that borrowed `Name`, so it may point into the helper’s local `Buffer`.
4. When `getNameFromIdOrString` returns, the local `Buffer` dies.
5. [`parseOMPTraitPropertyKind`](https://github.com/llvm/llvm-project/blob/d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5/clang/lib/Parse/ParseOpenMP.cpp#L841-L851): passes `Name` to [`checkForDuplicates`](https://github.com/llvm/llvm-project/blob/d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5/clang/lib/Parse/ParseOpenMP.cpp#L805-L810), which inserts it into `llvm::StringMap` and hashes the bytes behind the stale `StringRef`.

This is reachable with an OpenMP trait property name that combines:
- a non-basic UCN, to bypass the identifier-table fast path
- a line continuation, to force spelling cleanup into the caller scratch buffer
- a macro, so the pragma still presents a single identifier token to the OpenMP parser

### Reproducer

Build an ASan `clang` with no inlining:

```bash
cmake -S llvm -B build -G Ninja \
-DLLVM_ENABLE_PROJECTS=clang \
-DLLVM_TARGETS_TO_BUILD=X86 \
-DLLVM_USE_SANITIZER=Address \
-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
-DCMAKE_C_FLAGS_DEBUG='-O0 -fno-inline' \
-DCMAKE_CXX_FLAGS_DEBUG='-O0 -fno-inline' \
-DLLVM_INCLUDE_TESTS=OFF \
-DLLVM_INCLUDE_BENCHMARKS=OFF \
-DLLVM_INCLUDE_EXAMPLES=OFF \
-DLLVM_INCLUDE_DOCS=OFF

cmake --build build --target clang -j8
```

Create the testcase:
- `fo\` followed by a newline in the macro body forces line splicing and leaves the replacement token in the `needsCleaning()` path.
- `\u00E9` introduces a UCN into the replacement token, so `Tok.hasUCN()` is true and the stable identifier fast path is skipped.
- the macro keeps the OpenMP surface syntax acceptable while still delivering a single problematic identifier token to `getNameFromIdOrString`.

```bash
cat > /tmp/openmp_isa_ucn_macro.cpp <<'EOF'
void base();
void specialized();
#define BADISA fo\
\u00E9o
#pragma omp declare variant(specialized) match(device={isa(BADISA)})
void base();
void test() { base(); }
EOF
```

Run:

```bash
ASAN_OPTIONS=detect_stack_use_after_return=1:detect_leaks=0:halt_on_error=1 \
./build/bin/clang-23 \
-cc1 -fopenmp -fsyntax-only /tmp/openmp_isa_ucn_macro.cpp
```

### Observed result

ASan reports `stack-use-after-return`.

Representative frames from the clean-head run:

```text
ERROR: AddressSanitizer: stack-use-after-return
#12 (anonymous namespace)::checkForDuplicates(...)
#13 clang::Parser::parseOMPTraitPropertyKind(...)
...
#0 (anonymous namespace)::getNameFromIdOrString(...)
Poisoned local: `Buffer`
```

The binary I used for reproduction:

```text
clang version 23.0.0git (https://github.com/llvm/llvm-project.git d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5)
```

Contributor guide

Open the contributing guide

Research direction

Start in clang/lib/Parse/ParseOpenMP.cpp, tracing getNameFromIdOrString through parseOMPTraitPropertyKind and checkForDuplicates. Build the ASan clang configuration described in the issue and run the provided OpenMP macro reproducer. Done means the cleaned trait name no longer produces a stack-use-after-return and coverage exercises the UCN, line-continuation, and macro path.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, cpp
Domain
compilers, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.