Page MenuHomePhabricator

[Analysis] Ignore casts and unary ops for uninitialized values
ClosedPublic

Authored by void on Nov 30 2021, 11:24 PM.

Details

Summary

A series of unary operators and casts may obscure the variable we're
trying to analyze. Ignore them for the uninitialized value analysis.
Other checks determine if the unary operators result in a valid l-value.

Link: https://github.com/ClangBuiltLinux/linux/issues/1521

Diff Detail

Event Timeline

void requested review of this revision.Nov 30 2021, 11:24 PM
void created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptNov 30 2021, 11:24 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

I tested the problematic Linux kernel configuration that uncovered this issue with this patch and the issue was resolved. I tested an x86_64 allmodconfig build and did not see any warnings.

clang/lib/Analysis/UninitializedValues.cpp
594

Should "it's don't count" be "don't count"?

void updated this revision to Diff 391086.Dec 1 2021, 11:06 AM
void marked an inline comment as done.

Pretend that I've spoken English for most of my lyfe.

clang/lib/Analysis/UninitializedValues.cpp
594

You does not like mine English? :-P

void added a comment.Dec 6 2021, 7:45 AM

Friendly ping to reviewers. :-)

clang/lib/Analysis/UninitializedValues.cpp
819–820

Q: are there any unary operators that are not casts? For example isn't something like -x a unary negation operator? I worry that this could be an infinite loop as written.

If this fear is unfounded, then I think the code could be rewritten as:

while (const auto *UO = dyn_cast<UnaryOperator>(Ex))
  Ex = stripCasts(C, UO->getSubExpr());
void added inline comments.Dec 6 2021, 12:52 PM
clang/lib/Analysis/UninitializedValues.cpp
819–820

It's not just casts that I want to remove. In particular, the & and * operators need to be removed to get to the variable. It *should* eventually get to a variable or non-unary expression once it goes through all of the unary operators and casts.

Note there are other sema checks that determine if the expression is a proper l-value, so I can throw away the unary-ops.

nickdesaulniers added inline comments.Dec 6 2021, 4:32 PM
clang/lib/Analysis/UninitializedValues.cpp
819–820

Ok, my original concern seems fine.

I still think you should rewrite the loop as suggested. At the least, a dyn_cast shouldn't be necessary once we've already checked with isa; cast could simply be used. Or even better just one dyn_cast as suggested.

nickdesaulniers accepted this revision.Dec 6 2021, 4:32 PM
This revision is now accepted and ready to land.Dec 6 2021, 4:32 PM
void updated this revision to Diff 392281.Dec 6 2021, 9:48 PM

Simplify the loop.

nickdesaulniers accepted this revision.Dec 7 2021, 10:20 AM