How to fix null-deref-ps.c on FreeBSD?

This test fails on FreeBSD, and the following patch fixes it:

Index: test/Analysis/null-deref-ps.c

Hi Ben,

The analyzer knows that all functions with the 'noreturn' attribute don't return. What does the assert macro expand to on FreeBSD?

Hi Ben,

The analyzer knows that all functions with the 'noreturn' attribute don't
return. What does the assert macro expand to on FreeBSD?

The problem is that __assert does not have noreturn set on FreeBSD.
I'm in the process of getting that fixed - but as an apprentice
FreeBSD committer that's probably harder for me than anyone else, so
it may take a while :slight_smile:

In the meantime, I think the right answer is for me to patch
/usr/include/assert.h.

__assert isn’t the only “panic” function that isn’t marked with noreturn. Since it occurs so frequently, I can just add it to the list of hard-coded functions that the analyzer knows about. That will also make the analyzer more applicable to earlier versions of FreeBSD (should you want to see how far a bug goes back, for example).

Hi Ben,

The analyzer knows that all functions with the 'noreturn' attribute don't

return. What does the assert macro expand to on FreeBSD?

The problem is that __assert does not have noreturn set on FreeBSD.
I'm in the process of getting that fixed - but as an apprentice
FreeBSD committer that's probably harder for me than anyone else, so
it may take a while :slight_smile:

In the meantime, I think the right answer is for me to patch
/usr/include/assert.h.

__assert isn't the only "panic" function that isn't marked with noreturn.
Since it occurs so frequently, I can just add it to the list of hard-coded
functions that the analyzer knows about.

I wondered if such a thing existed. Where is it?

It's this hackish blob of code in GRExprEngine.cpp. Search for "panic". Eventually this code will get reworked to be less hackish, but we will always have to maintain a list of some of these functions somewhere.

I just added "_assert" to that list:

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20090216/012556.html

It joins the ranks of functions such as _assert_rtn, _assert_fail, dtrace_assfail and friends.

If a panic function is in user code, is the analyzer smart enough to
figure out that function never returns by looking at the code (ie all
branches lead to _exit() or another panic function)?

If not, I think this should be added. (Obviously this doesn't work for
library panic functions...)

-Alexei

The analyzer cannot do this right now, but that kind of inter-procedural analysis is something we'd like to do one day.

In general functions that cannot return should indicate that fact with a 'noreturn' attribute so that the compiler can generate better code. We certainly could add a local analysis check to warn users about functions that should be marked 'noreturn' because they always call a known panic function.

Some functions, however, are "conditional" panic functions, where they only abort a program when a passed argument is equal to a specified value. This actually happens quite a bit in the Postgresql codebase, which leads to a bunch of false positives from the analyzer. The solution for handling such functions is to do build up path-sensitive (and potentially context-sensitive) summaries for functions that indicate that "if argument X has the value 'error' then this function doesn't return". Then when we analyze a function that calls the panic function we just consult the summary to see if the function doesn't return given the current set of arguments.

In general, doing this kind of analysis doesn't just require inter-procedural analysis, but it requires doing a whole-program analysis across multiple source files. This is necessary because the definition for the panic function itself likely occurs in one specific file. Doing this kind of whole-program analysis requires some infrastructure that is a big TODO for the analyzer. Having this kind of whole-program analysis in place would also open the door for doing a lot more sophisticated checks.

Yes please - so does this TODO have any kind of shape yet?

Unfortunately no. I have been a little overwhelmed lately, and sadly it hasn't had any love from me yet. If anyone is interested, it would be great help to mock up a skeleton TODO that we could post to the website and revise over time.

I'd certainly be interested, but I'm not really sure where to start,
even. The kind of analysis I imagine would require either loading the
whole program up at once, or being able to go back to stuff you'd
already seen and do another pass over it, now that you know more.

Plus some kind of global database might be needed. And serialisation
of data structures?

It just seems there are many ways to approach this, and some clue
which is something of a prerequisite...