This is an archive of the discontinued LLVM Phabricator instance.

Ensure safestack overflow test doesn't segfault
ClosedPublic

Authored by dim on Dec 22 2015, 1:32 PM.

Details

Summary

In rL255491, the safestack overflow test was disabled for aarch64, since
it "is currently failing on an AArch64 buildbot with a segfault, but it
is currently passing on other configuration".

While testing on FreeBSD on x86, I also encountered a segfault. This is
because the fct() function actually writes before and after buffer,
and on FreeBSD this crashes because buffer is usually allocated at the
end of a page. That this runs correctly on Linux is probably just by
accident.

I propose to fix this by adding a pre and post buffer, to act as a
safety zone. The pre and post buffers must be accessed in an 'unsafe'
way, otherwise -fsanitize=safestack will allocate them on the safe
stack, and they will not bookend buffer itself. Therefore, I create
them large enough for fct(), and call it on both of them.

On FreeBSD, this makes the test run as expected, without segfaulting,
and I suppose this will also fix the segfault on AArch64. I do not have
AArch64 testing capabilities, so if someone could try that out, I would
be much obliged.

Diff Detail

Repository
rL LLVM

Event Timeline

dim updated this revision to Diff 43476.Dec 22 2015, 1:32 PM
dim retitled this revision from to Ensure safestack overflow test doesn't segfault.
dim updated this object.
dim added reviewers: zatrazz, pcc, kcc.
dim added subscribers: emaste, llvm-commits.
zatrazz edited edge metadata.Jan 5 2016, 12:38 PM

Another way would be to adjust fct to no pass the possible stack end by writing 6*sizeof(int) instead. I have no preference, so this change also LGTM.

I was not able to reproduce it yet on aarch64, but we can check on the faulty system (the buildbot) by removing the stable-runtime requirement.

dim added a comment.Jan 7 2016, 2:18 PM

Another way would be to adjust fct to no pass the possible stack end by writing 6*sizeof(int) instead. I have no preference, so this change also LGTM.

OK.

I was not able to reproduce it yet on aarch64, but we can check on the faulty system (the buildbot) by removing the stable-runtime requirement.

Shall I then remove the // REQUIRES: stable-runtime line from compiler-rt/trunk/test/safestack/overflow.c as part of this fix?

dim added a comment.Jan 7 2016, 2:20 PM

I'll just commit it now, the test can be re-enabled at some later time.

This revision was automatically updated to reflect the committed changes.