This is an archive of the discontinued LLVM Phabricator instance.

Use-after-dtor detection for trivial base classes.
ClosedPublic

Authored by eugenis on Feb 8 2022, 3:44 PM.

Details

Summary

-fsanitize-memory-use-after-dtor detects memory access after a
subobject is destroyed but its memory is not yet deallocated.
This is done by poisoning each object memory near the end of its destructor.

Subobjects (members and base classes) do this in their respective
destructors, and the parent class does the same for its members with
trivial destructors.

Inexplicably, base classes with trivial destructors are not handled at
all. This change fixes this oversight by adding the base class poisoning logic
to the parent class destructor.

Diff Detail

Event Timeline

eugenis requested review of this revision.Feb 8 2022, 3:44 PM
eugenis created this revision.
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 8 2022, 3:44 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
vitalybuka accepted this revision.Feb 8 2022, 5:49 PM
This revision is now accepted and ready to land.Feb 8 2022, 5:49 PM
eugenis updated this revision to Diff 408071.Feb 11 2022, 3:02 PM

Fix handling of empty base classes and suppress tail calls.

kda added inline comments.Feb 25 2022, 12:14 PM
clang/lib/CodeGen/CGClass.cpp
1869–1870

Maybe not "Ignore"?

1904–1905

not "ignore".

compiler-rt/test/msan/dtor-base-access.cpp
11–12

nit: inconsistent indent with line 18.

36

maybe in good faith '= nullptr'?

63

Is calling destructor preferred to 'delete g'? Seems like 'delete' would be obvious inversion of 'new' on line 56.

vitalybuka accepted this revision.Mar 15 2022, 4:16 PM

@eugenis I will prepare google3 and land it?

clang/lib/CodeGen/CGClass.cpp
1869–1870

@kda I assume it's about inconsistent comment about?

compiler-rt/test/msan/dtor-base-access.cpp
36

according standard declarations like this are always zero initialized already

63

after delete we can't do asserts below, which are the goal

Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2022, 4:16 PM
kda added inline comments.Mar 16 2022, 10:31 AM
clang/lib/CodeGen/CGClass.cpp
1869–1870

@vitalybuka: exactly. This does not look like it is ignoring trivial destructors anymore.

compiler-rt/test/msan/dtor-base-access.cpp
36

Okay. I was not aware of the standard. I am accustomed to making things clear, because everyone may not be aware of the standard.

kda accepted this revision.Mar 16 2022, 10:32 AM
kda added inline comments.
compiler-rt/test/msan/dtor-base-access.cpp
24

nit: add single space (consistent indent).

30

nit: add single space (consistent indent).

addressing some @kda comments

This revision was landed with ongoing or failed builds.Mar 16 2022, 6:20 PM
This revision was automatically updated to reflect the committed changes.