Page MenuHomePhabricator

MemoryBuffer: Add a missing error-check to getOpenFileImpl
ClosedPublic

Authored by labath on Aug 14 2019, 8:37 AM.

Details

Summary

In case the function was called with a desired read size *and* the file
was not an "mmap()" candidate, the function was falling back to a
"pread()", but it was failing to check the result of that system call.
This meant that the function would return "success" even though the read
operation failed, and it returned a buffer full of uninitialized memory.

Diff Detail

Repository
rL LLVM

Event Timeline

labath created this revision.Aug 14 2019, 8:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2019, 8:37 AM
Herald added a subscriber: kristina. · View Herald Transcript

PS: I discovered this when accidentally trying to read a directory, but it is not possible to write a portable test for that, as some BSDs will actually allow you to do that, so the test is written via a write-only file handle instead.

dblaikie accepted this revision.Aug 14 2019, 10:23 AM

Seems reasonable

This revision is now accepted and ready to land.Aug 14 2019, 10:23 AM
This revision was automatically updated to reflect the committed changes.

It looks like this changed caused 30 or so of the tests to fail on the Windows bot. It was already broken, so you wouldn't have gotten an email:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/7851

It looks like this changed caused 30 or so of the tests to fail on the Windows bot. It was already broken, so you wouldn't have gotten an email:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/7851

Oh my... I was doing this to fix a different LLDB bug :/. Lemme boot up windows to see what's happening here...

Ok, I have an idea what's going on. There's a difference in the behavior of readNativeFileSlice on windows&posix, and neither of them really do what MemoryBuffer expects here. I don't have time to fix this today so I'll revert and get back to this tomorrow. Thanks for the heads up.