[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
glider at google.com
Fri Mar 13 08:39:23 PDT 2015
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
More information about the llvm-commits
mailing list