Page MenuHomePhabricator

Defer CWD in MCContext lookup as late as possible.
Needs RevisionPublic

Authored by resistor on Feb 9 2016, 11:13 PM.

Details

Reviewers
chandlerc
Summary

Currently MCContext tries to lookup CWD on construction. This causes sandboxing violations when using LLVM in a daemon without filesystem access. The solution is defer CWD lookup until absolutely necessary.

Diff Detail

Repository
rL LLVM

Event Timeline

resistor updated this revision to Diff 47424.Feb 9 2016, 11:13 PM
resistor retitled this revision from to Defer CWD in MCContext lookup as late as possible..
resistor updated this object.
resistor added a reviewer: chandlerc.
resistor set the repository for this revision to rL LLVM.
resistor added a subscriber: llvm-commits.
chandlerc edited edge metadata.Feb 9 2016, 11:18 PM

The goal here makes perfect sense, but I wonder if this is the right fix... It seems almost too surgical, and makes it really easy to change MC in a way that yet again violates things.

I feel like the MCContext shouldn't be looking at the filesystem *at all*, and it should be the client that sets up the MCContext that provides a compilation directory, potentially via the filesystem query, where appropriate. Does that make sense here?

That makes sense to me in principle, but I know nothing of why CWD is needed for (some form of?) DWARF generation at all.

It's used to resolve relative paths encoded elsewhere in dwarf. I think all
the clients building an mccontext either have an analog (clang) or can
reasonably directly query their cwd (llc and friends).

Agreed, this can be hoisted out fairly easily.

It's used to resolve relative paths encoded elsewhere in dwarf. I think all
the clients building an mccontext either have an analog (clang) or can
reasonably directly query their cwd (llc and friends).

So during an LTO build you will get the CWD of wherever the linker was invoked?
Shouldn't this get passed in by the frontend as metadata?

chandlerc requested changes to this revision.Apr 6 2016, 11:15 PM
chandlerc edited edge metadata.

Marking this as needing changes along the lines of what I suggested or a further change along the lines of what Adrian suggested.

This revision now requires changes to proceed.Apr 6 2016, 11:15 PM