Skip to content

Commit b38f577

Browse files
mstorsjomstorsjo
committedOct 18, 2019
[LLD] [COFF] Try to report source locations for duplicate symbols
This fixes the second part of PR42407. For files with dwarf debug info, it manually loads and iterates .debug_info to find the declared location of variables, to allow reporting them. (This matches the corresponding code in the ELF linker.) For functions, it uses the existing getFileLineDwarf which uses LLVMSymbolizer for translating addresses to file lines. In object files with codeview debug info, only the source location of duplicate functions is printed. (And even there, only for the first input file. The getFileLineCodeView function requires the object file to be fully loaded and initialized to properly resolve source locations, but duplicate symbols are reported at a stage when the second object file isn't fully loaded yet.) Differential Revision: https://reviews.llvm.org/D68975 llvm-svn: 375218
1 parent 651f079 commit b38f577

9 files changed

+433
-22
lines changed
 

‎lld/COFF/InputFiles.cpp

+86-2
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,8 @@ Symbol *ObjFile::createRegular(COFFSymbolRef sym) {
382382
StringRef name;
383383
coffObj->getSymbolName(sym, name);
384384
if (sc)
385-
return symtab->addRegular(this, name, sym.getGeneric(), sc);
385+
return symtab->addRegular(this, name, sym.getGeneric(), sc,
386+
sym.getValue());
386387
// For MinGW symbols named .weak.* that point to a discarded section,
387388
// don't create an Undefined symbol. If nothing ever refers to the symbol,
388389
// everything should be fine. If something actually refers to the symbol
@@ -536,7 +537,7 @@ void ObjFile::handleComdatSelection(COFFSymbolRef sym, COMDATType &selection,
536537
// if the two comdat sections have e.g. different alignment.
537538
// Match that.
538539
if (leaderChunk->getContents() != newChunk.getContents())
539-
symtab->reportDuplicate(leader, this);
540+
symtab->reportDuplicate(leader, this, &newChunk, sym.getValue());
540541
break;
541542
}
542543

@@ -788,6 +789,89 @@ void ObjFile::initializeDependencies() {
788789
debugTypesObj = makeTpiSource(this);
789790
}
790791

792+
// Used only for DWARF debug info, which is not common (except in MinGW
793+
// environments). This returns an optional pair of file name and line
794+
// number for where the variable was defined.
795+
Optional<std::pair<StringRef, uint32_t>>
796+
ObjFile::getVariableLocation(StringRef var) {
797+
if (!dwarf) {
798+
dwarf = DWARFContext::create(*getCOFFObj());
799+
if (!dwarf)
800+
return None;
801+
initializeDwarf();
802+
}
803+
if (config->machine == I386)
804+
var.consume_front("_");
805+
auto it = variableLoc.find(var);
806+
if (it == variableLoc.end())
807+
return None;
808+
809+
// Take file name string from line table.
810+
std::string fileName;
811+
if (!it->second.lt->getFileNameByIndex(
812+
it->second.file, {},
813+
DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, fileName))
814+
return None;
815+
816+
return std::make_pair(saver.save(fileName), it->second.line);
817+
}
818+
819+
// Used only for DWARF debug info, which is not common (except in MinGW
820+
// environments). This initializes the dwarf, lineTables and variableLoc
821+
// members.
822+
void ObjFile::initializeDwarf() {
823+
for (std::unique_ptr<DWARFUnit> &cu : dwarf->compile_units()) {
824+
auto report = [](Error err) {
825+
handleAllErrors(std::move(err),
826+
[](ErrorInfoBase &info) { warn(info.message()); });
827+
};
828+
Expected<const DWARFDebugLine::LineTable *> expectedLT =
829+
dwarf->getLineTableForUnit(cu.get(), report);
830+
const DWARFDebugLine::LineTable *lt = nullptr;
831+
if (expectedLT)
832+
lt = *expectedLT;
833+
else
834+
report(expectedLT.takeError());
835+
if (!lt)
836+
continue;
837+
lineTables.push_back(lt);
838+
839+
// Loop over variable records and insert them to variableLoc.
840+
for (const auto &entry : cu->dies()) {
841+
DWARFDie die(cu.get(), &entry);
842+
// Skip all tags that are not variables.
843+
if (die.getTag() != dwarf::DW_TAG_variable)
844+
continue;
845+
846+
// Skip if a local variable because we don't need them for generating
847+
// error messages. In general, only non-local symbols can fail to be
848+
// linked.
849+
if (!dwarf::toUnsigned(die.find(dwarf::DW_AT_external), 0))
850+
continue;
851+
852+
// Get the source filename index for the variable.
853+
unsigned file = dwarf::toUnsigned(die.find(dwarf::DW_AT_decl_file), 0);
854+
if (!lt->hasFileAtIndex(file))
855+
continue;
856+
857+
// Get the line number on which the variable is declared.
858+
unsigned line = dwarf::toUnsigned(die.find(dwarf::DW_AT_decl_line), 0);
859+
860+
// Here we want to take the variable name to add it into variableLoc.
861+
// Variable can have regular and linkage name associated. At first, we try
862+
// to get linkage name as it can be different, for example when we have
863+
// two variables in different namespaces of the same object. Use common
864+
// name otherwise, but handle the case when it also absent in case if the
865+
// input object file lacks some debug info.
866+
StringRef name =
867+
dwarf::toString(die.find(dwarf::DW_AT_linkage_name),
868+
dwarf::toString(die.find(dwarf::DW_AT_name), ""));
869+
if (!name.empty())
870+
variableLoc.insert({name, {lt, file, line}});
871+
}
872+
}
873+
}
874+
791875
StringRef ltrim1(StringRef s, const char *chars) {
792876
if (!s.empty() && strchr(chars, s[0]))
793877
return s.substr(1);

‎lld/COFF/InputFiles.h

+14
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "llvm/ADT/DenseSet.h"
1717
#include "llvm/BinaryFormat/Magic.h"
1818
#include "llvm/DebugInfo/CodeView/TypeRecord.h"
19+
#include "llvm/DebugInfo/DWARF/DWARFContext.h"
1920
#include "llvm/LTO/LTO.h"
2021
#include "llvm/Object/Archive.h"
2122
#include "llvm/Object/COFF.h"
@@ -202,6 +203,9 @@ class ObjFile : public InputFile {
202203
// The .debug$T stream if there's one.
203204
llvm::Optional<llvm::codeview::CVTypeArray> debugTypes;
204205

206+
llvm::Optional<std::pair<StringRef, uint32_t>>
207+
getVariableLocation(StringRef var);
208+
205209
private:
206210
const coff_section* getSection(uint32_t i);
207211
const coff_section *getSection(COFFSymbolRef sym) {
@@ -212,6 +216,7 @@ class ObjFile : public InputFile {
212216
void initializeSymbols();
213217
void initializeFlags();
214218
void initializeDependencies();
219+
void initializeDwarf();
215220

216221
SectionChunk *
217222
readSection(uint32_t sectionNumber,
@@ -285,6 +290,15 @@ class ObjFile : public InputFile {
285290
// index. Nonexistent indices (which are occupied by auxiliary
286291
// symbols in the real symbol table) are filled with null pointers.
287292
std::vector<Symbol *> symbols;
293+
294+
std::unique_ptr<llvm::DWARFContext> dwarf;
295+
std::vector<const llvm::DWARFDebugLine::LineTable *> lineTables;
296+
struct VarLoc {
297+
const llvm::DWARFDebugLine::LineTable *lt;
298+
unsigned file;
299+
unsigned line;
300+
};
301+
llvm::DenseMap<StringRef, VarLoc> variableLoc;
288302
};
289303

290304
// This type represents import library members that contain DLL names

‎lld/COFF/SymbolTable.cpp

+63-9
Original file line numberDiff line numberDiff line change
@@ -520,15 +520,69 @@ void SymbolTable::addLazyObject(LazyObjFile *f, StringRef n) {
520520
f->fetch();
521521
}
522522

523-
void SymbolTable::reportDuplicate(Symbol *existing, InputFile *newFile) {
524-
std::string msg = "duplicate symbol: " + toString(*existing) + " in " +
525-
toString(existing->getFile()) + " and in " +
526-
toString(newFile);
523+
static std::string getSourceLocationBitcode(BitcodeFile *file) {
524+
std::string res("\n>>> defined at ");
525+
StringRef source = file->obj->getSourceFileName();
526+
if (!source.empty())
527+
res += source.str() + "\n>>> ";
528+
res += toString(file);
529+
return res;
530+
}
531+
532+
static std::string getSourceLocationObj(ObjFile *file, SectionChunk *sc,
533+
uint32_t offset, StringRef name) {
534+
Optional<std::pair<StringRef, uint32_t>> fileLine;
535+
if (sc)
536+
fileLine = getFileLine(sc, offset);
537+
if (!fileLine)
538+
fileLine = file->getVariableLocation(name);
539+
540+
std::string res;
541+
llvm::raw_string_ostream os(res);
542+
os << "\n>>> defined at ";
543+
if (fileLine)
544+
os << fileLine->first << ":" << fileLine->second << "\n>>> ";
545+
os << toString(file);
546+
return os.str();
547+
}
548+
549+
static std::string getSourceLocation(InputFile *file, SectionChunk *sc,
550+
uint32_t offset, StringRef name) {
551+
if (auto *o = dyn_cast<ObjFile>(file))
552+
return getSourceLocationObj(o, sc, offset, name);
553+
if (auto *b = dyn_cast<BitcodeFile>(file))
554+
return getSourceLocationBitcode(b);
555+
return "\n>>> defined at " + toString(file);
556+
}
557+
558+
// Construct and print an error message in the form of:
559+
//
560+
// lld-link: error: duplicate symbol: foo
561+
// >>> defined at bar.c:30
562+
// >>> bar.o
563+
// >>> defined at baz.c:563
564+
// >>> baz.o
565+
void SymbolTable::reportDuplicate(Symbol *existing, InputFile *newFile,
566+
SectionChunk *newSc,
567+
uint32_t newSectionOffset) {
568+
std::string msg;
569+
llvm::raw_string_ostream os(msg);
570+
os << "duplicate symbol: " << toString(*existing);
571+
572+
DefinedRegular *d = cast<DefinedRegular>(existing);
573+
if (d && isa<ObjFile>(d->getFile())) {
574+
os << getSourceLocation(d->getFile(), d->getChunk(), d->getValue(),
575+
existing->getName());
576+
} else {
577+
os << getSourceLocation(existing->getFile(), nullptr, 0, "");
578+
}
579+
os << getSourceLocation(newFile, newSc, newSectionOffset,
580+
existing->getName());
527581

528582
if (config->forceMultiple)
529-
warn(msg);
583+
warn(os.str());
530584
else
531-
error(msg);
585+
error(os.str());
532586
}
533587

534588
Symbol *SymbolTable::addAbsolute(StringRef n, COFFSymbolRef sym) {
@@ -568,16 +622,16 @@ Symbol *SymbolTable::addSynthetic(StringRef n, Chunk *c) {
568622
}
569623

570624
Symbol *SymbolTable::addRegular(InputFile *f, StringRef n,
571-
const coff_symbol_generic *sym,
572-
SectionChunk *c) {
625+
const coff_symbol_generic *sym, SectionChunk *c,
626+
uint32_t sectionOffset) {
573627
Symbol *s;
574628
bool wasInserted;
575629
std::tie(s, wasInserted) = insert(n, f);
576630
if (wasInserted || !isa<DefinedRegular>(s))
577631
replaceSymbol<DefinedRegular>(s, f, n, /*IsCOMDAT*/ false,
578632
/*IsExternal*/ true, sym, c);
579633
else
580-
reportDuplicate(s, f);
634+
reportDuplicate(s, f, c, sectionOffset);
581635
return s;
582636
}
583637

‎lld/COFF/SymbolTable.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class SymbolTable {
9191
Symbol *addAbsolute(StringRef n, COFFSymbolRef s);
9292
Symbol *addRegular(InputFile *f, StringRef n,
9393
const llvm::object::coff_symbol_generic *s = nullptr,
94-
SectionChunk *c = nullptr);
94+
SectionChunk *c = nullptr, uint32_t sectionOffset = 0);
9595
std::pair<DefinedRegular *, bool>
9696
addComdat(InputFile *f, StringRef n,
9797
const llvm::object::coff_symbol_generic *s = nullptr);
@@ -103,7 +103,9 @@ class SymbolTable {
103103
uint16_t machine);
104104
void addLibcall(StringRef name);
105105

106-
void reportDuplicate(Symbol *existing, InputFile *newFile);
106+
void reportDuplicate(Symbol *existing, InputFile *newFile,
107+
SectionChunk *newSc = nullptr,
108+
uint32_t newSectionOffset = 0);
107109

108110
// A list of chunks which to be added to .rdata.
109111
std::vector<Chunk *> localImportChunks;

‎lld/test/COFF/conflict-mangled.test

+6-2
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,13 @@
55
# RUN: not lld-link /out:%t.exe /demangle %t1.obj %t2.obj 2>&1 | FileCheck %s
66
# RUN: not lld-link /out:%t.exe /demangle:no %t1.obj %t2.obj 2>&1 | FileCheck --check-prefix=NODEMANGLE %s
77

8-
# NODEMANGLE: duplicate symbol: ?mangled@@YAHXZ in {{.+}}1.obj and in {{.+}}2.obj
8+
# NODEMANGLE: duplicate symbol: ?mangled@@YAHXZ
9+
# NODEMANGLE: defined at {{.+}}1.obj
10+
# NODEMANGLE: defined at {{.+}}2.obj
911

10-
# CHECK: duplicate symbol: int __cdecl mangled(void) in {{.+}}1.obj and in {{.+}}2.obj
12+
# CHECK: duplicate symbol: int __cdecl mangled(void)
13+
# CHECK: defined at {{.+}}1.obj
14+
# CHECK: defined at {{.+}}2.obj
1115

1216
--- !COFF
1317
header:

‎lld/test/COFF/conflict.test

+11-5
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
11
# REQUIRES: x86
22
# RUN: yaml2obj < %s > %t1.obj
33
# RUN: yaml2obj < %s > %t2.obj
4-
# RUN: not lld-link /out:%t.exe %t1.obj %t2.obj >& %t.log
5-
# RUN: FileCheck %s < %t.log
4+
# RUN: not lld-link /out:%t.exe %t1.obj %t2.obj 2>&1 | FileCheck --check-prefix=OBJ %s
65

76
# RUN: llvm-as -o %t.lto1.obj %S/Inputs/conflict.ll
87
# RUN: llvm-as -o %t.lto2.obj %S/Inputs/conflict.ll
9-
# RUN: not lld-link /out:%t.exe %t.lto1.obj %t.lto2.obj >& %t.log
10-
# RUN: FileCheck %s < %t.log
8+
# RUN: not lld-link /out:%t.exe %t.lto1.obj %t.lto2.obj 2>&1 | FileCheck --check-prefix=BC %s
119

12-
# CHECK: duplicate symbol: foo in {{.+}}1.obj and in {{.+}}2.obj
10+
# OBJ: duplicate symbol: foo
11+
# OBJ: defined at {{.+}}1.obj
12+
# OBJ: defined at {{.+}}2.obj
13+
14+
# BC: duplicate symbol: foo
15+
# BC: defined at {{.+}}conflict.ll
16+
# BC: {{.+}}1.obj
17+
# BC: defined at {{.+}}conflict.ll
18+
# BC: {{.+}}2.obj
1319

1420
--- !COFF
1521
header:

‎lld/test/COFF/duplicate-cv.s

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# REQUIRES: x86
2+
# RUN: llvm-mc -triple=x86_64-windows-msvc -filetype=obj -o %t.obj %s
3+
# RUN: cp %t.obj %t.dupl.obj
4+
# RUN: not lld-link /out:%t.exe %t.obj %t.dupl.obj 2>&1 | FileCheck %s
5+
6+
# CHECK: error: duplicate symbol: main
7+
# CHECK-NEXT: >>> defined at file1.cpp:2
8+
# CHECK-NEXT: >>> {{.*}}.obj
9+
# CHECK-NEXT: >>> defined at {{.*}}.obj
10+
11+
.cv_file 1 "file1.cpp" "EDA15C78BB573E49E685D8549286F33C" 1
12+
.cv_file 2 "file2.cpp" "EDA15C78BB573E49E685D8549286F33D" 1
13+
14+
.section .text,"xr",one_only,main
15+
.globl main
16+
main:
17+
.cv_func_id 0
18+
.cv_loc 0 1 1 0 is_stmt 0
19+
.cv_loc 0 1 2 0
20+
retq
21+
.Lfunc_end0:
22+
23+
.section .debug$S,"dr",associative,main
24+
.long 4
25+
.cv_linetable 0, main, .Lfunc_end0
26+
27+
.section .debug$S,"dr"
28+
.long 4
29+
.cv_filechecksums
30+
.cv_stringtable

0 commit comments

Comments
 (0)
Please sign in to comment.