Skip to content
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

[Bug Report] Use after move in run_operation.cpp #15073

Closed
marty1885 opened this issue Nov 15, 2024 · 5 comments
Closed

[Bug Report] Use after move in run_operation.cpp #15073

marty1885 opened this issue Nov 15, 2024 · 5 comments
Assignees
Labels
bug Something isn't working community P0

Comments

@marty1885
Copy link
Contributor

Describe the bug
This is a bug discovered by clang-analyzer following a long discussion on discord. In run_operation.cpp. The operation object is moved before calling operation.compute_output_shapes(). Causing a UB. I am not sure weather if it is causing any actual trouble now, but probably worth fixing.

Due to the nature of non-const functions. I am unable to fix myself as I lack the low level understanding of how this part of program works.

To Reproduce

In ttnn/cpp/ttnn/run_operation.cpp, line 372-374 and 437-437

Image

Image

Expected behavior
Should not trigger UB.

Screenshots
If applicable, add screenshots to help explain your problem.

Please complete the following environment information:

Additional context
Add any other context about the problem here.

@marty1885 marty1885 added the bug Something isn't working label Nov 15, 2024
@ayerofieiev-tt
Copy link
Member

Thank you! On it!

@ayerofieiev-tt ayerofieiev-tt self-assigned this Nov 15, 2024
@jvasilje
Copy link
Collaborator

Thanks @ayerofieiev-tt ! <3

@ayerofieiev-tt
Copy link
Member

Fixing this specific issue.

Then we should enable the clang-tidy check and fixing any issue that it surfaces.

-bugprone-use-after-move,

CC @blozano-tt @afuller-TT

@marty1885
Copy link
Contributor Author

Thanks for the quick patch! There shouldn't be any more as of now. I ran clang-analyzer over the entire codebase and this is the only use after move.

ayerofieiev-tt added a commit that referenced this issue Nov 15, 2024
### Ticket
#15073 

### Problem description
Recent changes cause a use after move

### What's changed
Fixing the issue.
@blozano-tt @afuller-TT will help to enable a corresponding clang-tidy
check

https://github.com/tenstorrent/tt-metal/blob/008c50a744ab398526f12992f9f4902283f9215a/.clang-tidy#L28


### Checklist
- [x] [Post commit
CI](https://github.com/tenstorrent/tt-metal/actions/runs/11850149066)
@marty1885
Copy link
Contributor Author

Close as patch merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working community P0
Projects
None yet
Development

No branches or pull requests

3 participants