[ 
https://issues.apache.org/jira/browse/MESOS-3335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15585813#comment-15585813
 ] 

Michael Park commented on MESOS-3335:
-------------------------------------

{noformat}
commit 92fb8e7ee2280b5b5784eeee0fd3d2f5f3e3814c
Author: Benjamin Bannier <benjamin.bann...@mesosphere.io>
Date:   Tue Oct 18 03:29:54 2016 -0400

    Removed the non-member pointer overloads of `FlagsBase::add`.

    Review: https://reviews.apache.org/r/46825/
{noformat}
{noformat}
commit f441eb9adb8c1443e62e10d17ed4019b66391168
Author: Benjamin Bannier <benjamin.bann...@mesosphere.io>
Date:   Tue Oct 18 04:48:10 2016 -0400

    Fully qualified addresses of flag members in `add` calls in mesos.

    While right now we can technically `add` variables to `Flags` classes
    which are not members, the in order to have correct copy semantics for
    `Flags` only member variables should be used.

    Here we changed all instances to a full pointer-to-member syntax in
    the current code.

    Review: https://reviews.apache.org/r/46824/
{noformat}
{noformat}
commit dde5eee7b11be8df874571316e29a9a25ae59150
Author: Benjamin Bannier <benjamin.bann...@mesosphere.io>
Date:   Tue Oct 18 03:29:39 2016 -0400

    Fully qualified addresses of flag members in `add` calls in libprocess.

    While right now we can technically `add` variables to `Flags` classes
    which are not members, the in order to have correct copy semantics for
    `Flags` only member variables should be used.

    Here we changed all instances to a full pointer-to-member syntax in
    the current code.

    Review: https://reviews.apache.org/r/46823/
{noformat}
{noformat}
commit 5aeecca345f80d5d34c9a8c8b64d460bcd773e30
Author: Benjamin Bannier <benjamin.bann...@mesosphere.io>
Date:   Tue Oct 18 03:29:14 2016 -0400

    Fully qualified addresses of flag members in `add` calls in stout.

    While right now we can technically `add` variables to `Flags` classes
    which are not members, the in order to have correct copy semantics for
    `Flags` only member variables should be used.

    Here we changed all instances to a full pointer-to-member syntax in
    the current code.

    Review: https://reviews.apache.org/r/52390/
{noformat}

> FlagsBase copy-ctor leads to dangling pointer.
> ----------------------------------------------
>
>                 Key: MESOS-3335
>                 URL: https://issues.apache.org/jira/browse/MESOS-3335
>             Project: Mesos
>          Issue Type: Bug
>            Reporter: Neil Conway
>            Assignee: Benjamin Bannier
>              Labels: mesosphere
>         Attachments: lambda_capture_bug.cpp
>
>
> Per [#3328], ubsan detects the following problem:
> [ RUN ] FaultToleranceTest.ReregisterCompletedFrameworks
> /mesos/3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp:303:25:
>  runtime error: load of value 33, which is not a valid value for type 'bool'
> I believe what is going on here is the following:
> * The test calls StartMaster(), which does MesosTest::CreateMasterFlags()
> * MesosTest::CreateMasterFlags() allocates a new master::Flags on the stack, 
> which is subsequently copy-constructed back to StartMaster()
> * The FlagsBase constructor is:
> bq. {{FlagsBase() { add(&help, "help", "...", false); }}}
> where "help" is a member variable -- i.e., it is allocated on the stack in 
> this case.
> * {{FlagsBase()::add}} captures {{&help}}, e.g.:
> {noformat}
> flag.stringify = [t1](const FlagsBase&) -> Option<std::string> {
>     return stringify(*t1);
>   };}}
> {noformat}
> * The implicit copy constructor for FlagsBase is just going to copy the 
> lambda above, i.e., the result of the copy constructor will have a lambda 
> that points into MesosTest::CreateMasterFlags()'s stack frame, which is bad 
> news.
> Not sure the right fix -- comments welcome. You could define a copy-ctor for 
> FlagsBase that does something gross (basically remove the old help flag and 
> define a new one that points into the target of the copy), but that seems, 
> well, gross.
> Probably not a pressing-problem to fix -- AFAICS worst symptom is that we end 
> up reading one byte from some random stack location when serving 
> {{state.json}}, for example.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to