This is an archive of the discontinued LLVM Phabricator instance.

[msan] Fix getmntent{_r} for empty /etc/fstab
ClosedPublic

Authored by zatrazz on May 3 2017, 6:33 AM.

Details

Summary

Some configuration (for instance default docker ubuntu images) uses
a default empty and invalid /etc/fstab configuration file. It makes
any call to getmntent return NULL and it leads to failures on
Msan-aarch64{-with-call}-Test/MemorySanitizer.getmntent{_r}.

This patch fixes it by creating a temporary file with some valid
entries (although not valid for the system) to use along with
setmntent/getmntent.

Diff Detail

Event Timeline

zatrazz created this revision.May 3 2017, 6:33 AM
eugenis added inline comments.May 3 2017, 11:00 AM
lib/msan/tests/msan_test.cc
2213

Either make this a constructor, or call it from the constructor, or add a check in the destructor so that it does not try to unlink an uninitialized path if Create was never called.

What happens if the test fails in one of the ASSERTs, will the file be unlinked? We build msan tests with -fno-exceptions.

zatrazz updated this revision to Diff 97850.May 4 2017, 10:30 AM
zatrazz added inline comments.May 4 2017, 10:30 AM
lib/msan/tests/msan_test.cc
2213

We can't really add the EXPECT_* macros on constructor, so I changed to a different logic: the Create returns a bool and set an internal file descriptor indicating the file was open correctly. The destructor close the file assuring the removal since it was unlinked after successful opening.

eugenis accepted this revision.May 8 2017, 2:03 PM
eugenis added inline comments.
lib/msan/tests/msan_test.cc
2213

if (fd != -1)

0 is a valid file descriptor

This revision is now accepted and ready to land.May 8 2017, 2:03 PM
zatrazz added inline comments.May 8 2017, 2:26 PM
lib/msan/tests/msan_test.cc
2213

Indeed, I will fix it.

zatrazz closed this revision.May 10 2017, 5:31 AM