Skip to content

[clang][ExprConst] Let diagnostics point to std::allocator calls #123744

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 24, 2025
Merged

[clang][ExprConst] Let diagnostics point to std::allocator calls #123744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions clang/lib/AST/ExprConstant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,7 @@ namespace {
struct StdAllocatorCaller {
unsigned FrameIndex;
QualType ElemType;
const Expr *Call;
explicit operator bool() const { return FrameIndex != 0; };
};

Expand All @@ -1155,7 +1156,7 @@ namespace {
if (CTSD->isInStdNamespace() && ClassII &&
ClassII->isStr("allocator") && TAL.size() >= 1 &&
TAL[0].getKind() == TemplateArgument::Type)
return {Call->Index, TAL[0].getAsType()};
return {Call->Index, TAL[0].getAsType(), Call->CallExpr};
}

return {};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So Call will be nullptr here and following some of the code from createHeapAlloc I am not sure that is safe. Maybe the combination can't happen but it is not obvious to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All callers of getStdAllocatorCaller() check that the returned value converts to true, and Call can't benullptr if that's the case.

Expand Down Expand Up @@ -7033,7 +7034,7 @@ static bool HandleOperatorNewCall(EvalInfo &Info, const CallExpr *E,

QualType AllocType = Info.Ctx.getConstantArrayType(
ElemType, Size, nullptr, ArraySizeModifier::Normal, 0);
APValue *Val = Info.createHeapAlloc(E, AllocType, Result);
APValue *Val = Info.createHeapAlloc(Caller.Call, AllocType, Result);
*Val = APValue(APValue::UninitArray(), 0, Size.getZExtValue());
Result.addArray(Info, E, cast<ConstantArrayType>(AllocType));
return true;
Expand Down
4 changes: 2 additions & 2 deletions clang/test/AST/ByteCode/new-delete.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ namespace std {
using size_t = decltype(sizeof(0));
template<typename T> struct allocator {
constexpr T *allocate(size_t N) {
return (T*)__builtin_operator_new(sizeof(T) * N); // both-note 2{{allocation performed here}} \
return (T*)__builtin_operator_new(sizeof(T) * N); // expected-note 2{{allocation performed here}} \
// #alloc
}
constexpr void deallocate(void *p) {
Expand Down Expand Up @@ -641,7 +641,7 @@ namespace OperatorNewDelete {
p = new int[1]; // both-note {{heap allocation performed here}}
break;
case 2:
p = std::allocator<int>().allocate(1);
p = std::allocator<int>().allocate(1); // ref-note 2{{heap allocation performed here}}
break;
}
switch (dealloc_kind) {
Expand Down
11 changes: 6 additions & 5 deletions clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ static_assert(alloc_from_user_code()); // expected-error {{constant expression}}

namespace std {
using size_t = decltype(sizeof(0));
// FIXME: It would be preferable to point these notes at the location of the call to allocator<...>::[de]allocate instead
template<typename T> struct allocator {
constexpr T *allocate(size_t N) {
return (T*)NEW(sizeof(T) * N); // expected-note 3{{heap allocation}} expected-note {{not deallocated}}
return (T*)NEW(sizeof(T) * N);
}
constexpr void deallocate(void *p) {
DELETE(p); // #dealloc expected-note 2{{'std::allocator<...>::deallocate' used to delete pointer to object allocated with 'new'}}
Expand Down Expand Up @@ -59,7 +58,7 @@ constexpr bool mismatched(int alloc_kind, int dealloc_kind) {
p = new int[1]; // expected-note {{heap allocation}}
break;
case 2:
p = std::allocator<int>().allocate(1);
p = std::allocator<int>().allocate(1); // expected-note 2{{heap allocation}}
break;
}
switch (dealloc_kind) {
Expand All @@ -81,8 +80,10 @@ static_assert(mismatched(2, 0)); // expected-error {{constant expression}} expec
static_assert(mismatched(2, 1)); // expected-error {{constant expression}} expected-note {{in call}}
static_assert(mismatched(2, 2));

constexpr int *escape = std::allocator<int>().allocate(3); // expected-error {{constant expression}} expected-note {{pointer to subobject of heap-allocated}}
constexpr int leak = (std::allocator<int>().allocate(3), 0); // expected-error {{constant expression}}
constexpr int *escape = std::allocator<int>().allocate(3); // expected-error {{constant expression}} expected-note {{pointer to subobject of heap-allocated}} \
// expected-note {{heap allocation performed here}}
constexpr int leak = (std::allocator<int>().allocate(3), 0); // expected-error {{constant expression}} \
// expected-note {{not deallocated}}
constexpr int no_lifetime_start = (*std::allocator<int>().allocate(1) = 1); // expected-error {{constant expression}} expected-note {{assignment to object outside its lifetime}}
constexpr int no_deallocate_nullptr = (std::allocator<int>().deallocate(nullptr), 1); // expected-error {{constant expression}} expected-note {{in call}}
// expected-note@#dealloc {{'std::allocator<...>::deallocate' used to delete a null pointer}}
Expand Down
Loading