[PATCH] [PECOFF] Handle .drectve section.

[Rui Ueyama]

================
Comment at: lib/ReaderWriter/PECOFF/ReaderCOFF.cpp:415
@@ +414,3 @@
+        return ec;
+      _directives = std::move(std::string(
+          reinterpret_cast<const char *>(&contents[0])));
----------------
Saleem Abdulrasool wrote:
> Rui Ueyama wrote:
> > Reid Kleckner wrote:
> > > Is it worth doing any kind of check that the contents are string-like and don't contain null bytes?
> > Good point. There seems to be actually no guarantee that the string in the section is terminated by NUL.
> Unfortunately, null bytes are valid in the .directve section.  It can be used to separate options to be passed to the linker.  Newlines are also valid terminators of options.  Effectively, the string needs to be stripped, and tokenised to parse the directives.
As to null bytes, "a .drectve section consists of a string of text that can be encoded as ANSI or UTF-8" says the MS COFF spec (section 5.2). It does not sound like it can contain multiple NUL-separated strings. I also checked the .drectve section created by cl.exe, and spaces were used to separate options there. So I think I doubt NUL can be a valid byte, except the padding bytes at the end of the section.

As to spaces arnd newlines, they should be being handled properly in this patch. TokenizeWindowsCommandLine() is the function to tokenize the string in the Windows manner.

================
Comment at: lib/ReaderWriter/PECOFF/ReaderCOFF.cpp:522
@@ +521,3 @@
+    llvm::raw_string_ostream stream(errorMessage);
+    bool parseFailed = WinLinkDriver::parse(argc, argv, *targetInfo, stream);
+    stream.flush();
----------------
kledzik at apple.com wrote:
> You've constructed a new argv[] here and called WinLinkDriver::parse() on it.  Normally, that constructs a whole new PECOFFTargetInfo.  Is your plan to create a new targetInfo  object and somehow merge that with the original one, or pass in the original one and let parse() modify it?  If the later, we probably need a parameter to parse() explicitly saying that more options are being appended after initial set up, because you may want to do error handling differently.  For instance, not allowing the output file to be renamed.  
> 
> Also, if more files or libraries are being added, whatever code is looping over the initial files (in parallel, thanks to Michael), will need to be somehow notified of the change.
> 
I'm trying to update the original TargetInfo. Your suggestion makes sense. I think in addition to add a new parameter to parse(), we need to mark the options in the tablegen file which are allowed to be used in the .drectve section as such, so that the parser can distinguish one from another. It should probably be a FIXME, as it'll be a rather large change to combine with this one, though.

As to the input files, we should move the InputFiles to TargetInfo (LinkerContext). It's logically fit, and also doing it makes it possible to update the InputFiles here. I'd like to do that in a different patch too. Does this sound good?


http://llvm-reviews.chandlerc.com/D1246