[cfe-dev] [PATCH] Wrap clang modules inside Mach-O/ELF/COFF containers

[Adrian Prantl]

> On Dec 10, 2014, at 3:01 PM, Ben Langmuir <blangmuir at apple.com> wrote:
> 
> 
>> On Dec 10, 2014, at 2:27 PM, Adrian Prantl <aprantl at apple.com> wrote:
>> 
>> Hi everyone,
>> 
>> As the first step in preparation for module debugging (see http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-November/040076.html) this patch turns the *.pcm files that are used to store clang modules and precompiled headers in a platform-dependent Mach-O/ELF/COFF container, so that eventually we will be able to store debug information alongside the module in the same file.
>> 
>> This is implemented by using the standard LLVM code generation machinery. Instead of directly writing to the output file, the serialized AST blob is attached to an empty llvm::Module as a ModuleFlag. The module is passed to the backend which emits the AST blob into a special “__clang_pch" section in TargetLoweringObjectFile*.
>> On the ASTReader side, any object file is transparently unwrapped and the BitstreamReader is pointed directly to the AST section.
>> 
>> Other than the .pcm files having an extra header inside, this patch is not meant to have any user-visible effects.
>> 
>> Known bugs: I still need to figure out how to make c-index-test link against and register the available targets (check-all passes, but the modules created by c-index-test currently are plain old .pcm files).
>> Open questions: I made up the name of the new __clang_pch section and the various flags on the different platforms on the spot. I’m open to better suggestions.
> 
> Once you’ve got c-index-test working with containers, is the intent to remove the fallback to reading raw AST files, or do you intend to keep it around?

Although the fallback is just one extra line of code, (so there wouldn’t be a large maintenance effort), I don’t really see a compelling use-case for it. I’d vote in favor of removing it.

-- adrian
> 
>> @@ -599,6 +600,25 @@ void PCHValidator::ReadCounter(const ModuleFile &M, unsigned Value) {
>> // AST reader implementation
>> //===----------------------------------------------------------------------===//
>> 
>> +void ASTReader::initStreamFileWithModule(llvm::MemoryBufferRef Buffer,
>> +                                         llvm::BitstreamReader &StreamFile) {
>> +  if (auto OF = llvm::object::ObjectFile::createObjectFile(Buffer))
>> +    // Unwrap the container if necessary.
>> +    for (auto &Section : OF->get()->sections()) {
>> +      StringRef Name;
>> +      Section.getName(Name);
>> +      if (Name == "__clang_pch") {
>> +        StringRef Buf;
>> +        Section.getContents(Buf);
>> +        return StreamFile.init((const unsigned char*)Buf.begin(),
>> +                               (const unsigned char*)Buf.end());
>> +      }
>> +    }
>> +
>> +  StreamFile.init((const unsigned char *)Buffer.getBufferStart(),
>> +                  (const unsigned char *)Buffer.getBufferEnd());
>> +}
>> +
> 
> 
> Is this sufficiently paranoid?  It looks like Section.getName() and Section.getContents() can both fail.  I guess we already fall over horribly if an AST file is malformed, so maybe this is no worse...
> 
> Nitpick: Can we add braces for the outer if? I was confused about the nesting.
> 
>> -  // Write the generated bitstream to "Out".
>> -  Out->write((char *)&Buffer.front(), Buffer.size());
>> +  std::string Error;
>> +  StringRef Triple = Ctx.getTargetInfo().getTriple().getTriple();
>> +  if (llvm::TargetRegistry::lookupTarget(Triple, Error)) {
> 
> <snip>
>> +  } else {
>> +    // FIXME: Fallback for c-index-test.
>> +    // Directly write the generated bitstream to "Out".
>> +    Out->write((char *)&Buffer.front(), Buffer.size());
>> +    Out->flush();
>> +  }
> 
> When c-index-test is updated, will this code change?  Is it possible to end up here for other reasons, like if the triple is bogus?  Or does that always get caught earlier?
> 
> Ben