[PATCH] D52749: [LLD][COFF] Fix CRT global initializers

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: COFF/Writer.cpp:1605
+        case 1:
+          return false;
+        case 0:
----------------
rnk wrote:
> aganea wrote:
> > aganea wrote:
> > > mstorsjo wrote:
> > > > mstorsjo wrote:
> > > > > Won't this override the internal ordering based on the section name suffixes like .CRT$ABC?
> > > > Or what actually does `getBufferIdentifier()` return - the name of the object file? In that case I'd say this is wrong. If it returns the section name it might be ok...
> > > @mstorsjo `getBufferIdentifier()` indeed returns the file name. However at this point, chunks were already grouped by specific section. Meaning that this function will sort only, say, `.CRT$XCU` chunks at once. Internal TU order is guaranteed by the test below (`return SA->SectionNumber < SB->SectionNumber;`)
> > > 
> > > The only side-effect I could see is that this change makes **deterministic **initialization of global initializers (which in this case, means the TUs will be called in lexicographic order, based on their relative/absolute path and filename). The C++ std says the behavior is "indefinite", but this makes it "definite", at least for LLD. What is your stance on this? @ruiu @zturner @pcc @rnk 
> > In an ideal world, it'd be nice if this behavior could be randomized, with a command-line flag. That could trap some (global initialization) issues earlier in the target program, by successive "link"-"run program" runs.
> You know, this might actually come back and bite us at some point. I agree that the linker should not provide guarantees about the order in which unrelated TUs initialize, but I am aware of a codebase that goes out of its way to maintain a particular ordering of source files in the build system to ensure that some files are initialized first. This codebase has used -fno-init-array on Linux for years to avoid dealing with the reversal of initialization order that happened when GCC moved from .ctors to .init_array.
> 
> I think, at the very least, Windows doesn't have anything like crt0.o, which is supposed to come first on the link line. The way that LLD and link.exe sort section names alphabetically makes that unnecessary.
> 
> I'm actually surprised that LLD doesn't number each object file so that we can just sort them by input order. It seems like that might be useful elsewhere.
> 
> All that said, I'd be fine with reordering input initializers alphabetically and seeing what happens. If anyone is relying on this behavior, they can use `#pragma init_seg` to mark the initializers that need to run first, and their code base will be less fragile as a result.
Ah, sorry, I missed that this was called at that stage. Then I have no objections.

A testcase that shows that ordering of different .CRT$FOO sections (with different FOO part) from different object files is preserved just for clarity, unless we already have that. But maybe there's no need...


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D52749