Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix undefined behavior in MuonIdProducer from empty pointer #35303

Merged
merged 2 commits into from
Sep 16, 2021
Merged

Fix undefined behavior in MuonIdProducer from empty pointer #35303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a86a0ca
fix issue 35036 in MuonIdProducer
trocino Sep 15, 2021
2577c56
fix issue 35036 in MuonIdProducer: code-checks patch
trocino Sep 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RecoMuon/MuonIdentification/interface/MuonShowerDigiFiller.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@

class MuonShowerDigiFiller {
public:
MuonShowerDigiFiller() {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this really needed with an empty body? it should already be the same as the implicit default.
please drop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an implicit default constructor? If I remove this line, I get a compilation error at the instantiation without parameters here. From the error message, it looks like there is an implicit copy constructor instead:

/.../unique_ptr.h: In instantiation of 'typename std::_MakeUniq<_Tp>::__single_object std::make_unique(_Args&& ...) [with _Tp = MuonShowerDigiFiller; _Args = {}; typename std::_MakeUniq<_Tp>::__single_object = std::unique_ptr<MuonShowerDigiFiller>]':

/.../plugins/MuonIdProducer.cc:93:67: required from here
/.../unique_ptr.h:962:30: error: no matching function for call to 'MuonShowerDigiFiller::MuonShowerDigiFiller()'
962 | { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }

In file included from /.../plugins/MuonIdProducer.h:49,
from /.../plugins/MuonIdProducer.cc:12:
/.../interface/MuonShowerDigiFiller.h:49:3: note: candidate: 'MuonShowerDigiFiller::MuonShowerDigiFiller(const edm::ParameterSet&, edm::ConsumesCollector&&)'
49 | MuonShowerDigiFiller(const edm::ParameterSet&, edm::ConsumesCollector&& iC);
/.../interface/MuonShowerDigiFiller.h:49:3: note: candidate expects 2 arguments, 0 provided

/.../interface/MuonShowerDigiFiller.h:47:7: note: candidate: 'MuonShowerDigiFiller::MuonShowerDigiFiller(const MuonShowerDigiFiller&)'
47 | class MuonShowerDigiFiller {
/.../interface/MuonShowerDigiFiller.h:47:7: note: candidate expects 1 argument, 0 provided

/.../interface/MuonShowerDigiFiller.h:47:7: note: candidate: 'MuonShowerDigiFiller::MuonShowerDigiFiller(MuonShowerDigiFiller&&)'
/.../interface/MuonShowerDigiFiller.h:47:7: note: candidate expects 1 argument, 0 provided

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for checking.
I forgot that the implicit default is created only if there is no other kind defined.

MuonShowerDigiFiller(const edm::ParameterSet&, edm::ConsumesCollector&& iC);

void getES(const edm::EventSetup& iSetup);
Expand Down
4 changes: 3 additions & 1 deletion RecoMuon/MuonIdentification/plugins/MuonIdProducer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ MuonIdProducer::MuonIdProducer(const edm::ParameterSet& iConfig)
if (fillShowerDigis_ && fillMatching_) {
edm::ParameterSet showerDigiParameters = iConfig.getParameter<edm::ParameterSet>("ShowerDigiFillerParameters");
theShowerDigiFiller_ = std::make_unique<MuonShowerDigiFiller>(showerDigiParameters, consumesCollector());
} else {
theShowerDigiFiller_ = std::make_unique<MuonShowerDigiFiller>(); // to be used to call fillDefault only
}

if (fillCaloCompatibility_) {
Expand Down Expand Up @@ -888,7 +890,7 @@ void MuonIdProducer::fillMuonId(edm::Event& iEvent,

matchedChamber.id = chamber.id;

if (fillShowerDigis_) {
if (fillShowerDigis_ && fillMatching_) {
theShowerDigiFiller_->fill(matchedChamber);
} else {
theShowerDigiFiller_->fillDefault(matchedChamber);
Expand Down