From 1a6fc0b8777058c0a5c36f19edf10e819c9dfab5 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Mon, 27 Jul 2026 10:17:56 +0000 Subject: [PATCH 1/2] [RF] Make RooAbsArg::setProxyNormSet public MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expose setProxyNormSet so that external code can make an object forget a normalization set previously passed to `getVal()`. This is needed when that set may not outlive the object (e.g. it lived on the caller's stack), since the proxies only keep a bare pointer to it. Symmetric to the already-public `RooAbsPdf::setActiveNormSet`. 🤖 Done with the help of AI. --- roofit/roofitcore/inc/RooAbsArg.h | 2 +- roofit/roofitcore/src/RooAbsArg.cxx | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) 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) { From 79ad705503d10f4af145116d76c139bf9a3dac3e Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Mon, 27 Jul 2026 10:18:06 +0000 Subject: [PATCH 2/2] [RF] Fix dangling stack normSet after xRooNode::BuildHistogram MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BuildHistogram evaluates the model with a stack-local `normSet`. RooFit proxies only cache a bare pointer to the last normalization set and re-use it later (e.g. RooProduct::evaluate reads back _compRSet.nset()), so once `normSet` goes out of scope those proxies dangle into freed stack memory. A subsequent evaluation then reads it, reported by valgrind as "use of uninitialised value" in RooAbsReal::getValV (nset->uniqueId()). `sterilize()` was meant to forget the passed normSet, but in >=6.27 that code is disabled (the uniqueId invalidity check only guards `RooAbsPdf::_normSet`, not the proxy pointers) and it only walks clients, whereas getVal() propagates the normSet down to the servers of the evaluated function. Reset the proxy normSet on the whole server tree of every function we called getVal() on, before `normSet` is destroyed. 🤖 Done with the help of AI. --- roofit/xroofit/src/xRooNode.cxx | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) 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)) {