https://github.com/devsw-prayas created https://github.com/llvm/llvm-project/pull/207529
Reading an uninitialized local variable whose address was never taken is undefined behavior under ISO C 6.3.2.1p2. `-fsanitize=undefined` currently misses this case. This is distinct from MSan — MSan tracks all uninitialized reads via shadow memory regardless of address-taken status; this check is purely static, AST-level, no shadow memory, no ABI implications. Adds `-fsanitize=uninitialized-read` (folded into `-fsanitize=undefined`) that fires before the load in `ScalarExprEmitter::VisitDeclRefExpr` when a local scalar `VarDecl` was never assigned and never had its address taken. A `RecursiveASTVisitor` walks the function body once per function, lazily cached. Inline asm output operands (`GCCAsmStmt`) are handled — they disqualify a variable from the candidate set, addressing the edge case raised in the issue thread. **Known limitations (future work):** - Scalars only — struct/array members go through `MemberExpr`/`AggExprEmitter` - Whole-function existence check, not path-sensitive — misses conditionally-initialized variables Fixes #201850. >From 54db283c59bdb45c14739e6c6ebfe389d0110b4d Mon Sep 17 00:00:00 2001 From: Prayas <[email protected]> Date: Thu, 2 Jul 2026 15:49:46 +0000 Subject: [PATCH] [UBSan] Add -fsanitize=uninitialized-read check (ISO C 6.3.2.1p2) --- clang/include/clang/Basic/Sanitizers.def | 4 +- clang/lib/CodeGen/CGExprScalar.cpp | 75 +++++++++++++++++++ clang/lib/CodeGen/CodeGenFunction.h | 5 ++ clang/lib/CodeGen/SanitizerHandler.h | 2 + compiler-rt/lib/ubsan/ubsan_checks.inc | 1 + compiler-rt/lib/ubsan/ubsan_handlers.cpp | 20 +++++ compiler-rt/lib/ubsan/ubsan_handlers.h | 8 ++ .../Uninitialized/uninitialized-read.c | 64 ++++++++++++++++ 8 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 compiler-rt/test/ubsan/TestCases/Uninitialized/uninitialized-read.c diff --git a/clang/include/clang/Basic/Sanitizers.def b/clang/include/clang/Basic/Sanitizers.def index da85431625026..518357ca11da3 100644 --- a/clang/include/clang/Basic/Sanitizers.def +++ b/clang/include/clang/Basic/Sanitizers.def @@ -114,6 +114,7 @@ SANITIZER("shift-exponent", ShiftExponent) SANITIZER_GROUP("shift", Shift, ShiftBase | ShiftExponent) SANITIZER("signed-integer-overflow", SignedIntegerOverflow) SANITIZER("unreachable", Unreachable) +SANITIZER("uninitialized-read", UninitializedRead) SANITIZER("vla-bound", VLABound) SANITIZER("vptr", Vptr) @@ -152,7 +153,8 @@ SANITIZER_GROUP("undefined", Undefined, FloatCastOverflow | IntegerDivideByZero | NonnullAttribute | Null | ObjectSize | PointerOverflow | Return | ReturnsNonnullAttribute | Shift | - SignedIntegerOverflow | Unreachable | VLABound | Function) + SignedIntegerOverflow | Unreachable | VLABound | UninitializedRead + | Function) // -fsanitize=undefined-trap is an alias for -fsanitize=undefined. SANITIZER_GROUP("undefined-trap", UndefinedTrap, Undefined) diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 18ed6570730f4..a178a88f20e25 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -29,6 +29,7 @@ #include "clang/AST/MatrixUtils.h" #include "clang/AST/ParentMapContext.h" #include "clang/AST/RecordLayout.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/StmtVisitor.h" #include "clang/Basic/CodeGenOptions.h" #include "clang/Basic/DiagnosticTrap.h" @@ -294,6 +295,53 @@ static bool CanElideOverflowCheck(ASTContext &Ctx, const BinOpInfo &Op) { (2 * Ctx.getTypeSize(RHSTy)) < PromotedSize; } +/// Walks a C function body once, collecting local scalar VarDecls that are +/// never assigned and never have their address taken. These are candidates +/// for the -fsanitize=uninitialized-read check (ISO C 6.3.2.1p2). +/// Scoped to C only — C++ lambdas, captures, and aggregates are out of scope. +class UninitLocalVarVisitor + : public RecursiveASTVisitor<UninitLocalVarVisitor> { + llvm::DenseMap<const VarDecl *, bool> &Candidates; + +public: + UninitLocalVarVisitor(llvm::DenseMap<const VarDecl *, bool> &Candidates) + : Candidates(Candidates) {} + + bool VisitVarDecl(VarDecl *VD) { + if (VD->isLocalVarDecl() && !VD->isStaticLocal() && !VD->hasInit() && + VD->getType()->isScalarType()) + Candidates[VD] = true; + return true; + } + + bool VisitBinaryOperator(BinaryOperator *BO) { + if (BO->isAssignmentOp()) + if (auto *DRE = dyn_cast<DeclRefExpr>(BO->getLHS()->IgnoreParens())) + if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) + Candidates.erase(VD); + return true; + } + + bool VisitUnaryOperator(UnaryOperator *UO) { + if (UO->getOpcode() == UO_AddrOf) + if (auto *DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr()->IgnoreParens())) + if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) + Candidates.erase(VD); + return true; + } + + bool VisitGCCAsmStmt(GCCAsmStmt *AS) { + // Output operands ("=r", "+r" etc.) initialize the variable: + // treat them the same as an assignment. + for (unsigned i = 0; i < AS->getNumOutputs(); ++i) + if (auto *DRE = dyn_cast<DeclRefExpr>( + AS->getOutputExpr(i)->IgnoreParenImpCasts())) + if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) + Candidates.erase(VD); + return true; + } +}; + class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, Value*> { CodeGenFunction &CGF; @@ -608,6 +656,33 @@ class ScalarExprEmitter Value *VisitDeclRefExpr(DeclRefExpr *E) { if (CodeGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E)) return CGF.emitScalarConstant(Constant, E); + + if (CGF.SanOpts.has(SanitizerKind::UninitializedRead) && + !CGF.getLangOpts().CPlusPlus) { + if (auto *VD = dyn_cast<VarDecl>(E->getDecl())) { + if (!CGF.UninitReadAnalysisDone) { + if (auto *FD = dyn_cast<FunctionDecl>(CGF.CurFuncDecl)) + if (const Stmt *Body = FD->getBody()) + UninitLocalVarVisitor(CGF.UninitReadCandidates) + .TraverseStmt(const_cast<Stmt *>(Body)); + CGF.UninitReadAnalysisDone = true; + } + if (CGF.UninitReadCandidates.count(VD)) { + auto CheckOrdinal = SanitizerKind::SO_UninitializedRead; + auto CheckHandler = SanitizerHandler::UninitializedRead; + SanitizerDebugLocation SanScope(&CGF, {CheckOrdinal}, CheckHandler); + CGF.EmitCheck({std::make_pair( + static_cast<llvm::Value *>(CGF.Builder.getFalse()), + CheckOrdinal)}, + CheckHandler, + {CGF.EmitCheckSourceLocation(E->getLocation()), + llvm::cast<llvm::Constant>( + CGF.Builder.CreateGlobalString(VD->getName()))}, + {}); + } + } + } + return EmitLoadOfLValue(E); } diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 6d0718c243812..a29c2810e44b9 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -591,6 +591,11 @@ class CodeGenFunction : public CodeGenTypeCache { ~SanitizerScope(); }; + /// Cache for -fsanitize=uninitialized-read analysis. + /// Populated lazily on first scalar read in a function body. + bool UninitReadAnalysisDone = false; + llvm::DenseMap<const VarDecl *, bool> UninitReadCandidates; + /// In C++, whether we are code generating a thunk. This controls whether we /// should emit cleanups. bool CurFuncIsThunk = false; diff --git a/clang/lib/CodeGen/SanitizerHandler.h b/clang/lib/CodeGen/SanitizerHandler.h index 871e17c22d3fa..a0ab426f9e85a 100644 --- a/clang/lib/CodeGen/SanitizerHandler.h +++ b/clang/lib/CodeGen/SanitizerHandler.h @@ -37,6 +37,8 @@ "Invalid Objective-C cast") \ SANITIZER_CHECK(LoadInvalidValue, load_invalid_value, 0, \ "Loaded an invalid or uninitialized value for the type") \ + SANITIZER_CHECK(UninitializedRead, uninitialized_read, 0, \ + "Read of an uninitialized local variable") \ SANITIZER_CHECK(MissingReturn, missing_return, 0, \ "Execution reached the end of a value-returning function " \ "without returning a value") \ diff --git a/compiler-rt/lib/ubsan/ubsan_checks.inc b/compiler-rt/lib/ubsan/ubsan_checks.inc index b1d09a9024e7e..c73d307df43e7 100644 --- a/compiler-rt/lib/ubsan/ubsan_checks.inc +++ b/compiler-rt/lib/ubsan/ubsan_checks.inc @@ -60,6 +60,7 @@ UBSAN_CHECK(NonPositiveVLAIndex, "non-positive-vla-index", "vla-bound") UBSAN_CHECK(FloatCastOverflow, "float-cast-overflow", "float-cast-overflow") UBSAN_CHECK(InvalidBoolLoad, "invalid-bool-load", "bool") UBSAN_CHECK(InvalidEnumLoad, "invalid-enum-load", "enum") +UBSAN_CHECK(UninitializedRead, "uninitialized-read", "uninitialized-read") UBSAN_CHECK(FunctionTypeMismatch, "function-type-mismatch", "function") UBSAN_CHECK(InvalidNullReturn, "invalid-null-return", "returns-nonnull-attribute") diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cpp b/compiler-rt/lib/ubsan/ubsan_handlers.cpp index 2614d49bf238b..7e62849debaff 100644 --- a/compiler-rt/lib/ubsan/ubsan_handlers.cpp +++ b/compiler-rt/lib/ubsan/ubsan_handlers.cpp @@ -573,6 +573,26 @@ void __ubsan::__ubsan_handle_load_invalid_value_abort(InvalidValueData *Data, Die(); } +static void handleUninitRead(UninitReadData *Data, ReportOptions Opts) { + SourceLocation Loc = Data->Loc.acquire(); + ErrorType ET = ErrorType::UninitializedRead; + if (ignoreReport(Loc, Opts, ET)) + return; + ScopedReport R(Opts, Loc, ET); + Diag(Loc, DL_Error, ET, "read of uninitialized local variable %0") + << Data->VarName; +} + +void __ubsan::__ubsan_handle_uninitialized_read(UninitReadData *Data) { + GET_REPORT_OPTIONS(false); + handleUninitRead(Data, Opts); +} +void __ubsan::__ubsan_handle_uninitialized_read_abort(UninitReadData *Data) { + GET_REPORT_OPTIONS(true); + handleUninitRead(Data, Opts); + Die(); +} + static void handleImplicitConversion(ImplicitConversionData *Data, ReportOptions Opts, ValueHandle Src, ValueHandle Dst) { diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.h b/compiler-rt/lib/ubsan/ubsan_handlers.h index 521caa96bc771..39756ef155593 100644 --- a/compiler-rt/lib/ubsan/ubsan_handlers.h +++ b/compiler-rt/lib/ubsan/ubsan_handlers.h @@ -135,6 +135,14 @@ struct InvalidValueData { /// \brief Handle a load of an invalid value for the type. RECOVERABLE(load_invalid_value, InvalidValueData *Data, ValueHandle Val) +struct UninitReadData { + SourceLocation Loc; + const char *VarName; +}; + +/// \brief Handle a read of an uninitialized local variable. +RECOVERABLE(uninitialized_read, UninitReadData *Data) + /// Known implicit conversion check kinds. /// Keep in sync with the enum of the same name in CGExprScalar.cpp enum ImplicitConversionCheckKind : unsigned char { diff --git a/compiler-rt/test/ubsan/TestCases/Uninitialized/uninitialized-read.c b/compiler-rt/test/ubsan/TestCases/Uninitialized/uninitialized-read.c new file mode 100644 index 0000000000000..e73ca396e86a4 --- /dev/null +++ b/compiler-rt/test/ubsan/TestCases/Uninitialized/uninitialized-read.c @@ -0,0 +1,64 @@ +// RUN: %clang -fsanitize=uninitialized-read %s -O0 -o %t +// RUN: %run %t 2>&1 | FileCheck %s +// RUN: %clang -fsanitize=undefined %s -O0 -o %t +// RUN: %run %t 2>&1 | FileCheck %s + +// Positive: scalar local — should trigger +// CHECK: uninitialized-read.c:[[@LINE+1]]:{{.*}}: runtime error: read of uninitialized local variable x +int scalar_uninit() { + int x; + return x; +} + +// Positive: pointer local — pointer is a scalar type in C, should trigger +// CHECK: uninitialized-read.c:[[@LINE+1]]:{{.*}}: runtime error: read of uninitialized local variable p +int *pointer_uninit() { + int *p; + return p; +} + +// Negative: address taken — C 6.3.2.1p2 does not apply, must NOT trigger +int scalar_addr_taken() { + int x; + (void)&x; + return x; +} + +// Known limitation — struct member access goes through MemberExpr/AggExprEmitter, +// not ScalarExprEmitter::VisitDeclRefExpr. Not detected. Future work. +struct S { + int x; +}; +int struct_member() { + struct S s; + return s.x; +} + +// Known limitation — array element access goes through ArraySubscriptExpr. +// Not detected. Future work. +int array_elem() { + int arr[5]; + return arr[0]; +} + +// Known limitation — path sensitivity: x is assigned on some paths but not +// others. Whole-function existence check sees an assignment and does not fire. +// Future work. +int path_sensitive(int cond) { + int x; + if (cond) + x = 5; + return x; +} + +int main() { + scalar_uninit(); + pointer_uninit(); + scalar_addr_taken(); + struct_member(); + array_elem(); + path_sensitive(0); + return 0; +} +// CHECK-NOT: runtime error: read of uninitialized local variable s +// CHECK-NOT: runtime error: read of uninitialized local variable arr _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
