This is an archive of the discontinued LLVM Phabricator instance.

[flang] Fixed crash on forward referenced `len` parameter
ClosedPublic

Authored by PeteSteinfeld on May 26 2020, 3:13 PM.

Details

Summary

It turned out that the root cause of the crash was that the length of the type
of the symbol being passed to SymbolLEN() had a bad expression. So I fixed
it by checking isExplicit() on the ParamValue before calling
GetExplicit(). The case where isExplicit() returned true and
GetExplicit() failed to get an expression indicates a prior error, so I just
returned std::nullopt from SymbolLEN() and avoided the call to the
constructor of DescriptorInquiry.

Diff Detail

Event Timeline

PeteSteinfeld created this revision.May 26 2020, 3:13 PM
Herald added a project: Restricted Project. · View Herald Transcript
tskeith added a project: Restricted Project.May 26 2020, 4:06 PM
tskeith accepted this revision.May 26 2020, 4:06 PM
This revision is now accepted and ready to land.May 26 2020, 4:06 PM
klausler requested changes to this revision.May 26 2020, 4:19 PM

Please leave the batteries installed in this particular smoke detector and see whether you can stop the annoying noise by extinguishing the fire instead. We don't want to be able to construct erroneous descriptor inquiries into things that aren't descriptors!

This revision now requires changes to proceed.May 26 2020, 4:19 PM
  1. Updating D80593: [flang] Fixed crash on forward referenced len parameter #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create

It turned out that the root cause of the crash was that the length of the type
of the symbol being passed to SymbolLEN() had a bad expression. So I fixed
it by checking isExplicit() on the ParamValue before calling
GetExplicit(). The case where isExplicit() returned true and
GetExplicit() failed to get an expression indicates a prior error, so I just
returned std::nullopt from SymbolLEN() and avoided the call to the
constructor of DescriptorInquiry.

I restored the calls to variable.cpp to CHECK(IsDescriptor(last)).

@klausler, you requested changes. Does the current state of this revision look good to you?

klausler accepted this revision.Jun 3 2020, 3:54 PM

If the CHECK() is back, I'm happy.

This revision is now accepted and ready to land.Jun 3 2020, 3:54 PM

@PeteSteinfeld Please update the summary.

PeteSteinfeld edited the summary of this revision. (Show Details)Jun 4 2020, 1:09 PM

@PeteSteinfeld Please update the summary.

Done.

This revision was automatically updated to reflect the committed changes.