Skip to content

[analyzer] Copy-constructor should trigger a load for check::Location #86403

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
steakhal opened this issue Mar 23, 2024 · 1 comment
Open

Comments

@steakhal
Copy link
Contributor

Recently I was investigating a new diagnostic caused by #72107 ([analyzer] Switch to PostStmt callbacks in ArrayBoundV2).

Eventually drilled down to this:

class Obj {};
void top() {
  Obj buf[10];
  Obj copy(buf[404]); // No issue with ArrayBound, but ArrayBoundV2 reports this
  (void)copy;
}

I was wondering if there is something wrong with check::Location given that we now report this with the PostStmt<ArraySubscriptExpr> callback.
It turns out that the copy-constructor takes the parameter by reference - no surprise. Consequently, there is no lvalue to rvalue conversion (aka. we don't have a read operation). However, one could argue that a copy-constructor should really do a load of the argument passed, thus begs the question:
Shouldn't we force model a "load" for copy-constructor, move-constructor, copy-assignment, move-assignment?

Any opinions?

@steakhal steakhal changed the title [analyzer] Copy-constructor should trigger a load [analyzer] Copy-constructor should trigger a load for check::Location Mar 23, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 23, 2024

@llvm/issue-subscribers-clang-static-analyzer

Author: Balazs Benics (steakhal)

Recently I was investigating a new diagnostic caused by #72107 ([analyzer] Switch to PostStmt callbacks in ArrayBoundV2).

Eventually drilled down to this:

class Obj {};
void top() {
  Obj buf[10];
  Obj copy(buf[404]); // No issue with ArrayBound, but ArrayBoundV2 reports this
  (void)copy;
}

I was wondering if there is something wrong with check::Location given that we now report this with the PostStmt&lt;ArraySubscriptExpr&gt; callback.
It turns out that the copy-constructor takes the parameter by reference - no surprise. Consequently, there is no lvalue to rvalue conversion (aka. we don't have a read operation). However, one could argue that a copy-constructor should really do a load of the argument passed, thus begs the question:
Shouldn't we force model a "load" for copy-constructor, move-constructor, copy-assignment, move-assignment?

Any opinions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants