[ProfData] Detect if zlib is available

[chenwj]

Summary

As discussed on [1], if the profile is compressed and llvm-profdata is not built with zlib support, the error message is not informative. Give a better error message if zlib is not available.

[1] http://lists.llvm.org/pipermail/llvm-dev/2017-July/115571.html

[davidxl]

lgtm

lib/ProfileData/InstrProf.cpp

435 ↗

(On Diff #107199)

remove {} here.

[chenwj]

address review comment.

@davidxl I have no commit access. Could you help me commit it if everything looks fine?

[dblaikie]

Test case?

[chenwj]

In D35586#814443, @dblaikie wrote:

Test case?

@dblaikie Would you like me to upload a compressed profile data, or ?

[dblaikie]

In D35586#814588, @chenwj wrote:

In D35586#814443, @dblaikie wrote:

Test case?

@dblaikie Would you like me to upload a compressed profile data, or ?

Yep, that'd probably be my approach - a checked in binary profile file with compressed data (including instructions in the .test file about how to reproduce the file in case anyone needs to maintain/modify/etc). (& you'll need "REQUIRES: nozlib" to ensure the test is only run when zlib is not available)

[chenwj]

@dblaikie I upload the profile data and test case. However, the error message I check including the full path of the profile data. Any idea on how can I improve this?

[davidxl]

test/tools/llvm-profdata/nocompress.test

5 ↗

(On Diff #107382)

CHECK: error: {{.*}}: Profile uses zlib compression ...

[dblaikie]

test/tools/llvm-profdata/nocompress.test

1–5 ↗

(On Diff #107382)

Include some text here describing how to produce the input file - in case anyone needs to regenerate it later, etc.

[chenwj]

@dblaikie Done.

[dblaikie]

test/tools/llvm-profdata/nocompress.test

1–10 ↗

(On Diff #107405)

You don't need to use a prefix for these lines - since this file isn't parsed by anything except Lit and FileCheck (which both only handle lines with prefixes) any non-prefix lines are ignored/irrelevant and can be freeform.

[chenwj]

test/tools/llvm-profdata/nocompress.test

1–10 ↗

(On Diff #107405)

I just copy and modify the instructions from c-general.test. Do you still want me remove those REGENERATE prefix?

[chenwj]

@dblaikie ping? :-)

[dblaikie]

In D35586#817181, @chenwj wrote:

@dblaikie ping? :-)

Oh, sorry - I already approved this. Approval with comments usually means "I assume the requested changes are simple/clear/obvious enough to not require another round of review", etc.

In any case - looks good to commit after the "REGENERATE:" prefixes are removed. (did you see the discussion on the mailing list? Not all emails get reflected in Phabricator, and I replied to your question about the REGENERATE Prefix directly on the mailing list - I removed it in the other test that used it, so please remove it from this one before committing)

[chenwj]

@dblaikie Done. I need your help here since I don't have commit access. Thanks. :)

[alekseyshl]

nocompress.test fails on a few targets here http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/6639

llvm/trunk/include/llvm/ProfileData/InstrProf.h

enum class instrprof_error {
   malformed,
   unknown_function,
   hash_mismatch,
   count_mismatch,
   counter_overflow,
   value_site_count_mismatch,
   compress_failed,
   uncompress_failed,
-  empty_raw_profile
+  empty_raw_profile,
+  zlib_unavailable
 };
 
 inline std::error_code make_error_code(instrprof_error E) {
   return std::error_code(static_cast<int>(E), instrprof_category());
 }
 
 class InstrProfError : public ErrorInfo<InstrProfError> {
 public:

llvm/trunk/lib/ProfileData/InstrProf.cpp

static std::string getInstrProfErrString(instrprof_error Err) {
   case instrprof_error::value_site_count_mismatch:
     return "Function value site count change detected (counter mismatch)";
   case instrprof_error::compress_failed:
     return "Failed to compress data (zlib)";
   case instrprof_error::uncompress_failed:
     return "Failed to uncompress data (zlib)";
   case instrprof_error::empty_raw_profile:
     return "Empty raw profile file";
+  case instrprof_error::zlib_unavailable:
+    return "Profile uses zlib compression but the profile reader was built without zlib support";
   }
   llvm_unreachable("A value of instrprof_error has no message.");
 }
 
 namespace {
 
 // FIXME: This class is only here to support the transition to llvm::Error. It
 // will be removed once this transition is complete. Clients should prefer to
while (P < EndP) {
     uint64_t UncompressedSize = decodeULEB128(P, &N);
     P += N;
     uint64_t CompressedSize = decodeULEB128(P, &N);
     P += N;
     bool isCompressed = (CompressedSize != 0);
     SmallString<128> UncompressedNameStrings;
     StringRef NameStrings;
     if (isCompressed) {
+      if (!llvm::zlib::isAvailable())
+        return make_error<InstrProfError>(instrprof_error::zlib_unavailable);
+
       StringRef CompressedNameStrings(reinterpret_cast<const char *>(P),
                                       CompressedSize);
       if (Error E =
               zlib::uncompress(CompressedNameStrings, UncompressedNameStrings,
                                UncompressedSize)) {
         consumeError(std::move(E));
         return make_error<InstrProfError>(instrprof_error::uncompress_failed);
       }

llvm/trunk/test/tools/llvm-profdata/nocompress.test

+You need a checkout of clang with compiler-rt to generate the
+binary file here.  These shell commands can be used to regenerate
+it.
+$ SRC=path/to/llvm
+$ CFE=$SRC/tools/clang
+$ TESTDIR=$SRC/test/tools/llvm-profdata
+$ CFE_TESTDIR=$CFE/test/Profile
+$ clang -o a.out -fprofile-instr-generate $CFE_TESTDIR/c-general.c
+$ LLVM_PROFILE_FILE=$TESTDIR/Inputs/compressed.profraw ./a.out
+
+RUN: not llvm-profdata show %p/Inputs/compressed.profraw -o %t 2>&1 | FileCheck %s
+
+REQUIRES: nozlib
+
+CHECK: error: {{.*}} Profile uses zlib compression but the profile reader was built without zlib support