Page MenuHomePhabricator

Implement poisoning of only class members in dtor, and verify emitted code for dtors of classes with virtual base dtors.
ClosedPublic

Authored by nmusgrave on Aug 11 2015, 11:12 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

nmusgrave updated this revision to Diff 31834.Aug 11 2015, 11:12 AM
nmusgrave updated this revision to Diff 31836.
nmusgrave retitled this revision from to Implement poisoning of only class members in dtor, and verify emitted code for dtors of classes with virtual base dtors..
nmusgrave updated this object.
nmusgrave added reviewers: eugenis, kcc.
  • Removed patch file containing extraneous changes
eugenis edited edge metadata.Aug 11 2015, 11:33 AM

I assume this makes the XFAIL-ed test in compiler-rt pass? Then you need to remove the XFAIL either in this change, or in another, simultaneously committed change.

Please check that this does the right thing for non-trivial class members. This should be tested in compiler-rt. In any case, this seems to be a change in the right direction.

lib/CodeGen/CGClass.cpp
1383 ↗(On Diff #31836)

So if there are 0 fields, you poison the entire object? This does not sound right.

1389 ↗(On Diff #31836)

There must be a way to do this with a single GEP, without bitcasting to int8ptrty. See how clang emits member address expression, like &(a->b).

test/CodeGenCXX/sanitize-dtor-derived-class.cpp
28 ↗(On Diff #31836)

Don't need {{.*}} at the end of the line.

nmusgrave updated this revision to Diff 31978.Aug 12 2015, 1:55 PM
nmusgrave marked an inline comment as done.
nmusgrave edited edge metadata.
  • clang tests to verify dtor sanitizing function emitted only in the last dtor of this class, and before base dtors are invoked
eugenis accepted this revision.Aug 12 2015, 2:21 PM
eugenis edited edge metadata.

LGTM with a nit

lib/CodeGen/CGClass.cpp
1396 ↗(On Diff #31978)

No need to emit the call is PoisonSize is 0.

This revision is now accepted and ready to land.Aug 12 2015, 2:21 PM
nmusgrave updated this revision to Diff 31984.Aug 12 2015, 2:29 PM
nmusgrave edited edge metadata.
  • Removed patch file containing extraneous changes
  • clang tests to verify dtor sanitizing function emitted only in the last dtor of this class, and before base dtors are invoked
  • skip poisoning if class has no fields
This revision was automatically updated to reflect the committed changes.