lkml.org 
[lkml]   [2021]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] gcov: fail build on gcov_info size mismatch
    From
    Date
    On 11.03.2021 19:38, Linus Torvalds wrote:
    > On Thu, Mar 11, 2021 at 5:07 AM Peter Oberparleiter
    > <oberpar@linux.ibm.com> wrote:
    >>
    >> This patch adds a compile-time check to ensure that the kernel's version
    >> of struct gcov_info has the same length as the one used by GCC as
    >> determined by looking at GCC's assembler output.
    >
    > So I don't think this is a bad idea, but if you end up test-compiling
    > something, could we not verify things a bit more?

    I thought about this as well, based on the idea to not only look at the
    size, but potentially also at the assembler-level definition of GCC's
    gcov_info version, and to compare that to how the kernel's version looks
    like. But then I thought that this still doesn't catch the cases where
    the semantics of a struct member changed.

    > If you actually build the object file, you should be able to then
    > check much more. You'll find the pointer to the struct gcov_info in
    > "__gcov_.fn", which is admittedly hard to then link against a test
    > program (because of that dot in the name that means that you can't
    > even use "attribute((alias..))" to generate some other name for it).

    This is an idea I hadn't considered - a user space test program that
    accesses GCC's gcov_info data using the kernel's definition would be
    able to fail a lot more gracefully than the kernel could.

    Getting a pointer to the gcov_info struct in a program isn't actually
    that difficult: just provide a custom function named __gcov_init and
    don't link with libgcov. At program start constructor code will then
    call this function with the gcov_info pointer as parameter. This is
    exactly how the kernel collects all these gcov_info instances.

    > But then you could test not only the size, but you could verify that
    > the "filename" field matches, that the n_functions field should be 1
    > etc.

    Taking that idea a step further, a test program could exercise the exact
    same code that the kernel uses to generate a .gcda file from in-memory
    gcov_info data (or die/hang when there's a mismatch in the struct
    definition) and write that out. The resulting .gcda file could then be
    verified for correctness by passing it to GCC's gcov tool.

    This shouldn't be too difficult to achieve since there is already a
    kernel function that converts a gcov_info into .gcda file format
    in-memory with no actual dependency on kernel infrastructure. Of course
    this requires the gcov kernel code to be taught to live in user space
    with a healthy dose of ifdef-ing.

    Is there a convention on what is the lesser evil: adding multiple
    ifdef-endifs all over the place or moving code around for no apparent
    reason other than to gather all required parts in one place for a single
    ifdef?

    [...]

    > I dunno. The gcov code has obviously never actually done anything like
    > this before, so maybe I'm just taking the "we could verify
    > _something_" and my reaction is that there could be even more
    > verification if we really want to go down that rabbit hole..

    I'll try to come up with a new patch that introduces a test like
    described above. If that approach fails we can fall back to the original
    approach (plus the fix for the kernel test robot findings).


    Regards,
    Peter

    --
    Peter Oberparleiter
    Linux on Z Development - IBM Germany

    \
     
     \ /
      Last update: 2021-03-12 18:47    [W:3.566 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site