No runtime diff. No crashes.
8 disappeared:

• 4 core.CallAndMessage (1st arg is uninitialized)
• 2 alpha.deadcode.UnreachableCode (sinked before reaching it)
• 2 core.UndefinedBinaryOperatorResult

2 new reports:

• 1 core.UndefinedBinaryOperatorResult
• 1 alpha.security.ArrayBound

15k+ reports preserved.

I missed this commit because it was not tagged by "[analyzer]".
What is the community preference about tagging CSA commits?

LGTM, good job.

Thanks for updating the summary.

LGTM

We can't just do that check in evalCast because there are many additonal logic before we'd end up in makeNonLoc.

It feels like the parent revision is not correct.
Land it.

In the describeErrnoCheckState() the Errno_MustBeChecked is uncovered by tests.

The generated doc section looks great. The test coverage it excellent, but I would recommend adding tests for covering the following lines:

• ErrnoChecker.cpp:99
• ErrnoModeling.cpp:259
• ErrnoModeling.cpp:264

There are two other uncovered cases, but those are mainly defensive checks, so I don't mind them.

In D127874#3588786, @martong wrote:

If some checker placed a sink node into some block, the successors of the sink block was marked unreachable previously. This leads to an unpleasant situation from the user's point of view since a different bugreport gets correlated with the FPs caused by the sink node of that. This effectively means that for each unconditional sink, we will also have (preferable) an unreachable code report as well.

An example and/or a visualization would be useful for this.

It's quite hard. I could add step numbers so that I could mark which block is being entered and exited the visitation in the recursion.

@aaron.ballman @njames93

It looks great.
Let me check the test coverage and the generated docs page it everything looks great.

Please, consider updating the summary to clearly specify the motivation for making this change.
It describes what this patch intends to do, but I'm about the why.

Sorry for my late reply.

According to my measurements, this change has minor effects (1 new, 20 disappeared reports).
The interesting reports had really long bug paths, so I think there were some default eval called functions down the line which made this patch surface this way.

According to my measurements, this commit will change some reports, but nothing significant (<20) on our testset (except llvm which I did not include).
But more importantly, no taint reports disappeared nor were introduced.

This change introduces a single new report:

In D127838#3584911, @martong wrote:

Umm, why this can't be part of D127836 ?

remove dead code from MallocChecker

It looks everything fine, given that the alpha.unix.Errno will depend on the apimodeling.StdCLibraryFunctions.
This should do the expected errno state propagation by which the Errno checker can report diagnostics - WHILE the StdCLibraryFunctions still won't report diagnostics.
This means that the Errno part /diagnostics can be separately disabled from the diagnostics of StdCLibraryFunctionArgsChecker and other frontend checkers of the StdCLibraryFunctionsChecker modeling part.
Excellent.

I believe evalAssume would be a better fit for diagnosing such issues, but I can see your point that we don't have CheckerContext there to emit reports.
That being said, the check::Location is the best alternative.

Thanks @kazu. I evaluated your results and made a patch as D127836.
I think we can abandon this one.

Conflict markers fixed by 6ccc2733e72017999a94c10147a71ff595286080.

In D127732#3581883, @martong wrote:

LGTM

Do we have a tool to automatically discover unused member functions? (There might be so many of them in CSA...)

In D127742#3581874, @martong wrote:

Why not replace all getAs?

That's a massive change. I actually experimented with it and I see no advantage with that.

In D127306#3581814, @martong wrote:

In D127306#3580981, @steakhal wrote:

• Modify the GenericTaintChecker::isStdin() to look through derived symbols, to mitigate the effect of invalidations.

So, the taint property is still not propagated by the engine after the invalidation. BUT, since we have the

static bool isTaintedOrPointsToTainted(const Expr *E, .... {
  if (isTainted(State, E, C.getLocationContext()) || isStdin(E, C))
    return true;

condition and the modified isStdin, now we consider the Expr* associated to stdin as tainted. Please confirm my understanding is correct.

Use getOriginRegion().

We don't have the consensus to make me confident in landing this change.

ping again; @whisperity

rebase; ping

Please check this again @martong @xazax.hun.
I'll also conduct a measurement, investigating what report changes we experience with this change.

• Modify the GenericTaintChecker::isStdin() to look through derived symbols, to mitigate the effect of invalidations.

• Add the new diag::warn_analyzer_deprecated_option warning to the "deprecated-static-analyzer-flag" DiagGroup to prevent breaking the clang/test/Misc/warning-flags.c test file.

In D126067#3573056, @thakis wrote:

One of your changes broke check-clang: http://45.33.8.238/linux/78236/step_7.txt

Please take a look and revert for now if it takes a while to fix.

(Please run tests locally before committing.)

In D126215#3572984, @thakis wrote:

Looks like this breaks building clang-tidy: http://45.33.8.238/linux/78232/step_4.txt

Please take a look, and revert for now if it takes a while to fix.