[ 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 <benjamin.bann...@mesosphere.io> 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 <benjamin.bann...@mesosphere.io> 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)