Skip to content

Commit 2eea2de

Browse files
committed
[RF][HS3] Fix unsafe downcasts in importers
- Drop the pointless static_cast<RooSimultaneous*> around workspace.pdf() in importAnalysis: the result is stored as RooAbsPdf* anyway, and the cast is undefined behaviour when the pdf is not a RooSimultaneous. - Use dynamic_cast with a null check instead of an unchecked cast + deref in importDecay (resolutionModel) and importBinWidthFunction (histogram), so malformed input raises a clear error instead of crashing.
1 parent 2e8863d commit 2eea2de

2 files changed

Lines changed: 9 additions & 3 deletions

File tree

roofit/hs3/src/JSONFactories_RooFitCore.cxx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,10 @@ template <bool DivideByBinWidth>
232232
bool importBinWidthFunction(RooJSONFactoryWSTool *tool, const JSONNode &p)
233233
{
234234
std::string name(RooJSONFactoryWSTool::name(p));
235-
RooHistFunc *hf = static_cast<RooHistFunc *>(tool->request<RooAbsReal>(p["histogram"].val(), name));
235+
RooHistFunc *hf = dynamic_cast<RooHistFunc *>(tool->request<RooAbsReal>(p["histogram"].val(), name));
236+
if (!hf) {
237+
RooJSONFactoryWSTool::error("histogram '" + p["histogram"].val() + "' of '" + name + "' is not a RooHistFunc");
238+
}
236239
tool->wsEmplace<RooBinWidthFunction>(name, *hf, DivideByBinWidth);
237240
return true;
238241
}
@@ -325,6 +328,9 @@ bool importDecay(RooJSONFactoryWSTool *tool, const JSONNode &p)
325328
RooRealVar *t = tool->requestArg<RooRealVar>(p, "t");
326329
RooAbsReal *tau = tool->requestArg<RooAbsReal>(p, "tau");
327330
RooResolutionModel *model = dynamic_cast<RooResolutionModel *>(tool->requestArg<RooAbsPdf>(p, "resolutionModel"));
331+
if (!model) {
332+
RooJSONFactoryWSTool::error("resolutionModel of '" + name + "' is not a RooResolutionModel");
333+
}
328334
RooDecay::DecayType decayType = static_cast<RooDecay::DecayType>(p["decayType"].val_int());
329335
tool->wsEmplace<RooDecay>(name, *t, *tau, *model, decayType);
330336
return true;

roofit/hs3/src/RooJSONFactoryWSTool.cxx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,7 @@ void importAnalysis(const JSONNode &rootnode, const JSONNode &analysisNode, cons
667667
JSONNode const *pdfNameNode = mcAuxNode ? mcAuxNode->find("pdfName") : nullptr;
668668
std::string const pdfName = pdfNameNode ? pdfNameNode->val() : "simPdf";
669669

670-
RooAbsPdf *pdf = static_cast<RooSimultaneous *>(workspace.pdf(pdfName));
670+
RooAbsPdf *pdf = workspace.pdf(pdfName);
671671

672672
if (!pdf) {
673673
// if there is no simultaneous pdf, we can check whether there is only one pdf in the list
@@ -686,7 +686,7 @@ void importAnalysis(const JSONNode &rootnode, const JSONNode &analysisNode, cons
686686
}
687687
RooSimultaneous simPdf{simPdfName.c_str(), simPdfName.c_str(), pdfMap, indexCat};
688688
workspace.import(simPdf, RooFit::RecycleConflictNodes(true), RooFit::Silence(true));
689-
pdf = static_cast<RooSimultaneous *>(workspace.pdf(simPdfName));
689+
pdf = workspace.pdf(simPdfName);
690690
}
691691
}
692692

0 commit comments

Comments
 (0)