[
https://issues.apache.org/jira/browse/MESOS-3335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15438155#comment-15438155
]
Michael Park commented on MESOS-3335:
-------------------------------------
{noformat}
commit f3181999fd03c99078994855687749839eb5365f
Author: Benjamin Bannier <[email protected]>
Date: Thu Aug 25 13:53:15 2016 -0700
Avoided slicing of flags from `subprocess` in mesos.
While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).
Since `Option<T>` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option<T>` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.
Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.
Review: https://reviews.apache.org/r/46822/
{noformat}
{noformat}
commit d05625fd7dc8cdf8c0007f2d9ddd2d739cfd9f5d
Author: Benjamin Bannier <[email protected]>
Date: Thu Aug 25 13:53:10 2016 -0700
Avoided slicing of flags from `subprocess` in libprocess.
While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).
Since `Option<T>` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option<T>` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.
Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.
Review: https://reviews.apache.org/r/46821/
{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)
