[ASAN] Unit Test testTriggerEventAnalyzers failing with heap-buffer-overflow
#41045
Comments
|
A new Issue was created by @aandvalenzuela Andrea Valenzuela. @Dr15Jones, @perrotta, @dpiparo, @rappoccio, @makortel, @smuzaffar can you please review it and eventually sign/assign? Thanks. cms-bot commands are listed here |
|
assign hlt |
|
New categories assigned: hlt @missirol,@Martin-Grunewald you have been requested to review this Pull request/Issue and eventually sign? Thanks |
|
Here is the full ASAN report from the log |
|
Full ASAN report with line numbers: |
|
The unit test log also contains the following error message, which could be related to the error reported by ASAN: |
|
(Couldn't get to this issue yet, but I plan to have a look in the next days.) |
|
I started to have a look. The following is just my current understanding (might be incorrect/incomplete).
If you think I'm missing something, please let me know. [1] cmssw/HLTrigger/btau/plugins/HLTJetTag.cc Lines 109 to 110 in 357677c [2] diff --git a/HLTrigger/btau/plugins/HLTJetTag.cc b/HLTrigger/btau/plugins/HLTJetTag.cc
index 61f15b886dc..04c9f1fb8fc 100644
--- a/HLTrigger/btau/plugins/HLTJetTag.cc
+++ b/HLTrigger/btau/plugins/HLTJetTag.cc
@@ -107,6 +107,7 @@ bool HLTJetTag<T>::hltFilter(edm::Event& event,
int nJet = 0;
int nTag = 0;
for (auto const& jet : *h_JetTags) {
+if (jet.first.key() >= h_Jets->size()) throw cms::Exception("OutOfRange") << jet.first.key() << " >= " << h_Jets->size();
jetRef = TRef(h_Jets, jet.first.key());
LogTrace("") << "Jet " << nJet << " : Et = " << jet.first->et() << " , tag value = " << jet.second;
++nJet; |
|
It seems to me the cmssw/HLTrigger/btau/plugins/HLTJetTag.cc Lines 109 to 110 in 357677c would not make much sense. I suppose a minimal improvement would be for the module to check that the if (dependent.isNonnull() and dependent.id() != h_Jets.id()) {
// deal with the inconsistency in some way, e.g. by throwing an exception
} |
Okay, thanks for the insight. I think it's also clear that this assumption ends up being wrong. Just adding some printouts, one can see that
[1] Printout from step2 of wf |
|
From the printout it seems to me that in some cases the jets in |
I think it's the former. Apologies, I was sloppy with the printout in #41045 (comment), a clearer one is in [1] (in both cases, I'm printing from any instance of I don't claim to understand this in full yet, but I figured out what happens for one case ( [1] |
|
Thanks @missirol. I took a look of the jet consumption chain of It seems to me both of the Jets and JetTags chains go through many potential steps of selection, and therefore there is no prior guarantee that the jets in one would be a subset of the other for all events. In addition, I see the jet selectors ( |
|
Slowly converging on this issue.
Once there is data/MC processed with an updated HLT menu, the input file of the unit test could be updated again (with a proper input, more is tested, as this issue shows). By the way, thanks a lot for opening this issue (@aandvalenzuela). I doubt we would have caught this bug otherwise (and clearly, the bug went unnoticed for a long time). One naive question: why didn't 'anything' catch this earlier? It was sort-of caught by accident (in a unit test of something pretty unrelated, in ASAN). Was this issue triggering 'alarms' anywhere else (e.g. other tests in ASAN, or else)? If not, why was it going unnoticed? |
For reference, this check was done by adding a printout [1] in [1] diff --git a/HLTrigger/btau/plugins/HLTJetTag.cc b/HLTrigger/btau/plugins/HLTJetTag.cc
index 61f15b886dc..556a7acd578 100644
--- a/HLTrigger/btau/plugins/HLTJetTag.cc
+++ b/HLTrigger/btau/plugins/HLTJetTag.cc
@@ -101,6 +101,9 @@ bool HLTJetTag<T>::hltFilter(edm::Event& event,
<< dependent_provenance.branchName() << ", which has been dropped";
}
+edm::LogPrint("HLTJetTag") << moduleDescription().moduleLabel() << " : dep.isNonNull = " << dependent.isNonnull() << ", makesSense = "
+ << (dependent.isNonnull() and dependent.id() == h_Jets.id()) << " (" << h_Jets->size() << ", " << h_JetTags->size() << ")";
+
TRef jetRef;
// Look at all jets in decreasing order of Et. |
I don't remember seeing these symptoms elsewhere (but could easily just not remember). I can only guess along
|
|
(I'll try to answer, but my understanding is a bit limited here.)
I don't think so (and that was probably one of the reasons to introduce the unit test).
I don't think so. The products of the misconfigured modules are not consumed by anything at HLT, other than by instances of
I would be surprised if DQM was using/checking the products of the 7 filters in question (meaning, indirectly via I guess my question was even more naive, and is about this line in |
|
The items in #41045 (comment) are progressing, and the unit test itself is working in recent ASAN IBs.
@makortel , would you mind explaining this ? I did try to re-run an old HLT menu with ASAN using events where problematic modules like |
The |
|
+hlt The action items in #41045 (comment) have been tackled, modulo the completion of CMSHLT-2863. To do (for HLT): add a similar test which uses the GRun menu (as opposed to a fixed EDM file), to test in ASAN builds the |
|
This issue is fully signed and ready to be closed. |
|
please close |
Hello,
Unit Test
testTriggerEventAnalyzers(from moduleHLTrigger/HLTcore) is failing with heap-buffer-overflow. Here the summary:Find the full log in this link.
Thanks!
FYI, @missirol (I see you added this test to CMSSW)
The text was updated successfully, but these errors were encountered: