This is an archive of the discontinued LLVM Phabricator instance.

clang-cl: Don't store the cl compiler Tool on the stack (PR20131)
ClosedPublic

Authored by hans on Jun 26 2014, 9:58 AM.

Details

Summary

The Command will refer back to the Tool as its source, so it has to outlive the Command.

Having the Tool on the stack would cause us to crash when using "clang-cl -GR -fallback", because if the Command fails, Driver::ExecuteCompilation tries to peek at the Command's source.

Diff Detail

Event Timeline

hans updated this revision to Diff 10890.Jun 26 2014, 9:58 AM
hans retitled this revision from to clang-cl: Don't store the cl compiler Tool on the stack (PR20131).
hans updated this object.
hans edited the test plan for this revision. (Show Details)
hans added reviewers: rnk, hansw.
hans added a subscriber: Unknown Object (MLST).
rnk added inline comments.Jun 26 2014, 11:24 AM
include/clang/Driver/ToolChain.h
72 ↗(On Diff #10890)

You should be able to get away with forward declaring tools::visualstudio::Compile, and then using that type here and on the getter.

72 ↗(On Diff #10890)

Can we make the Clang tool own the CL tool when it's doing fallback? I think that should solve the lifetime issues without adding CL to the base Toolchain class.

hans added inline comments.Jun 26 2014, 12:51 PM
include/clang/Driver/ToolChain.h
72 ↗(On Diff #10890)

Yes, that seems nicer.

hans updated this revision to Diff 10900.Jun 26 2014, 12:51 PM

Make the cl tool a member of the Clang tool instead.

rnk accepted this revision.Jun 26 2014, 12:55 PM
rnk edited edge metadata.

lgtm

This revision is now accepted and ready to land.Jun 26 2014, 12:55 PM
hans closed this revision.Jun 26 2014, 1:07 PM
hans updated this revision to Diff 10901.

Closed by commit rL211802 (authored by @hans).