Page MenuHomePhabricator

[NFC][scudo] Add paramenters DCHECKs

Authored by vitalybuka on May 24 2021, 1:14 PM.

Diff Detail

Event Timeline

vitalybuka requested review of this revision.May 24 2021, 1:14 PM
vitalybuka created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2021, 1:14 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
hctim accepted this revision.May 24 2021, 1:22 PM
This revision is now accepted and ready to land.May 24 2021, 1:22 PM

This might trigger sign-compare warnings like

vitalybuka added a comment.EditedMay 24 2021, 6:04 PM

This might trigger sign-compare warnings like

It does not, as D103029 exposes gtest specific issue.
normally if (0 < uptr_var) ... should not produce the warning.

Scudo check has a different one, and I'll upload a patch.

This revision was landed with ongoing or failed builds.May 24 2021, 6:06 PM
This revision was automatically updated to reflect the committed changes.
pcc added inline comments.May 24 2021, 8:33 PM

This isn't right. The size is not required to be aligned to 16.


I'm not sure if this will always be true. Have you run the tests with MTE enabled?


Same as line 220.

vitalybuka added inline comments.May 24 2021, 9:18 PM

It's about return value.
This code returns aligned end?

this one passes DCHECK

for (uptr Size = 0; Size < 4 * archMemoryTagGranuleSize(); ++Size) {
    uptr Tagged = addFixedTag(P, 5);
    uptr TaggedEnd = Tagged + Size;

    EXPECT_EQ(roundUpTo(TaggedEnd, archMemoryTagGranuleSize()),
              storeTags(Tagged, TaggedEnd));

Yes, not all tests pass, with or without the patch, but nothing fails any of this DCHECKs.

QEMU itself crashes on nonaligned address
Shouldn't stg ptr be aligned?


We have few calls there and as I understand the code, they can't be misaligned at current call-sites.

pcc added inline comments.May 24 2021, 10:35 PM

Oh, right. This seems okay then.


They certainly all pass on Android. Perhaps they aren't passing for you because you're using glibc or qemu user mode or something.

Yes, looking at the ARM ARM the pointer is required to be aligned.


Okay then.