yronglin updated this revision to Diff 538712.
yronglin marked 2 inline comments as done.
yronglin added a comment.
Update comments in PseudoObjectExpr.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154784/new/
https://reviews.llvm.org/D154784
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/Expr.h
clang/include/clang/AST/Stmt.h
clang/lib/AST/Expr.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/test/SemaCXX/builtin-dump-struct.cpp
Index: clang/test/SemaCXX/builtin-dump-struct.cpp
===================================================================
--- clang/test/SemaCXX/builtin-dump-struct.cpp
+++ clang/test/SemaCXX/builtin-dump-struct.cpp
@@ -159,3 +159,28 @@
// expected-note@#Format {{no known conversion from 'int' to 'ConstexprString &' for 1st argument}}
}
#endif
+
+// Check that PseudoObjectExprBitfields:NumSubExprs doesn't overflow. This
+// would previously cause a crash.
+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); }
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ 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();
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -4842,7 +4842,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,
@@ -4873,8 +4874,15 @@
Expr *syntax, ArrayRef<Expr *> 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]);
Index: clang/include/clang/AST/Stmt.h
===================================================================
--- clang/include/clang/AST/Stmt.h
+++ 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 a 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 {
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -6278,11 +6278,12 @@
class PseudoObjectExpr final
: public Expr,
private llvm::TrailingObjects<PseudoObjectExpr, Expr *> {
- // PseudoObjectExprBits.NumSubExprs - The number of sub-expressions.
- // Always at least two, because the first sub-expression is the
- // syntactic form.
+ // PseudoObjectExprBits.ResultBits.NumSubExprs
+ // PseudoObjectExprBits.ResultBits.NumSubExprsIfNoResult
+ // - The number of sub-expressions. Always at least two, because the first
+ // sub-expression is the syntactic form.
- // PseudoObjectExprBits.ResultIndex - The index of the
+ // PseudoObjectExprBits.ResultBits.ResultIndex - The index of the
// sub-expression holding the result. 0 means the result is void,
// which is unambiguous because it's the index of the syntactic
// form. Note that this is therefore 1 higher than the value passed
@@ -6300,8 +6301,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 +6329,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<PseudoObjectExpr*>(this)->getResultExpr();
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -576,6 +576,8 @@
- Fixed false positive error diagnostic when pack expansion appears in template
parameters of a member expression.
(`#48731 <https://github.com/llvm/llvm-project/issues/48731>`_)
+- Fix crash caused by PseudoObjectExprBitfields: NumSubExprs overflow.
+ (`#63169 <https://github.com/llvm/llvm-project/issues/63169>_`)
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
