Skip to content

Commit

Permalink
merge main into amd-staging
Browse files Browse the repository at this point in the history
Change-Id: I165371edbfc212db7fdac6e5d7d2c7e750ac9ad3
  • Loading branch information
Jenkins committed Oct 15, 2024
2 parents 896ab66 + b0a2546 commit e9ae989
Show file tree
Hide file tree
Showing 801 changed files with 57,772 additions and 34,923 deletions.
3 changes: 1 addition & 2 deletions clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,7 @@ Example usage for a project using a compile commands database:
llvm::StringMap<std::vector<StringRef>> USRToBitcode;
Executor->get()->getToolResults()->forEachResult(
[&](StringRef Key, StringRef Value) {
auto R = USRToBitcode.try_emplace(Key, std::vector<StringRef>());
R.first->second.emplace_back(Value);
USRToBitcode[Key].emplace_back(Value);
});

// Collects all Infos according to their unique USR value. This map is added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name,
Options.get("WarnOnSizeOfCompareToConstant", true)),
WarnOnSizeOfPointerToAggregate(
Options.get("WarnOnSizeOfPointerToAggregate", true)),
WarnOnSizeOfPointer(Options.get("WarnOnSizeOfPointer", false)) {}
WarnOnSizeOfPointer(Options.get("WarnOnSizeOfPointer", false)),
WarnOnOffsetDividedBySizeOf(
Options.get("WarnOnOffsetDividedBySizeOf", true)) {}

void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
Expand All @@ -82,6 +84,8 @@ void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "WarnOnSizeOfPointerToAggregate",
WarnOnSizeOfPointerToAggregate);
Options.store(Opts, "WarnOnSizeOfPointer", WarnOnSizeOfPointer);
Options.store(Opts, "WarnOnOffsetDividedBySizeOf",
WarnOnOffsetDividedBySizeOf);
}

