diff --git a/roofit/roofitcore/inc/RooAbsArg.h b/roofit/roofitcore/inc/RooAbsArg.h index 46c1829637e57..ab6a659165f71 100644 --- a/roofit/roofitcore/inc/RooAbsArg.h +++ b/roofit/roofitcore/inc/RooAbsArg.h @@ -492,6 +492,7 @@ class RooAbsArg : public TNamed, public RooPrintable { RooAbsProxy *getProxy(Int_t index) const; Int_t numProxies() const; + void setProxyNormSet(const RooArgSet *nset); /// De-duplicated pointer to this object's name. /// This can be used for fast name comparisons. @@ -582,7 +583,6 @@ class RooAbsArg : public TNamed, public RooPrintable { void unRegisterProxy(RooArgProxy &proxy); void unRegisterProxy(RooSetProxy &proxy); void unRegisterProxy(RooListProxy &proxy); - void setProxyNormSet(const RooArgSet *nset); // Attribute list std::set _boolAttrib; // Boolean attributes diff --git a/roofit/roofitcore/src/RooAbsArg.cxx b/roofit/roofitcore/src/RooAbsArg.cxx index e5c948d99a422..ab4297ad9cad2 100644 --- a/roofit/roofitcore/src/RooAbsArg.cxx +++ b/roofit/roofitcore/src/RooAbsArg.cxx @@ -1284,7 +1284,10 @@ Int_t RooAbsArg::numProxies() const } /// Forward a change in the cached normalization argset -/// to all the registered proxies. +/// to all the registered proxies. Passing `nullptr` makes this object forget +/// any normalization set previously passed to `getVal()`, which is required if +/// that set may not outlive this object (e.g. it lived on the caller's stack), +/// as the proxies only keep a bare pointer to it. void RooAbsArg::setProxyNormSet(const RooArgSet *nset) { diff --git a/roofit/xroofit/src/xRooNode.cxx b/roofit/xroofit/src/xRooNode.cxx index bd856470a5a5d..9a6d815cd7562 100644 --- a/roofit/xroofit/src/xRooNode.cxx +++ b/roofit/xroofit/src/xRooNode.cxx @@ -9355,6 +9355,25 @@ TH1 *xRooNode::BuildHistogram(RooAbsLValue *v, bool empty, bool errors, int binS } } + // We evaluated the above with `normSet`, which lives on this function's stack. + // RooFit proxies only cache a bare pointer to the last normSet and re-use it + // later (e.g. RooProduct::evaluate reads back _compRSet.nset()), so before it + // goes out of scope we must clear it everywhere it could have been cached, + // otherwise a later evaluation dereferences freed stack ("use of uninitialised + // value" in valgrind). sterilize() only walks clients, but getVal() propagates + // the normSet to the servers of the evaluated function, so reset those here. + { + RooArgSet evaluatedNodes; + for (RooAbsArg *treeRoot : {static_cast(rar), static_cast(p), + static_cast(oldrar), _coefs.get()}) { + if (treeRoot) + treeRoot->treeNodeServerList(&evaluatedNodes); + } + for (RooAbsArg *node : evaluatedNodes) { + node->setProxyNormSet(nullptr); + } + } + if (oldrar) { std::vector extra; if (auto s = dynamic_cast(rar)) {