[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Having fun with the following C code (UB)



Shachar Shemesh <shachar@shemesh.biz> writes:
> On 13/04/14 05:39, Russ Allbery wrote:

>> One can make a good argument that such checks are exactly what you
>> should be doing.

> Then the answer is very simple. Write in Java.

There are a lot of reasons other than the absolute fastest performance you
can possibly muster to write something in C instead of Java.  For example,
if you're writing an Apache or PAM module, Java is not very useful, but
that doesn't mean you need micro-optimized signed integer math or need to
worry about the few instructions it takes to check parameters for whether
they're NULL.

There are some very experienced C programmers that make extensive use of
preconditions and checks.  For example, there was an article in
Communications of the ACM within the past year (sadly, I don't remember
exactly where and a quick search didn't help me out) that talked about a C
coding style that aimed for defining checks for every non-trivial
statement.  With some macro assistance, the resulting code wasn't too
ugly, and the result is rather appealing.  It's been something I've been
pondering experimenting with since reading that article.

> I am not a compiler writer, so I have no actual data. I suspect your
> common 20k line will yield about a thousand such warnings, the huge
> majority of which there will be nothing for you to do about.

> Also, it turns out gcc does have such an option. See
> http://www.airs.com/blog/archives/120. -Wstrict-overflow will let you
> know when the optimizer uses the assumption of no overflow to change
> other code.

Thanks for the pointer to that option!  I'd missed that.

I enabled -fstrict-overflow -Wstrict-overflow=5 -Werror in my standard
utility library, which has about 13,000 lines of C, and got one additional
warning, which was trivial to fix (and which was in a bit of code that has
a rather bad smell and has been on my list to rewrite when I get a
chance).  So I'm not horribly impressed by your doomsday worries.  :)
That said, I believe that warning flag does not catch all possible
overflows, just the ones where GCC happens to do an optimization.

I use all sorts of warning flags, with -Werror, that I've seen other
people claim are completely unworkable in practice, like -Wwrite-strings
and -Wsign-compare.  They're not.  They just require care and attention
and writing high-quality C code.

>> Put a different way, the answer to your question is quite different if
>> that function were instead:
>>
>> int compute_offset_into_network_packet( int a, int b )
>> {
>>     return a+b;
>> }
>>
>> No?

> In most cases, you will overflow the packet long before you overflow the
> integer.

Not if either a or b is coming from the network, which is the case I'm
concerned about.  You should do bounds-checking before using them to look
at offsets in a packet.  Tons and tons of security vulnerabilities have
happened due to lack of that bounds-checking, or getting the check wrong.
If there was a way for the compiler to check whether you've done that
bounds-checking before you start doing math with those values, that would
be very helpful.  Obviously, that warning might not be appropriate for all
code, but gcc has a rich pragmata system for handling that.

> If that's the case, the compiler won't help you.

That's the problem with C code in general: the compiler doesn't help you
enough.  Which is why I like seeing more warnings and smarter compilers
that can, from the code, work out what invariants you've established and
then warn you when you haven't checked for ones that may be important.

clang, for example, does a great job of this (far better than gcc) at
detecting variables that may be NULL at the point of use.

There are, obviously, limits, since the C language semantically doesn't
give either the author or the compiler a lot of help.  But there are still
opportunities for improvement.  Ten years ago, I would have said that a
lot of the diagnostics that clang provides were impossible in C because
there just wasn't enough information available to the compiler.  I was
wrong.

> Like I said before, I am not against the compilers warning about such
> cases. I just think that these warnings need to be done very carefully,
> or they become worse than useless.  As such, if you see a case in which
> you feel gcc (or clang, or whatever) should warn, by all means open a
> bug for it.  Just make sure you make it a "feature request" and not a
> "security hole" severity.  In other words, don't get mad merely because
> the compiler author did not read your mind.

I'll be sure to keep that in mind, since I've never reported a bug or
discussed issues with compiler writers before.

-- 
Russ Allbery (rra@debian.org)               <http://www.eyrie.org/~eagle/>


Reply to: