From 3901a064b3572a8aed9a7f650bc3c2633d8939a3 Mon Sep 17 00:00:00 2001 From: Patryk Stefanski Date: Wed, 18 Jun 2025 15:11:45 -0700 Subject: [PATCH] [BoundsSafety] Do not merge param/return types if there is no need to If pointers don't have any interesting attributes, there is no need to attempt to merge the types, and we can keep the type unchanged. This will let us keep the sugars used in the new declaration. rdar://153579566 (cherry picked from commit 035302950af0b6ac46793dba0c25c1839d1c7d86) --- clang/lib/AST/ASTContext.cpp | 15 ---------- clang/lib/Sema/SemaDecl.cpp | 30 +++++++++++++++---- .../system-header-merge-bounds-attributed.c | 10 +++++++ .../system-header-merge-bounds-attributed.h | 1 + 4 files changed, 36 insertions(+), 20 deletions(-) diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 973c2c84978c7..80171be23ddfd 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -3707,21 +3707,6 @@ QualType ASTContext::mergeBoundsSafetyPointerTypes( if (OrigDstTy.isNull()) OrigDstTy = DstTy; - // An ugly way to keep va_list typedef in DstTy if the merge type doesn't - // change. - // TODO: We need a general way of not stripping sugars. - QualType DesugaredDstTy; - if (const auto *TDT = dyn_cast(DstTy)) - DesugaredDstTy = TDT->desugar(); - else if (const auto *ET = dyn_cast(DstTy)) - DesugaredDstTy = ET->desugar(); - if (!DesugaredDstTy.isNull()) { - QualType MergeTy = mergeBoundsSafetyPointerTypes(DesugaredDstTy, SrcTy, - MergeFunctor, OrigDstTy); - if (MergeTy == DesugaredDstTy) - return DstTy; - } - // FIXME: a brittle hack to avoid skipping ValueTerminatedType outside // this PtrAutoAttr AttributedType. bool RecoverPtrAuto = false; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 22ecaace79d64..ca34eaee9ada7 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -4040,12 +4040,33 @@ static bool mergeFunctionDeclBoundsAttributes(FunctionDecl *New, } return MergeTy; }; + + auto hasBoundsAttributesAtAnyLevel = [](QualType Ty) -> bool { + while (Ty->isPointerType()) { + if (!Ty->isUnspecifiedPointerType()) + return true; + Ty = Ty->getPointeeType(); + } + return false; + }; + + auto mergePointersWithAttributes = [&](QualType NewTy, + QualType OldTy) -> QualType { + // Do not attempt to merge if there is no need to do so. This will let us + // keep the sugars used in the new declaration. + if (!hasBoundsAttributesAtAnyLevel(NewTy) && + !hasBoundsAttributesAtAnyLevel(OldTy)) + return NewTy; + return Self.Context.mergeBoundsSafetyPointerTypes(NewTy, OldTy, + mergeBoundsAttributes); + }; + QualType NewRetTy = New->getReturnType(); if (NewRetTy->isBoundsAttributedType() || NewRetTy->isValueTerminatedType()) return false; - QualType MergeRetTy = Self.Context.mergeBoundsSafetyPointerTypes( - New->getReturnType(), Old->getReturnType(), mergeBoundsAttributes); + QualType MergeRetTy = + mergePointersWithAttributes(New->getReturnType(), Old->getReturnType()); if (MergeRetTy.isNull()) return false; @@ -4055,9 +4076,8 @@ static bool mergeFunctionDeclBoundsAttributes(FunctionDecl *New, if (NewParmTy->isBoundsAttributedType() || NewParmTy->isValueTerminatedType()) return false; - QualType MergeParamTy = Self.Context.mergeBoundsSafetyPointerTypes( - NewParmTy, Old->getParamDecl(i)->getType(), - mergeBoundsAttributes); + QualType MergeParamTy = + mergePointersWithAttributes(NewParmTy, Old->getParamDecl(i)->getType()); if (MergeParamTy.isNull()) return false; if (const auto *CATy = MergeParamTy->getAs()) { diff --git a/clang/test/BoundsSafety/AST/system-header-merge-bounds-attributed.c b/clang/test/BoundsSafety/AST/system-header-merge-bounds-attributed.c index eed32ffe1f8aa..3e9adc3bde2ff 100644 --- a/clang/test/BoundsSafety/AST/system-header-merge-bounds-attributed.c +++ b/clang/test/BoundsSafety/AST/system-header-merge-bounds-attributed.c @@ -29,6 +29,9 @@ // CHECK: FunctionDecl {{.+}} eb // CHECK: |-ParmVarDecl {{.+}} eb_p 'void *' // CHECK: `-ParmVarDecl {{.+}} end 'void *' +// CHECK: FunctionDecl {{.+}} cb_array +// CHECK: |-ParmVarDecl {{.+}} array 'int *' +// CHECK: `-ParmVarDecl {{.+}} len 'int' // Check if we can override them. @@ -39,6 +42,7 @@ void cb_out_count(int *__counted_by(*len) cb_out_len_p, int *len); void cbn(int *__counted_by_or_null(len) cbn_p, int len); void sb(void *__sized_by(size) sb_p, int size); void eb(void *__ended_by(end) eb_p, void *end); +void cb_array(int array[__counted_by(len)], int len); // CHECK: FunctionDecl {{.+}} prev {{.+}} cb_in // CHECK: |-ParmVarDecl {{.+}} cb_in_p 'int *{{.*}} __counted_by(len)' @@ -58,6 +62,9 @@ void eb(void *__ended_by(end) eb_p, void *end); // CHECK: FunctionDecl {{.+}} prev {{.+}} eb // CHECK: |-ParmVarDecl {{.+}} used eb_p 'void *{{.*}} __ended_by(end)' // CHECK: `-ParmVarDecl {{.+}} used end 'void *{{.*}} /* __started_by(eb_p) */ ' +// CHECK: FunctionDecl {{.+}} prev {{.+}} cb_array +// CHECK: |-ParmVarDecl {{.+}} array 'int *{{.*}} __counted_by(len)' +// CHECK: `-ParmVarDecl {{.+}} used len 'int' // Check if the attributes are merged. @@ -82,3 +89,6 @@ void eb(void *__ended_by(end) eb_p, void *end); // CHECK: FunctionDecl {{.+}} prev {{.+}} eb // CHECK: |-ParmVarDecl {{.+}} used eb_p 'void *{{.*}} __ended_by(end)' // CHECK: `-ParmVarDecl {{.+}} used end 'void *{{.*}} /* __started_by(eb_p) */ ' +// CHECK: FunctionDecl {{.+}} prev {{.+}} cb_array +// CHECK: |-ParmVarDecl {{.+}} array 'int *{{.*}} __counted_by(len)' +// CHECK: `-ParmVarDecl {{.+}} used len 'int' diff --git a/clang/test/BoundsSafety/AST/system-header-merge-bounds-attributed.h b/clang/test/BoundsSafety/AST/system-header-merge-bounds-attributed.h index b3d0fcb78790e..abef43f41a6f0 100644 --- a/clang/test/BoundsSafety/AST/system-header-merge-bounds-attributed.h +++ b/clang/test/BoundsSafety/AST/system-header-merge-bounds-attributed.h @@ -6,3 +6,4 @@ void cb_out_count(int *cb_out_len_p, int *len); void cbn(int *cbn_p, int len); void sb(void *sb_p, int size); void eb(void *eb_p, void *end); +void cb_array(int array[], int len);