Skip to content

Commit ebb8966

Browse files
patrykstefanskipatrykstefanski
committed
[BoundsSafety] Rebuild bound attributed types when declaring a function with typeof/typedef
Declaring a function with typedef/typeof, which has bounds attributed types, creates a bounds attribute expression that refers to the parameters of the old function. void foo(int *__counted_by(len), int len); __typeof__(foo) bar; In the above example, the count expression in `bar` refers to `len` in the function `foo`. Instead, it should refer to `len` in `bar`. Moreover, declaring a function with typedef/typeof loses `DependerDeclsAttr`. In the above example, `len` in `bar` won't have the attribute. This change rebuilds the function type if the function is declared with typeof/typedef. This will rebuild the bounds attribute expression to refer to the parameters of the function being declared. Moreover, it creates `DependerDeclsAttr` for the dependent parameters. rdar://131339532 (cherry picked from commit 14fa725)
1 parent 0712898 commit ebb8966

File tree

8 files changed

+348
-47
lines changed

8 files changed

+348
-47
lines changed

‎clang/lib/Sema/DynamicCountPointerAssignmentAnalysis.cpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -570,8 +570,7 @@ class PreAssignCheck {
570570
}
571571

572572
assert(!Ty->getCountExpr()->HasSideEffects(Ctx));
573-
ExprResult CountR =
574-
DeclReplacer.TransformBoundsAttrExpr(Ty->getCountExpr());
573+
ExprResult CountR = DeclReplacer.TransformExpr(Ty->getCountExpr());
575574
if (CountR.isInvalid())
576575
return ExprError();
577576
Expr *Count = CountR.get();
@@ -781,7 +780,7 @@ class PreAssignCheck {
781780
// '__ended_by'. We may revisit this if we decide to expose '__started_by'
782781
// to users.
783782
if (auto *End = Ty->getEndPointer()) {
784-
if (!PushValidOrErr(DeclReplacer.TransformBoundsAttrExpr(End)))
783+
if (!PushValidOrErr(DeclReplacer.TransformExpr(End)))
785784
return ExprError();
786785
}
787786

@@ -969,7 +968,7 @@ class PreAssignCheck {
969968
Counts.push_back(Zero.get());
970969
}
971970

972-
ExprResult NewCount = DeclReplacer.TransformBoundsAttrExpr(Count);
971+
ExprResult NewCount = DeclReplacer.TransformExpr(Count);
973972
NewCount = SemaRef.DefaultLvalueConversion(NewCount.get());
974973
if (NewCount.isInvalid())
975974
return;
@@ -1025,7 +1024,7 @@ class PreAssignCheck {
10251024

10261025
assert(!RangePtr->HasSideEffects(SemaRef.Context));
10271026
// Since we materialize self assignments, we can reuse the materialized value.
1028-
ExprResult NewRangePtr = DeclReplacer.TransformBoundsAttrExpr(RangePtr);
1027+
ExprResult NewRangePtr = DeclReplacer.TransformExpr(RangePtr);
10291028
assert(!NewRangePtr.isInvalid());
10301029
// This is assuming the range has not been changed.
10311030
NewRangePtr = SemaRef.DefaultFunctionArrayLvalueConversion(NewRangePtr.get());

‎clang/lib/Sema/DynamicCountPointerAssignmentAnalysis.h‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -267,15 +267,6 @@ struct ReplaceDeclRefWithRHS : public TreeTransform<ReplaceDeclRefWithRHS> {
267267

268268
bool AlwaysRebuild() { return true; }
269269

270-
ExprResult TransformBoundsAttrExpr(Expr *E) {
271-
using ExpressionKind =
272-
Sema::ExpressionEvaluationContextRecord::ExpressionKind;
273-
EnterExpressionEvaluationContext EC(
274-
SemaRef, Sema::ExpressionEvaluationContext::PotentiallyEvaluated,
275-
nullptr, ExpressionKind::EK_AttrArgument);
276-
return TransformExpr(E);
277-
}
278-
279270
ExprResult TransformOpaqueValueExpr(OpaqueValueExpr *E) { return Owned(E); }
280271

281272
/// If we are replacing the value pointed to by DeclRef, we unwrap dereference

‎clang/lib/Sema/SemaDecl.cpp‎

Lines changed: 143 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10466,6 +10466,77 @@ static bool isStdBuiltin(ASTContext &Ctx, FunctionDecl *FD,
1046610466
}
1046710467
}
1046810468

10469+
static void rebuildBoundAttributedTypes(Sema &S, FunctionDecl *FD) {
10470+
struct ReplaceParams : public TreeTransform<ReplaceParams> {
10471+
using BaseTransform = TreeTransform<ReplaceParams>;
10472+
FunctionDecl *FD;
10473+
10474+
explicit ReplaceParams(Sema &SemaRef, FunctionDecl *FD)
10475+
: BaseTransform(SemaRef), FD(FD) {}
10476+
10477+
bool AlwaysRebuild() { return true; }
10478+
10479+
ExprResult TransformDeclRefExpr(DeclRefExpr *E) {
10480+
const auto *OldParam = dyn_cast<ParmVarDecl>(E->getDecl());
10481+
if (!OldParam)
10482+
return BaseTransform::TransformDeclRefExpr(E);
10483+
auto *NewParam = FD->getParamDecl(OldParam->getFunctionScopeIndex());
10484+
return SemaRef.BuildDeclRefExpr(NewParam, E->getType(), E->getValueKind(),
10485+
E->getNameInfo());
10486+
}
10487+
} Transform(S, FD);
10488+
10489+
auto RebuildBoundAttributeType = [&](QualType Ty) {
10490+
if (Ty->isBoundsAttributedType() ||
10491+
(Ty->isPointerType() &&
10492+
Ty->getPointeeType()->isBoundsAttributedType())) {
10493+
return Transform.TransformType(Ty);
10494+
}
10495+
return Ty;
10496+
};
10497+
10498+
QualType OldRetTy = FD->getReturnType();
10499+
QualType NewRetTy = RebuildBoundAttributeType(OldRetTy);
10500+
bool Updated = NewRetTy != OldRetTy;
10501+
10502+
llvm::SmallVector<QualType, 4> NewParamTys;
10503+
NewParamTys.reserve(FD->getNumParams());
10504+
for (unsigned I = 0; I < FD->getNumParams(); ++I) {
10505+
ParmVarDecl *Param = FD->getParamDecl(I);
10506+
QualType OldTy = Param->getType();
10507+
QualType NewTy = RebuildBoundAttributeType(OldTy);
10508+
10509+
NewParamTys.push_back(NewTy);
10510+
10511+
if (NewTy == OldTy)
10512+
continue;
10513+
10514+
Param->setType(NewTy);
10515+
Updated = true;
10516+
10517+
// If this param is a count attributed type or a pointer to it (Deref is
10518+
// true), we need to recreate DependerDeclsAttr for its dependent
10519+
// parameters.
10520+
bool Deref = false;
10521+
const auto *CATy = NewTy->getAs<CountAttributedType>();
10522+
if (!CATy && NewTy->isPointerType()) {
10523+
Deref = true;
10524+
CATy = NewTy->getPointeeType()->getAs<CountAttributedType>();
10525+
}
10526+
if (CATy)
10527+
S.AttachDependerDeclsAttr(Param, CATy, Deref);
10528+
}
10529+
10530+
if (!Updated)
10531+
return;
10532+
10533+
// TODO: We could keep the typeof/typedef sugars.
10534+
const auto *OldFPT = FD->getType()->castAs<FunctionProtoType>();
10535+
QualType NewFPT = S.Context.getFunctionType(NewRetTy, NewParamTys,
10536+
OldFPT->getExtProtoInfo());
10537+
FD->setType(NewFPT);
10538+
}
10539+
1046910540
NamedDecl*
1047010541
Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
1047110542
TypeSourceInfo *TInfo, LookupResult &Previous,
@@ -11025,11 +11096,64 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
1102511096
// @endcode
1102611097

1102711098
// Synthesize a parameter for each argument type.
11028-
for (const auto &AI : FT->param_types()) {
11029-
ParmVarDecl *Param =
11030-
BuildParmVarDeclForTypedef(NewFD, D.getIdentifierLoc(), AI);
11031-
Param->setScopeInfo(0, Params.size());
11032-
Params.push_back(Param);
11099+
11100+
/* TO_UPSTREAM(BoundsSafety) ON */
11101+
if (getLangOpts().BoundsSafetyAttributes) {
11102+
// If the parameters are used as dependent variables, we can synthesize
11103+
// named parameters for use in the declaration. Named dependent variables
11104+
// let us generate better diagnostics for the user (e.g.,
11105+
// __counted_by(len) instead of __counted_by()). Otherwise, just
11106+
// synthesize unnamed parameters.
11107+
llvm::SmallVector<const ParmVarDecl *, 4> NamedParams(FT->getNumParams(),
11108+
nullptr);
11109+
auto AddNamedParamFromDependentType = [&](QualType Ty) {
11110+
const auto *BAT = Ty->getAs<BoundsAttributedType>();
11111+
if (!BAT && Ty->isPointerType())
11112+
BAT = Ty->getPointeeType()->getAs<BoundsAttributedType>();
11113+
if (!BAT)
11114+
return;
11115+
for (const auto &DD : BAT->dependent_decls()) {
11116+
if (const auto *PVD = dyn_cast<ParmVarDecl>(DD.getDecl()))
11117+
NamedParams[PVD->getFunctionScopeIndex()] = PVD;
11118+
}
11119+
};
11120+
AddNamedParamFromDependentType(FT->getReturnType());
11121+
for (QualType ParamTy : FT->param_types())
11122+
AddNamedParamFromDependentType(ParamTy);
11123+
11124+
// Temporarily put parameter variables in the translation unit, not the
11125+
// enclosing context. This is what Sema::ActOnParamDeclarator() already
11126+
// does when declaring a function without a typedef/typeof. In
11127+
// BoundsSafety, we do it to allow creating references to the parameters
11128+
// when rebuilding the count expression. Otherwise, creating such
11129+
// references triggers an error.
11130+
for (unsigned I = 0; I < FT->getNumParams(); ++I) {
11131+
QualType Ty = FT->getParamType(I);
11132+
const ParmVarDecl *NamedParam = NamedParams[I];
11133+
ParmVarDecl *Param;
11134+
if (!NamedParam) {
11135+
Param = BuildParmVarDeclForTypedef(Context.getTranslationUnitDecl(),
11136+
D.getIdentifierLoc(), Ty);
11137+
} else {
11138+
SourceLocation Loc = D.getIdentifierLoc();
11139+
Param = ParmVarDecl::Create(Context, Context.getTranslationUnitDecl(),
11140+
Loc, Loc, NamedParam->getIdentifier(), Ty,
11141+
Context.getTrivialTypeSourceInfo(Ty, Loc),
11142+
SC_None, nullptr);
11143+
Param->setImplicit();
11144+
}
11145+
Param->setScopeInfo(0, Params.size());
11146+
Params.push_back(Param);
11147+
}
11148+
}
11149+
/* TO_UPSTREAM(BoundsSafety) OFF */
11150+
else {
11151+
for (const auto &AI : FT->param_types()) {
11152+
ParmVarDecl *Param =
11153+
BuildParmVarDeclForTypedef(NewFD, D.getIdentifierLoc(), AI);
11154+
Param->setScopeInfo(0, Params.size());
11155+
Params.push_back(Param);
11156+
}
1103311157
}
1103411158
} else {
1103511159
assert(R->isFunctionNoProtoType() && NewFD->getNumParams() == 0 &&
@@ -11040,6 +11164,20 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
1104011164
NewFD->setParams(Params);
1104111165

1104211166
/* TO_UPSTREAM(BoundsSafety) ON*/
11167+
if (getLangOpts().BoundsSafetyAttributes) {
11168+
if (!D.isFunctionDeclarator() && R->isFunctionProtoType()) {
11169+
// The function is being declared with a typedef/typeof. We need to
11170+
// rebuild the bound attributed types, so that their expression refer the
11171+
// to the new parameters.
11172+
rebuildBoundAttributedTypes(*this, NewFD);
11173+
11174+
for (ParmVarDecl *Param : Params) {
11175+
assert(Param->getDeclContext() == Context.getTranslationUnitDecl());
11176+
Param->setDeclContext(NewFD);
11177+
}
11178+
}
11179+
}
11180+
1104311181
if (getLangOpts().BoundsSafety) {
1104411182
// This is a good place to make sure that no array parameter has decayed
1104511183
// to a __single pointer. Annoyingly, we can't do this when parameters are

‎clang/lib/Sema/SemaExpr.cpp‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7851,7 +7851,7 @@ bool Sema::CheckDynamicCountSizeForAssignment(
78517851
if (!DependentValues.empty()) {
78527852
if (LHSMemberBase)
78537853
Transform.MemberBase = LHSMemberBase;
7854-
ExprResult CountExprRes = Transform.TransformBoundsAttrExpr(CountExpr);
7854+
ExprResult CountExprRes = Transform.TransformExpr(CountExpr);
78557855
if (CountExprRes.isInvalid())
78567856
return false;
78577857
CountExpr = CountExprRes.get();
@@ -22339,14 +22339,6 @@ void diagnoseUncapturableValueReferenceOrBinding(Sema &S, SourceLocation loc,
2233922339
if (!S.getLangOpts().CPlusPlus && !S.CurContext->isFunctionOrMethod())
2234022340
return;
2234122341

22342-
/* TO_UPSTREAM(BoundsSafety) ON*/
22343-
if (S.getLangOpts().BoundsSafety) {
22344-
// Allow capturing variables/fields in bounds attr expr context.
22345-
if (S.isAttrContext() && (isa<VarDecl>(var) || isa<FieldDecl>(var)))
22346-
return;
22347-
}
22348-
/* TO_UPSTREAM(BoundsSafety) OFF*/
22349-
2235022342
unsigned ValueKind = isa<BindingDecl>(var) ? 1 : 0;
2235122343
unsigned ContextKind = 3; // unknown
2235222344
if (isa<CXXMethodDecl>(VarDC) &&
@@ -25162,8 +25154,7 @@ ExprResult Sema::ActOnBoundsSafetyCall(ExprResult Call) {
2516225154
QualType ParamType = ParamTypes[I];
2516325155
BoundsCheckBuilder Builder(OVEs);
2516425156
if (auto *DCPT = ParamType->getAs<CountAttributedType>()) {
25165-
ExprResult CountExpr =
25166-
Transform.TransformBoundsAttrExpr(DCPT->getCountExpr());
25157+
ExprResult CountExpr = Transform.TransformExpr(DCPT->getCountExpr());
2516725158
if (!CountExpr.get())
2516825159
return ExprError();
2516925160
Builder.setAccessCount(CountExpr.get(), DCPT->isCountInBytes(),
@@ -25173,7 +25164,7 @@ ExprResult Sema::ActOnBoundsSafetyCall(ExprResult Call) {
2517325164
// will not have an end pointer, just a start pointer. The start pointer
2517425165
// will generate the entire breadth of required checks.
2517525166
if (auto *EndPtr = DRPT->getEndPointer()) {
25176-
ExprResult Upper = Transform.TransformBoundsAttrExpr(EndPtr);
25167+
ExprResult Upper = Transform.TransformExpr(EndPtr);
2517725168
if (!Upper.get())
2517825169
return ExprError();
2517925170
Builder.setAccessLowerBound(OVEs[I]);
@@ -25197,8 +25188,7 @@ ExprResult Sema::ActOnBoundsSafetyCall(ExprResult Call) {
2519725188
// for flexible array member pointers.
2519825189
QualType ResultTy = ResultExpr->getType();
2519925190
if (auto *DCPT = ResultTy->getAs<CountAttributedType>()) {
25200-
ExprResult ReturnCount =
25201-
Transform.TransformBoundsAttrExpr(DCPT->getCountExpr());
25191+
ExprResult ReturnCount = Transform.TransformExpr(DCPT->getCountExpr());
2520225192
if (!ReturnCount.get())
2520325193
return ExprError();
2520425194
ReturnCount = DefaultLvalueConversion(ReturnCount.get());
@@ -25213,7 +25203,7 @@ ExprResult Sema::ActOnBoundsSafetyCall(ExprResult Call) {
2521325203
if (!(ResultExpr = Promoted.get()))
2521425204
return ExprError();
2521525205
} else if (auto *DRPT = ResultTy->getAs<DynamicRangePointerType>()) {
25216-
ExprResult Upper = Transform.TransformBoundsAttrExpr(DRPT->getEndPointer());
25206+
ExprResult Upper = Transform.TransformExpr(DRPT->getEndPointer());
2521725207
if (!Upper.get())
2521825208
return ExprError();
2521925209
auto FA = BoundsSafetyPointerAttributes::bidiIndexable();

‎clang/lib/Sema/TreeTransform.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7469,7 +7469,7 @@ QualType TreeTransform<Derived>::TransformCountAttributedType(
74697469
if (getDerived().AlwaysRebuild() || InnerTy != OldTy->desugar() ||
74707470
OldCount != NewCount) {
74717471
/* TO_UPSTREAM(BoundsSafety) ON */
7472-
if (SemaRef.getLangOpts().hasBoundsSafety()) {
7472+
if (SemaRef.getLangOpts().BoundsSafetyAttributes) {
74737473
Result = SemaRef.BuildCountAttributedType(
74747474
InnerTy, NewCount, OldTy->isCountInBytes(), OldTy->isOrNull());
74757475
} else {

‎clang/test/BoundsSafety/AST/typedef-function-attrs-late-parsed-ret.c‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,12 @@ void foo(
9292
// CHECK: | `-AttributedType {{.+}} 'int *__single' sugar
9393
// CHECK: | `-PointerType {{.+}} 'int *__single'
9494
// CHECK: | `-BuiltinType {{.+}} 'int'
95-
// CHECK: |-FunctionDecl {{.+}} g_cb 'cb_t':'int *__single __counted_by(count)(int)'
96-
// CHECK: | `-ParmVarDecl {{.+}} implicit 'int'
97-
// CHECK: |-FunctionDecl {{.+}} g_sb 'sb_t':'void *__single __sized_by(size)(int)'
98-
// CHECK: | `-ParmVarDecl {{.+}} implicit 'int'
99-
// CHECK: |-FunctionDecl {{.+}} g_eb 'eb_t':'int *__single __ended_by(end)(int *__single)'
100-
// CHECK: | `-ParmVarDecl {{.+}} implicit 'int *__single'
95+
// CHECK: |-FunctionDecl {{.+}} g_cb 'int *__single __counted_by(count)(int)'
96+
// CHECK: | `-ParmVarDecl {{.+}} implicit used count 'int'
97+
// CHECK: |-FunctionDecl {{.+}} g_sb 'void *__single __sized_by(size)(int)'
98+
// CHECK: | `-ParmVarDecl {{.+}} implicit used size 'int'
99+
// CHECK: |-FunctionDecl {{.+}} g_eb 'int *__single __ended_by(end)(int *__single)'
100+
// CHECK: | `-ParmVarDecl {{.+}} implicit used end 'int *__single'
101101
// CHECK: |-VarDecl {{.+}} g_cb_ptr 'int *__single __counted_by(count)(*__single)(int)'
102102
// CHECK: |-VarDecl {{.+}} g_sb_ptr 'void *__single __sized_by(size)(*__single)(int)'
103103
// CHECK: |-VarDecl {{.+}} g_eb_ptr 'int *__single __ended_by(end)(*__single)(int *__single)'
@@ -116,14 +116,14 @@ void foo(
116116
// CHECK: |-ParmVarDecl {{.+}} a_ptr_eb 'eb_t *__single'
117117
// CHECK: `-CompoundStmt {{.+}}
118118
// CHECK: |-DeclStmt
119-
// CHECK: | `-FunctionDecl {{.+}} l_cb 'cb_t':'int *__single __counted_by(count)(int)'
120-
// CHECK: | `-ParmVarDecl {{.+}} implicit 'int'
119+
// CHECK: | `-FunctionDecl {{.+}} l_cb 'int *__single __counted_by(count)(int)'
120+
// CHECK: | `-ParmVarDecl {{.+}} implicit used count 'int'
121121
// CHECK: |-DeclStmt
122-
// CHECK: | `-FunctionDecl {{.+}} l_sb 'sb_t':'void *__single __sized_by(size)(int)'
123-
// CHECK: | `-ParmVarDecl {{.+}} implicit 'int'
122+
// CHECK: | `-FunctionDecl {{.+}} l_sb 'void *__single __sized_by(size)(int)'
123+
// CHECK: | `-ParmVarDecl {{.+}} implicit used size 'int'
124124
// CHECK: |-DeclStmt
125-
// CHECK: | `-FunctionDecl {{.+}} l_eb 'eb_t':'int *__single __ended_by(end)(int *__single)'
126-
// CHECK: | `-ParmVarDecl {{.+}} implicit 'int *__single'
125+
// CHECK: | `-FunctionDecl {{.+}} l_eb 'int *__single __ended_by(end)(int *__single)'
126+
// CHECK: | `-ParmVarDecl {{.+}} implicit used end 'int *__single'
127127
// CHECK: |-DeclStmt
128128
// CHECK: | `-VarDecl {{.+}} l_cb_ptr 'int *__single __counted_by(count)(*__single)(int)'
129129
// CHECK: |-DeclStmt

0 commit comments

Comments
 (0)