Skip to content

[CodeGen][counted_by] See past parentheses and no-op casts #151266

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 2 commits into from
Jul 30, 2025
Merged

[CodeGen][counted_by] See past parentheses and no-op casts #151266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3e6589
[CodeGen][counted_by] See past parentheses and no-op casts
bwendling Jul 30, 2025
c741ca7
Merge branch 'llvm:main' into counted-by-bdos-bug
bwendling Jul 30, 2025
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
3 changes: 2 additions & 1 deletion clang/lib/CodeGen/CGBuiltin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,8 @@ llvm::Value *CodeGenFunction::emitCountedByPointerSize(
assert(E->getCastKind() == CK_LValueToRValue &&
"must be an LValue to RValue cast");

const MemberExpr *ME = dyn_cast<MemberExpr>(E->getSubExpr());
const MemberExpr *ME =
dyn_cast<MemberExpr>(E->getSubExpr()->IgnoreParenNoopCasts(getContext()));
if (!ME)
return nullptr;

Expand Down
77 changes: 77 additions & 0 deletions clang/test/CodeGen/attr-counted-by-for-pointers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,3 +471,80 @@ size_t test9(struct annotated_sized_ptr *p, int index) {
size_t test10(struct annotated_sized_ptr *p, int index) {
return __bdos(&((unsigned int *) p->buf)[index]);
}

struct pr151236_struct {
int *a __counted_by(a_count);
short a_count;
};

// SANITIZE-WITH-ATTR-LABEL: define dso_local range(i64 -262144, 262137) i64 @test11(
// SANITIZE-WITH-ATTR-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0]] {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: [[COUNTED_BY_GEP:%.*]] = getelementptr inbounds nuw i8, ptr [[P]], i64 8
// SANITIZE-WITH-ATTR-NEXT: [[COUNTED_BY_LOAD:%.*]] = load i16, ptr [[COUNTED_BY_GEP]], align 4
// SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = icmp sgt i16 [[COUNTED_BY_LOAD]], -1
// SANITIZE-WITH-ATTR-NEXT: [[COUNT:%.*]] = sext i16 [[COUNTED_BY_LOAD]] to i64
// SANITIZE-WITH-ATTR-NEXT: [[ARRAY_SIZE:%.*]] = shl nsw i64 [[COUNT]], 3
// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = select i1 [[TMP0]], i64 [[ARRAY_SIZE]], i64 0
// SANITIZE-WITH-ATTR-NEXT: ret i64 [[TMP1]]
//
// NO-SANITIZE-WITH-ATTR-LABEL: define dso_local range(i64 -262144, 262137) i64 @test11(
// NO-SANITIZE-WITH-ATTR-SAME: ptr noundef readonly captures(none) [[P:%.*]]) local_unnamed_addr #[[ATTR1]] {
// NO-SANITIZE-WITH-ATTR-NEXT: entry:
// NO-SANITIZE-WITH-ATTR-NEXT: [[COUNTED_BY_GEP:%.*]] = getelementptr inbounds nuw i8, ptr [[P]], i64 8
// NO-SANITIZE-WITH-ATTR-NEXT: [[COUNTED_BY_LOAD:%.*]] = load i16, ptr [[COUNTED_BY_GEP]], align 4
// NO-SANITIZE-WITH-ATTR-NEXT: [[COUNT:%.*]] = sext i16 [[COUNTED_BY_LOAD]] to i64
// NO-SANITIZE-WITH-ATTR-NEXT: [[ARRAY_SIZE:%.*]] = shl nsw i64 [[COUNT]], 3
// NO-SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = icmp sgt i16 [[COUNTED_BY_LOAD]], -1
// NO-SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = select i1 [[TMP0]], i64 [[ARRAY_SIZE]], i64 0
// NO-SANITIZE-WITH-ATTR-NEXT: ret i64 [[TMP1]]
//
// SANITIZE-WITHOUT-ATTR-LABEL: define dso_local range(i64 0, -1) i64 @test11(
// SANITIZE-WITHOUT-ATTR-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0]] {
// SANITIZE-WITHOUT-ATTR-NEXT: entry:
// SANITIZE-WITHOUT-ATTR-NEXT: ret i64 -2
//
// NO-SANITIZE-WITHOUT-ATTR-LABEL: define dso_local range(i64 0, -1) i64 @test11(
// NO-SANITIZE-WITHOUT-ATTR-SAME: ptr noundef readonly captures(none) [[P:%.*]]) local_unnamed_addr #[[ATTR1]] {
// NO-SANITIZE-WITHOUT-ATTR-NEXT: entry:
// NO-SANITIZE-WITHOUT-ATTR-NEXT: ret i64 -2
//
size_t test11(struct pr151236_struct *p) {
return __bdos(p->a) + __bdos((p->a));
Copy link
Collaborator

@shafik shafik Jul 30, 2025

Choose a reason for hiding this comment

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

Maybe also worth trying a comma operator inside the parens e.g. (0,p->a)

I noted that had the same behavior in the original example: https://godbolt.org/z/j9765v75z

Copy link
Collaborator Author

@bwendling bwendling Jul 30, 2025

Choose a reason for hiding this comment

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

I'm not super worried about that construction, and it can get into very tricky situations. For instance, only the final expression is the "result" of the comma operator, the preceding expressions could be arbitrarily complex, and they cannot have side-effects. This is a can of worms that I want to keep shut (as I hope such usage is extremely rare)... :-)

Copy link
Collaborator

@shafik shafik Aug 1, 2025

Choose a reason for hiding this comment

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

So there are two concerns here 1) What is the behavior we expect and documenting that, via tests one way of doing that 2) Making sure we don't crash or some other bad behavior.

I spend a lot of time triaging and folks do plenty of stuff we don't expect and or discourage and if we can predict some of them in advance that saves us time later on.

Copy link
Collaborator Author

@bwendling bwendling Aug 1, 2025

Choose a reason for hiding this comment

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

What I want is to reject that behavior. It's completely abnormal. But whatever. I'll send a patch.

}

