[PATCH] D42475: [ELF] Add warnings for various symbols that cannot be ordered

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: ELF/Driver.cpp:598
+    if (!Names.insert(S) && Config->WarnSymbolOrdering)
+      warn("symbol ordering file: symbol '" + S + "' specified multiple times");
+  }
----------------
grimar wrote:
> jhenderson wrote:
> > ruiu wrote:
> > > I prefer the following style
> > > 
> > >   warn(MB.getBufferIdentifier() + ": duplicate symbol: " + S);
> > > 
> > > because (1) it is easier to parse by script, and (2) easier to read if symbol name is extremely long (which is actually common for C++ programs).
> > I have a slight concern that "duplicate symbol" could cause confusion with a multiply-defined symbol error. How about "duplicate ordered symbol"? Also, unless we store the order file name somewhere (e.g. in the Config variable), we can't use it in the other warnings, so I've used the owning file of the symbol instead. That still leaves the "no such symbol warning" however without an easily identified file.
> I think we often include command line option name to error message, for example: 
> 
> ```
> error("max-page-size: value isn't a power of 2");
> ...
> error("-image-base: number expected, but got " + S);
> ```
> 
> So to avoid confusion and make message shorter, we could probably use something like:
> `warn("--warn-symbol-ordering: " + MB.getBufferIdentifier() + ": duplicate symbol: " + S);`
This seems like a bit of a new case to me, so it's unclear what should be done. At the moment, there broadly appear to be four kinds of error/warning messages that are somewhat related to this, based on a quick reading of the LLD code:
  # Messages for problems with object files/archives. These are usually of the form "<object or source file name>: message"
  # Messages for problems with reading/opening input or output files. These are usually of the form "cannot open <file name> ..."
  # Messages for problems with input switches. These are of the form "<switch name>: message"
  # Messages for problems with linker script parsing/evaluation. These are of the "<script file name>: message"
There are also quite a few that don't really fit any pattern, mostly either missing any file name or with the file name in the middle or end of the message, which should probably be one of the other formats. Unfortunately, the closest equivalent to our situation (i.e. a message regarding the contents of a non-script or object input) is the version script warning for duplicate symbol, which does not mention the version script's filename at all, although the message is clear that the duplicate symbol is in the version script, not elsewhere. The next closest is probably the linker script situation (since it is a non-object/archive input), where "-T/--script" is not mentioned in the message, which is the form I've gone for (and explicitly mentioned "ordered"), but I'm happy to change the format to include both switch name and file name if that's the consensus.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D42475