[PATCH] Fix a crash that occurs when PWD is invalid.
atrick at apple.com
Mon Dec 9 15:48:13 PST 2013
On Dec 9, 2013, at 3:09 PM, Eric Christopher <echristo at gmail.com> wrote:
> On Mon, Dec 9, 2013 at 3:02 PM, Andrew Trick <atrick at apple.com> wrote:
>> On Dec 9, 2013, at 2:50 PM, Eric Christopher <echristo at gmail.com> wrote:
>> On Mon, Dec 9, 2013 at 1:58 PM, Filip Pizlo <fpizlo at apple.com> wrote:
>> On Dec 9, 2013, at 12:42 PM, Andrew Trick <atrick at apple.com> wrote:
>> On Dec 9, 2013, at 12:01 PM, Eric Christopher <echristo at gmail.com> wrote:
>> Out of curiosity why isn't it able to get a compilation dir?
>> Probably one of these.
>> The getcwd() function will fail if:
>> [EACCES] Read or search permission was denied for a component
>> of the pathname.
>> [ENOENT] A component of the pathname no longer exists.
>> Phil might know for sure.
>> I'm not sure, actually. ;-) When I hit the bug it was probably ENOENT
>> caused by weirdness in our JSC test harness.
>> with $PWD set to anything remotely sensible. Moreover, it's my
>> understanding that libraries and frameworks in general never assume anything
>> about the sanity of $PWD unless they are asked to do relative path IO. This
>> isn't just a JSC bug: if the claim is that LLVM is a JIT library then it's
>> probably best if LLVM gracefully handles not having a sane $PWD.
>> That's not quite what I'm talking about, FWIW I agree that you
>> shouldn't have to have an environment at all :)
>> Basically the lines of code in MCDwarf.cpp are supposed to be when
>> generating dwarf for assembly files and I wasn't under the impression
>> you were compiling .s files. :)
>> That code should probably have some decent fallback in it, but I don't
>> think a null DW_AT_compilation_dir is the correct one. It should omit
>> the attribute if it can't get a valid value.
>> You think we should omit the entire compile_unit DIE if we don’t find the
>> absolute path? If we simply omit the DIE, do you think any tools could be
>> confused? Is it worth adding a special case for a situation that cannot
>> conceivably happen in the context of compiling .s files? I’d be happy to
>> simply assert that CompilationDir is non-empty if that’s preferable to
>> emitting a DIE with an empty path.
> No, I think you should omit the DW_AT_compilation_dir attribute on the
> DIE if it's empty. I still don't understand how you're getting down
> this path, but…
Maybe the problem was my commit comment:
MCJIT needs to be able to run in hostile environments, even when PWD
is invalid. There's no need to crash MCJIT in this case.
The obvious fix is to simply leave MCContext's CompilationDir empty
when PWD can't be determined. This way, MCJIT clients,
and other clients that link with LLVM don’t need a valid working directory.
If we do want to guarantee valid CompilationDir, that should be done
only for clients of getCompilationDir(). This is as simple as checking
for an empty string.
The only current use of getCompilationDir is EmitGenDwarfInfo. This case won’t
conceivably run with an invalid working dir, and if it did (hypothetically), skipping
the AT_comp_dir attribute is a reasonable way to handle it.
I’m not sure how to “omit the attribute” other than emitting a NULL byte in its place.
More information about the llvm-commits