Index: Common/ErrorHandler.cpp =================================================================== --- Common/ErrorHandler.cpp +++ Common/ErrorHandler.cpp @@ -84,15 +84,39 @@ [&](ErrorInfoBase &EIB) { error(EIB.message()); }); } -void ErrorHandler::print(StringRef S, raw_ostream::Colors C) { - *ErrorOS << LogName << ": "; - if (ColorDiagnostics) { - ErrorOS->changeColor(C, true); - *ErrorOS << S; - ErrorOS->resetColor(); +void ErrorHandler::print(const Twine &Msg, const Twine &Origin, + const Twine &LevelText, + const raw_ostream::Colors Color) { + newline(ErrorOS, Msg); + + // Visual Studio diagnostics must conform to several formatting rules that + // change the output from the default: + // - line/column information must be paranthesized. + // - Origin provides the source file and line information. While Origin can be + // blank when source information is not available, we default to the lld + // executable name without the path and '.exe. extensions. The current lld + // behaviour is to use the full lld executable path, but visual studio + // interprets this as a path to the source file. Locally we store this as + // LogName, but this is a reference set with Driver->diagnosticSrcDefault(). + // + // For details on the Visual Studio diagnostics format see: + // https://blogs.msdn.microsoft.com/msbuild/2006/11/02/msbuild-visual-studio-aware-error-messages-and-message-formats/ + if (VSDiagnostics) { + std::string OriginStr = (Origin.str().empty()) ? LogName : Origin.str(); + *ErrorOS << OriginStr << " : "; } else { - *ErrorOS << S; + *ErrorOS << LogName << " : "; } + + if (ColorDiagnostics) + ErrorOS->changeColor(Color); + + *ErrorOS << LevelText; + + if (ColorDiagnostics) + ErrorOS->resetColor(); + + *ErrorOS << ": " << Msg << "\n"; } void ErrorHandler::log(const Twine &Msg) { @@ -108,27 +132,22 @@ outs().flush(); } -void ErrorHandler::warn(const Twine &Msg) { +void ErrorHandler::warn(const Twine &Msg, const Twine &Origin) { if (FatalWarnings) { - error(Msg); + error(Msg, Origin); return; } std::lock_guard<std::mutex> Lock(Mu); - newline(ErrorOS, Msg); - print("warning: ", raw_ostream::MAGENTA); - *ErrorOS << Msg << "\n"; + print(Msg, Origin, "warning", raw_ostream::MAGENTA); } -void ErrorHandler::error(const Twine &Msg) { +void ErrorHandler::error(const Twine &Msg, const Twine &Origin) { std::lock_guard<std::mutex> Lock(Mu); - newline(ErrorOS, Msg); if (ErrorLimit == 0 || ErrorCount < ErrorLimit) { - print("error: ", raw_ostream::RED); - *ErrorOS << Msg << "\n"; + print(Msg, Origin, "error", raw_ostream::RED); } else if (ErrorCount == ErrorLimit) { - print("error: ", raw_ostream::RED); *ErrorOS << ErrorLimitExceededMsg << "\n"; if (ExitEarly) exitLld(1); @@ -137,7 +156,7 @@ ++ErrorCount; } -void ErrorHandler::fatal(const Twine &Msg) { - error(Msg); +void ErrorHandler::fatal(const Twine &Msg, const Twine &Origin) { + error(Msg, Origin); exitLld(1); } Index: ELF/Driver.h =================================================================== --- ELF/Driver.h +++ ELF/Driver.h @@ -29,6 +29,8 @@ void main(ArrayRef<const char *> Args); void addFile(StringRef Path, bool WithLOption); void addLibrary(StringRef Name); + void setDiagnosticSrcDefault(const StringRef srcDefault); + const StringRef diagnosticSrcDefault(); private: void readConfigs(llvm::opt::InputArgList &Args); @@ -36,6 +38,7 @@ void inferMachineType(); template <class ELFT> void link(llvm::opt::InputArgList &Args); + // True if we are in --whole-archive and --no-whole-archive. bool InWholeArchive = false; @@ -46,13 +49,18 @@ bool InBinary = false; std::vector<InputFile *> Files; + + // String for diagnostic Src parameter, when there is no source file. + // Previously stored as 'LogName' in the ErrorHandler class. + std::string DiagnosticSourceDefault = "lld"; }; // Parses command line options. class ELFOptTable : public llvm::opt::OptTable { public: ELFOptTable(); - llvm::opt::InputArgList parse(ArrayRef<const char *> Argv); + llvm::opt::InputArgList parse(StringRef linkerExeName, + ArrayRef<const char *> Argv); }; // Create enum with OPT_xxx values for each option in Options.td Index: ELF/Driver.cpp =================================================================== --- ELF/Driver.cpp +++ ELF/Driver.cpp @@ -346,7 +346,11 @@ void LinkerDriver::main(ArrayRef<const char *> ArgsArr) { ELFOptTable Parser; - opt::InputArgList Args = Parser.parse(ArgsArr.slice(1)); + + // Extract the executable name for diagnostic print outs + StringRef linkerExecutableName = ArgsArr[0]; + opt::InputArgList Args = Parser.parse(linkerExecutableName, ArgsArr.slice(1)); + // Interpret this flag early because error() depends on them. errorHandler().ErrorLimit = args::getInteger(Args, OPT_error_limit, 20); @@ -1403,3 +1407,13 @@ // Write the result to the file. writeResult<ELFT>(); } + +void LinkerDriver::setDiagnosticSrcDefault(const StringRef srcDefault) +{ + DiagnosticSourceDefault = srcDefault; +} + +const StringRef LinkerDriver::diagnosticSrcDefault() +{ + return DiagnosticSourceDefault; +} Index: ELF/DriverUtils.cpp =================================================================== --- ELF/DriverUtils.cpp +++ ELF/DriverUtils.cpp @@ -112,7 +112,8 @@ } // Parses a given list of options. -opt::InputArgList ELFOptTable::parse(ArrayRef<const char *> Argv) { +opt::InputArgList ELFOptTable::parse(StringRef linkerExeName, + ArrayRef<const char *> Argv) { // Make InputArgList from string vectors. unsigned MissingIndex; unsigned MissingCount; @@ -129,7 +130,20 @@ concatLTOPluginOptions(Vec); Args = this->ParseArgs(Vec, MissingIndex, MissingCount); + // We need to set a Visual Studio compatible default source, + // which is the diagnostic message prefix or 'origin', before any calls to + // error(), warn() or fatal(). + if (Args.hasArg(OPT_visual_studio_diagnostics_format)) { + Driver->setDiagnosticSrcDefault( + sys::path::stem(sys::path::filename(linkerExeName))); + errorHandler().VSDiagnostics = true; + } else { + Driver->setDiagnosticSrcDefault(linkerExeName); + } + + errorHandler().LogName = Driver->diagnosticSrcDefault(); handleColorDiagnostics(Args); + if (MissingCount) error(Twine(Args.getArgString(MissingIndex)) + ": missing argument"); Index: ELF/InputFiles.cpp =================================================================== --- ELF/InputFiles.cpp +++ ELF/InputFiles.cpp @@ -81,6 +81,11 @@ // Concatenates arguments to construct a string representing an error location. static std::string createFileLineMsg(StringRef Path, unsigned Line) { + // Visual Studio diagnostics format stipulates paranthesized line and + // column information. + if (errorHandler().VSDiagnostics) + return Path.str() + " (" + std::to_string(Line) + ")"; + std::string Filename = path::filename(Path); std::string Lineno = ":" + std::to_string(Line); if (Filename == Path) Index: ELF/InputSection.cpp =================================================================== --- ELF/InputSection.cpp +++ ELF/InputSection.cpp @@ -243,10 +243,15 @@ } // This function is intended to be used for constructing an error message. -// The returned message looks like this: +// By default, the returned message looks like this: // // foo.c:42 (/home/alice/possibly/very/long/path/foo.c:42) // +// If Visual Studio diagnostics compatibility is enabled the format +// is different: +// +// /home/alice/possibly/very/long/path/foo.c (42) +// // Returns an empty string if there's no way to get line info. std::string InputSectionBase::getSrcMsg(const Symbol &Sym, uint64_t Offset) { // Synthetic sections don't have input files. @@ -703,7 +708,7 @@ ": has non-ABS relocation " + toString(Type) + " against symbol '" + toString(Sym) + "'"; if (Expr != R_PC) { - error(Msg); + error(Msg, errorHandler().LogName); return; } @@ -714,7 +719,7 @@ // relocations without any errors and relocate them as if they were at // address 0. For bug-compatibilty, we accept them with warnings. We // know Steel Bank Common Lisp as of 2018 have this bug. - warn(Msg); + warn(Msg, errorHandler().LogName); Target->relocateOne(BufLoc, Type, SignExtend64<Bits>(Sym.getVA(Addend - Offset))); continue; Index: ELF/Options.td =================================================================== --- ELF/Options.td +++ ELF/Options.td @@ -355,6 +355,9 @@ def z: JoinedOrSeparate<["-"], "z">, MetaVarName<"<option>">, HelpText<"Linker option extensions">; +def visual_studio_diagnostics_format : F<"vs-diagnostics">, + HelpText<"Format diagnostics for Visual Studio compatiblity">; + // Aliases def: Separate<["-"], "f">, Alias<auxiliary>, HelpText<"Alias for --auxiliary">; def: F<"call_shared">, Alias<Bdynamic>, HelpText<"Alias for --Bdynamic">; Index: ELF/Relocations.cpp =================================================================== --- ELF/Relocations.cpp +++ ELF/Relocations.cpp @@ -643,11 +643,11 @@ if ((Config->UnresolvedSymbols == UnresolvedPolicy::Warn && CanBeExternal) || Config->NoinhibitExec) { - warn(Msg); + warn(Msg, Src); return false; } - error(Msg); + error(Msg, Src); return true; } Index: ELF/SymbolTable.cpp =================================================================== --- ELF/SymbolTable.cpp +++ ELF/SymbolTable.cpp @@ -490,7 +490,8 @@ static void reportDuplicate(Symbol *Sym, InputFile *NewFile) { if (!Config->AllowMultipleDefinition) error("duplicate symbol: " + toString(*Sym) + "\n>>> defined in " + - toString(Sym->File) + "\n>>> defined in " + toString(NewFile)); + toString(Sym->File) + "\n>>> defined in " + toString(NewFile), + toString(NewFile)); } static void reportDuplicate(Symbol *Sym, InputFile *NewFile, @@ -504,13 +505,24 @@ return; } - // Construct and print an error message in the form of: + // Construct and print an error message. + // + // In default diagnostics mode the format is: // // ld.lld: error: duplicate symbol: foo // >>> defined at bar.c:30 // >>> bar.o (/home/alice/src/bar.o) // >>> defined at baz.c:563 // >>> baz.o in archive libbaz.a + // + // In Visual Studio compatible diagnostics mode the format is: + // + // bar.c (30): error: duplicate symbol: foo + // >>> defined at bar.c:30 + // >>> bar.o (/home/alice/src/bar.o) + // >>> defined at baz.c:563 + // >>> baz.o in archive libbaz.a + auto *Sec1 = cast<InputSectionBase>(D->Section); std::string Src1 = Sec1->getSrcMsg(*Sym, D->Value); std::string Obj1 = Sec1->getObjMsg(D->Value); @@ -524,7 +536,7 @@ if (!Src2.empty()) Msg += Src2 + "\n>>> "; Msg += Obj2; - error(Msg); + error(Msg, Sec1->getSrcMsg(*Sym, D->Value)); } Symbol *SymbolTable::addRegular(StringRef Name, uint8_t StOther, uint8_t Type, Index: include/lld/Common/ErrorHandler.h =================================================================== --- include/lld/Common/ErrorHandler.h +++ include/lld/Common/ErrorHandler.h @@ -92,29 +92,36 @@ bool ExitEarly = true; bool FatalWarnings = false; bool Verbose = false; + bool VSDiagnostics = false; - void error(const Twine &Msg); - LLVM_ATTRIBUTE_NORETURN void fatal(const Twine &Msg); + void error(const Twine &Msg, const Twine &Origin); + LLVM_ATTRIBUTE_NORETURN void fatal(const Twine &Msg, const Twine &Origin); void log(const Twine &Msg); void message(const Twine &Msg); - void warn(const Twine &Msg); + void warn(const Twine &Msg, const Twine &Origin); std::unique_ptr<llvm::FileOutputBuffer> OutputBuffer; private: - void print(StringRef S, raw_ostream::Colors C); + void print(const Twine &Msg, const Twine &Origin, const Twine &LevelText, + const raw_ostream::Colors Color); }; /// Returns the default error handler. ErrorHandler &errorHandler(); -inline void error(const Twine &Msg) { errorHandler().error(Msg); } -inline LLVM_ATTRIBUTE_NORETURN void fatal(const Twine &Msg) { - errorHandler().fatal(Msg); +inline void error(const Twine &Msg, const Twine &Origin = "") { + errorHandler().error(Msg, Origin); +} +inline LLVM_ATTRIBUTE_NORETURN void fatal(const Twine &Msg, + const Twine &Origin = "") { + errorHandler().fatal(Msg, Origin); } inline void log(const Twine &Msg) { errorHandler().log(Msg); } inline void message(const Twine &Msg) { errorHandler().message(Msg); } -inline void warn(const Twine &Msg) { errorHandler().warn(Msg); } +inline void warn(const Twine &Msg, const Twine &Origin = "") { + errorHandler().warn(Msg, Origin); +} inline uint64_t errorCount() { return errorHandler().ErrorCount; } LLVM_ATTRIBUTE_NORETURN void exitLld(int Val); Index: test/ELF/Inputs/vs-diagnostics-bad-relocation.test =================================================================== --- /dev/null +++ test/ELF/Inputs/vs-diagnostics-bad-relocation.test @@ -0,0 +1,34 @@ +!ELF +FileHeader: + Class: ELFCLASS32 + Data: ELFDATA2LSB + Type: ET_REL + Machine: EM_386 +Sections: + - Type: SHT_PROGBITS + Name: .text + Flags: [ ] + AddressAlign: 0x04 + Content: "0000" + - Type: SHT_PROGBITS + Name: .debug_info + Flags: [ ] + AddressAlign: 0x04 + Content: "0000" + - Type: SHT_REL + Name: .rel.debug_info + Link: .symtab + Info: .debug_info + Relocations: + - Offset: 0 + Symbol: _start + Type: 0xFF + - Offset: 4 + Symbol: _start + Type: 0xFF +Symbols: + Global: + - Name: _start + Type: STT_FUNC + Section: .text + Value: 0x0 Index: test/ELF/Inputs/vs-diagnostics-multiply-defined.s =================================================================== --- /dev/null +++ test/ELF/Inputs/vs-diagnostics-multiply-defined.s @@ -0,0 +1,3 @@ +.file "vs-diagnostics-multiply-defined.s" +.globl wump +wump: Index: test/ELF/Inputs/vs-diagnostics-non-abs-reloc.test =================================================================== --- /dev/null +++ test/ELF/Inputs/vs-diagnostics-non-abs-reloc.test @@ -0,0 +1,34 @@ +!ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_REL + Machine: EM_X86_64 +Sections: + - Type: SHT_PROGBITS + Name: .text + Flags: [ ] + AddressAlign: 0x04 + Content: "0000" + - Type: SHT_PROGBITS + Name: .debug_info + Flags: [ ] + AddressAlign: 0x04 + Content: "0000" + - Type: SHT_REL + Name: .rel.debug_info + Link: .symtab + Info: .debug_info + Relocations: + - Offset: 0 + Symbol: _start + Type: 0x02 + - Offset: 4 + Symbol: _start + Type: 0x02 +Symbols: + Global: + - Name: _start + Type: STT_FUNC + Section: .text + Value: 0x0 Index: test/ELF/vs-diagnostics.s =================================================================== --- /dev/null +++ test/ELF/vs-diagnostics.s @@ -0,0 +1,133 @@ +## We repeat all tests with and without the vs-diagnostics flag enabled +## to demonstrate the output differences. +# REQUIRES: x86 +# RUN: rm -rf %t.dir && mkdir -p %t.dir + +## Check error message output when source information is provided. +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.dir/vs-diagnostics.o +## Without --vs-diagnostics: +# RUN: not ld.lld %t.dir/vs-diagnostics.o 2>&1 | \ +# RUN: FileCheck --strict-whitespace -check-prefix=SOURCE -D=PATH=%t.dir %s + +# SOURCE: {{.+}}ld.lld{{(.EXE|.exe)?}} : error: undefined symbol: flump +# SOURCE-NEXT: >>> referenced by vs-diagnostics.s:1215 +# SOURCE-NEXT: >>> [[PATH]]{{\\|\/}}vs-diagnostics.o:(.text+0x1){{$}} +# SOURCE-SAME: {{[[:space:]]$}} +# SOURCE-NEXT: {{.+}}ld.lld{{(.EXE|.exe)?}} : error: undefined symbol: foo +# SOURCE-NEXT: >>> referenced by vs-diagnostics.s:1066 +# SOURCE-NEXT: >>> [[PATH]]{{\\|\/}}vs-diagnostics.o:(.text+0x6) + +## With --vs-diagnostics: +# RUN: not ld.lld --vs-diagnostics %t.dir/vs-diagnostics.o 2>&1 | \ +# RUN: FileCheck --strict-whitespace -check-prefix=VSSOURCE -D=PATH=%t.dir %s + +# VSSOURCE: vs-diagnostics.s (1215) : error: undefined symbol: flump +# VSSOURCE-NEXT: >>> referenced by vs-diagnostics.s +# VSSOURCE-NEXT: >>> [[PATH]]{{\\|\/}}vs-diagnostics.o:(.text+0x1){{$}} +# VSSOURCE-SAME: {{[[:space:]]$}} +# VSSOURCE-NEXT: vs-diagnostics.s (1066) : error: undefined symbol: foo +# VSSOURCE-NEXT: >>> referenced by vs-diagnostics.s +# VSSOURCE-NEXT: >>> [[PATH]]{{\\|\/}}vs-diagnostics.o:(.text+0x6) + +## Check error message output for duplicate symbols +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/vs-diagnostics-multiply-defined.s \ +# RUN: -o %t.dir/vs-diagnostics-multiply-defined.o +## Without --vs-diagnostics: +# RUN: not ld.lld %t.dir/vs-diagnostics-multiply-defined.o %t.dir/vs-diagnostics-multiply-defined.o 2>&1 | \ +# RUN: FileCheck --strict-whitespace -check-prefix=NOSOURCE -D=PATH=%t.dir %s + +# NOSOURCE: {{.+}}ld.lld{{(.EXE|.exe)?}} : error: duplicate symbol: wump +# NOSOURCE-NEXT: >>> defined at vs-diagnostics-multiply-defined.s +# NOSOURCE-NEXT: >>> [[PATH]]{{\\|\/}}vs-diagnostics-multiply-defined.o:(.text+0x0) +# NOSOURCE-NEXT: >>> defined at vs-diagnostics-multiply-defined.s +# NOSOURCE-NEXT: >>> [[PATH]]{{\\|\/}}vs-diagnostics-multiply-defined.o:(.text+0x0) + +## With --vs-diagnostics: +# RUN: not ld.lld --vs-diagnostics %t.dir/vs-diagnostics-multiply-defined.o %t.dir/vs-diagnostics-multiply-defined.o 2>&1 | \ +# RUN: FileCheck --strict-whitespace -check-prefix=VSNOSOURCE -D=PATH=%t.dir %s + +# VSNOSOURCE: vs-diagnostics-multiply-defined.s : error: duplicate symbol: wump +# VSNOSOURCE-NEXT: >>> defined at vs-diagnostics-multiply-defined.s +# VSNOSOURCE-NEXT: >>> [[PATH]]{{\\|\/}}vs-diagnostics-multiply-defined.o:(.text+0x0) +# VSNOSOURCE-NEXT: >>> defined at vs-diagnostics-multiply-defined.s +# VSNOSOURCE-NEXT: >>> [[PATH]]{{\\|\/}}vs-diagnostics-multiply-defined.o:(.text+0x0) + +## Check warning message output for a non-ABS relocation R_X86_64_PC32 against symbol '_start' +# RUN: yaml2obj %p/Inputs/vs-diagnostics-non-abs-reloc.test -o %t.dir/vs-diagnostics-non-abs-reloc.o + +## without --vs-diagnostics +# RUN: ld.lld %t.dir/vs-diagnostics-non-abs-reloc.o 2>&1 | FileCheck -check-prefix=NONABS -D=PATH=%t.dir %s +# NONABS: {{.+}}ld.lld{{(.EXE|.exe)?}} : warning: [[PATH]]/vs-diagnostics-non-abs-reloc.o:(.debug_info+0x0): has non-ABS relocation R_X86_64_PC32 against symbol '_start' +# NONABS-NEXT: (.debug_info+0x4): has non-ABS relocation R_X86_64_PC32 against symbol '_start' + +## with --vs-diagnostics +# RUN: ld.lld --vs-diagnostics %t.dir/vs-diagnostics-non-abs-reloc.o 2>&1 | FileCheck -check-prefix=VSNONABS -D=PATH=%t.dir %s +# VSNONABS: ld.lld : warning: [[PATH]]/vs-diagnostics-non-abs-reloc.o:(.debug_info+0x0): has non-ABS relocation R_X86_64_PC32 against symbol '_start' +# VSNONABS-NEXT: (.debug_info+0x4): has non-ABS relocation R_X86_64_PC32 against symbol '_start' + +## Check error message output for a bad relocation type (value 0xff) +# RUN: yaml2obj %p/Inputs/vs-diagnostics-bad-relocation.test -o %t.dir/vs-diagnostics-bad-relocation.o + +## without --vs-diagnostics +# RUN: not ld.lld -gdb-index %t.dir/vs-diagnostics-bad-relocation.o -o %t.dir/vs-diagnostics-bad-relocation.exe 2>&1 | FileCheck -check-prefix=BADRELOC %s +# BADRELOC: {{.+}}ld.lld{{(.EXE|.exe)?}} : error: {{.*}}vs-diagnostics-bad-relocation.o:(.debug_info+0x0): has non-ABS relocation Unknown (255) against symbol '_start' + +## with vs-diagnostics +# RUN: not ld.lld --vs-diagnostics -gdb-index %t.dir/vs-diagnostics-bad-relocation.o -o %t.dir/vs-diagnostics-bad-relocation.exe 2>&1 | FileCheck -check-prefix=VSBADRELOC -D=PATH=%t.dir %s +# VSBADRELOC: ld.lld : error: {{.*}}vs-diagnostics-bad-relocation.o:(.debug_info+0x0): has non-ABS relocation Unknown (255) against symbol '_start' + +## Check that we have configured the diagnostic prefix early enough +## so that the output is VS conformant when reporting an unknown command line +## argument. + +## Without --vs-diagnostics: +# RUN: not ld.lld --nonexistantflag %t.o 2>&1 | \ +# RUN: FileCheck -check-prefix=EARLY %s + +# EARLY: {{.+}}ld.lld{{(.EXE|.exe)?}} : error: unknown argument: --nonexistantflag + +## With -- vs-diagnostics: +# RUN: not ld.lld --nonexistantflag --vs-diagnostics %t.o 2>&1 | \ +# RUN: FileCheck -check-prefix=VSEARLY %s + +# VSEARLY: ld.lld : error: unknown argument: --nonexistantflag + +.file 1 "vs-diagnostics.s" +.text + .globl _start +_start: + .loc 1 1215 + call flump + .loc 1 1066 + call foo +end_start: + +.section .debug_abbrev,"",@progbits +.byte 1 # Abbreviation Code +.byte 17 # DW_TAG_compile_unit +.byte 0 # DW_CHILDREN_no +.byte 16 # DW_AT_stmt_list +.byte 23 # DW_FORM_sec_offset +.byte 17 # DW_AT_low_pc +.byte 1 # DW_FORM_addr +.byte 18 # DW_AT_high_pc +.byte 6 # DW_FORM_data4 +.byte 0 # EOM(1) +.byte 0 # EOM(2) +.byte 0 # EOM(3) + +.section .debug_info,"",@progbits +.long .end_debug_info - .start_debug_info # Length of Unit +.start_debug_info: +.short 4 # DWARF version number +.long .debug_abbrev # Offset Into Abbrev. Section +.byte 8 # Address Size (in bytes) +.byte 1 # Abbrev [1] 0xb:0x19 DW_TAG_compile_unit +.long .Lline_table_start0 # DW_AT_stmt_list +.quad _start +.long end_start - _start +.end_debug_info: + +.section .debug_line,"",@progbits + +.Lline_table_start0: