Use a smaller type for the relocation type and move it to a location in
the structure where it avoids wasted padding bytes.
Technically ELF could use up to four bytes for the type.
But until now only types up to number 43 have been defined.
Reduce the size of struct reloc on x86_64 from 120 to 112 bytes.
This structure is allocated a lot and never freed.
This reduces maximum memory usage while processing vmlinux.o from
3035668 KB to 2919716 KB (-3.8%) on my notebooks "localmodconfig".
Signed-off-by: Thomas Weißschuh <[email protected]>
---
tools/objtool/elf.c | 3 +++
tools/objtool/include/objtool/elf.h | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index ee355beb0d82..182452adaa71 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -1474,5 +1474,8 @@ void elf_close(struct elf *elf)
void elf_reloc_set_type(struct reloc *reloc, int type)
{
+ if (type >= (1U << (8 * sizeof(reloc->type))))
+ WARN("reloc->type out of range: %d", type);
+
reloc->type = type;
}
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 33ec6cf72325..2b5becad5a0a 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -77,10 +77,10 @@ struct reloc {
struct symbol *sym;
struct list_head sym_reloc_entry;
unsigned long offset;
- unsigned int type;
s64 addend;
int idx;
bool jump_table_start;
+ unsigned char type;
};
void elf_reloc_set_type(struct reloc *reloc, int type);
--
2.39.0
Hi, Thomas!
Is this likely to cause most RELOC failures? As shown in the following
example:
#include <bfd.h>
#include <stdio.h>
int main(void)
{
printf("%d\n", BFD_RELOC_S12Z_OPR);
return 0;
}
The BFD_RELOC_S12Z_OPR equal to 2366.
Best wishes.
Rong Tao
Hi Rong,
On Thu, Dec 29, 2022 at 09:33:32AM +0800, Rong Tao wrote:
> Is this likely to cause most RELOC failures? As shown in the following
> example:
>
> #include <bfd.h>
> #include <stdio.h>
>
> int main(void)
> {
> printf("%d\n", BFD_RELOC_S12Z_OPR);
> return 0;
> }
>
> The BFD_RELOC_S12Z_OPR equal to 2366.
I'm not familiar with libbfd, so please correct me if I'm wrong.
To me BFD_RELOC_S12Z_OPR looks like a value that is only used by libbfd.
objtool does not use libbfd.
The only values that objtools should see for the relocation types are
the R_* constants from elf.h as they are used by the compiler and linker
in the raw elf binary files.
These never exceed the value 255, one byte. Indeed they seem to have
been explicitly chosen like this.
$ grep 'define R_.*NUM' /usr/include/elf.h
#define R_68K_NUM 43
#define R_386_NUM 44
#define R_SPARC_NUM 253
#define R_MIPS_NUM 128
#define R_ALPHA_NUM 46
#define R_ARM_NUM 256
#define R_390_NUM 62
#define R_CRIS_NUM 20
#define R_X86_64_NUM 43
#define R_MN10300_NUM 35
#define R_M32R_NUM 256 /* Keep this the last entry. */
#define R_TILEPRO_NUM 130
#define R_TILEGX_NUM 130
#define R_RISCV_NUM 59
These _NUM constants are the highest actually used values, plus one.
So all real values are smaller than 256.
Thomas
Hi, Thomas:
In /usr/include/elf.h has:
#define ELF32_R_TYPE(val) ((val) & 0xff)
#define ELF64_R_TYPE(i) ((i) & 0xffffffff)
^^^^^^^^^^
So, I still feel that keeping 'unsigned int' is a good option. Can we just
use __attribute__((packed)) for wasted padding bytes?
Reviewed-by: Rong Tao <[email protected]>
Hi Rong,
On Thu, Dec 29, 2022 at 11:29:07AM +0800, Rong Tao wrote:
> Hi, Thomas:
>
> In /usr/include/elf.h has:
>
> #define ELF32_R_TYPE(val) ((val) & 0xff)
> #define ELF64_R_TYPE(i) ((i) & 0xffffffff)
> ^^^^^^^^^^
>
> So, I still feel that keeping 'unsigned int' is a good option. Can we just
> use __attribute__((packed)) for wasted padding bytes?
As the struct contains addresses, packing it will create a ton of
compiler warnings and will be much more likely to break than reducing
the width of 'type'.
Given that reducing the width of 'type'
* is currently absolutely safe,
* is unlikely to break in the future, as the elf designers seem to be
actively trying to stay within this range anyways,
* saves 8 bytes from a very heavily allocated struct, having a
measurable impact on the build process,
I continue to propose this aproach.
Let's let the maintainers decide.
Thomas
On Tue, Dec 27, 2022 at 04:01:01PM +0000, Thomas Weißschuh wrote:
> void elf_reloc_set_type(struct reloc *reloc, int type)
> {
> + if (type >= (1U << (8 * sizeof(reloc->type))))
> + WARN("reloc->type out of range: %d", type);
> +
> reloc->type = type;
> }
> diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
> index 33ec6cf72325..2b5becad5a0a 100644
> --- a/tools/objtool/include/objtool/elf.h
> +++ b/tools/objtool/include/objtool/elf.h
> @@ -77,10 +77,10 @@ struct reloc {
> struct symbol *sym;
> struct list_head sym_reloc_entry;
> unsigned long offset;
> - unsigned int type;
> s64 addend;
> int idx;
> bool jump_table_start;
> + unsigned char type;
> };
I'd rather not because
a) the helper function isn't very intuitive and we'll probably forget
to use it
b) some arches need more than 256 types (see for example aarch64 which
objtool may need to support one of these days)
That said, feel free to rearrange the struct fields to at least get some
of those bytes back.
--
Josh