[PATCH] D69251: [LLVMDebugInfoPDB] - Use cantFail() instead of assert().

Mon Oct 21 05:12:00 PDT 2019

grimar created this revision.
grimar added reviewers: thakis, zturner, rnk.
grimar added a comment.

Fail log is below.

  ********************
  FAIL: LLVM :: tools/llvm-pdbutil/injected-sources-native.test (33257 of 33768)
  ******************** TEST 'LLVM :: tools/llvm-pdbutil/injected-sources-native.test' FAILED ********************
  Script:
  --
  : 'RUN: at line 4';   /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content    /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/InjectedSource.pdb | /home/umb/LLVM/build/bin/FileCheck /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test
  : 'RUN: at line 6';   /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content    /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/ClassLayoutTest.pdb | /home/umb/LLVM/build/bin/FileCheck --check-prefix=NEGATIVE /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test
  : 'RUN: at line 33';   /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content    /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/dotnet_hashonly.pdb | /home/umb/LLVM/build/bin/FileCheck --check-prefix=HASH /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test
  : 'RUN: at line 45';   /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content    /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/dotnet_contents_uncompressed.pdb | /home/umb/LLVM/build/bin/FileCheck --check-prefix=UNCOMP /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test
  : 'RUN: at line 62';   /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content    /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/dotnet_contents_compressed.pdb | /home/umb/LLVM/build/bin/FileCheck --check-prefix=COMP /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test
  --
  Exit Code: 2
  
  Command Output (stderr):
  --
  Expected<T> must be checked before access or destruction.
  Expected<T> value was in success state. (Note: Expected<T> values in success mode must still be checked prior to being destroyed).
  Stack dump:
  0.	Program arguments: /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/InjectedSource.pdb 
   #0 0x000055b389798f5a llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/home/umb/LLVM/build/bin/llvm-pdbutil+0x2acf5a)
   #1 0x000055b389796c84 llvm::sys::RunSignalHandlers() (/home/umb/LLVM/build/bin/llvm-pdbutil+0x2aac84)
   #2 0x000055b389796e05 SignalHandler(int) (/home/umb/LLVM/build/bin/llvm-pdbutil+0x2aae05)
   #3 0x00007faef95ebdd0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12dd0)
   #4 0x00007faef8cb4077 raise /build/glibc-B9XfQf/glibc-2.28/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
   #5 0x00007faef8c95535 abort /build/glibc-B9XfQf/glibc-2.28/stdlib/abort.c:81:7
   #6 0x000055b389587813 llvm::Expected<llvm::StringRef>::fatalUncheckedExpected() const (/home/umb/LLVM/build/bin/llvm-pdbutil+0x9b813)
   #7 0x000055b3896a4eb8 llvm::pdb::(anonymous namespace)::NativeInjectedSource::getFileName() const (/home/umb/LLVM/build/bin/llvm-pdbutil+0x1b8eb8)
   #8 0x000055b3895bb8ed dumpPretty(llvm::StringRef) (/home/umb/LLVM/build/bin/llvm-pdbutil+0xcf8ed)
   #9 0x000055b3895501c0 main (/home/umb/LLVM/build/bin/llvm-pdbutil+0x641c0)
  #10 0x00007faef8c9709b __libc_start_main /build/glibc-B9XfQf/glibc-2.28/csu/../csu/libc-start.c:342:3
  #11 0x000055b38956645a _start (/home/umb/LLVM/build/bin/llvm-pdbutil+0x7a45a)
  FileCheck error: '-' is empty.
  FileCheck command line:  /home/umb/LLVM/build/bin/FileCheck /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test


Currently injected-sources-native.test fails with "Expected<T> value was in success state.
(Note: Expected<T> values in success mode must still be checked prior to being destroyed)"
when llvm is compiled with LLVM_ENABLE_ABI_BREAKING_CHECKS in Release.

The problem is that `getStringForID` returns `Expected<StringRef>` and `Expected` value must always
be checked, even if it is in success state. Checking with `assert` only helps in Debug and is wrong.


https://reviews.llvm.org/D69251

Files:
  lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp


Index: lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp
===================================================================

--- lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp
+++ lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp
@@ -46,30 +46,31 @@
   uint64_t getCodeByteSize() const override { return Entry.FileSize; }
 
   std::string getFileName() const override {
-    auto Name = Strings.getStringForID(Entry.FileNI);
-    assert(Name && "InjectedSourceStream should have rejected this");
-    return *Name;
+    StringRef Ret = cantFail(Strings.getStringForID(Entry.FileNI),
+                             "InjectedSourceStream should have rejected this");
+    return Ret;
   }
 
   std::string getObjectFileName() const override {
-    auto ObjName = Strings.getStringForID(Entry.ObjNI);
-    assert(ObjName && "InjectedSourceStream should have rejected this");
-    return *ObjName;
+    StringRef Ret = cantFail(Strings.getStringForID(Entry.ObjNI),
+                             "InjectedSourceStream should have rejected this");
+    return Ret;
   }
 
   std::string getVirtualFileName() const override {
-    auto VName = Strings.getStringForID(Entry.VFileNI);
-    assert(VName && "InjectedSourceStream should have rejected this");
-    return *VName;
+    StringRef Ret = cantFail(Strings.getStringForID(Entry.VFileNI),
+                             "InjectedSourceStream should have rejected this");
+    return Ret;
   }
 
   uint32_t getCompression() const override { return Entry.Compression; }
 
   std::string getCode() const override {
     // Get name of stream storing the data.
-    auto VName = Strings.getStringForID(Entry.VFileNI);
-    assert(VName && "InjectedSourceStream should have rejected this");
-    std::string StreamName = ("/src/files/" + *VName).str();
+    StringRef VName =
+        cantFail(Strings.getStringForID(Entry.VFileNI),
+                 "InjectedSourceStream should have rejected this");
+    std::string StreamName = ("/src/files/" + VName).str();
 
     // Find stream with that name and read its data.
     // FIXME: Consider validating (or even loading) all this in


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69251.225855.patch
Type: text/x-patch
Size: 2144 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191021/0b3e9ea2/attachment.bin>
