[PATCH] D20286: [Coverage] Fix an issue where improper coverage mapping data could be loaded for an inline function.
Igor Kudrin via llvm-commits
llvm-commits at lists.llvm.org
Thu May 19 10:11:53 PDT 2016
ikudrin added inline comments.
================
Comment at: lib/ProfileData/Coverage/CoverageMappingReader.cpp:297
@@ -296,1 +296,3 @@
+Error RawCoverageMappingDummyChecker::isDummy(bool &Result) {
+ // A dummy coverage mapping data consists of just one region with zero count.
----------------
vsk wrote:
> nit, could you use `Expected<bool>` here? That should let you simplify this a bit.
Thanks! I'll update the patch tomorrow.
================
Comment at: lib/ProfileData/Coverage/CoverageMappingReader.cpp:351
@@ +350,3 @@
+ if (Hash) {
+ Result = false;
+ return Error::success();
----------------
davidxl wrote:
> Should this be moved above the if(Hash) so that caller does not need to initialize it.
The caller doesn't need to initialize it. If Hash is 0, RawCoverageMappingDummyChecker::isDummy() does its part of work.
================
Comment at: lib/ProfileData/Coverage/CoverageMappingReader.cpp:408
@@ +407,3 @@
+ }
+ // Update the existent record if it's a dummy and the new record is real.
+ size_t OldRecordIndex = InsertResult.first->second;
----------------
vsk wrote:
> nit, existing
Thanks!
================
Comment at: test/tools/llvm-cov/Inputs/prefer_used_to_unused.cpp:5
@@ +4,2 @@
+ return sampleFunc(5) + simpleFunc(5);
+}
----------------
davidxl wrote:
> do you need another source file that includes prefer_used_to_unused.h but does not call sampleFunc nor simpleFunc in order to force creation of dummies?
I use `prefer_used_to_unused.h` itself for this, see the comment at line 6 there.
http://reviews.llvm.org/D20286
More information about the llvm-commits
mailing list