This is an archive of the discontinued LLVM Phabricator instance.

Do not insert asan paddings after fields that have flexible arrays.
ClosedPublic

Authored by kcc on Oct 22 2014, 5:56 PM.

Details

Summary

We should avoid a tail padding not only if the last field
has zero size but also if the last field is a struct with a flexible array.

If/when http://reviews.llvm.org/D5478 is committed,
this will also handle the case of structs with zero-sized arrays.

Diff Detail

Event Timeline

kcc updated this revision to Diff 15288.Oct 22 2014, 5:56 PM
kcc retitled this revision from to Do not insert asan paddings after fields that have flexible arrays..
kcc updated this object.
kcc edited the test plan for this revision. (Show Details)
kcc added reviewers: majnemer, rsmith.
kcc added a subscriber: Unknown Object (MLST).
majnemer edited edge metadata.Oct 22 2014, 6:03 PM

Do you handle stuff like the following correctly?

struct S {
  int Array[1]; // This field is actually variable length.
};

I would recommend that you do not put padding after the last field if it has one or zero as the array bound.

kcc updated this revision to Diff 15338.Oct 23 2014, 11:06 AM
kcc edited edge metadata.

Changed the patch so that it does not check the field size any more,
but only relies on hasFlexibleArrayMember().
Now the loop iterating over the fields actually knows which element is the last one.
This way we will avoid instrumenting flexible arrays and structs that contain those.

Combined with the David's patch http://reviews.llvm.org/D5478
this will automatically handle cases like
struct S {int Array[0]; }
and
struct S {int Array[1]; }
as well as the cases where S is the last field of another struct.

PTAL

rsmith accepted this revision.Oct 27 2014, 12:28 PM
rsmith edited edge metadata.

LGTM

lib/AST/RecordLayoutBuilder.cpp
1336

Remove this comment.

This revision is now accepted and ready to land.Oct 27 2014, 12:28 PM
kcc closed this revision.Oct 27 2014, 12:44 PM