bug 9418 is about a crash which happens during mathml generation after
copying a math macro to the clipboard. The bug report shows several
problems, which I'll address eventually, but the actual crash happens
because Buffer::updateMacroInstances() iterates through the insets from
begin to end. Thus, if a macro \a calls another macro \b with arguments
(which needs to be defined before \a), and \b is no longer found for some
reason (in this case because it was not part of the copied text), \b is
replaced by an unknown macro without arguments, and the arguments are
converted to other math insets. However, \a has just been updated, and an
ArgumentProxy pointing to the (now deleted) argument of \b has just been
created. This ArgumentProxy holds a reference to freed memory, and the fact
that the crash does not always happen is pure luck (sometimes the memory is
not immediately reused).
This problem would be easily avoided if one would iterate backwards through
the macro insets (a macro can never reference another one which follows
later), but unfortunately I failed to implement that, since I do not
understand the DocIterator stuff well enough. I tried several versions, both
using nextInset() and prevInset() in DocIterator::backwardInset() and
Buffer::updateMacroInstances(), but nothing worked. The attached patcdh
shows one failed attempt. Does anybody have a clue how to implement a
backward inset iteration?
Georg
PS: The math macro stuff is far too complicated.
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index f9da416..1dce385 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -3404,10 +3404,10 @@ void Buffer::updateMacroInstances(UpdateType utype) const
{
LYXERR(Debug::MACROS, "updateMacroInstances for "
<< d->filename.onlyFileName());
- DocIterator it = doc_iterator_begin(this);
- it.forwardInset();
- DocIterator const end = doc_iterator_end(this);
- for (; it != end; it.forwardInset()) {
+ DocIterator it = doc_iterator_end(this);
+ it.backwardInset();
+ DocIterator const beg = doc_iterator_begin(this);
+ for (; it != beg; it.backwardInset()) {
// look for MathData cells in InsetMathNest insets
InsetMath * minset = it.nextInset()->asInsetMath();
if (!minset)
@@ -3416,8 +3416,8 @@ void Buffer::updateMacroInstances(UpdateType utype) const
// update macro in all cells of the InsetMathNest
DocIterator::idx_type n = minset->nargs();
MacroContext mc = MacroContext(this, it);
- for (DocIterator::idx_type i = 0; i < n; ++i) {
- MathData & data = minset->cell(i);
+ for (DocIterator::idx_type i = n; i > 0; --i) {
+ MathData & data = minset->cell(i-1);
data.updateMacros(0, mc, utype);
}
}
diff --git a/src/DocIterator.cpp b/src/DocIterator.cpp
index e574115..d4bcad1 100644
--- a/src/DocIterator.cpp
+++ b/src/DocIterator.cpp
@@ -449,6 +449,28 @@ void DocIterator::backwardPos()
}
+void DocIterator::backwardInset()
+{
+ backwardPos();
+
+ while (!empty() && !nextInset()) {
+ if (inTexted()) {
+ Paragraph const & par = paragraph();
+ pos_type & pos = top().pos();
+ if (pos > 0 && pos == top().lastpos())
+ --pos;
+ if (pos < top().lastpos()) {
+ while (pos > 0 && !par.isInset(pos))
+ --pos;
+ if (pos > 0)
+ break;
+ }
+ }
+ backwardPos();
+ }
+}
+
+
bool DocIterator::hasPart(DocIterator const & it) const
{
// it can't be a part if it is larger
diff --git a/src/DocIterator.h b/src/DocIterator.h
index 7796819..20ca8fe 100644
--- a/src/DocIterator.h
+++ b/src/DocIterator.h
@@ -202,8 +202,7 @@ public:
/// move backward one paragraph
void backwardPar();
/// move backward one inset
- /// FIXME: This is not implemented!
- //void backwardInset();
+ void backwardInset();
/// are we some 'extension' (i.e. deeper nested) of the given iterator
bool hasPart(DocIterator const & it) const;
