[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

[James Nagurne via Phabricator via cfe-commits]

JamesNagurne added a comment.

In D65907#1645474 <https://reviews.llvm.org/D65907#1645474>, @arphaman wrote:

> In D65907#1643800 <https://reviews.llvm.org/D65907#1643800>, @JamesNagurne wrote:
>
> > In D65907#1643650 <https://reviews.llvm.org/D65907#1643650>, @arphaman wrote:
> >
> > > No the windows test failure was different, there were no Deps at all. I'm currently investigating it on a windows VM.
> > >
> > > @JamesNagurne I think there's some issue with the working directory, which is not added in your case. Which platform are you running your build/test on? Which cmake options are you using?
> >
> >
> > I apologize for not giving such information in the first reply. Unfortunately this isn't an easy remote reproduction, as our ToolChain and some integral changes aren't upstreamed. This is an embedded ARM cross-compiled on Linux. Might be able to reproduce with arm-none-none-eabi.
> >  LLVM is built as an external project. Looking at the build system, it looks like we have the CMAKE_ARGS:
> >
> >  
> >   -DLLVM_DEFAULT_TARGET_TRIPLE=arm-ti-none-eabi
> >   -DLLVM_EXTERNAL_CLANG_SOURCE_DIR=${CMAKE_SOURCE_DIR}/llvm-project/clang
> >   -DLLVM_TARGETS_TO_BUILD=ARM
> >   -DCMAKE_BUILD_TYPE=Release
> >   -DCMAKE_CXX_COMPILER=clang++
> >   -DCMAKE_C_COMPILER=clang
> >   -DLLVM_USE_LINKER=gold
> >   -GNinja
> >   
> >
> > Nothing suspicious, except maybe the default triple and ARM target.
> >  Looking at our (not upstream) toolchain file, we do have some RTLibs, LibInternal, libcxx, and System include changes, along with a -nostdsysteminc to avoid pulling host includes into our cross compiler. However, none of this should affect general "#include" behavior, correct?
> >  Another glance at your changes don't seem to affect any target-specific handling either, at least directly.
>
>
> Yes, I don't see anything in your changes that is an obvious thing to blame.
>
> > I might have to just bite the bullet and drop into the test with a debugger.
>
> I fixed the Windows issue, and it was completely unrelated to your issue. It looks like the FileManager isn't constructing absolute paths when accessing the files, which is somewhat unexpected. It would be useful to debug invocations of `getFileEntryRef`, to see if the `InterndFilename` in the function is getting changed into an absolute path or not.


We found and fixed the issue on our end. Turns out we had a nefarious little piece of code in AddClangSystemIncludeArgs:

  if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
    SmallString<128> Dir(getDriver().SysRoot);
    llvm::sys::path::append(Dir, "include");
    addSystemInclude(DriverArgs, CC1Args, Dir.str());
  }

Turns out that, as a cross compiler, we had set SysRoot to the empty string. So, we were adding "-internal-isystem" "include" to the cc1 arguments, which somehow caused the test to fail. I don't know precisely why it exposes issues in your test, but the code is clearly busted, so we've removed it.

I appreciate the responses as we've worked through the problems on our end!


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65907/new/

https://reviews.llvm.org/D65907