This is an archive of the discontinued LLVM Phabricator instance.

CodeGen: Properly null-check typeid expressions
ClosedPublic

Authored by majnemer on Jul 8 2014, 12:59 AM.

Details

Summary

Thoroughly check for a pointer dereference which yields a glvalue. Look
through casts, comma operators, conditional operators, paren
expressions, etc.

Diff Detail

Event Timeline

majnemer updated this revision to Diff 11143.Jul 8 2014, 12:59 AM
majnemer retitled this revision from to CodeGen: Properly null-check typeid expressions.
majnemer updated this object.
majnemer added a reviewer: rsmith.
majnemer added a subscriber: Unknown Object (MLST).
rsmith added inline comments.Jul 13 2014, 4:53 PM
lib/CodeGen/CGExprCXX.cpp
1618

This doesn't handle

((T*)nullptr)[0]

Its semantics are defined in terms of a desugaring to

*((T*)nullptr + 0)

... so I suppose we should probably handle it here, but it's not entirely clear.

1619

I think this will insert a null check for

T *p;
typeid( (const T&)(T)*p )

... which doesn't seem strictly necessary (though superfluous null checks here seem pretty harmless).

majnemer updated this revision to Diff 11390.Jul 14 2014, 9:46 AM
  • Address Richard's comments.
majnemer added inline comments.Jul 14 2014, 9:47 AM
lib/CodeGen/CGExprCXX.cpp
1618

I've updated isGLValueFromPointerDeref to handle this.

1619

I can live with inserting an extra null check here.

majnemer updated this revision to Diff 11471.Jul 15 2014, 3:13 PM
  • Address Richard's comments.
rsmith added inline comments.Jul 15 2014, 4:39 PM
lib/CodeGen/CGExprCXX.cpp
1622

Maybe add a test for the xvalue case; something like typeid((T&&)*(T*)nullptr) ?

1636

How could this not be the case? Also, you have no test coverage for binary conditionals.

1652–1653

Maybe extend this to say that it's not clear what this means, but we use the most generous interpretation.

majnemer closed this revision.Jul 18 2014, 1:02 PM
majnemer updated this revision to Diff 11667.

Closed by commit rL213401 (authored by @majnemer).

lib/CodeGen/CGExprCXX.cpp
1622

Done.

1636

A test has been added, this code has been merged with the ConditionalOperator case above.

1652–1653

Done.