[PATCH] D17361: [PGO] Add support to enable testing multiple supported targets
Vedant Kumar via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 25 12:44:02 PST 2016
I'm not sure what you mean by object symtab. It's true that sizeof(__llvm_profile_data) is 40, but clang generates assembly that looks like:
.globl _a1 ## @a1
.align 3
_a1:
.space 36
So that's how the linker knows the object's 'real' size and alignment.
I thought the DataSize field in the raw header referred to the number of profile data entries in the profile, and check-profile doesn't seem broken. Have you seen data entries being dropped?
vedant
> On Feb 25, 2016, at 12:29 PM, Xinliang David Li <davidxl at google.com> wrote:
>
> Actually this is still not quite right. The Data object size should
> be 40 bytes (in object symtab). I wonder how the linker gets the 36
> byte.
>
> Your patch can fix the crash, but the recorded number of Data (aka
> DataSize field in the raw header) will still be wrong -- the last
> function's data will be dropped.
>
> David
>
> On Tue, Feb 23, 2016 at 12:41 PM, Xinliang David Li <davidxl at google.com> wrote:
>> Makes sense. The address of the Data is 8 byte aligned, but the size of the
>> object is still 36 bytes.
>>
>> David
>>
>> On Tue, Feb 23, 2016 at 12:25 PM, Vedant Kumar <vsk at apple.com> wrote:
>>>
>>> I see one more spot where this could be an issue in InstrProfiling.c:
>>>
>>> __llvm_profile_reset_counters()
>>> ...
>>> - for (DI = DataBegin; DI != DataEnd; ++DI) {
>>> + for (DI = DataBegin; DI < DataEnd; ++DI) {
>>>
>>> I can fix this up too.
>>>
>>> vedant
>>>
>>>> On Feb 23, 2016, at 12:22 PM, Vedant Kumar via llvm-commits
>>>> <llvm-commits at lists.llvm.org> wrote:
>>>>
>>>> Yes, it is linker-related but not a linker bug.
>>>>
>>>> __llvm_profile_data is only required to _start_ at an 8-byte aligned
>>>> address.
>>>>
>>>> We shouldn't ever need to access past byte 36 of this structure (on
>>>> i386).
>>>>
>>>> In the case of instrprof_basic.c, we have 3 __llvm_profile_data objects.
>>>> So the linker allocates 40 + 40 + 36 = 116 bytes.
>>>>
>>>> The iterator in __llvm_profile_gather_value_data walks past the end of
>>>> the section. Here is a fix:
>>>>
>>>> - for (I = (__llvm_profile_data *)DataBegin; I != DataEnd; ++I) {
>>>> + for (I = (__llvm_profile_data *)DataBegin; I < DataEnd; ++I) {
>>>>
>>>> Is this good to commit?
>>>>
>>>> vedant
>>>>
>>>>
>>>>> On Feb 23, 2016, at 11:24 AM, Xinliang David Li <davidxl at google.com>
>>>>> wrote:
>>>>>
>>>>> Is the root cause of the issue understood now?
>>>>>
>>>>> thanks,
>>>>>
>>>>> David
>>>>>
>>>>> On Mon, Feb 22, 2016 at 11:55 AM, Xinliang David Li
>>>>> <davidxl at google.com> wrote:
>>>>>> On Mon, Feb 22, 2016 at 11:37 AM, Vedant Kumar <vsk at apple.com> wrote:
>>>>>>> vsk added a comment.
>>>>>>>
>>>>>>> Hi David,
>>>>>>>
>>>>>>> This commit enables tests on 64-bit Darwin which have apparently been
>>>>>>> failing. I now see:
>>>>>>>
>>>>>>> ******************** TEST 'Profile-i386 :: instrprof-basic.c' FAILED
>>>>>>> ********************
>>>>>>> Script:
>>>>>>> --
>>>>>>> /Users/vk/Desktop/llvm/Debug+Asserts/./bin/clang -arch i386
>>>>>>> -fprofile-instr-generate -o
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-basic.c.tmp
>>>>>>> -O3
>>>>>>> /Users/vk/Desktop/llvm/projects/compiler-rt/test/profile/instrprof-basic.c
>>>>>>> env
>>>>>>> LLVM_PROFILE_FILE=/Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-basic.c.tmp.profraw
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-basic.c.tmp
>>>>>>> llvm-profdata merge -o
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-basic.c.tmp.profdata
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-basic.c.tmp.profraw
>>>>>>> /Users/vk/Desktop/llvm/Debug+Asserts/./bin/clang -arch i386
>>>>>>> -fprofile-instr-use=/Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-basic.c.tmp.profdata
>>>>>>> -o - -S -emit-llvm
>>>>>>> /Users/vk/Desktop/llvm/projects/compiler-rt/test/profile/instrprof-basic.c |
>>>>>>> FileCheck
>>>>>>> /Users/vk/Desktop/llvm/projects/compiler-rt/test/profile/instrprof-basic.c
>>>>>>> --
>>>>>>> Exit Code: 139
>>>>>>>
>>>>>>> Command Output (stderr):
>>>>>>> --
>>>>>>>
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-basic.c.script:
>>>>>>> line 4: 20417 Segmentation fault: 11 env
>>>>>>> LLVM_PROFILE_FILE=/Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-basic.c.tmp.profraw
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-basic.c.tmp
>>>>>>>
>>>>>>> The segmentation fault is in `__llvm_profile_gather_value_data()`.
>>>>>>> Accessing `I->Values` dereferences `0x1`:
>>>>>>>
>>>>>>> (lldb) p *I
>>>>>>> (__llvm_profile_data) $11 = {
>>>>>>> NameRef = 10864
>>>>>>> FuncHash = 48378511633216
>>>>>>> CounterPtr = 0x00000000
>>>>>>> FunctionPointer = 0x0000df1e
>>>>>>> Values = 0x00000001
>>>>>>> NumCounters = 0
>>>>>>> NumValueSites = ([0] = 13723)
>>>>>>> }
>>>>>>>
>>>>>>
>>>>>> Looks like the prof data iterator is returning some garbage entry. For
>>>>>> that test, it should have only 3 entries. If it iterates more
>>>>>> entries, it is likely a linker bug on darwin.
>>>>>>
>>>>>>
>>>>>>> There's a second issue with corrupt raw profiles:
>>>>>>>
>>>>>>> ********************
>>>>>>> FAIL: Profile-i386 :: instrprof-without-libc.c (39 of 96)
>>>>>>> ******************** TEST 'Profile-i386 :: instrprof-without-libc.c'
>>>>>>> FAILED ********************
>>>>>>> Script:
>>>>>>> --
>>>>>>> /Users/vk/Desktop/llvm/Debug+Asserts/./bin/clang -arch i386
>>>>>>> -fprofile-instr-generate -DCHECK_SYMBOLS -O3 -o
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-without-libc.c.tmp.symbols
>>>>>>> /Users/vk/Desktop/llvm/projects/compiler-rt/test/profile/instrprof-without-libc.c
>>>>>>> llvm-nm
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-without-libc.c.tmp.symbols
>>>>>>> | FileCheck
>>>>>>> /Users/vk/Desktop/llvm/projects/compiler-rt/test/profile/instrprof-without-libc.c
>>>>>>> --check-prefix=CHECK-SYMBOLS
>>>>>>> /Users/vk/Desktop/llvm/Debug+Asserts/./bin/clang -arch i386
>>>>>>> -fprofile-instr-generate -O3 -o
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-without-libc.c.tmp
>>>>>>> /Users/vk/Desktop/llvm/projects/compiler-rt/test/profile/instrprof-without-libc.c
>>>>>>>
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-without-libc.c.tmp
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-without-libc.c.tmp.profraw
>>>>>>> llvm-profdata merge -o
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-without-libc.c.tmp.profdata
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-without-libc.c.tmp.profraw
>>>>>>> /Users/vk/Desktop/llvm/Debug+Asserts/./bin/clang -arch i386
>>>>>>> -fprofile-instr-use=/Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-without-libc.c.tmp.profdata
>>>>>>> -o - -S -emit-llvm
>>>>>>> /Users/vk/Desktop/llvm/projects/compiler-rt/test/profile/instrprof-without-libc.c
>>>>>>> | FileCheck
>>>>>>> /Users/vk/Desktop/llvm/projects/compiler-rt/test/profile/instrprof-without-libc.c
>>>>>>> --
>>>>>>> Exit Code: 1
>>>>>>>
>>>>>>> Command Output (stderr):
>>>>>>> --
>>>>>>> error:
>>>>>>> /Users/vk/Desktop/llvm/build/projects/compiler-rt/test/profile/Profile-i386/Output/instrprof-without-libc.c.tmp.profraw:
>>>>>>> Invalid instrumentation profile data (file header is corrupt)
>>>>>>>
>>>>>>> This happens because `ProfileSize = 34359738544` (pretty close to 32
>>>>>>> GB, which seems wrong):
>>>>>>>
>>>>>>> frame #0: 0x00000001001fa851
>>>>>>> llvm-profdata`llvm::RawInstrProfReader<unsigned
>>>>>>> int>::readHeader(this=0x0000000100b00000, Header=0x0000000100a00110) + 801
>>>>>>> at InstrProfReader.cpp:337
>>>>>>> 334
>>>>>>> 335 auto *Start = reinterpret_cast<const char
>>>>>>> *>(&Header);
>>>>>>> 336 if (Start + ProfileSize > DataBuffer->getBufferEnd())
>>>>>>> -> 337 return error(instrprof_error::bad_header);
>>>>>>>
>>>>>>
>>>>>> It is likely that ValueDataSize in the header has bogus value. It is
>>>>>> likely related to the bug above -- some bogus function prof data entry
>>>>>> gets processed.
>>>>>>
>>>>>>
>>>>>>> I don't know if this last issue is affected by this commit, but I'm
>>>>>>> not sure why these tests are unsupported (they don't seem Linux-specific):
>>>>>>
>>>>>> Looks like i386/darwin was never tested before? This just exposed some
>>>>>> latent bug.
>>>>>>
>>>>>> thanks,
>>>>>>
>>>>>> David
>>>>>>
>>>>>>>
>>>>>>> UNSUPPORTED: Profile-x86_64 :: Linux/coverage_ctors.cpp (1 of 32)
>>>>>>> UNSUPPORTED: Profile-x86_64 :: Linux/coverage_dtor.cpp (2 of 32)
>>>>>>> UNSUPPORTED: Profile-x86_64 :: Linux/instrprof-basic.c (3 of 32)
>>>>>>> UNSUPPORTED: Profile-x86_64 :: Linux/coverage_test.cpp (4 of 32)
>>>>>>> UNSUPPORTED: Profile-x86_64 :: Linux/coverage_shared.test (5 of 32)
>>>>>>> UNSUPPORTED: Profile-x86_64 :: Linux/instrprof-comdat.test (6 of 32)
>>>>>>> UNSUPPORTED: Profile-x86_64 :: Linux/instrprof-dlopen.test (7 of 32)
>>>>>>> UNSUPPORTED: Profile-x86_64 ::
>>>>>>> Linux/instrprof-dynamic-one-shared.test (8 of 32)
>>>>>>> UNSUPPORTED: Profile-x86_64 ::
>>>>>>> Linux/instrprof-dynamic-two-shared.test (9 of 32)
>>>>>>>
>>>>>>> For reference, I configure llvm with: `-DCMAKE_CROSSCOMPILING=False
>>>>>>> -DLLVM_TARGET_ARCH="X86" -DLLVM_TARGETS_TO_BUILD="X86;AArch64;ARM"
>>>>>>> -DCMAKE_BUILD_TYPE:STRING=Debug -DLLVM_ENABLE_ASSERTIONS=True
>>>>>>> -DLLVM_INCLUDE_TESTS=True -DLLVM_ENABLE_WARNINGS=False -G 'Ninja' ../`.
>>>>>>>
>>>>>>> AFAICT (checking -###), the compiler is grabbing the correct copy of
>>>>>>> compiler-rt and it has the right slice:
>>>>>>>
>>>>>>> $ ninja check-profile [...]
>>>>>>> $ file
>>>>>>> "/Users/vk/Desktop/llvm/build/bin/../lib/clang/3.9.0/lib/darwin/libclang_rt.profile_osx.a"
>>>>>>>
>>>>>>> /Users/vk/Desktop/llvm/build/bin/../lib/clang/3.9.0/lib/darwin/libclang_rt.profile_osx.a:
>>>>>>> Mach-O universal binary with 3 architectures: [i386: current ar archive
>>>>>>> random library] [x86_64: current ar archive random library] [x86_64h:
>>>>>>> current ar archive random library]
>>>>>>>
>>>>>>> /Users/vk/Desktop/llvm/build/bin/../lib/clang/3.9.0/lib/darwin/libclang_rt.profile_osx.a
>>>>>>> (for architecture i386): current ar archive random library
>>>>>>>
>>>>>>> /Users/vk/Desktop/llvm/build/bin/../lib/clang/3.9.0/lib/darwin/libclang_rt.profile_osx.a
>>>>>>> (for architecture x86_64): current ar archive random library
>>>>>>>
>>>>>>> /Users/vk/Desktop/llvm/build/bin/../lib/clang/3.9.0/lib/darwin/libclang_rt.profile_osx.a
>>>>>>> (for architecture x86_64h): current ar archive random library
>>>>>>>
>>>>>>> I'll try to get to the bottom of this, but any help is appreciated.
>>>>>>>
>>>>>>>
>>>>>>> Repository:
>>>>>>> rL LLVM
>>>>>>>
>>>>>>> http://reviews.llvm.org/D17361
>>>>>>>
>>>>>>>
>>>>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
More information about the llvm-commits
mailing list