[llvm] r258188 - Fix a coverage reading bug

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 20 16:01:02 PST 2016


On Wed, Jan 20, 2016 at 3:56 PM, Xinliang David Li <davidxl at google.com>
wrote:

> It was added but got reverted in r258314 due to an ARM build bot
> failure. Briefly looking at the bot failure looks like some BFD linker
> bug (old version perhaps). I need more investigation before re-install
> the tests.
>

Why does adding a test for this require putting something in compiler-rt?

Also, for the moment, can you revert this patch until you have time to
investigate the test failure? Experience shows that failing to check in
tests with the code that fixes the bug results in tests ultimately never
getting added.

-- Sean Silva


>
> David
>
> On Wed, Jan 20, 2016 at 3:49 PM, Sean Silva <chisophugis at gmail.com> wrote:
> >
> >
> > On Tue, Jan 19, 2016 at 1:18 PM, Xinliang David Li via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >>
> >> Author: davidxl
> >> Date: Tue Jan 19 15:18:12 2016
> >> New Revision: 258188
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=258188&view=rev
> >> Log:
> >> Fix a coverage reading bug
> >>
> >> function record pointer is not advanced when
> >> duplicate entry is found.
> >>
> >> Test case to be added.
> >
> >
> > Did this ever get added? In LLVM the general rule is that if your commit
> > fixes a bug then the test case should be included in the same commit (or
> > there should be an explanation for why it was impractical to add).
> >
> > -- Sean Silva
> >
> >>
> >>
> >> Modified:
> >>     llvm/trunk/lib/ProfileData/CoverageMappingReader.cpp
> >>
> >> Modified: llvm/trunk/lib/ProfileData/CoverageMappingReader.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/CoverageMappingReader.cpp?rev=258188&r1=258187&r2=258188&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/ProfileData/CoverageMappingReader.cpp (original)
> >> +++ llvm/trunk/lib/ProfileData/CoverageMappingReader.cpp Tue Jan 19
> >> 15:18:12 2016
> >> @@ -396,8 +396,10 @@ public:
> >>        // function name. This is useful to ignore the redundant records
> >> for the
> >>        // functions with ODR linkage.
> >>        NameRefType NameRef = CFR->template getFuncNameRef<Endian>();
> >> -      if (!UniqueFunctionMappingData.insert(NameRef).second)
> >> +      if (!UniqueFunctionMappingData.insert(NameRef).second) {
> >> +        CFR++;
> >>          continue;
> >> +      }
> >>
> >>        StringRef FuncName;
> >>        if (std::error_code EC =
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160120/22789ef2/attachment.html>


More information about the llvm-commits mailing list