Recursive Object Deserialization bug?

[william....@intel.com]

I am trying to place nodes uniformly in a circle around a fixed point, but getting a parse error trying to create the position allocator.

The code I wrote to do it looks something like this:

    NodeContainer myNodes;
    // ... code to configure myNodes ..
    std::ostringstream myAngles;
    myAngles
           << "ns3::SequentialRandomVariable[Min=0.0|Max=6.28|Consecutive=1|"
           << "Increment=ns3::ConstantRandomVariable[Constant="
           << 6.28 / myNodes.GetN()
           << "]]";
    mobility.SetPositionAllocator ("ns3::RandomDiscPositionAllocator",
           "X", DoubleValue (10.0),
           "Y", DoubleValue (10.0),
           "Rho" , StringValue ("ns3::ConstantRandomVariable[Constant=8.0]"),
           "Theta", StringValue (myAngles.str()));
    mobility.SetMobilityModel ("ns3::ConstantPositionMobilityModel");
    mobility.Install (myNodes);

I get a SIGSEGV running the code above.  When I debug into it, ns3::PointerValue::DeserializeFromString is trying to create an object of type "[ns3::ConstantRandomVariable[Constant=1.256", which ultimately causes ns3::IidManager::GetConstructor to crash.

The StringValue that gets passed to SetPositionAllocator is "ns3::SequentialRandomVariable[Min=0.0|Max=6.28|Consecutive=1|Increment=ns3::ConstantRandomVariable[Constant=1.256]]".  The problem is that string parameter for the `Rho` attribute has nested `[` and ']' characters in it.  When ns3::PointerValue::DeserialzeFromString tries to deserialize the "Rho" attribute, `std::istream & operator >> (std::istream &is, ObjectFactory &factory)` defined in src/core/model/object-factory.cc finds the closing ']' character using std::string::find, which gets the first occurrence of ']' rather than the second.  When it tries to parse the value to use for Increment, the closing ']' is missing, so parsing fails.

If I change operator >> to use `std::string::rfind` to get the closing ']' instead of `std::string::find`, the above code seems to work, and the NS3 unit tests all pass.  Here a patch of the change:

--- a/src/core/model/object-factory.cc    Mon Dec 05 17:37:33 2016 -0800
+++ b/src/core/model/object-factory.cc    Wed Dec 14 11:42:29 2016 -0800
@@ -126,7 +126,7 @@
   is >> v;
   std::string::size_type lbracket, rbracket;
   lbracket = v.find ("[");
-  rbracket = v.find ("]");
+  rbracket = v.rfind ("]");
   if (lbracket == std::string::npos && rbracket == std::string::npos)
     {
       factory.SetTypeId (v);

Is this a bug?  That is, are recursive objects allowed in an attribute definition?  Is there a case where using `rfind` instead of `find` will break something else?

FWIW, I am using version 3.26.

Thanks,
Bill.

[Tom Henderson]

Thanks, Bill, for a detailed report and patch.  This is the first such usage that I've seen, so perhaps it is a (fixable) bug.  I created a new issue in our tracker for it:

https://www.nsnam.org/bugzilla/show_bug.cgi?id=2586

- Tom

[pdbarnes]

Recursive attributes strings is really interesting.

In this particular case, why do you need ConstantRandomVariable[] instead of just the explicit constant, 1.256?

Peter

On Wednesday, December 14, 2016 at 2:16:26 PM UTC-8, Tom Henderson wrote:

On 12/14/2016 12:23 PM, william....@intel.com wrote:

I get a SIGSEGV running the code above.  When I debug into it, ns3::PointerValue::DeserializeFromString is trying to create an object of type "[ns3::ConstantRandomVariable[Constant=1.256", which ultimately causes ns3::IidManager::GetConstructor to crash.

[william....@intel.com]

I started with "ns3::SequentialRandomVariable[Min=0.0|Max=6.28|Consecutive=1|Increment=1.256]" as the value for the "Theta" attribute, but I get a SIGSEGV in the ObjectFactory that way.  It apparently gets an error creating an Object based on the string "1.256" when it deserializes the Increment parameter.

I did not see anything in the Doxygen documentation (https://www.nsnam.org/docs/doxygen/classns3_1_1_sequential_random_variable.html) saying what type Increment should take, but ns3::SequentialRandomVariable::GetInterval() returns an ns3::RandomVariableStream.  Not knowing any better, I guessed that I could probably just put a ns3::ConstantRandomVariable there using the same syntax as the top level attribute value :-)

Before I got the crash on Increment, I got a similar SIGSEGV when I tried to use "Consecutive=1.0" instead of "Consecutive=1" (double where uint32_t is expected).  So it was kind of fresh in my head that a type mismatch will cause a SIGSEGV, and that I might not have all the types right.

As a secondary issue, it might be nice to get a "type mismatch error" of some kind instead of a SIGSEGV when the type is wrong.

Is there an easier way to get nodes uniformly and deterministically distributed around a circle?  This way seemed pretty easy...

Thanks,
Bill.

[william....@intel.com]

On Wednesday, December 14, 2016 at 2:16:26 PM UTC-8, Tom Henderson wrote:

I am not sure the syntax for a container, but that seems like the most likely to cause problems.  For example,

    container[[obj1][obj2][obj3]]

The outer part will parse fine, but the brackets on the inner part will be in correct.  Just doing find will fail in this case, too, so rfind is not really any worse.

Bill.

[Tom Henderson]

On 12/14/2016 06:19 PM, william....@intel.com wrote:
> I started with
> "ns3::SequentialRandomVariable[Min=0.0|Max=6.28|Consecutive=1|Increment=1.256]"
> as the value for the "Theta" attribute, but I get a SIGSEGV in the
> ObjectFactory that way. It apparently gets an error creating an Object
> based on the string "1.256" when it deserializes the Increment parameter.
>
> I did not see anything in the Doxygen documentation
> (https://www.nsnam.org/docs/doxygen/classns3_1_1_sequential_random_variable.html)
> saying what type Increment should take, but
> ns3::SequentialRandomVariable::GetInterval() returns an
> ns3::RandomVariableStream. Not knowing any better, I guessed that I
> could probably just put a ns3::ConstantRandomVariable there using the
> same syntax as the top level attribute value :-)
>
> Before I got the crash on Increment, I got a similar SIGSEGV when I
> tried to use "Consecutive=1.0" instead of "Consecutive=1" (double where
> uint32_t is expected). So it was kind of fresh in my head that a type
> mismatch will cause a SIGSEGV, and that I might not have all the types
> right.
>
> As a secondary issue, it might be nice to get a "type mismatch error" of
> some kind instead of a SIGSEGV when the type is wrong.

Point noted (will look into it).

>
> Is there an easier way to get nodes uniformly and deterministically
> distributed around a circle? This way seemed pretty easy...

I would be inclined to use a ListPositionAllocator and just load the
desired points into that allocator, using spherical to cartesian
conversion to obtain the points.

- Tom