Commit Graph

7 Commits

Author SHA1 Message Date
Kevin Enderby 7bd8d99497 Thread Expected<...> up from libObject’s getType() for symbols to allow llvm-objdump to produce a good error message.
Produce another specific error message for a malformed Mach-O file when a symbol’s
section index is more than the number of sections.  The existing test case in test/Object/macho-invalid.test
for macho-invalid-section-index-getSectionRawName now reports the error with the message indicating
that a symbol at a specific index has a bad section index and that bad section index value.

Again converting interfaces to Expected<> from ErrorOr<> does involve
touching a number of places. Where the existing code reported the error with a
string message or an error code it was converted to do the same.

Also there some were bugs in the existing code that did not deal with the
old ErrorOr<> return values.  So now with Expected<> since they must be
checked and the error handled, I added a TODO and a comment:
"// TODO: Actually report errors helpfully" and a call something like
consumeError(NameOrErr.takeError()) so the buggy code will not crash
since needed to deal with the Error.

llvm-svn: 268298
2016-05-02 20:28:12 +00:00
Kevin Enderby 81e8b7d949 Thread Expected<...> up from libObject’s getName() for symbols to allow llvm-objdump to produce a good error message.
Produce another specific error message for a malformed Mach-O file when a symbol’s
string index is past the end of the string table.  The existing test case in test/Object/macho-invalid.test
for macho-invalid-symbol-name-past-eof now reports the error with the message indicating
that a symbol at a specific index has a bad sting index and that bad string index value.
 
Again converting interfaces to Expected<> from ErrorOr<> does involve
touching a number of places. Where the existing code reported the error with a
string message or an error code it was converted to do the same.  There is some
code for this that could be factored into a routine but I would like to leave that for
the code owners post-commit to do as they want for handling an llvm::Error.  An
example of how this could be done is shown in the diff in
lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h which had a Check() routine
already for std::error_code so I added one like it for llvm::Error .

Also there some were bugs in the existing code that did not deal with the
old ErrorOr<> return values.  So now with Expected<> since they must be
checked and the error handled, I added a TODO and a comment:
“// TODO: Actually report errors helpfully” and a call something like
consumeError(NameOrErr.takeError()) so the buggy code will not crash
since needed to deal with the Error.

Note there fixes needed to lld that goes along with this that I will commit right after this.
So expect lld not to built after this commit and before the next one.

llvm-svn: 266919
2016-04-20 21:24:34 +00:00
Kevin Enderby 5afbc1cda7 Fix a crash in running llvm-objdump -t with an invalid Mach-O file already
in the test suite. While this is not really an interesting tool and option to run
on a Mach-O file to show the symbol table in a generic libObject format
it shouldn’t crash.

The reason for the crash was in MachOObjectFile::getSymbolType() when it was
calling MachOObjectFile::getSymbolSection() without checking its return value
for the error case.

What makes this fix require a fair bit of diffs is that the method getSymbolType() is
in the class ObjectFile defined without an ErrorOr<> so I needed to add that all
the sub classes.  And all of the uses needed to be updated and the return value
needed to be checked for the error case.

The MachOObjectFile version of getSymbolType() “can” get an error in trying to
come up with the libObject’s internal SymbolRef::Type when the Mach-O symbol
symbol type is an N_SECT type because the code is trying to select from the
SymbolRef::ST_Data or SymbolRef::ST_Function values for the SymbolRef::Type.
And it needs the Mach-O section to use isData() and isBSS to determine if
it will return SymbolRef::ST_Data.

One other possible fix I considered is to simply return SymbolRef::ST_Other
when MachOObjectFile::getSymbolSection() returned an error.  But since in
the past when I did such changes that “ate an error in the libObject code” I
was asked instead to push the error out of the libObject code I chose not
to implement the fix this way.

As currently written both the COFF and ELF versions of getSymbolType()
can’t get an error.  But if isReservedSectionNumber() wanted to check for
the two known negative values rather than allowing all negative values or
the code wanted to add the same check as in getSymbolAddress() to use
getSection() and check for the error then these versions of getSymbolType()
could return errors.

At the end of the day the error printed now is the generic “Invalid data was
encountered while parsing the file” for object_error::parse_failed.  In the
future when we thread Lang’s new TypedError for recoverable error handling
though libObject this will improve.  And where the added // Diagnostic(…
comment is, it would be changed to produce and error message
like “bad section index (42) for symbol at index 8” for this case.

llvm-svn: 264187
2016-03-23 20:27:00 +00:00
Reid Kleckner c038e2db4d [Symbolizer] Don't use PE symbol tables to override PDB symbols
Summary:
PE files are stripped by default, and only contain the names of exported
symbols.

The actual reason that we bother to do this override by default is
actually due to a quirk of the way -gline-tables-only is implemented, so
I phrased the check as "if we are symbolizing from dwarf, do the symtab
override".

This fixes lots of Windows ASan tests that I broke in r250582.

Reviewers: samsonov

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D14594

llvm-svn: 253051
2015-11-13 17:00:36 +00:00
Alexey Samsonov e46bd74147 [LLVMSymbolize] Simplify SymbolizableObjectFile::symbolizeInlinedCode(). NFC.
llvm-svn: 251672
2015-10-30 00:02:55 +00:00
Alexey Samsonov 76f7ecb83a [LLVMSymbolize] Move printing the description of a global into a separate function. NFC.
llvm-svn: 251669
2015-10-29 23:49:19 +00:00
Alexey Samsonov 8df3a07aa8 [LLVMSymbolize] Move ModuleInfo into a separate class (SymbolizableModule).
Summary:
This is mostly NFC. It is a first step in cleaning up LLVMSymbolize
library. It removes "ModuleInfo" class which bundles together ObjectFile
and its debug info context in favor of:
  * abstract SymbolizableModule in public headers;
  * SymbolizableObjectFile subclass in implementation.

Additionally, SymbolizableObjectFile is now created via factory, so we
can properly detect object parsing error at this stage instead of keeping
the broken half-parsed object. As a next step, we would be able to
propagate the error all the way back to the library user.

Further improvements might include:
  * factoring out the logic of finding appropriate file with debug info
    for a given object file, and caching all parsed object files into a
    separate class [A].
  * factoring out DILineInfo rendering [B].
This would make what is now a heavyweight "LLVMSymbolizer" a relatively
straightforward class, that calls into [A] to turn filepath into a
SymbolizableModule, delegates actual symbolization to concrete SymbolizableModule
implementation, and lets [C] render the result.

Reviewers: dblaikie, echristo, rafael

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D14099

llvm-svn: 251662
2015-10-29 22:21:37 +00:00