Skip to content

[BoundsSafety] make alloc_size imply __sized_by_or_null return type #10991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions clang/docs/InternalsManual.rst
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,17 @@ Description:
This is useful when the argument could be a string in some cases, but
another class in other cases, and it needs to be quoted consistently.

**"unquoted" format**

Example:
``"conflicting attributes were '%select{__counted_by|__sized_by|__counted_by_or_null|__sized_by_or_null}0(%unquoted1)' and '%select{__counted_by|__sized_by|__counted_by_or_null|__sized_by_or_null}2(%unquoted3)'"``
Class:
``Expr, Decl``
Description:
This is a simple formatter which omits the single quotes from a class
that would normally be printed quoted. This is useful when the diagnostic
uses the argument to construct a larger type or expression.

.. _internals-producing-diag:

Producing the Diagnostic
Expand Down
16 changes: 16 additions & 0 deletions clang/include/clang/AST/Attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,22 @@ class ParamIdx {
assertComparable(I);
return Idx == I.Idx;
}
/* TO_UPSTREAM(BoundsSafety) ON */
/// Unlike operator== which requires isValid to be true for both objects,
/// this relaxes that constraint and returns true when comparing two
/// invalid objects. An invalid object does not equal any valid object.
bool equals(const ParamIdx &I) const {
assert(HasThis == I.HasThis &&
"ParamIdx must be for the same function to be compared");
if (isValid() != I.isValid())
return false;
if (!isValid()) {
assert(!I.isValid());
return true;
}
return Idx == I.Idx;
}
/* TO_UPSTREAM(BoundsSafety) OFF */
bool operator!=(const ParamIdx &I) const {
assertComparable(I);
return Idx != I.Idx;
Expand Down
14 changes: 14 additions & 0 deletions clang/include/clang/AST/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -2605,6 +2605,7 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
bool isBidiIndexablePointerType() const;
bool isUnspecifiedPointerType() const;
bool isSafePointerType() const;
bool isImplicitSafePointerType() const;
/* TO_UPSTREAM(BoundsSafety) OFF */
bool isSignableType(const ASTContext &Ctx) const;
bool isSignablePointerType() const;
Expand Down Expand Up @@ -8839,6 +8840,19 @@ inline bool Type::isSafePointerType() const {
isValueTerminatedType());
}

/* TO_UPSTREAM(BoundsSafety) ON */
inline bool Type::isImplicitSafePointerType() const {
if (auto AT = this->getAs<AttributedType>()) {
if (AT->getAttrKind() == attr::PtrAutoAttr) {
assert(isSafePointerType());
return true;
}
return AT->getEquivalentType()->isImplicitSafePointerType();
}
return false;
}
/* TO_UPSTREAM(BoundsSafety) OFF */

