Skip to content

Comments

Fixes #1074#1685

Open
Vedant2005goyal wants to merge 1 commit intovgvassilev:masterfrom
Vedant2005goyal:issue-1074
Open

Fixes #1074#1685
Vedant2005goyal wants to merge 1 commit intovgvassilev:masterfrom
Vedant2005goyal:issue-1074

Conversation

@Vedant2005goyal
Copy link
Contributor

I have configured the clad both reverse mode and the forward mode to use 'getZeroInit()' for initialization and assignments of variable. I have changed the test suite to align with the changes made. While running the tests locally with ASAN enabled i encountered use-after-poison crash in clad/lib/Analyses/UsefulAnalyzer.cpp. And the fix of that problem (in the UsefulAnalyzer.cpp) is also made in this PR.
Below is the stack trace of the crash which has been resolved:-

Screenshot 2025-12-24 at 11 59 12 PM

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

void UsefulAnalyzer::AnalyzeCFGBlock(const CFGBlock& block) {
for (auto ib = block.end(); ib != block.begin() - 1; ib--) {
if (ib->getKind() == clang::CFGElement::Statement) {
for (const auto& Element : llvm::reverse(block)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "llvm::reverse" is directly included [misc-include-cleaner]

lib/Differentiator/UsefulAnalyzer.cpp:1:

+ #include <llvm/ADT/STLExtras.h>

if (ib->getKind() == clang::CFGElement::Statement) {
for (const auto& Element : llvm::reverse(block)) {
if (Element.getKind() == clang::CFGElement::Statement) {
const clang::Stmt* S = Element.castAs<clang::CFGStmt>().getStmt();
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "clang::Stmt" is directly included [misc-include-cleaner]

lib/Differentiator/UsefulAnalyzer.cpp:1:

+ #include <clang/AST/Stmt.h>

@Vedant2005goyal Vedant2005goyal force-pushed the issue-1074 branch 2 times, most recently from 4411956 to 01f3196 Compare December 24, 2025 19:03
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@codecov
Copy link

codecov bot commented Dec 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@Vedant2005goyal Vedant2005goyal force-pushed the issue-1074 branch 2 times, most recently from 0faa242 to 50b23e4 Compare December 25, 2025 18:11
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

if (auto* IL =
dyn_cast<IntegerLiteral>(Init->IgnoreParenImpCasts())) {
if (IL->getValue() == 0)
VD->setInit(getZeroInit(VD->getType()));
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't this happen in DSClone when building the declaration statement?

Copy link
Contributor Author

@Vedant2005goyal Vedant2005goyal Jan 25, 2026

Choose a reason for hiding this comment

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

Thanks for the review!
That makes sense handling the getZeroInit during the declaration is cleaner and more efficient way than patching in the later stages.
I will make the changes accordingly and push the update.

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants