Page MenuHomePhabricator

Fix testing for end of stream in bitstream reader.
ClosedPublic

Authored by kschimpf on Jul 21 2015, 10:00 AM.

Details

Summary

This fixes a bug found while working on the bitcode reader. In
particular, the method BitstreamReader::AtEndOfStream doesn't always
behave correctly when processing a data streamer. The method
fillCurWord doesn't properly set CurWord/BitsInCurWord if the data
streamer was already at eof, but GetBytes had not yet set the
ObjectSize field of the streaming memory object.

This patch fixes this problem, and provides a test to show that
this problem has been fixed.

Diff Detail

Repository
rL LLVM

Event Timeline

kschimpf updated this revision to Diff 30268.Jul 21 2015, 10:00 AM
kschimpf retitled this revision from to Fix testing for end of stream in bitstream reader..
kschimpf updated this object.
kschimpf added reviewers: dschuff, jvoung, rafael, filcab.
kschimpf added a subscriber: llvm-commits.

Will remove unnecessary include in StreamingMemoryObject.h

include/llvm/Support/StreamingMemoryObject.h
20 ↗(On Diff #30268)

Delete this!

rafael added inline comments.Jul 22 2015, 1:07 PM
include/llvm/Support/StreamingMemoryObject.h
56 ↗(On Diff #30268)

I don't think constexpr is supported everywhere. Why do you need it?

filcab added inline comments.Jul 25 2015, 12:31 AM
unittests/Bitcode/BitReaderTest.cpp
109 ↗(On Diff #30268)

Why std::move() a StringRef?

kschimpf updated this revision to Diff 31041.Jul 30 2015, 9:41 AM
  • Fix issues raised.
  • Fix invalid-abbrev.bc to include 0's at end to close module block.

Note that I had to add a few zero-valued bytes to the end of (binary) test case test/Bitcode/Input/invalid-abbrev.bc. These zeros are needed to correctly make the bitcode reader terminate (it wasn't reading the last byte correctly due to eof bug fixed by this CL).

include/llvm/Support/StreamingMemoryObject.h
56 ↗(On Diff #30268)

It isn't necessary. removing.

unittests/Bitcode/BitReaderTest.cpp
109 ↗(On Diff #30268)

This is a leftover from when I created the memory buffer using a different constructor of a memory buffer. Removing.

filcab accepted this revision.Jul 30 2015, 8:59 PM
filcab edited edge metadata.
This revision is now accepted and ready to land.Jul 30 2015, 8:59 PM
This revision was automatically updated to reflect the committed changes.
kschimpf marked an inline comment as done.Aug 3 2015, 12:46 PM

Looking into cause of test failure under compilation with ASAN. Also creating patch to remove constexpr from unittests/Bitcode/BitReaderTest.cpp.