The error reporting function are not consistent.
Before this change:
* They had inconsistent naming (e.g. 'error' vs 'report_error').
* Some of them reported the object name, others - dont.
* Some of them accepted the case when there was no error. (i.e. error code or Error had a success value).
This patch tries to cleanup it a bit.
It also renames report_error -> reportError, report_warning -> reportWarning
and removes a full stop from messages.
Differential revision: https://reviews.llvm.org/D66418
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@369515 91177308-0d34-0410-b5e6-96231b3b80d8
This patch makes a change for test/Object tests responsible
for relocations.
* 2 tests were moved to llvm-readobj/llvm-objdump folders:
Object/elf-reloc-no-sym.test -> tools/llvm-readobj/elf-reloc-no-sym.test
Object/objdump-reloc-shared.test -> tools/llvm-objdump/relocations-in-nonreloc.test
* A prerecompiled binary was removed and these tests were refactored.
Differential revision: https://reviews.llvm.org/D66291
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@369342 91177308-0d34-0410-b5e6-96231b3b80d8
Fix: Add a `consumeError` call removed by mistake to 'printStackSize',
this should fix the "Expected<T> must be checked before access or destruction." reported by following bot:
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/9743/steps/stage%201%20check/logs/stdio
Original commit message:
Currently we have the following functions for error reporting:
LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg);
void reportError(Error Err, StringRef Input);
void reportWarning(Twine Msg);
void reportWarning(StringRef Input, Error Err);
void warn(llvm::Error Err);
void error(std::error_code EC);
Problems are: naming is inconsistent, arguments order is inconsistent,
some of the functions looks excessive.
After applying this patch we have:
void reportError(Error Err, StringRef Input);
void reportError(std::error_code EC, StringRef Input);
void reportWarning(Error Err, StringRef Input);
I'd be happy to remove reportError(std::error_code EC, StringRef Input) too, but it
is used by COFF heavily.
Test cases were updated, they show an improvement introduced.
Differential revision: https://reviews.llvm.org/D66286
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@369194 91177308-0d34-0410-b5e6-96231b3b80d8
urrently we have the following functions for error reporting:
--
LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg);
void reportError(Error Err, StringRef Input);
void reportWarning(Twine Msg);
void reportWarning(StringRef Input, Error Err);
void warn(llvm::Error Err);
void error(std::error_code EC);
---
Problems are: naming is inconsistent, arguments order is inconsistent,
some of the functions looks excessive.
After applying this patch we have:
---
LLVM_ATTRIBUTE_NORETURN void reportError(Error Err, StringRef Input);
LLVM_ATTRIBUTE_NORETURN void reportError(std::error_code EC, StringRef Input);
void reportWarning(Error Err, StringRef Input);
---
I'd be happy to remove reportError(std::error_code EC, StringRef Input) too, but it
is used by COFF heavily.
Test cases were updated, they show an improvement introduced.
Differential revision: https://reviews.llvm.org/D66286
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@369190 91177308-0d34-0410-b5e6-96231b3b80d8
There are 2 similar openbsd-phdrs.elf-x86-64 objects committed and
used in test/Object and test/tools/llvm-objdump test cases.
There is no reason to have them, we can use YAML instead. Patch does that.
Differential revision: https://reviews.llvm.org/D66342
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@369189 91177308-0d34-0410-b5e6-96231b3b80d8
Changes: no changes. A fix for the clang code will be landed right on top.
Original commit message:
SectionRef::getName() returns std::error_code now.
Returning Expected<> instead has multiple benefits.
For example, it forces user to check the error returned.
Also Expected<> may keep a valuable string error message,
what is more useful than having a error code.
(Object\invalid.test was updated to show the new messages printed.)
This patch makes a change for all users to switch to Expected<> version.
Note: in a few places the error returned was ignored before my changes.
In such places I left them ignored. My intention was to convert the interface
used, and not to improve and/or the existent users in this patch.
(Though I think this is good idea for a follow-ups to revisit such places
and either remove consumeError calls or comment each of them to clarify why
it is OK to have them).
Differential revision: https://reviews.llvm.org/D66089
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@368826 91177308-0d34-0410-b5e6-96231b3b80d8
SectionRef::getName() returns std::error_code now.
Returning Expected<> instead has multiple benefits.
For example, it forces user to check the error returned.
Also Expected<> may keep a valuable string error message,
what is more useful than having a error code.
(Object\invalid.test was updated to show the new messages printed.)
This patch makes a change for all users to switch to Expected<> version.
Note: in a few places the error returned was ignored before my changes.
In such places I left them ignored. My intention was to convert the interface
used, and not to improve and/or the existent users in this patch.
(Though I think this is good idea for a follow-ups to revisit such places
and either remove consumeError calls or comment each of them to clarify why
it is OK to have them).
Differential revision: https://reviews.llvm.org/D66089
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@368812 91177308-0d34-0410-b5e6-96231b3b80d8
In some cases a symbol might have section index == SHN_XINDEX.
This is an escape value indicating that the actual section header index
is too large to fit in the containing field.
Then the SHT_SYMTAB_SHNDX section is used. It contains the 32bit values
that stores section indexes.
ELF gABI says that there can be multiple SHT_SYMTAB_SHNDX sections,
i.e. for example one for .symtab and one for .dynsym
(1) https://groups.google.com/forum/#!topic/generic-abi/-XJAV5d8PRg
(2) DT_SYMTAB_SHNDX: http://www.sco.com/developers/gabi/latest/ch5.dynamic.html
In this patch I am only supporting a single SHT_SYMTAB_SHNDX associated
with a .symtab. This is a more or less common case which is used a few tests I saw in LLVM.
I decided not to create the SHT_SYMTAB_SHNDX section as "implicit",
but implement is like a kind of regular section for now.
i.e. tools do not recreate this section or its content, like they do for
symbol table sections, for example. That should allow to write all kind of
possible broken test cases for our needs and keep the output closer to requested.
Differential revision: https://reviews.llvm.org/D65446
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@368272 91177308-0d34-0410-b5e6-96231b3b80d8
Currently, we have a code duplication in llvm-readobj which was introduced in D63266.
The duplication was introduced to allow llvm-readobj to dump the partially
broken object. Methods in ELFFile<ELFT> perform a strict validation of the inputs,
what is itself good, but not for dumper tools, that might want to dump the information,
even if some pieces are broken/unexpected.
This patch introduces a warning handler which can be passed to ELFFile<ELFT> methods
and can allow skipping the non-critical errors when needed/possible.
For demonstration, I removed the duplication from llvm-readobj and implemented a warning using
the new custom warning handler. It also deduplicates the strings printed, making the output less verbose.
Differential revision: https://reviews.llvm.org/D65515
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@368260 91177308-0d34-0410-b5e6-96231b3b80d8
Mips/elf-flags.yaml and Mips/elf-abi.yaml are tests that intention was to
show that yaml2obj/obj2yaml are able to read/dump MIPS specific e_flags.
They were not complete, contained an excessive YAML parts and were placed
at a wrong location.
I removed them and created the obj2yaml/elf-mips-eflags.yaml instead.
Differential revision: https://reviews.llvm.org/D65807
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@368140 91177308-0d34-0410-b5e6-96231b3b80d8
When e_shstrndx is equal to SHN_XINDEX,
the index of the section string table section should
be taken from the sh_link field of the section
header at index 0.
If sh_link is broken, e.g. contains an index that is
larger than number of sections, then error is reported.
This error message was untested before.
Differential revision: https://reviews.llvm.org/D65391
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@368139 91177308-0d34-0410-b5e6-96231b3b80d8
Remove redundant `yaml2obj-elf-file-headers-with-e_flags.yaml` test
case. The same functionality is checked by the `Mips/elf-flags.yaml`.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@368023 91177308-0d34-0410-b5e6-96231b3b80d8
There are multiple yaml2obj-* tests in llvm/test/Object
folder. This is not correct place to have them and my intention
was to move them out to test\tools\yaml2obj folder. I reviewed
them, made some changes, and my comments are below.
For all tests I:
Added comments when needed.
Moved them from llvm/test/Object to yaml2obj tests.
Another changes performed:
1) yaml2obj-invalid.yaml. It was a test for an invalid YAML input.
I just moved it.
2) yaml2obj-coff-multi-doc.test/yaml2obj-elf-multi-doc.test:
these were a tests for testing --docnum=x functionality,
one was for COFF and one for ELF. I merged them into one.
3) yaml2obj-elf-bits-endian.test:
I removed its 4 YAML inputs (merged into the main test).
4) yaml2obj-readobj.test:
This file has a long history. It was added to check the
"parsing of header charactestics" initially. Then was used to test
how yaml2obj writes the relocations. Then was upgraded to check how
yaml2obj handle "-o" option. I think it should be heavily splitted
and refactored in a separate patch. For now I leaved it as is, but restyled
to reduce the changes in a follow-ups.
5) yaml2obj-elf-alignment.yaml: its intention was to check we
can set sh-addralign field. I moved, renamed (to elf-sh-addralign.yaml)
and updated this test.
6) yaml2obj-elf-file-headers.yaml: I removed it.
It's intention was to check that
yaml2obj handles OS/ABI and ELF type (e.g Relocatable).
We are testing this already, for example in D64800. We might want
to add a better (more complete) test, but keeping the existent test
does not have much sense I think.
7) yaml2obj-elf-file-headers-with-e_flags.yaml: I would describe its intention
as "testing MIPS e_flags". It is far from being complete and tests only
a few flags. I leaved it alone for now.
8) yaml2obj-elf-rel.yaml: its intention is to check the MIPS32 relocations.
We have a version for MIPS64 here: test\Object\Mips\elf-mips64-rel.yaml
Seems them both are incomplete. I leaved them alone for now.
9) yaml2obj-elf-rel-noref.yaml: was introduced to check the support of arm32
R_ARM_V4BX relocatiion. I leaved it alone for now.
10) yaml2obj-elf-section-basic.yaml: it just checked that we are able to recognise
trivial fields like section 'Name', 'Type', 'Flags' and others. All of our yaml2obj
tests are heavily using it. I just removed this test.
11) yaml2obj-elf-section-invalid-size.yaml: its intention was to check the
"Section size must be greater than or equal to the content size" error.
I moved this test to `tools\yaml2obj\section-size-content.yaml'
12) yaml2obj-elf-symbol-basic.yaml: its intention seems was to support declarations
of the symbols in yaml2obj. I removed it. We use this in almost each test we already have.
13) yaml2obj-elf-symbol-LocalGlobalWeak.yaml: its intention was to check that we can
declare different symbol bindings. I moved it to tools\yaml2obj\elf-symbol-binding.yaml.
14) yaml2obj-coff-invalid-alignment.test: check that error is reported for a too large coff
section alignment. Moved it to tools\yaml2obj\coff-invalid-alignment.test
15) yaml2obj-elf-symbol-visibility.yaml: tests ELF symbols visibility. I improved it and
moved to tools\yaml2obj\elf-symbol-visibility.yaml and tools\obj2yaml\elf-symbol-visibility.yaml
Differential revision: https://reviews.llvm.org/D65652
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@367988 91177308-0d34-0410-b5e6-96231b3b80d8
This patch removes test/Object/X86/no-start-symbol.test (rewrites
it to use YAML and moves it to llvm-objdump tests folder).
(This test was initially introduced in rL239039, but now there
is no reason to keep the precompiled binary it seems).
DIfferential revision: https://reviews.llvm.org/D65136
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@366896 91177308-0d34-0410-b5e6-96231b3b80d8
This patch touches a few test cases:
It removes dtflags.elf-x86-64 binary and elf-dtflags.test.
elf-dtflags.test is excessive because we have the
elf-dynamic-tags.test which test all non-machine specific tags.
It removes testing of --dynamic-table from test\Object\readobj-shared-object.test
(we have the elf-dynamic-tags.test for that), and simplifies this test case.
It moves testing of the headers from readobj-shared-object.test
to elf-file-headers.test.
Adds test/tools/llvm-readobj/elf-file-types.test and test/tools/llvm-readobj/elf-loadname.test.
It opens road for removing the readobj-shared-object.test completely soon.
Differential revision: https://reviews.llvm.org/D65068
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@366895 91177308-0d34-0410-b5e6-96231b3b80d8
With a fix of the issue found by UBSan.
Original commit message:
This eliminates a one error untested and
also introduces a error for one more possible case
which lead to crash previously.
Differential revision: https://reviews.llvm.org/D64987
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@366886 91177308-0d34-0410-b5e6-96231b3b80d8
When e_shstrndx is broken, it is impossible to get a section name.
In this patch I improved the error message we show and
added tests for Object and for llvm-readelf/llvm-readobj
Message was changed in two places:
1) llvm-readelf/llvm-readobj previously used a code from Object/ELF.h,
now they have a modified version of it (it has less checks and allows
dumping broken things).
2) Code in Object/ELF.h is still used for generic cases.
Differential revision: https://reviews.llvm.org/D64714
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@366203 91177308-0d34-0410-b5e6-96231b3b80d8
test/Object is not correct place to have tests that check obj2yaml
functionality, because we have test/tools/obj2yaml folder for that.
In this patch I merged a few test cases with their YAMLs from Inputs
folder, converted one of binary inputs and moved them to
tools/obj2yaml folder.
There are still another tests that might need the same, so it is initial step.
Differential revision: https://reviews.llvm.org/D64555
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365891 91177308-0d34-0410-b5e6-96231b3b80d8
It does not make sence to stop dumping the object if the broken
dynamic section was found. In this patch I changed the behavior from
"report an error" to "report a warning". This matches GNU.
Differential revision: https://reviews.llvm.org/D64472
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365762 91177308-0d34-0410-b5e6-96231b3b80d8
This patch removes trivial-object-test.elf-i386,
trivial-object-test.elf-x86-64 and trivial-object-test2.elf-x86-64
precompiled objects from test/Object/Inputs folder.
I adjusted the existent test cases to use YAML instead.
Differential revision: https://reviews.llvm.org/D64206
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365348 91177308-0d34-0410-b5e6-96231b3b80d8
The errors coming from ELF.h are usually not very
useful because they are uninformative. This patch is a
first step to improve the situation.
I tested this patch with a run of check-llvm and found
that few messages are untested. In this patch, I did not
add more tests but marked all such cases with a "TODO" comment.
For all tested messages I extended the error text to
provide more details (see test cases changed).
Differential revision: https://reviews.llvm.org/D64014
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365183 91177308-0d34-0410-b5e6-96231b3b80d8
Object/corrupt.test has the same purpose as Object/invalid.test:
it tests the behavior on invalid inputs.
In this patch I converted it to YAML, merged into invalid.test,
added comments and removed a few precompiled binaries.
Differential revision: https://reviews.llvm.org/D63927
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@364916 91177308-0d34-0410-b5e6-96231b3b80d8
Object/invalid.test is a test case that is used to check the behavior of tools
when broken inputs are used.
The most often tool tested there is llvm-readobj. I think we might want to move
such tests to test\tools\llvm-readobj. For now this patch converts
many sub-tests to use YAML and removes 12 binaries from the inputs.
Differential revision: https://reviews.llvm.org/D63762
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@364522 91177308-0d34-0410-b5e6-96231b3b80d8
The weak alias should have the characteristics set to
`IMAGE_EXTERN_WEAK_SEARCH_ALIAS` to indicate that the weak external here
is a symbol alias and that the symbol is aliased to a locally defined
symbol. We were previously setting the characteristics to
`IMAGE_EXTERN_WEAK_SEARCH_LIBRARY` which indicates that the symbol
should be looked for in the libraries.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@364370 91177308-0d34-0410-b5e6-96231b3b80d8
This is a follow-up for D62809.
Content and Size fields should be optional as was discussed in comments
of the D62809's thread. With that, we can describe a specific string table and
symbol table sections in a more correct way and also show appropriate errors.
The patch adds lots of test cases where the behavior is described in details.
Differential revision: https://reviews.llvm.org/D62957
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@362931 91177308-0d34-0410-b5e6-96231b3b80d8
llvm-lib now needs a `target triple` for bitcode, so add a new file
that's like trivial.ll but has one, and use that in the test.
(trivial.ll had a comment that looked like it wasn't supposed to be used
in tests directly, so I don't want to change that file.)
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@362809 91177308-0d34-0410-b5e6-96231b3b80d8
It is now possible after D61937 was landed and was discussed
in it's review comments. It is not consistent with GNU, which
does not output .dynamic section content in this case for
no visible reason.
Differential revision: https://reviews.llvm.org/D62179
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@361943 91177308-0d34-0410-b5e6-96231b3b80d8
This is a result of what I found during my work on https://bugs.llvm.org/show_bug.cgi?id=41679.
Previously LLVM readelf took the information about .dynamic section
from its PT_DYNAMIC segment only. GNU tools have a bit different logic.
They also use the information from the .dynamic section header if it is available.
This patch changes the code to improve the compatibility with the GNU Binutils.
Differential revision: https://reviews.llvm.org/D61937
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@361165 91177308-0d34-0410-b5e6-96231b3b80d8
This patch implements a limited form of autolinking primarily designed to allow
either the --dependent-library compiler option, or "comment lib" pragmas (
https://docs.microsoft.com/en-us/cpp/preprocessor/comment-c-cpp?view=vs-2017) in
C/C++ e.g. #pragma comment(lib, "foo"), to cause an ELF linker to automatically
add the specified library to the link when processing the input file generated
by the compiler.
Currently this extension is unique to LLVM and LLD. However, care has been taken
to design this feature so that it could be supported by other ELF linkers.
The design goals were to provide:
- A simple linking model for developers to reason about.
- The ability to to override autolinking from the linker command line.
- Source code compatibility, where possible, with "comment lib" pragmas in other
environments (MSVC in particular).
Dependent library support is implemented differently for ELF platforms than on
the other platforms. Primarily this difference is that on ELF we pass the
dependent library specifiers directly to the linker without manipulating them.
This is in contrast to other platforms where they are mapped to a specific
linker option by the compiler. This difference is a result of the greater
variety of ELF linkers and the fact that ELF linkers tend to handle libraries in
a more complicated fashion than on other platforms. This forces us to defer
handling the specifiers to the linker.
In order to achieve a level of source code compatibility with other platforms
we have restricted this feature to work with libraries that meet the following
"reasonable" requirements:
1. There are no competing defined symbols in a given set of libraries, or
if they exist, the program owner doesn't care which is linked to their
program.
2. There may be circular dependencies between libraries.
The binary representation is a mergeable string section (SHF_MERGE,
SHF_STRINGS), called .deplibs, with custom type SHT_LLVM_DEPENDENT_LIBRARIES
(0x6fff4c04). The compiler forms this section by concatenating the arguments of
the "comment lib" pragmas and --dependent-library options in the order they are
encountered. Partial (-r, -Ur) links are handled by concatenating .deplibs
sections with the normal mergeable string section rules. As an example, #pragma
comment(lib, "foo") would result in:
.section ".deplibs","MS",@llvm_dependent_libraries,1
.asciz "foo"
For LTO, equivalent information to the contents of a the .deplibs section can be
retrieved by the LLD for bitcode input files.
LLD processes the dependent library specifiers in the following way:
1. Dependent libraries which are found from the specifiers in .deplibs sections
of relocatable object files are added when the linker decides to include that
file (which could itself be in a library) in the link. Dependent libraries
behave as if they were appended to the command line after all other options. As
a consequence the set of dependent libraries are searched last to resolve
symbols.
2. It is an error if a file cannot be found for a given specifier.
3. Any command line options in effect at the end of the command line parsing apply
to the dependent libraries, e.g. --whole-archive.
4. The linker tries to add a library or relocatable object file from each of the
strings in a .deplibs section by; first, handling the string as if it was
specified on the command line; second, by looking for the string in each of the
library search paths in turn; third, by looking for a lib<string>.a or
lib<string>.so (depending on the current mode of the linker) in each of the
library search paths.
5. A new command line option --no-dependent-libraries tells LLD to ignore the
dependent libraries.
Rationale for the above points:
1. Adding the dependent libraries last makes the process simple to understand
from a developers perspective. All linkers are able to implement this scheme.
2. Error-ing for libraries that are not found seems like better behavior than
failing the link during symbol resolution.
3. It seems useful for the user to be able to apply command line options which
will affect all of the dependent libraries. There is a potential problem of
surprise for developers, who might not realize that these options would apply
to these "invisible" input files; however, despite the potential for surprise,
this is easy for developers to reason about and gives developers the control
that they may require.
4. This algorithm takes into account all of the different ways that ELF linkers
find input files. The different search methods are tried by the linker in most
obvious to least obvious order.
5. I considered adding finer grained control over which dependent libraries were
ignored (e.g. MSVC has /nodefaultlib:<library>); however, I concluded that this
is not necessary: if finer control is required developers can fall back to using
the command line directly.
RFC thread: http://lists.llvm.org/pipermail/llvm-dev/2019-March/131004.html.
Differential Revision: https://reviews.llvm.org/D60274
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@360984 91177308-0d34-0410-b5e6-96231b3b80d8
George Rimar