CODEWITHSUNDEEP
c++c++11
charlie_hustle
charlie_hustle

Reputation: 3

C++ Memory Corruption When Using String Literals

I'm a newcomer to C++ and have written a small program to learn more about OOD and polymorphism.

In summary I have a base class (WeeklyCompCalculator) containing a single member variable (string description) that is passed in via a constructor call in the derived class (FixedCompCalculator).

When I initialize the base class with a const string (FIXED_SCHEME) everything works fine: 

FixedCompCalculator::FixedCompCalculator() : WeeklyCompCalculator(FIXED_SCHEME) {}

image

However, when I attempt to initialize with a string literal, the description member is corrupted:

FixedCompCalculator::FixedCompCalculator() : WeeklyCompCalculator("$600 Weekly") {}

image

I have included the relevant bits of code below. An interesting observation is that if I modify shoe_comp.cpp to use raw pointers instead of shared_ptrs, the issue goes away.

comp_calculator.h

namespace ShoeCompUtil {
  const string FIXED_SCHEME = "$600 Weekly";
  class WeeklyCompCalculator {
  public:
    WeeklyCompCalculator(const string&);
    virtual ~WeeklyCompCalculator();
    virtual double calculate( const int shoePrice, const int weeklyHours, const int weeklyUnits ) const = 0;
    string getDescription() const;
  private:
    const string& description;
  };

  class FixedCompCalculator : public WeeklyCompCalculator {
  public:
    FixedCompCalculator();
    virtual double calculate( const int shoePrice, const int weeklyHours, const int weeklyUnits ) const;
  };
};

comp_calculator.cpp

WeeklyCompCalculator::WeeklyCompCalculator(const string& descr): description{descr} {}
WeeklyCompCalculator::~WeeklyCompCalculator(){}
string WeeklyCompCalculator::getDescription() const {
  const string descr{description};
  return descr;
}

FixedCompCalculator::FixedCompCalculator() : WeeklyCompCalculator("$600 Weekly") {}
double FixedCompCalculator::calculate( const int shoePrice, const int weeklyHours, const int weeklyUnits ) const 
{ 
  return 600; 
}

shoe_comp.cpp

cout << "Computing best compensation scheme given shoe_price = " << shoePrice << ", weeklyHours = " << weeklyHours << ", weeklyUnits = " << weeklyUnits << endl;
vector<shared_ptr<WeeklyCompCalculator>> calculators { make_shared<FixedCompCalculator>(), make_shared<HybridCompCalculator>(), make_shared<CommissionCompCalculator>() };
double maxComp = -1;
string maxDescr = "";
for( const shared_ptr<WeeklyCompCalculator>& calc : calculators ){
  double compHere = calc->calculate( shoePrice, weeklyHours, weeklyUnits );
  if( compHere > maxComp ){
    maxComp = compHere;
    maxDescr = calc->getDescription();
  }
}
cout << "Optimal compensation scheme is " << maxDescr << ", weekly comp = " << maxComp << endl;

Upvotes: 0

Views: 586

Answers (1)

Sam Varshavchik
Sam Varshavchik

Reputation: 118300

Your derived class constructor passes a string literal to the base class:

FixedCompCalculator::FixedCompCalculator() : WeeklyCompCalculator("$600 Weekly")

The base class's constructor takes a const std::string & as a parameter to its constructor.

WeeklyCompCalculator(const string&);

Because your parameter is actually a string literal, a temporary std::string object gets constructed, and a reference to it gets passed to the base class's constructor.

The base class constructor takes the reference it receives, as its argument, and saves it as its class's member.

Because the class member ends up being a reference to a temporary object, as soon as the derived class finishes constructing (slightly earlier, to be pedantically correct, but it's not relevant here), the temporary object gets destroyed, and the base class ends up with a reference to a destroyed object, and using this reference from that point on results in undefined behavior.

The simplest fix is to have WeeklyCompCalculator's class member to be an explicit object instance, and not a reference:

  private:
    const string description;
  };

Now, the description class member is guaranteed to exist as long as the instance of its class exists.

Upvotes: 2

Related Questions