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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to