[compiler-rt] r231413 - [sanitizer] Reconstruct the function that dumps block/edge coverage, hopefully making it more robust. Also increase the allowed coverage size on 32-bit.

[Alexander Potapenko]

Kuba's analysis is correct.
Because of this bug coverage doesn't work on Mac at all, so I'm taking a look.

On Tue, Mar 10, 2015 at 2:19 AM, Kostya Serebryany <kcc at google.com> wrote:
> Can we simply disable this test on Mac for now?
> I don't have Mac expertise, glider is on vacation, and this code is not used
> by anyone yet on Mac.
>
> On Mon, Mar 9, 2015 at 4:13 PM, Kuba Brecka <kuba.brecka at gmail.com> wrote:
>>
>> It passes on i386, but fails on x86_64. The current state is that it
>> produces an XPASS on i386, but if we remove the XFAIL line, we would get a
>> failure on x86_64.
>>
>> Kuba
>>
>> Sent from my iPhone
>>
>> On Mar 9, 2015, at 11:55 PM, Kostya Serebryany <kcc at google.com> wrote:
>>
>> Interesting. in r225281 added XFAIL: darwin
>> Do you actually mean that the test now started passing?
>> From your link: "Exit Code: 0"
>>
>>
>> On Mon, Mar 9, 2015 at 2:25 PM, Kuba Brecka <kuba.brecka at gmail.com> wrote:
>>>
>>> Hi Kostya,
>>>
>>> I believe this recent change (r231413) caused a subtle change in behavior
>>> that is causing a test failure on the Darwin buildbot at
>>> http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA_check/2083/console.
>>> The test is failing for a few days already, but it’s hard to tell which
>>> exact patch caused the regression, because the bot was offline for a day and
>>> a half when it occurred.
>>>
>>> I think the cause is this:  The old code in CovDump() iterates over the
>>> process map, but it intentionally skips over segments that are not
>>> executable.  The new code in DumpOffsets() asks GetModuleNameAndOffsetForPC
>>> instead, which contains a bug/feature that is triggered for in a 64-bit
>>> binary when a 4GB __PAGEZERO (with protection=0) segment is present.  In
>>> that case GetModuleNameAndOffsetForPC will say that the resulting offset is
>>> larger than 4GB, and the subsequent check in DumpOffsets that asserts
>>> `offset > 0xffffffffU` will ignore that item and not print it into the
>>> coverage file.
>>>
>>> I also realized that simply ignoring __PAGEZERO in the Darwin
>>> implementation of process maps is not going to work easily, because there
>>> seems to be other code that relies on this behavior (I can’t tell if that’s
>>> a bug or a feature).  What do you think would be way to fix this?
>>>
>>> Thanks,
>>> Kuba
>>>
>>> Author: kcc
>>> Date: Thu Mar  5 16:19:25 2015
>>> New Revision: 231413
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=231413&view=rev
>>> Log:
>>> [sanitizer] Reconstruct the function that dumps block/edge coverage,
>>> hopefully making it more robust. Also increase the allowed coverage size on
>>> 32-bit.
>>>
>>> Modified:
>>>     compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc
>>>
>>> compiler-rt/trunk/test/asan/TestCases/Linux/coverage-module-unloaded.cc
>>>     compiler-rt/trunk/test/asan/TestCases/Linux/coverage-sandboxing.cc
>>>     compiler-rt/trunk/test/asan/TestCases/Linux/coverage.cc
>>>
>>> Modified:
>>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc?rev=231413&r1=231412&r2=231413&view=diff
>>>
>>> ==============================================================================
>>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc
>>> (original)
>>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc
>>> Thu Mar  5 16:19:25 2015
>>> @@ -79,6 +79,8 @@ class CoverageData {
>>>    void DumpTrace();
>>>
>>>
>>
>



-- 
Alexander Potapenko
Software Engineer
Google Moscow