// SANITIZE-WITH-ATTR-LABEL: define dso_local range(i64 -262144, 262137) i64 @test12(
// SANITIZE-WITH-ATTR-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0]] {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: [[COUNTED_BY_GEP:%.*]] = getelementptr inbounds nuw i8, ptr [[P]], i64 8
// SANITIZE-WITH-ATTR-NEXT: [[COUNTED_BY_LOAD:%.*]] = load i16, ptr [[COUNTED_BY_GEP]], align 4
// SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = icmp sgt i16 [[COUNTED_BY_LOAD]], -1
// SANITIZE-WITH-ATTR-NEXT: [[COUNT:%.*]] = sext i16 [[COUNTED_BY_LOAD]] to i64
// SANITIZE-WITH-ATTR-NEXT: [[ARRAY_SIZE:%.*]] = shl nsw i64 [[COUNT]], 3
// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = select i1 [[TMP0]], i64 [[ARRAY_SIZE]], i64 0
// SANITIZE-WITH-ATTR-NEXT: ret i64 [[TMP1]]
//
// NO-SANITIZE-WITH-ATTR-LABEL: define dso_local range(i64 -262144, 262137) i64 @test12(
// NO-SANITIZE-WITH-ATTR-SAME: ptr noundef readonly captures(none) [[P:%.*]]) local_unnamed_addr #[[ATTR1]] {
// NO-SANITIZE-WITH-ATTR-NEXT: entry:
// NO-SANITIZE-WITH-ATTR-NEXT: [[COUNTED_BY_GEP:%.*]] = getelementptr inbounds nuw i8, ptr [[P]], i64 8
// NO-SANITIZE-WITH-ATTR-NEXT: [[COUNTED_BY_LOAD:%.*]] = load i16, ptr [[COUNTED_BY_GEP]], align 4
// NO-SANITIZE-WITH-ATTR-NEXT: [[COUNT:%.*]] = sext i16 [[COUNTED_BY_LOAD]] to i64
// NO-SANITIZE-WITH-ATTR-NEXT: [[ARRAY_SIZE:%.*]] = shl nsw i64 [[COUNT]], 3
// NO-SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = icmp sgt i16 [[COUNTED_BY_LOAD]], -1
// NO-SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = select i1 [[TMP0]], i64 [[ARRAY_SIZE]], i64 0
// NO-SANITIZE-WITH-ATTR-NEXT: ret i64 [[TMP1]]
//
// SANITIZE-WITHOUT-ATTR-LABEL: define dso_local range(i64 0, -1) i64 @test12(
// SANITIZE-WITHOUT-ATTR-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0]] {
// SANITIZE-WITHOUT-ATTR-NEXT: entry:
// SANITIZE-WITHOUT-ATTR-NEXT: ret i64 -2
//
// NO-SANITIZE-WITHOUT-ATTR-LABEL: define dso_local range(i64 0, -1) i64 @test12(
// NO-SANITIZE-WITHOUT-ATTR-SAME: ptr noundef readonly captures(none) [[P:%.*]]) local_unnamed_addr #[[ATTR1]] {
// NO-SANITIZE-WITHOUT-ATTR-NEXT: entry:
// NO-SANITIZE-WITHOUT-ATTR-NEXT: ret i64 -2
//
size_t test12(struct pr151236_struct *p) {
return __bdos(p->a) + __bdos(((int *)p->a));
}