https://github.com/AhmedKamel10 created https://github.com/llvm/llvm-project/pull/219780
Fixes #217636. Per C99 6.9.2p2 / C23 6.9.3p1, a file-scope `thread_local`/`_Thread_local`/`__thread` declaration is always a full definition, never tentative. Clang wasn't checking for TLS storage before classifying file-scope declarations without initializers as `TentativeDefinition`, so this was accepted: ```c ``` code thread_local int x; thread_local int x; // GCC rejects this; Clang didn't ``` **Changes:** 1- In `Decl.cpp` I changed the function `isThisDeclarationADefinition()` to exclude TLS vars from the tentative-definition branch and to classify them as `Definition`. 2- In `SemaDecl.cpp` in the function `MergeVarDecl`, the C path never checked for redefinition on `Definition` (only the C++ part did). This was unreachable for TLS vars until the first change. Also added the check `New->getTLSKind() != VarDecl::TLS_None && New->isThisDeclarationADefinition() == VarDecl::Definition`, that is similar to the existing C++ path logic: if `New` is a `Definition` and a previous `Definition` exists in the chain, diagnose via the `checkVarDeclRedefinition`. Added tests to `tentative-decls.c` and ran `check-clang-sema`. **Open question** Marking as draft pending this question: `clang/test/Sema/attr-tls_model.c` and `clang/test/Sema/PowerPC/aix-attr-tls_model.c` both repeat `static __thread int y __attribute((tls_model(...)))` several times to test different attribute values, relying on the old tentative treatment to avoid a redefinition error. With this fix, they now fail. Question: Should redeclaring a TLS variable purely to attach attributes stay legal, even though it's the same pattern this issue asks to reject in general? If yes, guidance on how to distinguish the two cases would help. If no, I'll update those two tests as part of this PR. cc @shafik @AaronBallman >From 0109e5d1bc40ea1e2b5bfcdd17b36141d2f786a7 Mon Sep 17 00:00:00 2001 From: ahmedkamel10 <[email protected]> Date: Sun, 30 Aug 2026 09:50:50 +0300 Subject: [PATCH] [Clang] Prevent thread_local from creating tentative definitions (#217636) --- clang/lib/AST/Decl.cpp | 2 +- clang/lib/Sema/SemaDecl.cpp | 8 +++++++- clang/test/Sema/tentative-decls.c | 3 +++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index c9524dc82588a..a9c39a95a93d6 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -2316,7 +2316,7 @@ VarDecl::isThisDeclarationADefinition(ASTContext &C) const { // and without a storage class specifier or the scs 'static', constitutes // a tentative definition. // No such thing in C++. - if (!C.getLangOpts().CPlusPlus && isFileVarDecl()) + if (!C.getLangOpts().CPlusPlus && isFileVarDecl() && getTLSKind() == TLS_None) return TentativeDefinition; // What's left is (in C, block-scope) declarations without initializers or diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 07c6157ab8f31..33839827a6d6e 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -4963,7 +4963,13 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { // [basic.def]p2 for details, but the basic idea is: if the old declaration // contains the extern specifier and doesn't have an initializer, it's fine // in C++. - if (Old->getStorageClass() != SC_Extern || Old->hasInit()) { + if (New->getTLSKind() != VarDecl::TLS_None && New->isThisDeclarationADefinition() == VarDecl::Definition) { + VarDecl* Def = Old->getDefinition(); + if(Def && checkVarDeclRedefinition(Def, New)){ + return; + } + } + else if (Old->getStorageClass() != SC_Extern || Old->hasInit()) { Diag(New->getLocation(), diag::warn_cxx_compat_tentative_definition) << New; Diag(Old->getLocation(), diag::note_previous_declaration); diff --git a/clang/test/Sema/tentative-decls.c b/clang/test/Sema/tentative-decls.c index 94d21bdbf94da..fd05e880d028f 100644 --- a/clang/test/Sema/tentative-decls.c +++ b/clang/test/Sema/tentative-decls.c @@ -63,3 +63,6 @@ static int b0; static int a0[] = { 4 }; static int b0 = 5; + +_Thread_local int tls_redef; // expected-note {{previous definition is here}} +_Thread_local int tls_redef; // expected-error {{redefinition of 'tls_redef'}} \ No newline at end of file _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
