[PATCH] D17361: [PGO] Add support to enable testing multiple supported targets

Thu Feb 25 15:05:09 PST 2016

On Thu, Feb 25, 2016 at 2:56 PM, Vedant Kumar <vsk at apple.com> wrote:

> You're right that for the instrprof-basic test DataSize is 2 on
> i386-Darwin (not 3 as it should be).
>
> I'm not an expert on the standard, but I don't think it mandates the
> padding. It doesn't seem like this is CodeGen bug, although I'd be happy to
> have someone else chime in.
>

The padding is part of the object. What is the value of
sizeof(__llvm_profile_data) ? It should be 40.

David

>
> I'm looking into a fix right now.
>
> vedant
>
>
> > On Feb 25, 2016, at 12:54 PM, Xinliang David Li <davidxl at google.com>
> wrote:
> >
> > On Thu, Feb 25, 2016 at 12:44 PM, Vedant Kumar <vsk at apple.com> wrote:
> >> I'm not sure what you mean by object symtab.
> >
> > 40 is the size recorded in object file's symtab (I only checked ELF)
> >
> >> 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.
> >
> > Is this a code gen bug? On Linux, the symbol will have an explicit
> > '.size 40' directive in the assembly file.
> >
> >>
> >> 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?
> >
> > The function gets dropped may not be examined in our test.  Can you
> > check for 32 bit dwarwin, what DataSize is recorded in the header?
> >
> > David
> >>
> >> 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
> >>>>>
> >>>>
> >>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160225/d3c5d65f/attachment.html>