This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Fix a crash when reading an empty index file.
ClosedPublic

Authored by hokein on Jan 8 2019, 7:06 AM.

Details

Summary

Unfortunately, yaml::Input::setCurrentDocument() and yaml::Input::nextDocument() are
internal APIs, the way we use them may cause a nullptr accessing when
processing an empty YAML file.

Diff Detail

Repository
rL LLVM

Event Timeline

hokein created this revision.Jan 8 2019, 7:06 AM
ilya-biryukov accepted this revision.Jan 8 2019, 7:15 AM

LGTM. Thanks for fixing this.

A few questions for the long-term:

  • Should we consider removing the YAML support altogether?
  • If we want to keep, are there non-private APIs that we could use? (I assume we need them to avoid keeping the whole parsed YAML tree in memory, right?)
This revision is now accepted and ready to land.Jan 8 2019, 7:15 AM
hokein added a comment.Jan 8 2019, 7:27 AM

LGTM. Thanks for fixing this.

A few questions for the long-term:

  • Should we consider removing the YAML support altogether?

The motivation of the YAML is for debugging purpose (compared with the binary format), we might want to switch to another human-readable format like json.

  • If we want to keep, are there non-private APIs that we could use? (I assume we need them to avoid keeping the whole parsed YAML tree in memory, right?)

This is the best way we've found so far. The public YAML APIs don't seem provide enough flexibility to read an Variant element (but I may miss something).

This revision was automatically updated to reflect the committed changes.