void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
Expand Down Expand Up @@ -307,7 +311,8 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
offsetOfExpr()))
.bind("sizeof-in-ptr-arithmetic-scale-expr");
const auto PtrArithmeticIntegerScaleExpr = binaryOperator(
hasAnyOperatorName("*", "/"),
WarnOnOffsetDividedBySizeOf ? binaryOperator(hasAnyOperatorName("*", "/"))
: binaryOperator(hasOperatorName("*")),
// sizeof(...) * sizeof(...) and sizeof(...) / sizeof(...) is handled
// by this check on another path.
hasOperands(expr(hasType(isInteger()), unless(SizeofLikeScaleExpr)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class SizeofExpressionCheck : public ClangTidyCheck {
const bool WarnOnSizeOfCompareToConstant;
const bool WarnOnSizeOfPointerToAggregate;
const bool WarnOnSizeOfPointer;
const bool WarnOnOffsetDividedBySizeOf;
};

} // namespace clang::tidy::bugprone
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ Changes in existing checks
- Improved :doc:`bugprone-sizeof-expression
<clang-tidy/checks/bugprone/sizeof-expression>` check to find suspicious
usages of ``sizeof()``, ``alignof()``, and ``offsetof()`` when adding or
subtracting from a pointer.
subtracting from a pointer directly or when used to scale a numeric value.

- Improved :doc:`bugprone-unchecked-optional-access
<clang-tidy/checks/bugprone/unchecked-optional-access>` to support
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,17 @@ and the result is typically unintended, often out of bounds.
``Ptr + sizeof(T)`` will offset the pointer by ``sizeof(T)`` elements,
effectively exponentiating the scaling factor to the power of 2.

Similarly, multiplying or dividing a numeric value with the ``sizeof`` of an
element or the whole buffer is suspicious, because the dimensional connection
between the numeric value and the actual ``sizeof`` result can not always be
deduced.
While scaling an integer up (multiplying) with ``sizeof`` is likely **always**
an issue, a scaling down (division) is not always inherently dangerous, in case
the developer is aware that the division happens between an appropriate number
of _bytes_ and a ``sizeof`` value.
Turning :option:`WarnOnOffsetDividedBySizeOf` off will restrict the
warnings to the multiplication case.

This case also checks suspicious ``alignof`` and ``offsetof`` usages in
pointer arithmetic, as both return the "size" in bytes and not elements,
potentially resulting in doubly-scaled offsets.
Expand Down Expand Up @@ -299,3 +310,9 @@ Options
``sizeof`` is an expression that produces a pointer (except for a few
idiomatic expressions that are probably intentional and correct).
This detects occurrences of CWE 467. Default is `false`.

.. option:: WarnOnOffsetDividedBySizeOf

When `true`, the check will warn on pointer arithmetic where the
element count is obtained from a division with ``sizeof(...)``,
e.g., ``Ptr + Bytes / sizeof(*T)``. Default is `true`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t -- \
// RUN: -config='{CheckOptions: { \
// RUN: bugprone-sizeof-expression.WarnOnOffsetDividedBySizeOf: false \
// RUN: }}'

typedef __SIZE_TYPE__ size_t;

void situational14(int *Buffer, size_t BufferSize) {
int *P = &Buffer[0];
while (P < Buffer + BufferSize / sizeof(*Buffer)) {
// NO-WARNING: This test opted out of "P +- N */ sizeof(...)" warnings.
++P;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -352,21 +352,30 @@ void good13(void) {
int Buffer[BufferSize];

int *P = &Buffer[0];
while (P < (Buffer + sizeof(Buffer) / sizeof(int))) {
while (P < Buffer + sizeof(Buffer) / sizeof(int)) {
// NO-WARNING: Calculating the element count of the buffer here, which is
// safe with this idiom (as long as the types don't change).
++P;
}

while (P < (Buffer + sizeof(Buffer) / sizeof(Buffer[0]))) {
while (P < Buffer + sizeof(Buffer) / sizeof(Buffer[0])) {
// NO-WARNING: Calculating the element count of the buffer here, which is
// safe with this idiom.
++P;
}

while (P < (Buffer + sizeof(Buffer) / sizeof(*P))) {
while (P < Buffer + sizeof(Buffer) / sizeof(*P)) {
// NO-WARNING: Calculating the element count of the buffer here, which is
// safe with this idiom.
++P;
}
}

void situational14(int *Buffer, size_t BufferSize) {
int *P = &Buffer[0];
while (P < Buffer + BufferSize / sizeof(*Buffer)) {
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+' operator
// CHECK-MESSAGES: :[[@LINE-2]]:21: note: '+' in pointer arithmetic internally scales with 'sizeof(int)' == {{[0-9]+}}
++P;
}
}
42 changes: 16 additions & 26 deletions clang/include/clang/AST/OpenACCClause.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,32 +119,6 @@ class OpenACCSeqClause : public OpenACCClause {
}
};

// Not yet implemented, but the type name is necessary for 'seq' diagnostics, so
// this provides a basic, do-nothing implementation. We still need to add this
// type to the visitors/etc, as well as get it to take its proper arguments.
class OpenACCVectorClause : public OpenACCClause {
protected:
OpenACCVectorClause(SourceLocation BeginLoc, SourceLocation EndLoc)
: OpenACCClause(OpenACCClauseKind::Vector, BeginLoc, EndLoc) {
llvm_unreachable("Not yet implemented");
}

public:
static bool classof(const OpenACCClause *C) {
return C->getClauseKind() == OpenACCClauseKind::Vector;
}

static OpenACCVectorClause *
Create(const ASTContext &Ctx, SourceLocation BeginLoc, SourceLocation EndLoc);

child_range children() {
return child_range(child_iterator(), child_iterator());
}
const_child_range children() const {
return const_child_range(const_child_iterator(), const_child_iterator());
}
};

/// Represents a clause that has a list of parameters.
class OpenACCClauseWithParams : public OpenACCClause {
/// Location of the '('.
Expand Down Expand Up @@ -531,6 +505,22 @@ class OpenACCWorkerClause : public OpenACCClauseWithSingleIntExpr {
SourceLocation EndLoc);
};

class OpenACCVectorClause : public OpenACCClauseWithSingleIntExpr {
protected:
OpenACCVectorClause(SourceLocation BeginLoc, SourceLocation LParenLoc,
Expr *IntExpr, SourceLocation EndLoc);

public:
static bool classof(const OpenACCClause *C) {
return C->getClauseKind() == OpenACCClauseKind::Vector;
}

static OpenACCVectorClause *Create(const ASTContext &Ctx,
SourceLocation BeginLoc,
SourceLocation LParenLoc, Expr *IntExpr,
SourceLocation EndLoc);
};

class OpenACCNumWorkersClause : public OpenACCClauseWithSingleIntExpr {
OpenACCNumWorkersClause(SourceLocation BeginLoc, SourceLocation LParenLoc,
Expr *IntExpr, SourceLocation EndLoc);
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -12700,7 +12700,7 @@ def err_acc_gang_dim_value
def err_acc_num_arg_conflict
: Error<"'num' argument to '%0' clause not allowed on a 'loop' construct "
"associated with a 'kernels' construct that has a "
"'%select{num_gangs|num_workers}1' "
"'%select{num_gangs|num_workers|vector_length}1' "
"clause">;
def err_acc_clause_in_clause_region
: Error<"loop with a '%0' clause may not exist in the region of a '%1' "
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Basic/OpenACCClauses.def
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ VISIT_CLAUSE(Reduction)
VISIT_CLAUSE(Self)
VISIT_CLAUSE(Seq)
VISIT_CLAUSE(Tile)
VISIT_CLAUSE(Vector)
VISIT_CLAUSE(VectorLength)
VISIT_CLAUSE(Wait)
VISIT_CLAUSE(Worker)
Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Sema/SemaOpenACC.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ class SemaOpenACC : public SemaBase {
/// permits us to implement the restriction of no further 'gang' or 'worker'
/// clauses.
SourceLocation LoopWorkerClauseLoc;
/// If there is a current 'active' loop construct with a 'vector' clause on it
/// (on any sort of construct), this has the source location for it. This
/// permits us to implement the restriction of no further 'gang', 'vector', or
/// 'worker' clauses.
SourceLocation LoopVectorClauseLoc;

// Redeclaration of the version in OpenACCClause.h.
using DeviceTypeArgument = std::pair<IdentifierInfo *, SourceLocation>;
Expand Down Expand Up @@ -679,6 +684,7 @@ class SemaOpenACC : public SemaBase {
OpenACCDirectiveKind DirKind;
SourceLocation OldLoopGangClauseOnKernelLoc;
SourceLocation OldLoopWorkerClauseLoc;
SourceLocation OldLoopVectorClauseLoc;
llvm::SmallVector<OpenACCLoopConstruct *> ParentlessLoopConstructs;
LoopInConstructRAII LoopRAII;

Expand Down
7 changes: 7 additions & 0 deletions clang/lib/AST/ByteCode/Interp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,13 @@ bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
if (D->isConstexpr())
return true;

// If we're evaluating the initializer for a constexpr variable in C23, we may
// only read other contexpr variables. Abort here since this one isn't
// constexpr.
if (const auto *VD = dyn_cast_if_present<VarDecl>(S.EvaluatingDecl);
VD && VD->isConstexpr() && S.getLangOpts().C23)
return Invalid(S, OpPC);

QualType T = D->getType();
bool IsConstant = T.isConstant(S.getASTContext());
if (T->isIntegralOrEnumerationType()) {
Expand Down
65 changes: 46 additions & 19 deletions clang/lib/AST/ByteCode/InterpBuiltin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,39 +333,48 @@ static bool interp__builtin_copysign(InterpState &S, CodePtr OpPC,
}

static bool interp__builtin_fmin(InterpState &S, CodePtr OpPC,
const InterpFrame *Frame, const Function *F) {
const InterpFrame *Frame, const Function *F,
bool IsNumBuiltin) {
const Floating &LHS = getParam<Floating>(Frame, 0);
const Floating &RHS = getParam<Floating>(Frame, 1);

Floating Result;

// When comparing zeroes, return -0.0 if one of the zeroes is negative.
if (LHS.isZero() && RHS.isZero() && RHS.isNegative())
Result = RHS;
else if (LHS.isNan() || RHS < LHS)
Result = RHS;
else
Result = LHS;
if (IsNumBuiltin) {
Result = llvm::minimumnum(LHS.getAPFloat(), RHS.getAPFloat());
} else {
// When comparing zeroes, return -0.0 if one of the zeroes is negative.
if (LHS.isZero() && RHS.isZero() && RHS.isNegative())
Result = RHS;
else if (LHS.isNan() || RHS < LHS)
Result = RHS;
else
Result = LHS;
}

S.Stk.push<Floating>(Result);
return true;
}

static bool interp__builtin_fmax(InterpState &S, CodePtr OpPC,
const InterpFrame *Frame,
const Function *Func) {
const InterpFrame *Frame, const Function *Func,
bool IsNumBuiltin) {
const Floating &LHS = getParam<Floating>(Frame, 0);
const Floating &RHS = getParam<Floating>(Frame, 1);

Floating Result;

// When comparing zeroes, return +0.0 if one of the zeroes is positive.
if (LHS.isZero() && RHS.isZero() && LHS.isNegative())
Result = RHS;
else if (LHS.isNan() || RHS > LHS)
Result = RHS;
else
Result = LHS;
if (IsNumBuiltin) {
Result = llvm::maximumnum(LHS.getAPFloat(), RHS.getAPFloat());
} else {
// When comparing zeroes, return +0.0 if one of the zeroes is positive.
if (LHS.isZero() && RHS.isZero() && LHS.isNegative())
Result = RHS;
else if (LHS.isNan() || RHS > LHS)
Result = RHS;
else
Result = LHS;
}

S.Stk.push<Floating>(Result);
return true;
Expand Down Expand Up @@ -1701,7 +1710,16 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
case Builtin::BI__builtin_fminl:
case Builtin::BI__builtin_fminf16:
case Builtin::BI__builtin_fminf128:
if (!interp__builtin_fmin(S, OpPC, Frame, F))
if (!interp__builtin_fmin(S, OpPC, Frame, F, /*IsNumBuiltin=*/false))
return false;
break;

case Builtin::BI__builtin_fminimum_num:
case Builtin::BI__builtin_fminimum_numf:
case Builtin::BI__builtin_fminimum_numl:
case Builtin::BI__builtin_fminimum_numf16:
case Builtin::BI__builtin_fminimum_numf128:
if (!interp__builtin_fmin(S, OpPC, Frame, F, /*IsNumBuiltin=*/true))
return false;
break;

Expand All @@ -1710,7 +1728,16 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
case Builtin::BI__builtin_fmaxl:
case Builtin::BI__builtin_fmaxf16:
case Builtin::BI__builtin_fmaxf128:
if (!interp__builtin_fmax(S, OpPC, Frame, F))
if (!interp__builtin_fmax(S, OpPC, Frame, F, /*IsNumBuiltin=*/false))
return false;
break;

case Builtin::BI__builtin_fmaximum_num:
case Builtin::BI__builtin_fmaximum_numf:
case Builtin::BI__builtin_fmaximum_numl:
case Builtin::BI__builtin_fmaximum_numf16:
case Builtin::BI__builtin_fmaximum_numf128:
if (!interp__builtin_fmax(S, OpPC, Frame, F, /*IsNumBuiltin=*/true))
return false;
break;

Expand Down
31 changes: 27 additions & 4 deletions clang/lib/AST/OpenACCClause.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ bool OpenACCClauseWithCondition::classof(const OpenACCClause *C) {
bool OpenACCClauseWithSingleIntExpr::classof(const OpenACCClause *C) {
return OpenACCNumWorkersClause::classof(C) ||
OpenACCVectorLengthClause::classof(C) ||
OpenACCWorkerClause::classof(C) || OpenACCCollapseClause::classof(C) ||
OpenACCAsyncClause::classof(C);
OpenACCVectorClause::classof(C) || OpenACCWorkerClause::classof(C) ||
OpenACCCollapseClause::classof(C) || OpenACCAsyncClause::classof(C);
}
OpenACCDefaultClause *OpenACCDefaultClause::Create(const ASTContext &C,
OpenACCDefaultClauseKind K,
Expand Down Expand Up @@ -424,11 +424,24 @@ OpenACCWorkerClause *OpenACCWorkerClause::Create(const ASTContext &C,
return new (Mem) OpenACCWorkerClause(BeginLoc, LParenLoc, IntExpr, EndLoc);
}

OpenACCVectorClause::OpenACCVectorClause(SourceLocation BeginLoc,
SourceLocation LParenLoc,
Expr *IntExpr, SourceLocation EndLoc)
: OpenACCClauseWithSingleIntExpr(OpenACCClauseKind::Vector, BeginLoc,
LParenLoc, IntExpr, EndLoc) {
assert((!IntExpr || IntExpr->isInstantiationDependent() ||
IntExpr->getType()->isIntegerType()) &&
"Int expression type not scalar/dependent");
}

OpenACCVectorClause *OpenACCVectorClause::Create(const ASTContext &C,
SourceLocation BeginLoc,
SourceLocation LParenLoc,
Expr *IntExpr,
SourceLocation EndLoc) {
void *Mem = C.Allocate(sizeof(OpenACCVectorClause));
return new (Mem) OpenACCVectorClause(BeginLoc, EndLoc);
void *Mem =
C.Allocate(sizeof(OpenACCVectorClause), alignof(OpenACCVectorClause));
return new (Mem) OpenACCVectorClause(BeginLoc, LParenLoc, IntExpr, EndLoc);
}

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -662,3 +675,13 @@ void OpenACCClausePrinter::VisitWorkerClause(const OpenACCWorkerClause &C) {
OS << ")";
}
}

void OpenACCClausePrinter::VisitVectorClause(const OpenACCVectorClause &C) {
OS << "vector";

if (C.hasIntExpr()) {
OS << "(length: ";
printExpr(C.getIntExpr());
OS << ")";
}
}
Loading

0 comments on commit e9ae989

Please sign in to comment.