lkml.org 
[lkml]   [2006]   [Apr]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: Compiling C++ modules
    Kyle Moffett wrote:
    > On Apr 25, 2006, at 03:08:02, Avi Kivity wrote:
    >> Kyle Moffett wrote:
    >>> The "advantages" of the former over the latter:
    >>>
    >>> (1) Without exceptions (which are fragile in a kernel), the former
    >>> can't return an error instead of initializing the Foo.
    >> Don't discount exceptions so fast. They're exactly what makes the
    >> code clearer and more robust.
    >
    > Except making exceptions work in the kernel is exceptionally
    > nontrivial (sorry about the pun).
    My experience with exceptions in kernel-like code (a distributed
    filesystem) was excellent.
    >
    >> A very large proportion of error handling consists of:
    >> - detect the error
    >> - undo local changes (freeing memory and unlocking spinlocks)
    >> - propagate the error/
    >>
    >> Exceptions make that fully automatic. The kernel uses a mix of gotos
    >> and alternate returns which bloat the code and are incredibly error
    >> prone. See the recent 2.6.16.x for examples.
    >
    > You talk about code bloat here. Which of the following fits better
    > into a 4k stack?
    The C++ code. See below.
    > Which of the following shows the flow of code better?
    Once you accept the idea that an exception can occur (almost) anywhere,
    the C++ code shows you what the code does in the normal case without
    obfuscating it with error handling. Pretend that after every semicolon
    there is a comment of the form:

    /* possible exceptional return */

    once you think like that, you can see what the code actually does rather
    than how it handles errors. A 15-line function can do something
    meaningful, not just call two functions.
    >
    > C version:
    > int result;
    > spin_lock(&lock);
    >
    > result = do_something();
    > if (result)
    > goto out;
    >
    > result = do_something_else();
    > if (result)
    > goto out;
    >
    > out:
    > spin_unlock(&lock);
    > return result;
    >
    > C++ version:
    > int result;
    not needed unless you actually return something.
    > TakeLock l(&lock);
    >
    > do_something();
    > do_something_else();
    >
    > First of all, that extra TakeLock object chews up stack, at least 4 or
    > 8 bytes of it, depending on your word size.
    No, it's optimized out. gcc notices that &lock doesn't change and that
    'l' never escapes the function.
    > Secondly with standard integer error returns you have one or two
    > easily-predictable assembly instructions at each step of the way,
    > whereas with exceptions you trade the absence of error handling in the
    > rest of the code for a lot of extra instructions at the exception
    > throw and catch points.
    The extra code is out of line (not even an if (unlikely())). So yes,
    total code grows, but the exceptional paths can be in a .text.exception
    section and not consume cache or TLB space.
    > Secondly, while the former is much longer it shows _explicitly_
    > exactly where the flow of code can go. In an OS kernel, that is
    > critical; your debugability is directly dependent on how easy it is
    > to see where the flow of code is going.
    >
    I can only say that I had very positive experience with code that used
    exceptions. Having less code to view actually improves visibility.
    >>> (2) You can't control when you initialize the Foo. For example in
    >>> this code, the "Foo item;" declarations seem to be trivially
    >>> relocatable, even if they're not.
    >>> spin_lock(&foo_lock);
    >>> Foo item1;
    >>> Foo item2;
    >>> spin_unlock(&foo_lock);
    >>
    >> They only seem relocatable with your C glasses on. Put on your C++
    >> glasses (much thicker), and initialization no longer seems trivially
    >> movable.
    >
    > This is a really _really_ bad idea for a kernel. Having simple
    > declaration statements have big side effects (like the common TakeLock
    > object example I gave above) is bound to lead to people screwing up
    > and forgetting about the side effects. In C it's impossible to miss
    > the side effects of a statement; function calls are obvious, as is
    > global memory modification.
    >
    In C++ you just have to treat declarations as executable statements.
    Just as you can't compile the code with a C compiler, you can't read it
    with a C mindset. Once you get used to it, it isn't surprising at all.
    >> On the other hand, you can replace the C code
    >>
    >> {
    >> Foo item1, item2;
    >> int r;
    >>
    >> spin_lock(&foo_lock);
    >> if ((r = foo_init(&item1)) < 0) {
    >> spin_unlock(&foo_lock);
    >> return r;
    >> }
    >> if ((r = foo_init(&item2)) < 0) {
    >> foo_destroy(&item1);
    >> spin_unlock(&foo_lock);
    >> return r;
    >> }
    >> foo_destroy(&item2);
    >> foo_destroy(&item1);
    >> spin_unlock(&foo_lock);
    >> return 0;
    >> }
    >>
    >> with
    >>
    >> {
    >> spinlock_t::guard foo_guard(foo_lock);
    >> Foo item1;
    >> Foo item2;
    >> }
    >
    > Let me point out _again_ how unobvious and fragile the flow of code
    > there is. Not to mention the fact that the C++ compiler can easily
    > notice that item1 and item2 are never used and optimize them out entirely.
    Excellent! If there are no side effects, I want it out. If there are
    side effects, it won't optimize them out.
    > You also totally missed the "int flags" argument you're supposed to
    > pass to object specifying allocation parameters,
    There is no allocation here (both the C and the C++ code allocate on the
    stack.

    Should you want to allocate from the heap, try this:

    {
    spinlock_t::guard g(some_lock);
    auto_ptr<Foo> item(new (gfp_mask) Foo); /* or pass a kmem_cache_t */
    item->do_something();
    item->do_something_else();
    return item.release();
    }

    contrast with

    {
    Foo *item = 0;
    int r;

    spin_lock(&some_lock);
    item = kmalloc(sizeof(Foo), gfp_flags);
    if (!item) {
    item = PTR_ERR(ENOMEM);
    goto out;
    }
    if ((r = foo_do_something(item))) {
    foo_destroy(item);
    kfree(item);
    item = PTR_ERR(-r);
    goto out;
    }
    if ((r = foo_do_something_else(item))) {
    foo_destroy(item);
    kfree(item);
    item = PTR_ERR(-r);
    }
    out:
    spin_unlock(&some_lock);
    return item;
    }

    (oops, you wrote another version further on...)
    > not to mention the fact that you just allocated 2 objects of unknown
    > size on the stack (which is limited to 4k).
    So did the C code. I was just side-by-side comparisons of *equivalent* code.
    > AND there's the fact that the order of destruction of foo_guard,
    > item1, and item2 is implementation-defined and can't easily be relied
    > upon without adding massive amounts of excess braces:
    It is well defined. guard ctor, item1 ctor, item2 ctor, item2 dtor,
    item1 dtor, guard dtor.

    It is also well defined that if a constructor is executed for an
    automatic variable, so will its destructor. That makes the "guard" work
    so well with exceptions.
    >
    > {
    > spinlock_t::guard foo_guard(&foo_lock);
    > {
    > Foo item1;
    > {
    > Foo item2;
    > }
    > }
    > }
    >
    Absolutely unneeded braces there.
    > Also, your spinlock_t::guard chews up stack space that otherwise
    > wouldn't be used. It would be much better to rewrite your above C
    > function like this:
    >
    > {
    > struct foo *item1, *item2;
    > int result;
    > spin_lock(&foo_lock);
    >
    > item1 = kmalloc(sizeof(*item1), GFP_KERNEL);
    > item2 = kmalloc(sizeof(*item2), GFP_KERNEL);
    > if (!item1 || !item2)
    > goto out;
    >
    > result = foo_init(item1, GFP_KERNEL);
    > if (result)
    > goto out;
    >
    > result = foo_init(item2, GFP_KERNEL);
    > if (result)
    > goto out;
    >
    > out:
    > /* If alloc and init went ok, register them */
    > if (item1 && item2 && !result) {
    > result = register_foo_pair(item1, item2);
    > }
    >
    > /* If anything failed, free resources */
    > if (!item1 || !item2 || result) {
    > kfree(item1);
    > kfree(item2);
    > }
    >
    > spin_unlock(&foo_lock);
    > return result;
    > }
    >
    >> 14 lines vs 3, one variable eliminated. How many potential security
    >> vulnerabilities? How much time freed to work on the algorithm/data
    >> structure, not on error handling?
    >
    > Yeah, sure, yours is 3 lines when you omit the following:
    > (1) Handling allocation flags like GFP_KERNEL
    done
    > (2) Not allocating things on the stack
    done
    > (3) Proper cleanup ordering
    done, without lifting a finger
    > (4) Reference counting, garbage collection, or another way to
    > selectively free the allocated objects based on success or failure of
    > other code.
    Reference counting is ridiculously to do in C++. I'll spare you the details.
    >
    > Those are all critical things that we want to force people to think
    > about; in many cases the exact ordering of operations _is_ important
    > and that needs to be specified _and_ commented on. How often do you
    > think people write comments talking about things that don't even
    > appear in the source code?
    C++ preserves the order (unless it can prove the order doesn't matter,
    same as C)
    >
    > Also, since when is error handling _not_ a critical part of the
    > algorithm? You can see in my more complicated example that you only
    > want to free the items if the registration was unsuccessful. How do
    > you handle that without adding a refcount to everything (bloat) or
    > implementing garbage collection (worse bloat).
    Use auto_ptr<>.

    Yes, error handling is critical. That's why I want language help. Just
    like I don't want to:
    - calculate the carry for 64 bit addition on a 32 bit machine
    - calculate structure offsets for structs (even though struct layout is
    important)

    I faced exactly these problems (fibre channel and ethernet cables pulled
    out during I/O) and C++ made error handling easier, not more difficult.

    >
    >>> Does that actually make it any easier to understand the code? How
    >>> does it make it more obvious to be able to write a "+" operator that
    >>> allocates memory?
    >>
    >> Not all C++ features need to be used in the kernel. In fact, not all
    >> C++ features need to be used, period. Ever tried to understand code
    >> which uses overloaded operator,() (the comma operator)?
    >
    > The very fact that the language provides such features mean that
    > people would try to get code using them into the kernel. Have you
    > ever looked at all the ugly debugging macros that various people use?
    > The C preprocessor provides few features at all, and yet people still
    > abuse those, I don't see why C++ would be any different.
    Probably not. I agree C++ is much more abusable.

    BTW, under C++ preprocessor abuse drops significantly as it can be
    replaced by safer and cleaner constructs.

    --
    Do not meddle in the internals of kernels, for they are subtle and quick to panic.

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-04-25 18:48    [W:0.051 / U:64.080 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site