This is an archive of the discontinued LLVM Phabricator instance.

[flang] Fix a crash when creating generics from a copy
ClosedPublic

Authored by PeteSteinfeld on Jul 9 2020, 9:40 AM.

Details

Summary

When a program unit creates a generic based on one defined in a module, the
function CopyFrom() is called to create the GenericDetails. This function
copied the specificProcs_ but failed to copy the bindingNames_. If the
function CheckGeneric() then gets called, it tries to index into the empty
binding names and causes the crash.

I fixed this by adding code to CopyFrom() to copy the binding names.

I also added a test that causes the crash.

Diff Detail

Event Timeline

PeteSteinfeld created this revision.Jul 9 2020, 9:40 AM
Herald added a project: Restricted Project. · View Herald Transcript
PeteSteinfeld added a project: Restricted Project.Jul 9 2020, 9:41 AM
tskeith added inline comments.Jul 9 2020, 11:07 AM
flang/lib/Semantics/symbol.cpp
211

I think that specificProcs_ and bindingNames_ are supposed to be parallel vectors; at least that is the assumption in CheckHelper::CheckGeneric. So this should be written as a single loop that pushes onto the two lists at the same time. As it's written it looks like the two loops might push different numbers of elements on the two lists.

One thing that suggests that the above assumption is wrong is the existence of this constructor: GenericDetails(const SymbolVector &specificProcs);. But I'm not sure it is ever used, so it would be good if you can delete it as part of this change.

PeteSteinfeld marked an inline comment as done.Jul 9 2020, 2:47 PM
PeteSteinfeld added inline comments.
flang/lib/Semantics/symbol.cpp
211

After doing some testing, I believe that you're correct that they're supposed to be parallel vectors. I'll put some calls to CHECK() into CopyFrom() to verify that their sizes match and copy them as pairs.

You're also correct that the constructor GenericDetails(const SymbolVector &specificProcs) is not used. I'll delete it.

Thanks for the guidance!

With Tim's guidance, I changed the code in CopyFrom() to treat the procs and
binding names as a pair and also removed the constructor for GenericDetails
that took a vector of procs, since it wasn't used anywhere.

tskeith accepted this revision.Jul 9 2020, 3:06 PM
This revision is now accepted and ready to land.Jul 9 2020, 3:06 PM
This revision was automatically updated to reflect the committed changes.