inline bool Type::isPointerTypeWithBounds() const {
const auto *PT = dyn_cast<PointerType>(CanonicalType);
return PT && PT->getPointerAttributes().hasUpperBound();
Expand Down
17 changes: 16 additions & 1 deletion clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -3444,6 +3444,9 @@ def err_attribute_only_once_per_parameter : Error<
"%0 attribute can only be applied once per parameter">;
def err_mismatched_uuid : Error<"uuid does not match previous declaration">;
def note_previous_uuid : Note<"previous uuid specified here">;
/* TO_UPSTREAM(BoundsSafety) ON */
def err_mismatched_alloc_size : Error<"'alloc_size' attribute does not match previous declaration">;
/* TO_UPSTREAM(BoundsSafety) OFF */
def warn_attribute_pointers_only : Warning<
"%0 attribute only applies to%select{| constant}1 pointer arguments">,
InGroup<IgnoredAttributes>;
Expand Down Expand Up @@ -12808,7 +12811,19 @@ let CategoryName = "BoundsSafety Pointer Attributes Issue" in {
def err_bounds_safety_conflicting_pointer_attributes : Error<
"%select{array|pointer}0 cannot have more than one %select{bound|type|count|end|terminator}1 attribute">;
def note_bounds_safety_conflicting_pointer_attribute_args : Note<
"conflicting arguments for %select{count|end|terminator}0 were %1 and %2">;
"conflicting arguments for %select{end|terminator}0 were %1 and %2">;
def warn_bounds_safety_ignored_implicit_sized_by_or_null : Warning<
"implicit __sized_by_or_null attribute ignored because of explicit __sized_by">;
def note_bounds_safety_overriding_implicit_sized_by_or_null_silence : Note<
"add _Nonnull qualifier to return type to silence this warning">;
def note_bounds_safety_conflicting_count_attribute : Note<
"conflicting attributes were '%select{__counted_by|__sized_by|__counted_by_or_null|__sized_by_or_null}0(%unquoted1)' and '%select{__counted_by|__sized_by|__counted_by_or_null|__sized_by_or_null}2(%unquoted3)'">;
def note_bounds_safety_implicit_size_from_alloc_size_attr : Note<
"function return type implicitly __sized_by%select{|_or_null}0 because of the function's 'alloc_size' %select{and 'returns_nonnull' attributes|attribute}0">;
def err_invalid_return_type_for_alloc_size : Error<
"invalid return type %0 for function with alloc_size attribute; '__sized_by_or_null(%unquoted1)' or '__sized_by(%unquoted1)' required">;
def note_attribute_inherited : Note<
"attribute inherited from previous declaration here">;
def warn_bounds_safety_duplicate_pointer_attributes : Warning<
"%select{array|pointer}0 annotated with %select{__unsafe_indexable|__bidi_indexable|__indexable|__single|__terminated_by}1 "
"multiple times. Annotate only once to remove this warning">, InGroup<BoundsSafetyRedundantAttribute>;
Expand Down
7 changes: 7 additions & 0 deletions clang/include/clang/Sema/Attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ inline const ParmVarDecl *getFunctionOrMethodParam(const Decl *D,
return nullptr;
}

/* TO_UPSTREAM(BoundsSafety) ON */
inline ParmVarDecl *getFunctionOrMethodParam(Decl *D, unsigned Idx) {
return const_cast<ParmVarDecl *>(
getFunctionOrMethodParam(const_cast<const Decl *>(D), Idx));
}
/* TO_UPSTREAM(BoundsSafety) OFF */

inline QualType getFunctionOrMethodParamType(const Decl *D, unsigned Idx) {
if (const FunctionType *FnTy = D->getFunctionType())
return cast<FunctionProtoType>(FnTy)->getParamType(Idx);
Expand Down
5 changes: 4 additions & 1 deletion clang/include/clang/Sema/ParsedAttr.h
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,10 @@ class ParsedAttributes : public ParsedAttributesView {
void takeAllFrom(ParsedAttributes &Other) {
assert(&Other != this &&
"ParsedAttributes can't take attributes from itself");
addAll(Other.begin(), Other.end());
/* TO_UPSTREAM(BoundsSafety) ON
* Upstream uses addAll() instead, but that changes the attribute order. */
addAllAtEnd(Other.begin(), Other.end());
/* TO_UPSTREAM(BoundsSafety) OFF */
Other.clearListOnly();
pool.takeAllFrom(Other.pool);
}
Expand Down
9 changes: 9 additions & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -5390,6 +5390,9 @@ class Sema final : public SemaBase {
MinSizeAttr *mergeMinSizeAttr(Decl *D, const AttributeCommonInfo &CI);
OptimizeNoneAttr *mergeOptimizeNoneAttr(Decl *D,
const AttributeCommonInfo &CI);
/* TO_UPSTREAM(BoundsSafety) ON */
AllocSizeAttr *mergeAllocSizeAttr(NamedDecl *D, const AllocSizeAttr &ASA);
/* TO_UPSTREAM(BoundsSafety) OFF */
InternalLinkageAttr *mergeInternalLinkageAttr(Decl *D, const ParsedAttr &AL);
InternalLinkageAttr *mergeInternalLinkageAttr(Decl *D,
const InternalLinkageAttr &AL);
Expand Down Expand Up @@ -5485,6 +5488,12 @@ class Sema final : public SemaBase {

void DiagnoseUnknownAttribute(const ParsedAttr &AL);

/* TO_UPSTREAM(BoundsSafety) ON */
QualType
PostProcessBoundsSafetyAllocSizeAttribute(FunctionDecl *EnclosingDecl,
QualType FTy);
/* TO_UPSTREAM(BoundsSafety) OFF */

/// DeclClonePragmaWeak - clone existing decl (maybe definition),
/// \#pragma weak needs a non-definition decl and source may not have one.
NamedDecl *DeclClonePragmaWeak(NamedDecl *ND, const IdentifierInfo *II,
Expand Down
5 changes: 5 additions & 0 deletions clang/lib/AST/ASTDiagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,11 @@ void clang::FormatASTNodeDiagnosticArgument(
}
}

/* TO_UPSTREAM(BoundsSafety) ON */
if (Modifier == "unquoted")
NeedQuotes = false;
/* TO_UPSTREAM(BoundsSafety) OFF */

if (NeedQuotes) {
Output.insert(Output.begin()+OldEnd, '\'');
Output.push_back('\'');
Expand Down
16 changes: 0 additions & 16 deletions clang/lib/Sema/BoundsSafetySuggestions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -490,22 +490,6 @@ bool UnsafeOperationVisitor::FindSingleEntity(
// Don't support indirect calls for now.
return false;
}
if (DirectCallDecl->hasAttr<AllocSizeAttr>() &&
IsReallySinglePtr(DirectCallDecl->getReturnType())) {
// Functions declared like
// void * custom_malloc(size_t s) __attribute__((alloc_size(1)))
//
// are currently are annotated as returning `void *__single` rather
// than `void *__sized_by(s)`. To make the right thing happen at call
// sites `BoundsSafetyPointerPromotionExpr` is used to generate a pointer
// with the appropriate bounds from the `void *__single`. For functions
// like this the warning needs to be suppressed because from the user's
// perspective the returned value is not actually __single.
//
// This code path can be deleted once allocating functions are properly
// annotated with __sized_by_or_null. rdar://117114186
return false;
}

assert(IsReallySinglePtr(DirectCallDecl->getReturnType()));

Expand Down
Loading