[PATCH] D41269: [COFF] Warn for locally imported symbols

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: COFF/SymbolTable.cpp:102
         LocalImportChunks.push_back(cast<DefinedLocalImport>(Sym)->getChunk());
+        UndefsToLocalImports.try_emplace(Sym, D);
         continue;
----------------
ruiu wrote:
> smeenai wrote:
> > ruiu wrote:
> > > smeenai wrote:
> > > > ruiu wrote:
> > > > > Can you just call warn() here?
> > > > I definitely want to report the object file which is importing the symbol (link.exe does even better and reports the function that's doing the import, but I believe reporting the object file is the best we can do). As far as I can tell, the only way to figure out the object file(s) that are doing the import is to loop through the symbols in `Config->GCRoot` and every object file, as is done below (in the loops I've modified). I could duplicate those loops here to inline the warning, but it seemed better to avoid the duplication and reuse the existing loops.
> > > I see. But the way you are using the map is not correct. Sym and D are always the same pointer value. You can use a set instead of map.
> > I thought Sym would be the original (undefined) symbol, and D would be the corresponding defined local symbol. The pointer values are definitely different in my testing, and e.g. `Sym->getFile()` is NULL even after the `replaceSymbol` call.
> Ah, my bad. They are different symbols indeed.
Why don't you do `LocalImports[Sym] = D`?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D41269