[llvm-objdump] Add warning messages if disassembly + source for problematic inputs

[mmpozulp]

Summary

Addresses https://bugs.llvm.org/show_bug.cgi?id=41905

[grimar]

Generally looks good to me (with 2 nits), please wait for other opinions too.

llvm/tools/llvm-objdump/llvm-objdump.cpp

560 ↗

(On Diff #201437)

I'd simplify:

if (MissingSources.insert(LineInfo.FileName).second)
  warn("failed to find source " + LineInfo.FileName);

596 ↗

(On Diff #201437)

You do not need Twine( here.

[mmpozulp]

llvm/tools/llvm-objdump/llvm-objdump.cpp

596 ↗

(On Diff #201437)

It appears that I have created a problem by adding the new warn(Twine) function. If I delete Twine( here, I get

llvm-objdump.cpp:594:61: error: call of overloaded 'warn(const char [48])' is ambiguous
       warn("failed to parse debug info which may be missing");
                                                             ^
llvm-objdump.cpp:375:6: note: candidate: 'void llvm::warn(llvm::StringRef)'
 void warn(StringRef Message) {
      ^~~~
llvm-objdump.cpp:380:6: note: candidate: 'void llvm::warn(llvm::Twine)'
 void warn(Twine Message) {

I needed warn(Twine) for calls like warn("a message " + VariableName) and warn(formatv("a message {0}", VariableName)) which create Twines.

Is there a way to change the warn() API to resolve the ambiguity? That way the compiler would not have to force users to pick between warn(Twine) and warn(StringRef) with a tedious Twine() or StringRef() constructor call.

[mmpozulp]

Incorporate feedback from @grimar to simplify an if-statement.

[grimar]

llvm/tools/llvm-objdump/llvm-objdump.cpp

596 ↗

(On Diff #201437)

I think we just need to change our void warn(StringRef Message) to void warn(const Twine &Message).

(note: Twine.h says: "Twines should only be used accepted as const references in arguments, when an API wishes to accept possibly-concatenated strings.")

[jhenderson]

llvm/test/tools/llvm-objdump/X86/source-interleave-invalid-source.test

11 ↗

(On Diff #202077)

I'm concerned that there might be an interleaving issue here, since stderr is normally unbuffered. This could make the test flaky, since the output ordering is important. Perhaps it would be wise to redirect stderr to a file, and check the file contents in a separate FileCheck run (using --input-file)?

15 ↗

(On Diff #202077)

I'd add a check for the file name in the message here. This can be done by the following:

# RUN: ... | FileCheck %s ... -DFILE=%t2.o
...
# WARN: ... in [[FILE]]

where FILE is an arbitrary variable name.

llvm/tools/llvm-objdump/llvm-objdump.cpp

526 ↗

(On Diff #202077)

These new comments (and the two above) are missing trailing full stops.

591 ↗

(On Diff #202077)

Delete this extra blank line.

592 ↗

(On Diff #202077)

I see this was there before, but "<invalid>" seems like a magic string. Is there a constant it is stored in to avoid having "<invalid>" explicitly mentioned in multiple places?

[jhenderson]

llvm/tools/llvm-objdump/llvm-objdump.cpp

594 ↗

(On Diff #202077)

Include the file name in this error message somewhere.

[mmpozulp]

Incorporate excellent feedback from @grimar and @jhenderson. Thanks!

[grimar]

I have probably no more comments except 2 nits.

llvm/include/llvm/DebugInfo/DIContext.h

32 ↗

(On Diff #203944)

Variable names should start from the upper case (LLVM coding style).

llvm/tools/llvm-objdump/llvm-objdump.cpp

376 ↗

(On Diff #203944)

I guess you need to keep errs().flush().

[grimar]

llvm/tools/llvm-objdump/llvm-objdump.cpp

376 ↗

(On Diff #203944)

I am not sure though. It seems should be better for debugging to keep it, i.e. I think this flush
allows to see a warning in the console output as soon it appears, what can probably be useful sometimes.
But I am not sure it is an important bit.

At the same time it seems that in error(Twine Message) above it is used before exit(1) to
flush the messages before application termination. But it is not used in void report_error(StringRef File, Twine Message) below.
It can be a bug probably.

[jhenderson]

llvm/include/llvm/DebugInfo/DIContext.h

32 ↗

(On Diff #203944)

Just to add, the 'k' really isn't a useful part of the name. We don't use that style of variable naming.

Also, I don't think you need to define them in the header. You just need to add a declaration with extern.

llvm/test/tools/llvm-objdump/X86/source-interleave-no-debug-info.test

12 ↗

(On Diff #203944)

Hmm... Not sure about this one. I think we need a different message in that case, since '<invalid>' isn't actually a file.

llvm/tools/llvm-objdump/llvm-objdump.cpp

525 ↗

(On Diff #203944)

Unnecessary extra blank line

[jhenderson]

Hi @mmpozulp, just wondering if you are going to come back to this some time soon, or if you need any further assistance with this issue?

[mmpozulp]

Hey @jhenderson, thanks for asking! I will have some questions for you once I look at the code again. I return from vacation on Wednesday. :)

[mmpozulp]

Rebase on master

[mmpozulp]

Taking off on a camping trip for the week. If anyone wants these features upstream'd before I get back please feel free to pick up where I left off, otherwise I'll be back at next week. :)

[mmpozulp]

Rebase on master

[mmpozulp]

Change global variable names, update warning message, delete extra newline.

[mmpozulp]

llvm/include/llvm/DebugInfo/DIContext.h

32 ↗

(On Diff #203944)

Where should I put the definition?

Also, I just realized that these use static constructors, which the LLVM Coding Standards prohibit. I could use the Scott Meyers singleton

static StringRef &DILineInfoBadString() {
  static StringRef S("<invalid>");
  return S;
}
static StringRef &Addr2LineBadString() {
  static StringRef S("??");
  return S;
}

but it forces references to change from DILineInfoBadString to DILineInfoBadString() and trips this compile warning for some TUs (e.g. lib/DebugInfo/DWARF/DWARFVerifier.cpp)

warning: ‘llvm::StringRef& llvm::Addr2LineBadString()’ defined but not used [-Wunused-function]

when I build with gcc 9.1.0. The warnings should go away when I move the definition out of the header.

llvm/test/tools/llvm-objdump/X86/source-interleave-no-debug-info.test

12 ↗

(On Diff #203944)

I think failed to parse debug info is a bit better than failed to parse debug info from file "<invalid>". Here's what the user sees

[mike@gannon build]$ ./bin/llvm-objdump --source test/tools/llvm-objdump/X86/Output/source-interleave-no-debug-info.test.tmp2.o

test/tools/llvm-objdump/X86/Output/source-interleave-no-debug-info.test.tmp2.o:	file format ELF64-x86-64


Disassembly of section .text:

0000000000000000 foo:
./bin/llvm-objdump: warning: failed to parse debug info
       0: 55                           	pushq	%rbp
       1: 48 89 e5                     	movq	%rsp, %rbp
       4: 8b 05 00 00 00 00            	movl	(%rip), %eax
       a: 5d                           	popq	%rbp
       b: c3                           	retq
       c: 90                           	nop
       d: 90                           	nop
       e: 90                           	nop
       f: 90                           	nop

0000000000000010 main:
      10: 55                           	pushq	%rbp
      11: 48 89 e5                     	movq	%rsp, %rbp
      14: 53                           	pushq	%rbx
      15: 48 83 ec 18                  	subq	$24, %rsp
      19: c7 45 f4 00 00 00 00         	movl	$0, -12(%rbp)
      20: 48 c7 45 e8 00 00 00 00      	movq	$0, -24(%rbp)
      28: 8b 1d 00 00 00 00            	movl	(%rip), %ebx
      2e: e8 cd ff ff ff               	callq	-51 <foo>
      33: 01 d8                        	addl	%ebx, %eax
      35: 48 83 c4 18                  	addq	$24, %rsp
      39: 5b                           	popq	%rbx
      3a: 5d                           	popq	%rbp
      3b: c3                           	retq

[jhenderson]

llvm/include/llvm/DebugInfo/DIContext.h

32 ↗

(On Diff #203944)

Perhaps they could be static members of DILineInfo. To avoid the static constructor issue, they could also be const char * const variables, since they can't change.

llvm/test/tools/llvm-objdump/X86/source-interleave-no-debug-info.test

12 ↗

(On Diff #203944)

They'll only see this if standard output and standard error are printed to the same stream. I think you need something indicating the file in most cases, where possible.

Thinking about your previous version (reporting "invalid"), that sounds like something's wrong. Why isn't it able to print the object file name here? How could it be in an invalid state in that context? Sure, the debug info in the object file might be invalid, but that doesn't affect the file name of the object.

9 ↗

(On Diff #208880)

Nit: put a space after >2, here and elsewhere.

[mmpozulp]

Incorporate feedback from @jhenderson

[mmpozulp]

llvm/test/tools/llvm-objdump/X86/source-interleave-no-debug-info.test

12 ↗

(On Diff #203944)

Why isn't it able to print the object file name here?

In this test, we "failed to parse debug info" because it was stripped from the object file.

[mike@gannon build]$ ./bin/llvm-dwarfdump test/tools/llvm-objdump/X86/Output/source-interleave-no-debug-info.test.tmp2.o
test/tools/llvm-objdump/X86/Output/source-interleave-no-debug-info.test.tmp2.o:	file format ELF64-x86-64

.debug_info contents:

Thus, LineInfo.FileName == "<invalid>" for every instruction. Here is the output from dwarfdump on the unstripped file (that is, before running llvm-objcopy --strip-debug). The filename /home/mike/ws/llvm/llvm-project/llvm/test/tools/llvm-objdump/X86/Inputs/source-interleave-x86_64.c is present.

[mike@gannon build]$ ./bin/llvm-dwarfdump test/tools/llvm-objdump/X86/Output/source-interleave-no-debug-info.test.tmp.o
test/tools/llvm-objdump/X86/Output/source-interleave-no-debug-info.test.tmp.o:	file format ELF64-x86-64

.debug_info contents:
0x00000000: Compile Unit: length = 0x00000089 version = 0x0004 abbr_offset = 0x0000 addr_size = 0x08 (next unit at 0x0000008d)

0x0000000b: DW_TAG_compile_unit
              DW_AT_producer	("clang version 4.0.0")
              DW_AT_language	(DW_LANG_C99)
              DW_AT_name	("source-interleave-x86_64.c")
              DW_AT_stmt_list	(0x00000000)
              DW_AT_comp_dir	("/home/mike/ws/llvm/llvm-project/llvm/test/tools/llvm-objdump/X86/Inputs")
              DW_AT_GNU_pubnames	(true)
              DW_AT_low_pc	(0x0000000000000000)
              DW_AT_high_pc	(0x000000000000003c)

0x0000002a:   DW_TAG_variable
                DW_AT_name	("a")
                DW_AT_type	(0x0000003f "int")
                DW_AT_external	(true)
                DW_AT_decl_file	("/home/mike/ws/llvm/llvm-project/llvm/test/tools/llvm-objdump/X86/Inputs/source-interleave-x86_64.c")
                DW_AT_decl_line	(1)
                DW_AT_location	(DW_OP_addr 0x0)

0x0000003f:   DW_TAG_base_type
                DW_AT_name	("int")
                DW_AT_encoding	(DW_ATE_signed)
                DW_AT_byte_size	(0x04)

0x00000046:   DW_TAG_subprogram
                DW_AT_low_pc	(0x0000000000000000)
                DW_AT_high_pc	(0x000000000000000c)
                DW_AT_frame_base	(DW_OP_reg6 RBP)
                DW_AT_name	("foo")
                DW_AT_decl_file	("/home/mike/ws/llvm/llvm-project/llvm/test/tools/llvm-objdump/X86/Inputs/source-interleave-x86_64.c")
                DW_AT_decl_line	(2)
                DW_AT_type	(0x0000003f "int")
                DW_AT_external	(true)

0x0000005f:   DW_TAG_subprogram
                DW_AT_low_pc	(0x0000000000000010)
                DW_AT_high_pc	(0x000000000000003c)
                DW_AT_frame_base	(DW_OP_reg6 RBP)
                DW_AT_name	("main")
                DW_AT_decl_file	("/home/mike/ws/llvm/llvm-project/llvm/test/tools/llvm-objdump/X86/Inputs/source-interleave-x86_64.c")
                DW_AT_decl_line	(6)
                DW_AT_type	(0x0000003f "int")
                DW_AT_external	(true)

0x00000078:     DW_TAG_variable
                  DW_AT_location	(DW_OP_fbreg -24)
                  DW_AT_name	("b")
                  DW_AT_decl_file	("/home/mike/ws/llvm/llvm-project/llvm/test/tools/llvm-objdump/X86/Inputs/source-interleave-x86_64.c")
                  DW_AT_decl_line	(7)
                  DW_AT_type	(0x00000087 "int*")

0x00000086:     NULL

0x00000087:   DW_TAG_pointer_type
                DW_AT_type	(0x0000003f "int")

0x0000008c:   NULL

How do we recover the filename after stripping?

[jhenderson]

llvm/test/tools/llvm-objdump/X86/source-interleave-no-debug-info.test

12 ↗

(On Diff #203944)

Isn't the name you want to report in this message the object file, not the source? That's the thing that's useful in this context (e.g. it could be "warning: unable to find debug information for object.o"). The problem here is in the object, not the source.

[mmpozulp]

Emit object file name in warning message. Duh. Thanks, @jhenderson!

[jhenderson]

Cool, nearly there. Just one more suggestion.

llvm/tools/llvm-objdump/llvm-objdump.cpp

621 ↗

(On Diff #209419)

It's probably worth sticking with the "failed to parse..." phrasing in case there was a genuine problem parsing it:
"failed to parse debug information for " + ObjectFilename.

Bonus points if it makes sense to use the infromation in the ExpectedLinkInfo error in this case.

[mmpozulp]

Use original wording of "failed to parse..."

[mmpozulp]

llvm/tools/llvm-objdump/llvm-objdump.cpp

621 ↗

(On Diff #209419)

I ran it in lldb and observed that in this case ExpectedLineInfo has no error, so according to the programmer's manual all it can tell us is "success":

If an Expected<T> value is in success mode then the takeError() method will return a success value.

[mmpozulp]

@jhenderson thanks so much for the feedback. Anything else?

[jhenderson]

llvm/tools/llvm-objdump/llvm-objdump.cpp

621 ↗

(On Diff #209419)

I'm not quite sure I follow. Above, ExpectedLineInfo is checked to determine if there is an error, but we then throw it away without updating the LineInfo FileName (see lines 614-617). Presumably this code exists precisely because the debug data couldn't be parsed, and the Expected error in this case will contain the reason why. There may be cases where the line info is successfully fetched, but with no file information, which we should warn about without any additional context, as happens now. Otherwise, it would be good to provide the extra context. Something like the following:

std::string Msg;
if (!ExpectedLineInfo) {
  // pseudo code
  Msg = ExpectedLineInfo.error.message();
  consumeError(ExpectedLineInfo.takeError());
}
else
  LineInfo = *ExpectedLineInfo;

if (LineInfo.FileName == DILineInfo::BadString) {
  if(!WarnedNoDebugInfo) {
    std::string Warning = "failed to parse debug information for " + ObjectFilename;
    if (!Msg.empty())
      Warning += ": " + Msg;
    warn(Warning);

[mmpozulp]

@jhenderson thanks for clarifying. I'll take a look and see what I can find.

[mmpozulp]

@jhenderson, can you think of an input that would cause Symbolizer->symbolizeCode(*Obj, Address) to return an Error?

I went a few calls deep underneath Symbolizer->symbolizeCode(*Obj, Address) and it looks like LLVMSymbolizer and DWARFContext return a default-constructed DILineInfo when they fail to parse the debug info. Can we cause a failure that returns an Error instead? If so we can check whether the Error is actually an ErrorInfo subclass which is how custom errors are created according to error handling Detailed Description. A better implementation of SourcePrinter::printSourceLine() might call handleErrors() instead of consumeError(). Grepping the code for handleErrors and handleAllErrors provides some example uses. First I need some inputs to cause errors so I can see whether any ErrorInfo subclasses are returned and if they have useful context which we can incorporate into our warning. This is my first time wading into LLVM's error handling so please let me know if you have any reading suggestions. In addition to the doxygen docs for Error.h I also found a section on error handling in the programmer's manual.

[jhenderson]

In D62462#1595071, @mmpozulp wrote:

@jhenderson, can you think of an input that would cause Symbolizer->symbolizeCode(*Obj, Address) to return an Error?

I went a few calls deep underneath Symbolizer->symbolizeCode(*Obj, Address) and it looks like LLVMSymbolizer and DWARFContext return a default-constructed DILineInfo when they fail to parse the debug info. Can we cause a failure that returns an Error instead? If so we can check whether the Error is actually an ErrorInfo subclass which is how custom errors are created according to error handling Detailed Description. A better implementation of SourcePrinter::printSourceLine() might call handleErrors() instead of consumeError(). Grepping the code for handleErrors and handleAllErrors provides some example uses. First I need some inputs to cause errors so I can see whether any ErrorInfo subclasses are returned and if they have useful context which we can incorporate into our warning. This is my first time wading into LLVM's error handling so please let me know if you have any reading suggestions. In addition to the doxygen docs for Error.h I also found a section on error handling in the programmer's manual.

I dug into the code myself, and there are a number of examples in the call tree where Expected/Error etc might be returned. For example, for a PPC ELF, when "SymbolizableObjectFile::create" is called, it tries to fetch the section name. If it fails to do so (e.g. because the sh_name field of the section header is invalid). There may well be many others too. I'm not sure how straightforward it would be to create this situation, but you could try using yaml2obj to create such an object for a test case (you can see an example of this error message in test/Object/invalid.test, looking for the "BROKEN-SECNAME" test case).

When it comes to handling the errors, you might want to look at logAllUnhandledErrors or toString in Error.h. The former can be used to write the error to a stream (which could be a string stream of some sort), whilst the latter converts the Error into a std::string, which you can then use.

It's worth noting that except for success values, ALL Error instances have an ErrorInfoBase member. The comments in the docs about ErrorInfo being what is used for custom errors is somewhat irrelevant to this case here, and simply means that you should inherit from ErrorInfo and not ErrorInfoBase when implementing custom error types. I'd therefore just assume that any non-success Error instancesdo have useful additional context and always use it.

Hopefully that gives you the information you need to go about my suggestion. I think in my pseudo code, you can just replace the line Msg = ExpectedLineInfo.error.message(); with Msg = toString(ExpectedLineInfo.takeError()). That would also avoid the need to call consumeError().

[mmpozulp]

Use error message from Symbolizer->symbolizeCode() in "failed to parse debug information" warning

[mmpozulp]

Thanks James. I was able to induce an error by modifying SymbolizableObjectFile::create to unconditionally return object_error::unexpected_eof which produced

./bin/llvm-objdump: warning: failed to parse debug information for tmp2.o: The end of the file was unexpectedly encountered

[mmpozulp]

Run git-clang-format

[jhenderson]

LGTM.

[mmpozulp]

Reverted after the buildbots informed me that I caused a test to fail
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/29488

[jhenderson]

In D62462#1605750, @mmpozulp wrote:

Reverted after the buildbots informed me that I caused a test to fail
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/29488

Are you sure this build failure was caused by your change and not just a flaky test? The latter seems more likely to me, looking at the test failure. It doesn't look like anything to do with what you've done.

[mmpozulp]

Replacing svn diff with git diff.

[mmpozulp]

Reverted in r367816 because the modified tests failed on windows http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/27368

[mmpozulp]

Add single quotes so [[FILE]] is now '[[FILE]]'.
@grimar can you see if this fixes the windows failures?

[grimar]

In D62462#1614349, @mmpozulp wrote:

Add single quotes so [[FILE]] is now '[[FILE]]'.
@grimar can you see if this fixes the windows failures?

Will answer very soon.

[MaskRay]

Failing Tests (2):

LLVM :: tools/llvm-objdump/X86/source-interleave-invalid-source.test
LLVM :: tools/llvm-objdump/X86/source-interleave-missing-source.test

This has been reverted twice now... (I think you can relax the DFILE check instead of doing a full revert)

$ "-DFILE=C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\test\tools\llvm-objdump\X86/Inputs/source-interleave-x86_64.c"

warning: debug info line number 9999 exceeds the number of lines in C:/ps4-buildslave2/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/llvm.src/test/tools/llvm-objdump/X86/Inputs\source-interleave-x86_64.c

This looks like a path separator problem.

[mmpozulp]

Abandon [[FILE]] use in tests.
@grimar please try this on your windows box.

If someone with a windows system wants to try
adding [[FILE]] back in these tests then go for it.
Getting it to work might be complicated. I'm
definitely not going to figure it out without my own
windows machine on which to test and iterate.

[RKSimon]

@mmpozulp This is failing on EXPENSIVE_CHECKS builds, please can you take a look:

http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/18985/steps/test-check-all/logs/stdio

[grimar]

In D62462#1614450, @mmpozulp wrote:

Abandon [[FILE]] use in tests.
@grimar please try this on your windows box.

Tests from test/tools/llvm-objdump now pass for me.

[jhenderson]

llvm/test/tools/llvm-objdump/X86/source-interleave-no-debug-info.test

12 ↗

(On Diff #213306)

Rather than {{.+}}, can this be {{.*}}2.o?

[mmpozulp]

Use {{.*}}2.o rather than {{.+}}

[mmpozulp]

Thanks, @grimar and @jhenderson. I feel ready for another re-commit. Think so too?

[jhenderson]

Yup, LGTM.

[jhenderson]

@mmpozulp, can I confirm that this is now in? It looks like Phabricator hasn't auto-closed this issue.

[mmpozulp]

In D62462#1627011, @jhenderson wrote:

@mmpozulp, can I confirm that this is now in? It looks like Phabricator hasn't auto-closed this issue.

It's in now.

[jhenderson]

In D62462#1630905, @mmpozulp wrote:

In D62462#1627011, @jhenderson wrote:

@mmpozulp, can I confirm that this is now in? It looks like Phabricator hasn't auto-closed this issue.

It's in now.

Great, thanks!

llvm/trunk/include/llvm/DebugInfo/DIContext.h

 #include <string>
 #include <tuple>
 #include <utility>
 
 namespace llvm {
 
 /// A format-neutral container for source line information.
 struct DILineInfo {
+  // DILineInfo contains "<invalid>" for function/filename it cannot fetch.
+  static constexpr const char *const BadString = "<invalid>";
+  // Use "??" instead of "<invalid>" to make our output closer to addr2line.
+  static constexpr const char *const Addr2LineBadString = "??";
   std::string FileName;
   std::string FunctionName;
   Optional<StringRef> Source;
   uint32_t Line = 0;
   uint32_t Column = 0;
   uint32_t StartLine = 0;
 
   // DWARF-specific.
   uint32_t Discriminator = 0;
 
-  DILineInfo() : FileName("<invalid>"), FunctionName("<invalid>") {}
+  DILineInfo() : FileName(BadString), FunctionName(BadString) {}
 
   bool operator==(const DILineInfo &RHS) const {
     return Line == RHS.Line && Column == RHS.Column &&
            FileName == RHS.FileName && FunctionName == RHS.FunctionName &&
            StartLine == RHS.StartLine && Discriminator == RHS.Discriminator;
   }
 
   bool operator!=(const DILineInfo &RHS) const {
     return !(*this == RHS);
   }
 
   bool operator<(const DILineInfo &RHS) const {
     return std::tie(FileName, FunctionName, Line, Column, StartLine,
                     Discriminator) <
            std::tie(RHS.FileName, RHS.FunctionName, RHS.Line, RHS.Column,
                     RHS.StartLine, RHS.Discriminator);
   }
 
   explicit operator bool() const { return *this != DILineInfo(); }
 
   void dump(raw_ostream &OS) {
     OS << "Line info: ";
-    if (FileName != "<invalid>")
+    if (FileName != BadString)
       OS << "file '" << FileName << "', ";
-    if (FunctionName != "<invalid>")
+    if (FunctionName != BadString)
       OS << "function '" << FunctionName << "', ";
     OS << "line " << Line << ", ";
     OS << "column " << Column << ", ";
     OS << "start line " << StartLine << '\n';
   }
 };
 
 using DILineInfoTable = SmallVector<std::pair<uint64_t, DILineInfo>, 16>;

 };
 
 /// Container for description of a global variable.
 struct DIGlobal {
   std::string Name;
   uint64_t Start = 0;
   uint64_t Size = 0;
 
-  DIGlobal() : Name("<invalid>") {}
+  DIGlobal() : Name(DILineInfo::BadString) {}
 };
 
 struct DILocal {
   std::string FunctionName;
   std::string Name;
   std::string DeclFile;
   uint64_t DeclLine = 0;
   Optional<int64_t> FrameOffset;

llvm/trunk/lib/DebugInfo/DWARF/DWARFContext.cpp

 
 DILineInfoTable DWARFContext::getLineInfoForAddressRange(
     object::SectionedAddress Address, uint64_t Size, DILineInfoSpecifier Spec) {
   DILineInfoTable  Lines;
   DWARFCompileUnit *CU = getCompileUnitForAddress(Address.Address);
   if (!CU)
     return Lines;
 
-  std::string FunctionName = "<invalid>";
   uint32_t StartLine = 0;
+  std::string FunctionName(DILineInfo::BadString);
   getFunctionNameAndStartLineForAddress(CU, Address.Address, Spec.FNKind,
                                         FunctionName, StartLine);
 
   // If the Specifier says we don't need FileLineInfo, just
   // return the top-most function at the starting address.
   if (Spec.FLIKind == FileLineInfoKind::None) {
     DILineInfo Result;
     Result.FunctionName = FunctionName;

llvm/trunk/lib/DebugInfo/Symbolize/DIPrinter.cpp

 #include <cstddef>
 #include <cstdint>
 #include <memory>
 #include <string>
 
 namespace llvm {
 namespace symbolize {
 
-// By default, DILineInfo contains "<invalid>" for function/filename it
-// cannot fetch. We replace it to "??" to make our output closer to addr2line.
-static const char kDILineInfoBadString[] = "<invalid>";
-static const char kBadString[] = "??";
-
 // Prints source code around in the FileName the Line.
 void DIPrinter::printContext(const std::string &FileName, int64_t Line) {
   if (PrintSourceContext <= 0)
     return;
 
   ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrErr =
       MemoryBuffer::getFile(FileName);
   if (!BufOrErr)
if (L >= FirstLine && L <= LastLine) {
       OS << *I << "\n";
     }
   }
 }
 
 void DIPrinter::print(const DILineInfo &Info, bool Inlined) {
   if (PrintFunctionNames) {
     std::string FunctionName = Info.FunctionName;
-    if (FunctionName == kDILineInfoBadString)
-      FunctionName = kBadString;
+    if (FunctionName == DILineInfo::BadString)
+      FunctionName = DILineInfo::Addr2LineBadString;
 
     StringRef Delimiter = PrintPretty ? " at " : "\n";
     StringRef Prefix = (PrintPretty && Inlined) ? " (inlined by) " : "";
     OS << Prefix << FunctionName << Delimiter;
   }
   std::string Filename = Info.FileName;
-  if (Filename == kDILineInfoBadString)
-    Filename = kBadString;
+  if (Filename == DILineInfo::BadString)
+    Filename = DILineInfo::Addr2LineBadString;
   else if (Basenames)
     Filename = llvm::sys::path::filename(Filename);
   if (!Verbose) {
     OS << Filename << ":" << Info.Line;
     if (Style == OutputStyle::LLVM)
       OS << ":" << Info.Column;
     OS << "\n";
     printContext(Filename, Info.Line);
DIPrinter &DIPrinter::operator<<(const DIInliningInfo &Info) {
   }
   for (uint32_t i = 0; i < FramesNum; i++)
     print(Info.getFrame(i), i > 0);
   return *this;
 }
 
 DIPrinter &DIPrinter::operator<<(const DIGlobal &Global) {
   std::string Name = Global.Name;
-  if (Name == kDILineInfoBadString)
-    Name = kBadString;
+  if (Name == DILineInfo::BadString)
+    Name = DILineInfo::Addr2LineBadString;
   OS << Name << "\n";
   OS << Global.Start << " " << Global.Size << "\n";
   return *this;
 }
 
 DIPrinter &DIPrinter::operator<<(const DILocal &Local) {
   OS << Local.FunctionName << '\n';
   OS << Local.Name << '\n';

llvm/trunk/test/tools/llvm-objdump/X86/source-interleave-invalid-source.test

 ## Test llvm-objdump's --source behaviour when a line number is greater than the
-## file length.
+## file length and ensure that we emit a warning.
 
 # RUN: sed -e "s,SRC_COMPDIR,%/p/Inputs,g" %p/Inputs/source-interleave.ll > %t.ll
 # RUN: sed -e "s,line: 7,line: 9999,g" %t.ll > %t2.ll
 
 # RUN: llc -o %t.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
 # RUN: llc -o %t2.o -filetype=obj -mtriple=x86_64-pc-linux %t2.ll
 
 # RUN: llvm-objdump --source %t.o | FileCheck %s --check-prefixes=CHECK,GOOD
-# RUN: llvm-objdump --source %t2.o | FileCheck %s --implicit-check-not="int *b = &a;"
+# RUN: llvm-objdump --source %t2.o 2> %t2.e | FileCheck %s --check-prefixes=CHECK --implicit-check-not="int *b = &a;"
+# RUN: FileCheck %s --input-file %t2.e --check-prefixes=WARN
 
 # CHECK:      main:
 # CHECK-NEXT: ; int main() {
+# WARN:       warning: debug info line number 9999 exceeds the number of lines in {{.*}}source-interleave-x86_64.c
 # GOOD:       ;   int *b = &a;
 # CHECK:      ;   return *b + foo();

llvm/trunk/test/tools/llvm-objdump/X86/source-interleave-missing-source.test

 ## Test that if the source cannot be found that disassembly is still printed,
-## and that no source is printed.
+## that no source is printed, and that we emit a warning.
 
 # RUN: sed -e "s,SRC_COMPDIR,%/t,g" %p/Inputs/source-interleave.ll > %t.ll
 # RUN: sed -e "s,SRC_COMPDIR,%/p/Inputs,g" %p/Inputs/source-interleave.ll > %t2.ll
 
 # RUN: llc -o %t.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
 # RUN: llc -o %t2.o -filetype=obj -mtriple=x86_64-pc-linux %t2.ll
 
-# RUN: llvm-objdump --source %t.o | FileCheck %s --implicit-check-not='main()'
+# RUN: llvm-objdump --source %t.o 2> %t.e | FileCheck %s --check-prefixes=CHECK --implicit-check-not='main()'
 # RUN: llvm-objdump --source %t2.o | FileCheck %s --check-prefixes=CHECK,SOURCE
+# RUN: FileCheck %s --input-file %t.e --check-prefixes=WARN
 
+# WARN:        warning: failed to find source {{.*}}source-interleave-x86_64.c
 # CHECK:       0000000000000010 main:
 # SOURCE-NEXT: ; int main() {
 # CHECK-NEXT:   10:   55                      pushq   %rbp

llvm/trunk/test/tools/llvm-objdump/X86/source-interleave-no-debug-info.test

 ## Test that if an object has no debug information, only the disassembly is
-## printed when --source is specified.
+## printed when --source is specified, and that we emit a warning.
 
 # RUN: sed -e "s,SRC_COMPDIR,%/p/Inputs,g" %p/Inputs/source-interleave.ll > %t.ll
 # RUN: llc -o %t.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
 # RUN: llvm-objcopy --strip-debug %t.o %t2.o
 
 # RUN: llvm-objdump --source %t.o | FileCheck %s --check-prefixes=CHECK,SOURCE
-# RUN: llvm-objdump --source %t2.o | FileCheck %s --implicit-check-not='main()'
+# RUN: llvm-objdump --source %t2.o 2> %t2.e | FileCheck %s --check-prefixes=CHECK --implicit-check-not='main()'
+# RUN: FileCheck %s --input-file %t2.e --check-prefixes=WARN
 
+# WARN:        warning: failed to parse debug information for {{.*}}2.o
 # CHECK:       0000000000000010 main:
 # SOURCE-NEXT: ; int main() {
 # CHECK-NEXT:   10:   55                      pushq   %rbp

llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.cpp

if (!IgnoreDWARFFlag) {
           Object->getFileName(), {VMAddress, Address.SectionIndex});
       if (!LineInfo) {
         handleAllErrors(LineInfo.takeError(), [](const ErrorInfoBase &E) {
           errs() << "Symbolizer failed to get line: " << E.message() << "\n";
         });
         continue;
       }
 
-      if (LineInfo->FileName == "<invalid>")
+      if (LineInfo->FileName == DILineInfo::BadString)
         continue;
     }
 
     IndirectInstructions.insert({VMAddress, Address.SectionIndex});
   }
 }
 
 void FileAnalysis::addInstruction(const Instr &Instruction) {

llvm/trunk/tools/llvm-objdump/llvm-objdump.h

 void printWeakBindTable(object::ObjectFile *O);
 void printRawClangAST(const object::ObjectFile *O);
 void printRelocations(const object::ObjectFile *O);
 void printDynamicRelocations(const object::ObjectFile *O);
 void printSectionHeaders(const object::ObjectFile *O);
 void printSectionContents(const object::ObjectFile *O);
 void printSymbolTable(const object::ObjectFile *O, StringRef ArchiveName,
                       StringRef ArchitectureName = StringRef());
-void warn(StringRef Message);
+void warn(Twine Message);
 LLVM_ATTRIBUTE_NORETURN void error(Twine Message);
 LLVM_ATTRIBUTE_NORETURN void report_error(StringRef File, Twine Message);
 LLVM_ATTRIBUTE_NORETURN void report_error(Error E, StringRef File);
 LLVM_ATTRIBUTE_NORETURN void
 report_error(Error E, StringRef FileName, StringRef ArchiveName,
              StringRef ArchitectureName = StringRef());
 LLVM_ATTRIBUTE_NORETURN void
 report_error(Error E, StringRef ArchiveName, const object::Archive::Child &C,

llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp

 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/GraphWriter.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"

 }
 
 LLVM_ATTRIBUTE_NORETURN void error(Twine Message) {
   WithColor::error(errs(), ToolName) << Message << ".\n";
   errs().flush();
   exit(1);
 }
 
-void warn(StringRef Message) {
-  WithColor::warning(errs(), ToolName) << Message << ".\n";
-  errs().flush();
-}
-
-static void warn(Twine Message) {
+void warn(Twine Message) {
   // Output order between errs() and outs() matters especially for archive
   // files where the output is per member object.
   outs().flush();
   WithColor::warning(errs(), ToolName) << Message << "\n";
   errs().flush();
 }
 
 LLVM_ATTRIBUTE_NORETURN void report_error(StringRef File, Twine Message) {

 }
 
 namespace {
 class SourcePrinter {
 protected:
   DILineInfo OldLineInfo;
   const ObjectFile *Obj = nullptr;
   std::unique_ptr<symbolize::LLVMSymbolizer> Symbolizer;
-  // File name to file contents of source
+  // File name to file contents of source.
   std::unordered_map<std::string, std::unique_ptr<MemoryBuffer>> SourceCache;
-  // Mark the line endings of the cached source
+  // Mark the line endings of the cached source.
   std::unordered_map<std::string, std::vector<StringRef>> LineCache;
+  // Keep track of missing sources.
+  StringSet<> MissingSources;
+  // Only emit 'no debug info' warning once.
+  bool WarnedNoDebugInfo;
 
 private:
   bool cacheSource(const DILineInfo& LineInfoFile);
 
 public:
   SourcePrinter() = default;
-  SourcePrinter(const ObjectFile *Obj, StringRef DefaultArch) : Obj(Obj) {
+  SourcePrinter(const ObjectFile *Obj, StringRef DefaultArch)
+      : Obj(Obj), WarnedNoDebugInfo(false) {
     symbolize::LLVMSymbolizer::Options SymbolizerOpts;
     SymbolizerOpts.PrintFunctions = DILineInfoSpecifier::FunctionNameKind::None;
     SymbolizerOpts.Demangle = false;
     SymbolizerOpts.DefaultArch = DefaultArch;
     Symbolizer.reset(new symbolize::LLVMSymbolizer(SymbolizerOpts));
   }
   virtual ~SourcePrinter() = default;
   virtual void printSourceLine(raw_ostream &OS,
                                object::SectionedAddress Address,
+                               StringRef ObjectFilename,
                                StringRef Delimiter = "; ");
 };
 
 bool SourcePrinter::cacheSource(const DILineInfo &LineInfo) {
   std::unique_ptr<MemoryBuffer> Buffer;
   if (LineInfo.Source) {
     Buffer = MemoryBuffer::getMemBuffer(*LineInfo.Source);
   } else {
     auto BufferOrError = MemoryBuffer::getFile(LineInfo.FileName);
-    if (!BufferOrError)
+    if (!BufferOrError) {
+      if (MissingSources.insert(LineInfo.FileName).second)
+        warn("failed to find source " + LineInfo.FileName);
+
       return false;
+    }
     Buffer = std::move(*BufferOrError);
   }
   // Chomp the file to get lines
   const char *BufferStart = Buffer->getBufferStart(),
              *BufferEnd = Buffer->getBufferEnd();
   std::vector<StringRef> &Lines = LineCache[LineInfo.FileName];
   const char *Start = BufferStart;
   for (const char *I = BufferStart; I != BufferEnd; ++I)
     if (*I == '\n') {
       Lines.emplace_back(Start, I - Start - (BufferStart < I && I[-1] == '\r'));
       Start = I + 1;
     }
   if (Start < BufferEnd)
     Lines.emplace_back(Start, BufferEnd - Start);
   SourceCache[LineInfo.FileName] = std::move(Buffer);
   return true;
 }
 
 void SourcePrinter::printSourceLine(raw_ostream &OS,
                                     object::SectionedAddress Address,
+                                    StringRef ObjectFilename,
                                     StringRef Delimiter) {
   if (!Symbolizer)
     return;
 
   DILineInfo LineInfo = DILineInfo();
   auto ExpectedLineInfo = Symbolizer->symbolizeCode(*Obj, Address);
+  std::string ErrorMessage;
   if (!ExpectedLineInfo)
-    consumeError(ExpectedLineInfo.takeError());
+    ErrorMessage = toString(ExpectedLineInfo.takeError());
   else
     LineInfo = *ExpectedLineInfo;
 
-  if ((LineInfo.FileName == "<invalid>") || LineInfo.Line == 0 ||
-      ((OldLineInfo.Line == LineInfo.Line) &&
+  if (LineInfo.FileName == DILineInfo::BadString) {
+    if (!WarnedNoDebugInfo) {
+      std::string Warning =
+          "failed to parse debug information for " + ObjectFilename.str();
+      if (!ErrorMessage.empty())
+        Warning += ": " + ErrorMessage;
+      warn(Warning);
+      WarnedNoDebugInfo = true;
+    }
+    return;
+  }
+
+  if (LineInfo.Line == 0 || ((OldLineInfo.Line == LineInfo.Line) &&
                              (OldLineInfo.FileName == LineInfo.FileName)))
     return;
 
   if (PrintLines)
     OS << Delimiter << LineInfo.FileName << ":" << LineInfo.Line << "\n";
   if (PrintSource) {
     if (SourceCache.find(LineInfo.FileName) == SourceCache.end())
       if (!cacheSource(LineInfo))
         return;
     auto LineBuffer = LineCache.find(LineInfo.FileName);
     if (LineBuffer != LineCache.end()) {
-      if (LineInfo.Line > LineBuffer->second.size())
+      if (LineInfo.Line > LineBuffer->second.size()) {
+        warn(formatv(
+            "debug info line number {0} exceeds the number of lines in {1}",
+            LineInfo.Line, LineInfo.FileName));
         return;
+      }
       // Vector begins at 0, line numbers are non-zero
       OS << Delimiter << LineBuffer->second[LineInfo.Line - 1] << '\n';
     }
   }
   OldLineInfo = LineInfo;
 }
 
 static bool isAArch64Elf(const ObjectFile *Obj) {

 
 class PrettyPrinter {
 public:
   virtual ~PrettyPrinter() = default;
   virtual void printInst(MCInstPrinter &IP, const MCInst *MI,
                          ArrayRef<uint8_t> Bytes,
                          object::SectionedAddress Address, raw_ostream &OS,
                          StringRef Annot, MCSubtargetInfo const &STI,
-                         SourcePrinter *SP,
+                         SourcePrinter *SP, StringRef ObjectFilename,
                          std::vector<RelocationRef> *Rels = nullptr) {
     if (SP && (PrintSource || PrintLines))
-      SP->printSourceLine(OS, Address);
+      SP->printSourceLine(OS, Address, ObjectFilename);
 
     size_t Start = OS.tell();
     if (!NoLeadingAddr)
       OS << format("%8" PRIx64 ":", Address.Address);
     if (!NoShowRawInsn) {
       OS << ' ';
       dumpBytes(Bytes, OS);
     }
if (!NoShowRawInsn) {
       OS << "\t";
       dumpBytes(Bytes.slice(0, 4), OS);
       OS << format("\t%08" PRIx32, opcode);
     }
   }
   void printInst(MCInstPrinter &IP, const MCInst *MI, ArrayRef<uint8_t> Bytes,
                  object::SectionedAddress Address, raw_ostream &OS,
                  StringRef Annot, MCSubtargetInfo const &STI, SourcePrinter *SP,
+                 StringRef ObjectFilename,
                  std::vector<RelocationRef> *Rels) override {
     if (SP && (PrintSource || PrintLines))
-      SP->printSourceLine(OS, Address, "");
+      SP->printSourceLine(OS, Address, ObjectFilename, "");
     if (!MI) {
       printLead(Bytes, Address.Address, OS);
       OS << " <unknown>";
       return;
     }
     std::string Buffer;
     {
       raw_string_ostream TempStream(Buffer);
auto PrintReloc = [&]() -> void {
         ++RelCur;
       }
     };
 
     while (!HeadTail.first.empty()) {
       OS << Separator;
       Separator = "\n";
       if (SP && (PrintSource || PrintLines))
-        SP->printSourceLine(OS, Address, "");
+        SP->printSourceLine(OS, Address, ObjectFilename, "");
       printLead(Bytes, Address.Address, OS);
       OS << Preamble;
       Preamble = "   ";
       StringRef Inst;
       auto Duplex = HeadTail.first.split('\v');
       if (!Duplex.second.empty()) {
         OS << Duplex.first;
         OS << "; ";

 };
 HexagonPrettyPrinter HexagonPrettyPrinterInst;
 
 class AMDGCNPrettyPrinter : public PrettyPrinter {
 public:
   void printInst(MCInstPrinter &IP, const MCInst *MI, ArrayRef<uint8_t> Bytes,
                  object::SectionedAddress Address, raw_ostream &OS,
                  StringRef Annot, MCSubtargetInfo const &STI, SourcePrinter *SP,
+                 StringRef ObjectFilename,
                  std::vector<RelocationRef> *Rels) override {
     if (SP && (PrintSource || PrintLines))
-      SP->printSourceLine(OS, Address);
+      SP->printSourceLine(OS, Address, ObjectFilename);
 
     if (MI) {
       SmallString<40> InstStr;
       raw_svector_ostream IS(InstStr);
 
       IP.printInst(MI, IS, "", STI);
 
       OS << left_justify(IS.str(), 60);

 };
 AMDGCNPrettyPrinter AMDGCNPrettyPrinterInst;
 
 class BPFPrettyPrinter : public PrettyPrinter {
 public:
   void printInst(MCInstPrinter &IP, const MCInst *MI, ArrayRef<uint8_t> Bytes,
                  object::SectionedAddress Address, raw_ostream &OS,
                  StringRef Annot, MCSubtargetInfo const &STI, SourcePrinter *SP,
+                 StringRef ObjectFilename,
                  std::vector<RelocationRef> *Rels) override {
     if (SP && (PrintSource || PrintLines))
-      SP->printSourceLine(OS, Address);
+      SP->printSourceLine(OS, Address, ObjectFilename);
     if (!NoLeadingAddr)
       OS << format("%8" PRId64 ":", Address.Address / 8);
     if (!NoShowRawInsn) {
       OS << "\t";
       dumpBytes(Bytes, OS);
     }
     if (MI)
       IP.printInst(MI, OS, "", STI);
#endif
         // provided
         MCInst Inst;
         bool Disassembled = DisAsm->getInstruction(
             Inst, Size, Bytes.slice(Index), SectionAddr + Index, DebugOut,
             CommentStream);
         if (Size == 0)
           Size = 1;
 
-        PIP.printInst(
-            *IP, Disassembled ? &Inst : nullptr, Bytes.slice(Index, Size),
-            {SectionAddr + Index + VMAAdjustment, Section.getIndex()}, outs(),
-            "", *STI, &SP, &Rels);
+        PIP.printInst(*IP, Disassembled ? &Inst : nullptr,
+                      Bytes.slice(Index, Size),
+                      {SectionAddr + Index + VMAAdjustment, Section.getIndex()},
+                      outs(), "", *STI, &SP, Obj->getFileName(), &Rels);
         outs() << CommentStream.str();
         Comments.clear();
 
         // Try to resolve the target of a call, tail call, etc. to a specific
         // symbol.
         if (MIA && (MIA->isCall(Inst) || MIA->isUnconditionalBranch(Inst) ||
                     MIA->isConditionalBranch(Inst))) {
           uint64_t Target;

llvm/trunk/tools/llvm-xray/func-id-helper.cpp

std::string FuncIdConversionHelper::SymbolOrNumber(int32_t FuncId) const {
 
   object::SectionedAddress ModuleAddress;
   ModuleAddress.Address = It->second;
   // TODO: set proper section index here.
   // object::SectionedAddress::UndefSection works for only absolute addresses.
   ModuleAddress.SectionIndex = object::SectionedAddress::UndefSection;
   if (auto ResOrErr = Symbolizer.symbolizeCode(BinaryInstrMap, ModuleAddress)) {
     auto &DI = *ResOrErr;
-    if (DI.FunctionName == "<invalid>")
+    if (DI.FunctionName == DILineInfo::BadString)
       F << "@(" << std::hex << It->second << ")";
     else
       F << DI.FunctionName;
   } else
     handleAllErrors(ResOrErr.takeError(), [&](const ErrorInfoBase &) {
       F << "@(" << std::hex << It->second << ")";
     });