[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.
Evgeniy Stepanov
eugeni.stepanov at gmail.com
Fri Apr 3 06:02:53 PDT 2015
See r234010.
On Fri, Apr 3, 2015 at 2:07 PM, Evgeniy Stepanov
<eugeni.stepanov at gmail.com> wrote:
>> Also increase the allowed coverage size on 32-bit.
>
> Why? This adds 200MB mapping on 32-bit, which is a significant chunk
> of the 3GB address space.
>
>
> On Fri, Mar 13, 2015 at 6:39 PM, Alexander Potapenko <glider at google.com> wrote:
>> Oh, looks like both atos and llvm-symbolizer expect the PCs to be
>> relative to the start of module in the virtual memory, that's why
>> everything breaks when we start ignoring __PAGEZERO.
>> We can possibly stick to VM offsets in the sanitizer reports and use
>> file offsets for the coverage, but this sounds counterintuitive,
>> because upon dumping those file offsets will need to be again
>> translated to VM addresses in order to symbolize them.
>>
>> It's a good idea to keep 32-bit offsets instead of 64-bit ones in the
>> coverage files for now (guess we'll have executables of more than 4Gb
>> in size really soon), but we'll need to store the base for these
>> 32-bit offsets in the coverage file and use it during symbolization.
>>
>> On Thu, Mar 12, 2015 at 3:42 PM, Alexander Potapenko <glider at google.com> wrote:
>>> I think there's a confusion between the offset from the beginning of
>>> the file on disk (which is natural and is what sancov.py expects) and
>>> the offset from the beginning of the module in the memory, which is
>>> what we get from PlatformFindModuleNameAndOffsetForAddress() on OSX.
>>>
>>> There's a LoadedModule class in sanitizer_common.h, which represents a
>>> single binary module in memory and encapsulates a series of address
>>> ranges and a single base address for the binary.
>>>
>>> The following code:
>>>
>>> 397 bool PlatformFindModuleNameAndOffsetForAddress(uptr address,
>>> 398 const char **module_name,
>>> 399 uptr
>>> *module_offset) override {
>>> 400 LoadedModule *module = FindModuleForAddress(address);
>>> 401 if (module == 0)
>>> 402 return false;
>>> 403 *module_name = module->full_name();
>>> 404 *module_offset = address - module->base_address();
>>> 405 return true;
>>> 406 }
>>>
>>> assumes that the address ranges are consecutive and the virtual memory
>>> address is always equal to file offset plus the base address.
>>>
>>> This isn't necessarily true, because generally the order of binary
>>> segments (in Mach-O terms) may be different on disk and in memory.
>>> Some segments (like __PAGEZERO) may have smaller size on disk than in
>>> memory. I suppose the same applies to Linux as well.
>>>
>>> What we can do here is make FindModuleForAddress() return the address
>>> range number for the LoadedModule object, and calculate
>>> |module_offset| using the beginning of that address range, not the
>>> module's base address.
>>>
>>> On Thu, Mar 12, 2015 at 2:03 PM, Alexander Potapenko <glider at google.com> wrote:
>>>> 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
>>>
>>>
>>>
>>> --
>>> Alexander Potapenko
>>> Software Engineer
>>> Google Moscow
>>
>>
>>
>> --
>> Alexander Potapenko
>> Software Engineer
>> Google Moscow
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list