diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -573,6 +573,8 @@ - Stop evaluating a constant expression if the condition expression which in switch statement contains errors. (`#63453 _`) +- Fix crash caused by PseudoObjectExprBitfields: NumSubExprs overflow. + (`#63169 _`) Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -6300,8 +6300,11 @@ PseudoObjectExpr(EmptyShell shell, unsigned numSemanticExprs); + bool hasResult() const { return PseudoObjectExprBits.HasResult; } + unsigned getNumSubExprs() const { - return PseudoObjectExprBits.NumSubExprs; + return hasResult() ? PseudoObjectExprBits.ResultBits.NumSubExprs + : PseudoObjectExprBits.NumSubExprsIfNoResult; } public: @@ -6325,15 +6328,16 @@ /// Return the index of the result-bearing expression into the semantics /// expressions, or PseudoObjectExpr::NoResult if there is none. unsigned getResultExprIndex() const { - if (PseudoObjectExprBits.ResultIndex == 0) return NoResult; - return PseudoObjectExprBits.ResultIndex - 1; + if (hasResult()) + return PseudoObjectExprBits.ResultBits.ResultIndex - 1; + return NoResult; } /// Return the result-bearing expression, or null if there is none. Expr *getResultExpr() { - if (PseudoObjectExprBits.ResultIndex == 0) - return nullptr; - return getSubExprsBuffer()[PseudoObjectExprBits.ResultIndex]; + if (hasResult()) + return getSubExprsBuffer()[PseudoObjectExprBits.ResultBits.ResultIndex]; + return nullptr; } const Expr *getResultExpr() const { return const_cast(this)->getResultExpr(); diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h --- a/clang/include/clang/AST/Stmt.h +++ b/clang/include/clang/AST/Stmt.h @@ -591,12 +591,26 @@ friend class ASTStmtReader; // deserialization friend class PseudoObjectExpr; + struct ResultBitfields { + // These don't need to be particularly wide, because they're + // strictly limited by the forms of expressions we permit. + unsigned NumSubExprs : 16; + unsigned ResultIndex : 16; + }; + unsigned : NumExprBits; - // These don't need to be particularly wide, because they're - // strictly limited by the forms of expressions we permit. - unsigned NumSubExprs : 8; - unsigned ResultIndex : 32 - 8 - NumExprBits; + // Whether the PseudoObjectExpr has result. + unsigned HasResult : 1; + + // This is an optimization for PseudoObjectExprBitfields, when there is no + // result in PseudoObjectExpr, we use 32 bits to store the number of + // subexpressions, otherwise, we use 16 bits to store the number of + // subexpressions, and use 16 bits to store the result indexes. + union { + ResultBitfields ResultBits; + unsigned NumSubExprsIfNoResult; + }; }; class SourceLocExprBitfields { diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -4848,7 +4848,8 @@ PseudoObjectExpr::PseudoObjectExpr(EmptyShell shell, unsigned numSemanticExprs) : Expr(PseudoObjectExprClass, shell) { - PseudoObjectExprBits.NumSubExprs = numSemanticExprs + 1; + PseudoObjectExprBits.HasResult = false; + PseudoObjectExprBits.NumSubExprsIfNoResult = numSemanticExprs + 1; } PseudoObjectExpr *PseudoObjectExpr::Create(const ASTContext &C, Expr *syntax, @@ -4879,8 +4880,15 @@ Expr *syntax, ArrayRef semantics, unsigned resultIndex) : Expr(PseudoObjectExprClass, type, VK, OK_Ordinary) { - PseudoObjectExprBits.NumSubExprs = semantics.size() + 1; - PseudoObjectExprBits.ResultIndex = resultIndex + 1; + + if (resultIndex == NoResult) { + PseudoObjectExprBits.HasResult = false; + PseudoObjectExprBits.NumSubExprsIfNoResult = semantics.size() + 1; + } else { + PseudoObjectExprBits.HasResult = true; + PseudoObjectExprBits.ResultBits.NumSubExprs = semantics.size() + 1; + PseudoObjectExprBits.ResultBits.ResultIndex = resultIndex + 1; + } for (unsigned i = 0, e = semantics.size() + 1; i != e; ++i) { Expr *E = (i == 0 ? syntax : semantics[i-1]); diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/clang/lib/Serialization/ASTReaderStmt.cpp @@ -1378,8 +1378,15 @@ void ASTStmtReader::VisitPseudoObjectExpr(PseudoObjectExpr *E) { VisitExpr(E); unsigned numSemanticExprs = Record.readInt(); - assert(numSemanticExprs + 1 == E->PseudoObjectExprBits.NumSubExprs); - E->PseudoObjectExprBits.ResultIndex = Record.readInt(); + assert(numSemanticExprs + 1 == E->getNumSubExprs()); + + unsigned ResultIndex = Record.readInt(); + if (ResultIndex) { + E->PseudoObjectExprBits.HasResult = true; + E->PseudoObjectExprBits.ResultBits.ResultIndex = ResultIndex; + } else { + E->PseudoObjectExprBits.HasResult = false; + } // Read the syntactic expression. E->getSubExprsBuffer()[0] = Record.readSubExpr(); diff --git a/clang/test/SemaCXX/builtin-dump-struct.cpp b/clang/test/SemaCXX/builtin-dump-struct.cpp --- a/clang/test/SemaCXX/builtin-dump-struct.cpp +++ b/clang/test/SemaCXX/builtin-dump-struct.cpp @@ -159,3 +159,27 @@ // expected-note@#Format {{no known conversion from 'int' to 'ConstexprString &' for 1st argument}} } #endif + +// Check that PseudoObjectExprBitfields:NumSubExprs doesn't overflow. +struct t1 { + int v0, v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11, v12, v13, v14, v15, v16, + v17, v18, v19, v20, v21, v22, v23, v24, v25, v26, v27, v28, v29, v30, v31, + v32, v33, v34, v35, v36, v37, v38, v39, v40, v41, v42, v43, v44, v45, v46, + v47, v48, v49, v50, v51, v52, v53, v54, v55, v56, v57, v58, v59, v60, v61, + v62, v63, v64, v65, v66, v67, v68, v69, v70, v71, v72, v73, v74, v75, v76, + v77, v78, v79, v80, v81, v82, v83, v84, v85, v86, v87, v88, v89, v90, v91, + v92, v93, v94, v95, v96, v97, v98, v99; +}; + +struct t2 { + t1 v0, v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11, v12, v13, v14, v15, v16, + v17, v18, v19, v20, v21, v22, v23, v24, v25, v26, v27, v28, v29, v30, v31, + v32, v33, v34, v35, v36, v37, v38, v39, v40, v41, v42, v43, v44, v45, v46, + v47, v48, v49, v50, v51, v52, v53, v54, v55, v56, v57, v58, v59, v60, v61, + v62, v63, v64, v65, v66, v67, v68, v69, v70, v71, v72, v73, v74, v75, v76, + v77, v78, v79, v80, v81, v82, v83, v84, v85, v86, v87, v88, v89, v90, v91, + v92, v93, v94, v95, v96, v97, v98, v99; +}; + +int printf(const char *, ...); +void f1(t2 w) { __builtin_dump_struct(&w, printf); }