This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker
ClosedPublic

Authored by ASDenysPetrov on Jul 11 2022, 9:19 AM.

Details

Summary

Introduce a new function clang_analyzer_value. It emits a report that in turn prints a RangeSet or APSInt associated with SVal. If there is no associated value, prints n/a.

Diff Detail

Event Timeline

ASDenysPetrov created this revision.Jul 11 2022, 9:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2022, 9:19 AM
ASDenysPetrov requested review of this revision.Jul 11 2022, 9:19 AM
NoQ added a comment.Jul 11 2022, 8:50 PM

Looks great!

Maybe clang_analyzer_range() instead?

Please add documentation to https://clang.llvm.org/docs/analyzer/developer-docs/DebugChecks.html

clang/test/Analysis/print-ranges.cpp
2

I suspect this test will crash when clang is built with Z3, because the Z3 constraint manager doesn't implement your new function yet.

ASDenysPetrov added a comment.EditedJul 12 2022, 11:48 PM

Maybe clang_analyzer_range() instead?

Thanks for the notes, @NoQ .
This was its first name. I refused. First, because it emits concrete integers as well and moreover we can extend it for arrays or strings e.g. Second, the ranges is just an implementation detail and an actual thing we want to see is an associated value.

Please add documentation to https://clang.llvm.org/docs/analyzer/developer-docs/DebugChecks.html

I'll do.

clang/test/Analysis/print-ranges.cpp
2

Agree. Is it enough REQUIRES: no-z3 or to add #ifdef ANALYZER_CM_Z3?

Constrained the test by adding llvm-lit REQUIRES command.
Documented a new function at https://clang.llvm.org/docs/analyzer/developer-docs/DebugChecks.html

NoQ added a comment.Jul 13 2022, 8:23 PM

Maybe clang_analyzer_range() instead?

This was its first name. I refused. First, because it emits concrete integers as well and moreover we can extend it for arrays or strings e.g. Second, the ranges is just an implementation detail and an actual thing we want to see is an associated value.

Ok how about clang_analyzer_constraint()? A concrete value could be thought of as constraint, and so can be range, or anything else any exotic constraint managers may decide to dump.

clang/test/Analysis/print-ranges.cpp
2

That should be good. My personal tradition in such cases is to double-check that this doesn't disable the test entirely.

Very good!

Maybe clang_analyzer_range() instead?

This was its first name. I refused. First, because it emits concrete integers as well and moreover we can extend it for arrays or strings e.g. Second, the ranges is just an implementation detail and an actual thing we want to see is an associated value.

Ok how about clang_analyzer_constraint()? A concrete value could be thought of as constraint, and so can be range, or anything else any exotic constraint managers may decide to dump.

We associate a value to a variable even if that is unconstrained. That value is the whole range of the variable's type. In this sense, I vote for clang_analyzer_value.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
126

printValue would be more general than printRange which is unique for the range based constraint manager.

clang/test/Analysis/print-ranges.cpp
1

Don't forget to pin the target/triple.

Stick to name clang_analyzer_value.
Change function name from printRange to printValue.
Make description more precise in the documentation.

This revision is now accepted and ready to land.Jul 15 2022, 12:36 AM
clang/test/Analysis/print-ranges.cpp
1

Why does this specific test need it?

2

I added REQUIRES: z3 it prints Unsupported, then added REQUIRES: no-z3 and it passed.

martong added inline comments.Jul 18 2022, 7:13 AM
clang/test/Analysis/print-ranges.cpp
1

I thought, the bit widths down below might change with different architectures and some build bots might break because of that.