Commit c6104028 authored by Yves Frederix's avatar Yves Frederix Committed by Brad King

Avoid occasional use-after-free when a variable watch is executed

Re-lookup a variable value when an associated VariableWatch is executed
in cmMakefile::GetDefinition.

This fixes a problem with 'def' sometimes becoming invalid due to memory
reallocation inside an std::vector. In this case, the problem was that
if the call to VariableAccessed actually executed a callback function,
the internal state of the makefile has changed due to the associated
function scope being pushed. This in turn implies that a new
cmDefinitions instance was pushed in cmMakefile::VarTree. As
cmLinkedTree is based on an std::vector, this push can have triggered
reallocation of its internal memory buffer. However, as the value of
'def', which was computed on method entry, actually points to a property
of one of the cmDefinitions instances in cmMakefile::VarTree,
reallocation can invalidate the value of 'def' so that it cannot simply
be returned at the end of the function. The solution implemented here is
to simply lookup the value of 'def' again.
parent b369959e
......@@ -2531,15 +2531,22 @@ const char* cmMakefile::GetDefinition(const std::string& name) const
cmVariableWatch* vv = this->GetVariableWatch();
if ( vv && !this->SuppressWatches )
{
if ( def )
{
vv->VariableAccessed(name, cmVariableWatch::VARIABLE_READ_ACCESS,
def, this);
}
else
{
bool const watch_function_executed =
vv->VariableAccessed(name,
cmVariableWatch::UNKNOWN_VARIABLE_READ_ACCESS, def, this);
def ? cmVariableWatch::VARIABLE_READ_ACCESS
: cmVariableWatch::UNKNOWN_VARIABLE_READ_ACCESS,
def, this);
if (watch_function_executed)
{
// A callback was executed and may have caused re-allocation of the
// variable storage. Look it up again for now.
// FIXME: Refactor variable storage to avoid this problem.
def = this->StateSnapshot.GetDefinition(name);
if(!def)
{
def = this->GetState()->GetInitializedCacheValue(name);
}
}
}
#endif
......
......@@ -96,7 +96,7 @@ void cmVariableWatch::RemoveWatch(const std::string& variable,
}
}
void cmVariableWatch::VariableAccessed(const std::string& variable,
bool cmVariableWatch::VariableAccessed(const std::string& variable,
int access_type,
const char* newValue,
const cmMakefile* mf) const
......@@ -112,5 +112,7 @@ void cmVariableWatch::VariableAccessed(const std::string& variable,
(*it)->Method(variable, access_type, (*it)->ClientData,
newValue, mf);
}
return true;
}
return false;
}
......@@ -42,7 +42,7 @@ public:
/**
* This method is called when variable is accessed
*/
void VariableAccessed(const std::string& variable, int access_type,
bool VariableAccessed(const std::string& variable, int access_type,
const char* newValue, const cmMakefile* mf) const;
/**
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment