Hi all,
reading the source code of ThrowErrorData,
it try to copy an ErrorData object into the correct memory context:
newedata = &errordata[errordata_stack_depth];
recursion_depth++;
oldcontext = MemoryContextSwitchTo(newedata->assoc_context);
/* Copy the supplied fields to the error stack entry. */
if (edata->sqlerrcode != 0)
newedata->sqlerrcode = edata->sqlerrcode;
if (edata->message)
newedata->message = pstrdup(edata->message);
if (edata->detail)
newedata->detail = pstrdup(edata->detail);
if (edata->detail_log)
newedata->detail_log = pstrdup(edata->detail_log);
.....
At the end of the function ThrowErrorData(), it calls
errfinish(edata->filename, edata->lineno, edata->funcname);
Inside errfinish, it just does a simple pointer assignment to set
filename and funcname field (via function set_stack_entry_location()).
So it might lead to an ErrorData object's filename, funcname are in
different memory context from edata->assoc_context, breaks the contract
of edata->assoc_context.
I tried but until now not manage to build a bad case that make the pointer
dangling. Still I feel it is a potential risk. We could also copy such two fields
into correct memory context like:
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 29643c51439..7859047df6a 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -820,9 +820,9 @@ set_stack_entry_location(ErrorData *edata,
filename = slash + 1;
}
- edata->filename = filename;
+ edata->filename = MemoryContextStrdup(edata->assoc_context, filename);
edata->lineno = lineno;
- edata->funcname = funcname;
+ edata->funcname = MemoryContextStrdup(edata->assoc_context, funcname);
}
Any ideas? Thanks!
Best,
Zhenghua Lyu
