Fix math macro crash (bug 9418)

In the test case the crash occured in mathml export of the temporary buffer,
because the macro was updated, and because one of the used other macros was
not copied, the macro argument was detached. However, the underlying problem
of the crash was a broken ArgumentProxy::mathMacro_ reference which became
invalid each time the ownng MathMacro was copied. In the bug test case the
copying happened due to resizing a std::vector, but any other copy would have
created the same problem. The crash did not always happen, because sometimes
the old freed memory was not immediately reused, so the invalid reference did
still point to usable data.

The fix is easy: Convert ArgumentProxy::mathMacro_ to a pointer and update it
always after creating a copy of the owner.

src/mathed/MathMacro.cpp

@@ -52,35 +52,37 @@ namespace lyx {
 class ArgumentProxy : public InsetMath {
 public:
 	///
-	ArgumentProxy(MathMacro & mathMacro, size_t idx)
+	ArgumentProxy(MathMacro * mathMacro, size_t idx)
 		: mathMacro_(mathMacro), idx_(idx) {}
 	///
-	ArgumentProxy(MathMacro & mathMacro, size_t idx, docstring const & def)
+	ArgumentProxy(MathMacro * mathMacro, size_t idx, docstring const & def)
 		: mathMacro_(mathMacro), idx_(idx)
 	{
 			asArray(def, def_);
 	}
 	///
+	void setOwner(MathMacro * mathMacro) { mathMacro_ = mathMacro; }
+	///
 	InsetCode lyxCode() const { return ARGUMENT_PROXY_CODE; }
 	///
 	void metrics(MetricsInfo & mi, Dimension & dim) const {
-		mathMacro_.macro()->unlock();
-		mathMacro_.cell(idx_).metrics(mi, dim);
+		mathMacro_->macro()->unlock();
+		mathMacro_->cell(idx_).metrics(mi, dim);
 
-		if (!mathMacro_.editMetrics(mi.base.bv)
-		    && mathMacro_.cell(idx_).empty())
+		if (!mathMacro_->editMetrics(mi.base.bv)
+		    && mathMacro_->cell(idx_).empty())
 			def_.metrics(mi, dim);
 
-		mathMacro_.macro()->lock();
+		mathMacro_->macro()->lock();
 	}
 	// FIXME Other external things need similar treatment.
 	///
-	void mathmlize(MathStream & ms) const { ms << mathMacro_.cell(idx_); }
+	void mathmlize(MathStream & ms) const { ms << mathMacro_->cell(idx_); }
 	///
-	void htmlize(HtmlStream & ms) const { ms << mathMacro_.cell(idx_); }
+	void htmlize(HtmlStream & ms) const { ms << mathMacro_->cell(idx_); }
 	///
 	void draw(PainterInfo & pi, int x, int y) const {
-		if (mathMacro_.editMetrics(pi.base.bv)) {
+		if (mathMacro_->editMetrics(pi.base.bv)) {
 			// The only way a ArgumentProxy can appear is in a cell of the
 			// MathMacro. Moreover the cells are only drawn in the DISPLAY_FOLDED
 			// mode and then, if the macro is edited the monochrome
@@ -90,22 +92,22 @@ public:
 			// here (and the assert triggers in pain.leaveMonochromeMode())
 			// it's a bug.
 			pi.pain.leaveMonochromeMode();
-			mathMacro_.cell(idx_).draw(pi, x, y);
+			mathMacro_->cell(idx_).draw(pi, x, y);
 			pi.pain.enterMonochromeMode(Color_mathbg, Color_mathmacroblend);
-		} else if (mathMacro_.cell(idx_).empty()) {
-			mathMacro_.cell(idx_).setXY(*pi.base.bv, x, y);
+		} else if (mathMacro_->cell(idx_).empty()) {
+			mathMacro_->cell(idx_).setXY(*pi.base.bv, x, y);
 			def_.draw(pi, x, y);
 		} else
-			mathMacro_.cell(idx_).draw(pi, x, y);
+			mathMacro_->cell(idx_).draw(pi, x, y);
 	}
 	///
 	size_t idx() const { return idx_; }
 	///
 	int kerning(BufferView const * bv) const
 	{
-		if (mathMacro_.editMetrics(bv)
-		    || !mathMacro_.cell(idx_).empty())
-			return mathMacro_.cell(idx_).kerning(bv);
+		if (mathMacro_->editMetrics(bv)
+		    || !mathMacro_->cell(idx_).empty())
+			return mathMacro_->cell(idx_).kerning(bv);
 		else
 			return def_.kerning(bv);
 	}
@@ -117,7 +119,7 @@ private:
 		return new ArgumentProxy(*this);
 	}
 	///
-	MathMacro & mathMacro_;
+	MathMacro * mathMacro_;
 	///
 	size_t idx_;
 	///
@@ -133,6 +135,50 @@ MathMacro::MathMacro(Buffer * buf, docstring const & name)
 {}
 
 
+MathMacro::MathMacro(MathMacro const & that)
+	: InsetMathNest(that), expanded_(that.buffer_)
+{
+	assign(that);
+}
+
+
+MathMacro & MathMacro::operator=(MathMacro const & that)
+{
+	if (&that == this)
+		return *this;
+	InsetMathNest::operator=(that);
+	assign(that);
+	return *this;
+}
+
+
+void MathMacro::assign(MathMacro const & that)
+{
+	name_ = that.name_;
+	displayMode_ = that.displayMode_;
+	expanded_ = that.expanded_;
+	definition_ = that.definition_;
+	attachedArgsNum_ = that.attachedArgsNum_;
+	optionals_ = that.optionals_;
+	nextFoldMode_ = that.nextFoldMode_;
+	macroBackup_ = that.macroBackup_;
+	macro_ = that.macro_;
+	editing_ = that.editing_;
+	requires_ = that.requires_;
+	needsUpdate_ = that.needsUpdate_;
+	isUpdating_ = that.isUpdating_;
+	appetite_ = that.appetite_;
+	// Update the pointers to our owner of all expanded macros.
+	// This needs to be called every time a copy of the owner is created
+	// (bug 9418).
+	for (size_t i = 0; i < expanded_.cell(0).size(); ++i) {
+		ArgumentProxy * p = dynamic_cast<ArgumentProxy *>(expanded_.cell(0)[i].nucleus());
+		if (p)
+			p->setOwner(this);
+	}
+}
+
+
 Inset * MathMacro::clone() const
 {
 	MathMacro * copy = new MathMacro(*this);
@@ -358,9 +404,9 @@ void MathMacro::updateRepresentation(Cursor * cur, MacroContext const & mc,
 	for (size_t i = 0; i < nargs(); ++i) {
 		ArgumentProxy * proxy;
 		if (i < defaults.size())
-			proxy = new ArgumentProxy(*this, i, defaults[i]);
+			proxy = new ArgumentProxy(this, i, defaults[i]);
 		else
-			proxy = new ArgumentProxy(*this, i);
+			proxy = new ArgumentProxy(this, i);
 		values[i].insert(0, MathAtom(proxy));
 	}
 	// expanding macro with the values

src/mathed/MathMacro.h

@@ -28,6 +28,10 @@ public:
 	/// A macro can be built from an existing template
 	MathMacro(Buffer * buf, docstring const & name);
 	///
+	MathMacro(MathMacro const &);
+	///
+	MathMacro & operator=(MathMacro const &);
+	///
 	virtual MathMacro * asMacro() { return this; }
 	///
 	virtual MathMacro const * asMacro() const { return this; }
@@ -156,6 +160,8 @@ private:
 	virtual Inset * clone() const;
 	///
 	bool editMode(BufferView const * bv) const;
+	/// Copy all members (except base class members)
+	void assign(MathMacro const &);
 	
 	/// name of macro
 	docstring name_;

status.21x

@@ -125,6 +125,8 @@ What's new
 
 - Fix insertion of spaces in macro definitions (bug 9432).
 
+- Fix crash when copying macros with arguments (bug 9418).
+
 - Fix name and hint of figure captions in documents using the class aastex.
 
 - Fix output encoding information for non-TeX fonts XeTeX/LuaTeX on preview