[PATCH] D17065: Defer CWD in MCContext lookup as late as possible.

Eric Christopher via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 22 14:25:27 PDT 2016


I'm good with this.

Thanks!

-eric

On Mon, Mar 21, 2016 at 10:31 PM Chandler Carruth <chandlerc at gmail.com>
wrote:

> On Mon, Mar 21, 2016 at 9:32 PM Justin Bogner <mail at justinbogner.com>
> wrote:
>
>> Chandler Carruth via llvm-commits <llvm-commits at lists.llvm.org> writes:
>> > Owen Anderson via llvm-commits <llvm-commits at lists.llvm.org> writes:
>> >> resistor created this revision.
>> >> resistor added a reviewer: chandlerc.
>> >> resistor added a subscriber: llvm-commits.
>> >> resistor set the repository for this revision to rL LLVM.
>> >>
>> >> 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.
>> >
>> > 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?
>>
>> For all intents and purposes, this is already done and the code Owen's
>> removing here is functionally dead.
>>
>> - Clang calls setCompilationDir (by passing -fdebug-compilation-dir to
>>   clang -cc1) with the result of current_path unless current_path fails,
>>   but it would just fail again if that happened.
>>
>> - opt and llc don't generate debug info, so they just pass along the
>>   AT_comp_dir directive in the IR if it's there.
>>
>> - llvm-mc does rely on this, but (1) it's a testing tool, and (2) it's
>>   trivial to make it call current_path.
>>
>> I think we just want to remove the call to current_path in LLVMContext's
>> constructor and add a call to setCompilationDir in llvm-mc, like in the
>> attached patch. WDYT?
>>
>
> LGTM, but should probably confirm with Eric as well.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160322/76a86892/attachment.html>


More information about the llvm-commits mailing list