[gold][PATCH] PR gold/18321: gold doesn't support SHF_COMPRESSED sections
H.J. Lu
hjl.tools@gmail.com
Fri May 15 00:22:00 GMT 2015
On Wed, May 13, 2015 at 11:51 PM, Cary Coutant <ccoutant@gmail.com> wrote:
>> This patch adds SHF_COMPRESSED section decompression to gold. OK for
>> master?
>
> + bool is_compressed
> + = (shdr.get_sh_flags() & elfcpp::SHF_COMPRESSED) != 0;
>
> Please match the style of surrounding code. In gold, when we break
> a line at '=', we break after the '=' and indent 4 spaces.
Fixed.
> - uint64_t uncompressed_size = get_uncompressed_size(contents, len);
> + obj->section_contents(i, &len, false);
>
> See above.
Fixed.
> + compression_header_size
> + = elfcpp::Elf_sizes<size>::chdr_size;
>
> This will fit on one line.
Removed.
> + uint64_t uncompressed_size
> + = get_uncompressed_size(contents + compression_header_size,
> + len - compression_header_size);
>
> In new-style compressed sections, the uncompressed size is in the
> compression header. See my comments below.
I changed it to
uint64_t uncompressed_size;
if (is_zcompressed)
uncompressed_size = get_uncompressed_size(contents, len);
else
{
elfcpp::Chdr<size, big_endian> chdr(contents);
uncompressed_size = chdr.get_ch_size();
}
to avoid passing size to big_endian and sh_flags to get_uncompressed_size.
> + info.flag = convert_to_section_size_type(shdr.get_sh_flags());
>
> You shouldn't be using convert_to_section_size_type on sh_flags.
>
> if (uncompressed_size != -1ULL)
> {
> + if (is_compressed)
> + {
> + typename elfcpp::Chdr<size, big_endian> chdr(contents);
> + if (chdr.get_ch_type() != elfcpp::ELFCOMPRESS_ZLIB
> + || chdr.get_ch_size() != uncompressed_size
> + || (chdr.get_ch_addralign()
> + != shdr.get_sh_addralign()))
> + return uncompressed_map;
> + }
> unsigned char* uncompressed_data = NULL;
> - if (decompress_if_needed && need_decompressed_section(name))
> + if (decompress_if_needed
> + && (is_compressed
> + || need_decompressed_section(name)))
> {
> uncompressed_data = new unsigned char[uncompressed_size];
> - if (decompress_input_section(contents, len,
> + if (decompress_input_section(contents +
> compression_header_size,
> + len -
> compression_header_size,
> uncompressed_data,
> uncompressed_size))
> info.contents = uncompressed_data;
>
> I don't see how this is going to work right. This patch doesn't do anything
> to decompress_input_section, so in an SHF_COMPRESS section, it looks like
> you're expecting both an Elf compression header *and* the old-style "ZLIB"
> plus 8 big-endian bytes. That's wrong. In a new-style compressed section,
> the raw compressed data should begin immediately after the compression
> header. For reference from the gABI proposal:
>
> ELFCOMPRESS_ZLIB
> The section data is compressed with the ZLIB compression algorithm.
> The compressed ZLIB data bytes begin with the byte immediately
> following the compression header, and extend to the end of the
> section. Additional documentation for ZLIB may be found at
> http://zlib.net/.
Fixed.
> I'd add a flag to get_uncompressed_size and decompress_input_section, let
> them deal with the difference between the two styles, and greatly simplify
> the logic in this function.
I changed to pass size and big endian to decompress_input_section.
> If the test case is passing, that makes me suspect that the assembler is
> also wrong.
>
> + if (memmem(names, sd->section_names_size, ".zdebug_", 8) != NULL
> + || memmem(names, sd->section_names_size, ".debug_", 7) != NULL)
>
> Scanning the section strings table for ".zdebug_" is a kludge, necessary
> only because we didn't have a section type or flag to scan for. Rather
> than extend this kludge to the ABI-compliant implementation, I'd rather
> scan the section headers for the SHF_COMPRESSED flag. If we find any,
> there's no need to look for ".zdebug_".
Removed.
> + if ((p->second.flag & elfcpp::SHF_COMPRESSED) != 0)
> + {
> + const int size = parameters->target().get_size();
> + if (size == 32)
> + compression_header_size = elfcpp::Elf_sizes<32>::chdr_size;
> + else if (size == 64)
> + compression_header_size = elfcpp::Elf_sizes<64>::chdr_size;
> + else
> + gold_unreachable();
> + }
> + else
> + compression_header_size = 0;
>
> This logic should be in decompress_input_section.
Done.
> + if (!decompress_input_section(buffer + compression_header_size,
> + buffer_size - compression_header_size,
> uncompressed_data,
> uncompressed_size))
>
> As above, decompress_input_section needs to deal with either an old-style
> compression header, or a new-style one -- not a combination of the two.
Done.
> struct Compressed_section_info
> {
> section_size_type size;
> + section_size_type flag;
>
> Why is this section_size_type? It should be elfcpp::Elf_Xword.
Done.
> + if ((shdr.get_sh_flags() & elfcpp::SHF_COMPRESSED) != 0)
> + compression_header_size = elfcpp::Elf_sizes<size>::chdr_size;
> + else
> + compression_header_size = 0;
> + if (!decompress_input_section(p + compression_header_size,
> + len - compression_header_size,
> + view, view_size))
>
> See above.
-
Changed.
Here is the updated patch. OK for master?
Thanks.
--
H.J.
----
This patch adds SHF_COMPRESSED section decompression to gold.
PR gold/18321
* compressed_output.h (decompress_input_section): Add arguments
for ELF class, big endian and sh_flags.
* compressed_output.cc (decompress_input_section): Likewise.
Support the SHF_COMPRESSED section.
* object.h (Compressed_section_info): Add flag to store sh_flags.
* object.cc (build_compressed_section_map): Check SHF_COMPRESSED
for uncompressed size. Store sh_flags in Compressed_section_info.
Pass size, big_endian and sh_flags to decompress_input_section.
(Sized_relobj_file<size, big_endian>::do_find_special_section):
Don't check ".zdebug_*" sections.
(Object::decompressed_section_contents): Pass ELF class, big
endian and sh_flags to decompress_input_section.
* reloc.cc (Sized_relobj_file<size, big_endian>::write_sections):
Likewise.
* output.cc (Output_section::Output_section): Clear the
SHF_COMPRESSED bit.
* testsuite/Makefile.am (check_DATA): Add
debug_msg_cdebug_gabi.err and gdb_index_test_2_gabi.stdout.
(MOSTLYCLEANFILES): Add debug_msg_cdebug_gabi.err and
gdb_index_test_2_gabi.stdout.
(debug_msg_cdebug_gabi.o): New.
(odr_violation1_cdebug_gabi.o): Likewise.
(odr_violation2_cdebug_gabi.o): Likewise.
(debug_msg_cdebug_gabi.err): Likewise.
(check_SCRIPTS): Add gdb_index_test_2_gabi.sh.
(gdb_index_test_cdebug_gabi.o): Likewise.
(gdb_index_test_2_gabi): Likewise.
(gdb_index_test_2_gabi.stdout): Likewise.
* testsuite/gdb_index_test_2_gabi.sh: New file.
* testsuite/Makefile.in: Regenerated.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-SHF_COMPRESSED-section-decompression-to-gold.patch
Type: text/x-patch
Size: 19807 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20150515/8b6eabdc/attachment.bin>
More information about the Binutils
mailing list