Page MenuHomePhabricator

[clangd] Bail-out when an empty compile flag is encountered
ClosedPublic

Authored by kadircet on Sep 16 2021, 8:44 AM.

Diff Detail

Event Timeline

kadircet created this revision.Sep 16 2021, 8:44 AM
kadircet requested review of this revision.Sep 16 2021, 8:44 AM
sammccall accepted this revision.Sep 16 2021, 12:39 PM

I do half wonder whether we're going to get 3 steps further and then crash again when we call Command.front() :-)

This change looks good.

It also seems tempting to inject an empty check in GlobalCompilationDatabase (where we convert vector<CompileCommand> -> optional.
But I can see a perverse situation where e.g. background indexing would "just fail" at the driver level today, but the change would lead to using a fallback command and attempting to parse some binary file. (Ugh, garbage in compile_commands...)

This revision is now accepted and ready to land.Sep 16 2021, 12:39 PM

I do half wonder whether we're going to get 3 steps further and then crash again when we call Command.front() :-)

I actually couldn't find any other places this could happen today (we seem to check for non-emptiness of compile commands most of the time). But I hadn't look into clang, apparently createInvocationFromCommandLine also assumes that.
Adding an assert and pushing the check to callers, as in theory they should probably check for it long before since many tools perform some sort of command line mangling. LMK if the check should reside in the callee instead.

kadircet updated this revision to Diff 373163.Sep 17 2021, 1:13 AM
  • Also handle OOB access while creating compiler invocation.
Herald added a project: Restricted Project. · View Herald TranscriptSep 17 2021, 1:13 AM