This is an archive of the discontinued LLVM Phabricator instance.

[lldb][AArch64] Don't check for VmFlags in smaps files
ClosedPublic

Authored by DavidSpickett on Apr 14 2021, 9:14 AM.

Details

Summary

AArch64 kernel builds default to having /smaps and
the "VmFlags" line was added in 3.8. Long before MTE
was supported.

So we can assume that if you're AArch64 with MTE,
you can run this test.

The previous method of checking had a race condition
where the process we read smaps for, could finish before
we get to read the file.

I explored some alternatives but in the end I think
it's fine to just assume we have what we need.

Diff Detail

Event Timeline

DavidSpickett created this revision.Apr 14 2021, 9:14 AM
DavidSpickett requested review of this revision.Apr 14 2021, 9:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2021, 9:14 AM

This fixes the underlying issue for the random failure of this test on the lldb bots.

The only way we get bitten here is if someone runs this on a system with smaps disabled but given that that removes your ability to actually debug MTE I don't see it happening.

DavidSpickett added inline comments.Apr 14 2021, 9:17 AM
lldb/packages/Python/lldbsuite/test/lldbtest.py
1314

When running on a buildbot via lit, this lldb-server could be one of hundreds of test instances. We get unlucky, the test finishes before we can read the file, and this fails.

Looks like buildkite had issues calling back to Phab yesterday so the builds didn't signal here. This test wouldn't get run in those anyway.

omjavaid accepted this revision.Apr 28 2021, 7:03 PM
omjavaid added inline comments.
lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py
42

I think this is LGTM as it is but if you want to add further reliability then may be set a unique executable name so that you can detect only its PID and then read smaps file. You can skip this test at this stage if target doesnt have smaps.

This revision is now accepted and ready to land.Apr 28 2021, 7:03 PM
This revision was landed with ongoing or failed builds.Apr 29 2021, 1:30 AM
This revision was automatically updated to reflect the committed changes.
DavidSpickett added inline comments.Apr 29 2021, 1:31 AM
lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py
42

That's clever, I didn't think of that. I might do that in a follow up.