[PATCH] D63518: BitStream reader: propagate errors

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added inline comments.


================
Comment at: clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp:619
+    // FIXME check that the enum is in range.
+    auto Code = static_cast<llvm::bitc::FixedAbbrevIDs>(MaybeCode.get());
+
----------------
This has caused problem for our sanitizer tests the last couple of days:

```
FAIL: Extra Tools Unit Tests :: clang-doc/./ClangDocTests/BitcodeTest.emitMethodInfoBitcode (20077 of 48746)
******************** TEST 'Extra Tools Unit Tests :: clang-doc/./ClangDocTests/BitcodeTest.emitMethodInfoBitcode' FAILED ********************
Note: Google Test filter = BitcodeTest.emitMethodInfoBitcode
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from BitcodeTest
[ RUN      ] BitcodeTest.emitMethodInfoBitcode
/local/repo/bbiswjenk/fem023-eiffel003/workspace/llvm/llvm-master-sanitize/clang-tools-extra/clang-doc/BitcodeReader.cpp:621:13: runtime error: load of value 9, which is not a valid value for type 'llvm::bitc::FixedAbbrevIDs'

```

This was seen when building trunk with clang 6.0.0 and LVM_USE_SANITIZER=Undefined

```
cmake -G Ninja '-DLLVM_ENABLE_PROJECTS='\''clang;clang-tools-extra'\''' -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON '-DCMAKE_CXX_FLAGS='\''-stdlib=libc++'\''' -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_USE_LINKER=gold -DLLVM_USE_SANITIZER=Undefined ../.
-- The C compiler identification is Clang 6.0.0
-- The CXX compiler identification is Clang 6.0.0
```

Afaict we can't cast the read value to FixedAbbrevIDs as we do not know yet if it matches one of the values defined in the enum, or if we will take the default case.


A similar switch exist at cfe/trunk/lib/Frontend/SerializedDiagnosticReader.cpp:127, but it is using a slightly different pattern:

```
unsigned Code;
Code = Res.get();
switch ((llvm::bitc::FixedAbbrevIDs)Code) 
```
I haven't seen any failures for SerializedDiagnosticReader. So either we lack test coverage for that function, or the sanitizer only introduce the check when using the static_cast (and declaring Code as an enum) as done here.



Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63518/new/

https://reviews.llvm.org/D63518