static analyzer bug && diagnostics about suspicious casts

Greetings,

recently I’ve found a bug in the static analyzer <http://llvm.org/bugs/show_bug.cgi?id=16308>, where it crashed processing code like this:

Greetings,

recently I’ve found a bug in the static analyzer <http://llvm.org/bugs/show_bug.cgi?id=16308>, where it crashed processing code like this:

struct A;
struct B{ virtual ~B(); };
class B2 : public B { };
void f(A *a) {
  B *b=(B *)a;
  B2 *b2=dynamic_cast<B2 *>(b);
}
-----
because A has incomplete type. Besides fixing the crash, I was thinking I could add a diagnostic which would detect patterns like this. The problem here is the reinterpret_cast to a non-standard-layout type, which invokes undefined behavior. Since this is fairly easy to detect, I started wondering if there is a reason it is not implemented yet. Is it because the pattern is too common in the code and would be an annoyance? Or am I missing something obvious here ?

As far as I know we have not tried adding this diagnostic. I am not sure what the false positive rate would be.

There's nothing wrong with reinterpret_cast'ing to an arbitrary object pointer type; it doesn't need to be standard-layout. You just have to be careful to not (1) increase the alignment requirements (which we should already warn about with -Wcast-align) or (2) do anything with the type-punned pointer except cast it back eventually (which is pretty hard to reason about).

And there are known encapsulation idioms which involve temporarily punning a pointer as a pointer to an incomplete type, so I'm not sure this is a terribly valuable warning to pursue even for your specific case.

Obligatory standards quote:

C++11 [expr.reinterpret.cast]p7:
  An object pointer can be explicitly converted to an object pointer of a different type. When a prvalue v of type “pointer to T1” is converted to the type “pointer to cv T2”, the result is static_cast<cv T2*>(static_cast<cv void*>(v)) if both T1 and T2 are standard-layout types (3.9) and the alignment requirements of T2 are no stricter than those of T1, or if either type is void. Converting a prvalue of type “pointer to T1” to the type “pointer to T2” (where T1 and T2 are object types and where the alignment requirements of T2 are no stricter than those of T1) and back to its original type yields the original pointer value. The result of any other such pointer conversion is unspecified.

John.

There's nothing wrong with reinterpret_cast'ing to an arbitrary object
pointer type; it doesn't need to be standard-layout. You just have to be
careful to not (1) increase the alignment requirements (which we should
already warn about with -Wcast-align) or (2) do anything with the
type-punned pointer except cast it back eventually (which is pretty hard to
reason about).

And there are known encapsulation idioms which involve temporarily punning
a pointer as a pointer to an incomplete type, so I'm not sure this is a
terribly valuable warning to pursue even for your specific case.

Obligatory standards quote:

C++11 [expr.reinterpret.cast]p7:
  An object pointer can be explicitly converted to an object pointer of a
different type. When a prvalue v of type “pointer to T1” is converted to
the type “pointer to cv T2”, the result is static_cast<cv
T2*>(static_cast<cv void*>(v)) if both T1 and T2 are standard-layout types
(3.9) and

I've read this paragraph before writing, and this part made me believe that
the types need to be standard layout. I'm not very good at standardese, so
I wasn't sure if this restriction applies to the following sentences or
not.

But I guess you're right. I think I'll abandon this warning idea.

the alignment requirements of T2 are no stricter than those of T1, or if

either type is void. Converting a prvalue of type “pointer to T1” to the
type “pointer to T2” (where T1 and T2 are object types and where the
alignment requirements of T2 are no stricter than those of T1) and back to
its original type yields the original pointer value. The result of any
other such pointer conversion is unspecified.

John.

regards,
pavel

It doesn’t; it’s a separate rule. The structure is basically: “If , the result is . If , the result is . The result of anything else is unspecified.” It seems weird only because is a subset of .

John.

Also of note: the "if both T1 and T2 are standard-layout types" clause
has been removed from more recent drafts of the standard, see DR1412.