gold: add --compress-debug-sections=zstd [PR 29641]
Commit Message
---
gold/NEWS | 2 ++
gold/compressed_output.cc | 59 +++++++++++++++++++++++++++++---------
gold/options.cc | 9 ++++++
gold/options.h | 4 +--
gold/testsuite/Makefile.am | 5 ++++
gold/testsuite/Makefile.in | 27 +++++++++++++++++
6 files changed, 90 insertions(+), 16 deletions(-)
Comments
On 2022-09-30, Fangrui Song wrote:
>---
> gold/NEWS | 2 ++
> gold/compressed_output.cc | 59 +++++++++++++++++++++++++++++---------
> gold/options.cc | 9 ++++++
> gold/options.h | 4 +--
> gold/testsuite/Makefile.am | 5 ++++
> gold/testsuite/Makefile.in | 27 +++++++++++++++++
> 6 files changed, 90 insertions(+), 16 deletions(-)
Ping :)
Alan, also this one for readelf https://sourceware.org/pipermail/binutils/2022-October/123240.html
On 2022-10-10, Fangrui Song wrote:
>On 2022-09-30, Fangrui Song wrote:
>>---
>>gold/NEWS | 2 ++
>>gold/compressed_output.cc | 59 +++++++++++++++++++++++++++++---------
>>gold/options.cc | 9 ++++++
>>gold/options.h | 4 +--
>>gold/testsuite/Makefile.am | 5 ++++
>>gold/testsuite/Makefile.in | 27 +++++++++++++++++
>>6 files changed, 90 insertions(+), 16 deletions(-)
>
>Ping :)
>
>Alan, also this one for readelf https://sourceware.org/pipermail/binutils/2022-October/123240.html
:)
> gold/testsuite/Makefile.am | 5 ++++
@@ -3120,6 +3120,15 @@ gdb_index_test_2_gabi:
gdb_index_test_cdebug_gabi.o gcctestdir/ld
$(CXXLINK) -Wl,--gdb-index $<
gdb_index_test_2_gabi.stdout: gdb_index_test_2_gabi
$(TEST_READELF) --debug-dump=gdb_index $< > $@
+check_SCRIPTS += gdb_index_test_2_zstd.sh
+check_DATA += gdb_index_test_2_zstd.stdout
+MOSTLYCLEANFILES += gdb_index_test_2_zstd.stdout gdb_index_test_2_zstd
+gdb_index_test_cdebug_zstd.o: gdb_index_test.cc
+ $(CXXCOMPILE) -O0 -g -Wa,--compress-debug-sections=zstd -c -o $@ $<
+gdb_index_test_2_zstd: gdb_index_test_cdebug_zstd.o gcctestdir/ld
+ $(CXXLINK) -Wl,--gdb-index $<
+gdb_index_test_2_zstd.stdout: gdb_index_test_2_zstd
+ $(TEST_READELF) --debug-dump=gdb_index $< > $@
These new tests should be conditioned by HAVE_ZSTD, shouldn't they?
Otherwise, looks good to me.
-cary
> > gold/testsuite/Makefile.am | 5 ++++
>
> @@ -3120,6 +3120,15 @@ gdb_index_test_2_gabi:
> gdb_index_test_cdebug_gabi.o gcctestdir/ld
> $(CXXLINK) -Wl,--gdb-index $<
> gdb_index_test_2_gabi.stdout: gdb_index_test_2_gabi
> $(TEST_READELF) --debug-dump=gdb_index $< > $@
> +check_SCRIPTS += gdb_index_test_2_zstd.sh
> +check_DATA += gdb_index_test_2_zstd.stdout
> +MOSTLYCLEANFILES += gdb_index_test_2_zstd.stdout gdb_index_test_2_zstd
> +gdb_index_test_cdebug_zstd.o: gdb_index_test.cc
> + $(CXXCOMPILE) -O0 -g -Wa,--compress-debug-sections=zstd -c -o $@ $<
> +gdb_index_test_2_zstd: gdb_index_test_cdebug_zstd.o gcctestdir/ld
> + $(CXXLINK) -Wl,--gdb-index $<
> +gdb_index_test_2_zstd.stdout: gdb_index_test_2_zstd
> + $(TEST_READELF) --debug-dump=gdb_index $< > $@
>
> These new tests should be conditioned by HAVE_ZSTD, shouldn't they?
>
> Otherwise, looks good to me.
Oops, this reply is for the other thread. Sorry!
-cary
> gold/NEWS | 2 ++
> gold/compressed_output.cc | 59 +++++++++++++++++++++++++++++---------
> gold/options.cc | 9 ++++++
> gold/options.h | 4 +--
> gold/testsuite/Makefile.am | 5 ++++
> gold/testsuite/Makefile.in | 27 +++++++++++++++++
> 6 files changed, 90 insertions(+), 16 deletions(-)
@@ -317,7 +348,7 @@ Output_compressed_section::set_final_data_size()
gold_assert(this->data_ == NULL);
this->set_data_size(uncompressed_size);
}
-}
+ }
The indentation here was correct before.
+check_PROGRAMS += flagstest_compress_debug_sections_zstd
+flagstest_compress_debug_sections_zstd: flagstest_debug.o gcctestdir/ld
+ $(CXXLINK) -o $@ $< -Wl,--compress-debug-sections=zstd
+ test -s $@
+
Please condition this new test on HAVE_ZSTD.
Thanks!
-cary
Applied.
Fix the following:
compressed_output.cc:86:8: error: assignment of read-only variable ‘size’
86 | size = ZSTD_compress(*compressed_data + header_size, size, uncompressed_data,
diff --git a/gold/compressed_output.cc b/gold/compressed_output.cc
index e9f12241f26..f192ddb5080 100644
--- a/gold/compressed_output.cc
+++ b/gold/compressed_output.cc
@@ -81,7 +81,7 @@ zstd_compress(int header_size, const unsigned char *uncompressed_data,
unsigned long uncompressed_size,
unsigned char **compressed_data, unsigned long *compressed_size)
{
- const size_t size = ZSTD_compressBound(uncompressed_size);
+ size_t size = ZSTD_compressBound(uncompressed_size);
*compressed_data = new unsigned char[size + header_size];
size = ZSTD_compress(*compressed_data + header_size, size, uncompressed_data,
uncompressed_size, ZSTD_CLEVEL_DEFAULT);
On Thu, Nov 10, 2022 at 11:40 PM Alan Modra <amodra@gmail.com> wrote:
>
> Applied.
> Fix the following:
> compressed_output.cc:86:8: error: assignment of read-only variable ‘size’
> 86 | size = ZSTD_compress(*compressed_data + header_size, size, uncompressed_data,
Sorry for my embarrassing last-minute error and thanks for fixing it :)
> diff --git a/gold/compressed_output.cc b/gold/compressed_output.cc
> index e9f12241f26..f192ddb5080 100644
> --- a/gold/compressed_output.cc
> +++ b/gold/compressed_output.cc
> @@ -81,7 +81,7 @@ zstd_compress(int header_size, const unsigned char *uncompressed_data,
> unsigned long uncompressed_size,
> unsigned char **compressed_data, unsigned long *compressed_size)
> {
> - const size_t size = ZSTD_compressBound(uncompressed_size);
> + size_t size = ZSTD_compressBound(uncompressed_size);
> *compressed_data = new unsigned char[size + header_size];
> size = ZSTD_compress(*compressed_data + header_size, size, uncompressed_data,
> uncompressed_size, ZSTD_CLEVEL_DEFAULT);
>
> --
> Alan Modra
> Australia Development Lab, IBM
@@ -1,4 +1,6 @@
* gold and dwp now support zstd compressed debug sections.
+* The new option --compress-debug-sections=zstd compresses debug sections with
+ zstd.
Changes in 1.16:
@@ -75,6 +75,26 @@ zlib_compress(int header_size,
}
}
+#if HAVE_ZSTD
+static bool
+zstd_compress(int header_size, const unsigned char *uncompressed_data,
+ unsigned long uncompressed_size,
+ unsigned char **compressed_data, unsigned long *compressed_size)
+{
+ size_t size = ZSTD_compressBound(uncompressed_size);
+ *compressed_data = new unsigned char[size + header_size];
+ size = ZSTD_compress(*compressed_data + header_size, size, uncompressed_data,
+ uncompressed_size, ZSTD_CLEVEL_DEFAULT);
+ if (ZSTD_isError(size))
+ {
+ delete[] *compressed_data;
+ return false;
+ }
+ *compressed_size = header_size + size;
+ return true;
+}
+#endif
+
// Decompress COMPRESSED_DATA of size COMPRESSED_SIZE, into a buffer
// UNCOMPRESSED_DATA of size UNCOMPRESSED_SIZE. Returns TRUE if it
// decompressed successfully, false if it failed. The buffer, of
@@ -226,15 +246,19 @@ Output_compressed_section::set_final_data_size()
this->write_to_postprocessing_buffer();
bool success = false;
- enum { none, gnu_zlib, gabi_zlib } compress;
+ enum { none, gnu_zlib, gabi_zlib, zstd } compress;
int compression_header_size = 12;
const int size = parameters->target().get_size();
if (strcmp(this->options_->compress_debug_sections(), "zlib-gnu") == 0)
compress = gnu_zlib;
- else if (strcmp(this->options_->compress_debug_sections(), "zlib-gabi") == 0
- || strcmp(this->options_->compress_debug_sections(), "zlib") == 0)
+ else if (strcmp(this->options_->compress_debug_sections(), "none") == 0)
+ compress = none;
+ else
{
- compress = gabi_zlib;
+ if (strcmp(this->options_->compress_debug_sections(), "zstd") == 0)
+ compress = zstd;
+ else
+ compress = gabi_zlib;
if (size == 32)
compression_header_size = elfcpp::Elf_sizes<32>::chdr_size;
else if (size == 64)
@@ -242,34 +266,41 @@ Output_compressed_section::set_final_data_size()
else
gold_unreachable();
}
- else
- compress = none;
- if (compress != none)
+ if (compress == gnu_zlib || compress == gabi_zlib)
success = zlib_compress(compression_header_size, uncompressed_data,
uncompressed_size, &this->data_,
&compressed_size);
+#if HAVE_ZSTD
+ else if (compress == zstd)
+ success = zstd_compress(compression_header_size, uncompressed_data,
+ uncompressed_size, &this->data_,
+ &compressed_size);
+#endif
if (success)
{
elfcpp::Elf_Xword flags = this->flags();
- if (compress == gabi_zlib)
+ if (compress == gabi_zlib || compress == zstd)
{
// Set the SHF_COMPRESSED bit.
flags |= elfcpp::SHF_COMPRESSED;
const bool is_big_endian = parameters->target().is_big_endian();
- uint64_t addralign = this->addralign();
+ const unsigned int ch_type = compress == zstd
+ ? elfcpp::ELFCOMPRESS_ZSTD
+ : elfcpp::ELFCOMPRESS_ZLIB;
+ uint64_t addralign = this->addralign ();
if (size == 32)
{
if (is_big_endian)
{
elfcpp::Chdr_write<32, true> chdr(this->data_);
- chdr.put_ch_type(elfcpp::ELFCOMPRESS_ZLIB);
+ chdr.put_ch_type(ch_type);
chdr.put_ch_size(uncompressed_size);
chdr.put_ch_addralign(addralign);
}
else
{
elfcpp::Chdr_write<32, false> chdr(this->data_);
- chdr.put_ch_type(elfcpp::ELFCOMPRESS_ZLIB);
+ chdr.put_ch_type(ch_type);
chdr.put_ch_size(uncompressed_size);
chdr.put_ch_addralign(addralign);
}
@@ -279,7 +310,7 @@ Output_compressed_section::set_final_data_size()
if (is_big_endian)
{
elfcpp::Chdr_write<64, true> chdr(this->data_);
- chdr.put_ch_type(elfcpp::ELFCOMPRESS_ZLIB);
+ chdr.put_ch_type(ch_type);
chdr.put_ch_size(uncompressed_size);
chdr.put_ch_addralign(addralign);
// Clear the reserved field.
@@ -288,7 +319,7 @@ Output_compressed_section::set_final_data_size()
else
{
elfcpp::Chdr_write<64, false> chdr(this->data_);
- chdr.put_ch_type(elfcpp::ELFCOMPRESS_ZLIB);
+ chdr.put_ch_type(ch_type);
chdr.put_ch_size(uncompressed_size);
chdr.put_ch_addralign(addralign);
// Clear the reserved field.
@@ -317,7 +348,7 @@ Output_compressed_section::set_final_data_size()
gold_assert(this->data_ == NULL);
this->set_data_size(uncompressed_size);
}
-}
+ }
// Write out a compressed section. If we couldn't compress, we just
// write it out as normal, uncompressed data.
@@ -1433,6 +1433,15 @@ General_options::finalize()
}
}
+#ifndef HAVE_ZSTD
+ if (strcmp(this->compress_debug_sections(), "zstd") == 0)
+ {
+ gold_error(_("--compress-debug-sections=zstd: gold is not built with "
+ "zstd support"));
+ this->set_compress_debug_sections("none");
+ }
+#endif
+
// --rosegment-gap implies --rosegment.
if (this->user_set_rosegment_gap())
this->set_rosegment(true);
@@ -770,8 +770,8 @@ class General_options
DEFINE_enum(compress_debug_sections, options::TWO_DASHES, '\0', "none",
N_("Compress .debug_* sections in the output file"),
- ("[none,zlib,zlib-gnu,zlib-gabi]"), false,
- {"none", "zlib", "zlib-gnu", "zlib-gabi"});
+ ("[none,zlib,zlib-gnu,zlib-gabi,zstd]"), false,
+ {"none", "zlib", "zlib-gnu", "zlib-gabi", "zstd"});
DEFINE_bool(copy_dt_needed_entries, options::TWO_DASHES, '\0', false,
N_("Not supported"),
@@ -1766,6 +1766,11 @@ flagstest_compress_debug_sections_gabi.cmp: flagstest_compress_debug_sections_ga
flagstest_compress_debug_sections_none.stdout > $@.tmp
mv -f $@.tmp $@
+check_PROGRAMS += flagstest_compress_debug_sections_zstd
+flagstest_compress_debug_sections_zstd: flagstest_debug.o gcctestdir/ld
+ $(CXXLINK) -o $@ $< -Wl,--compress-debug-sections=zstd
+ test -s $@
+
# The specialfile output has a tricky case when we also compress debug
# sections, because it requires output-file resizing.
check_PROGRAMS += flagstest_o_specialfile_and_compress_debug_sections
@@ -401,6 +401,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_compress_debug_sections_and_build_id_tree \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_compress_debug_sections_gnu \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_compress_debug_sections_gabi \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_compress_debug_sections_zstd \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_o_specialfile_and_compress_debug_sections \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_o_ttext_1 ver_test \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_2 ver_test_6 ver_test_8 \
@@ -1272,6 +1273,7 @@ libgoldtest_a_OBJECTS = $(am_libgoldtest_a_OBJECTS)
@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_compress_debug_sections_and_build_id_tree$(EXEEXT) \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_compress_debug_sections_gnu$(EXEEXT) \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_compress_debug_sections_gabi$(EXEEXT) \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_compress_debug_sections_zstd$(EXEEXT) \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_o_specialfile_and_compress_debug_sections$(EXEEXT) \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_o_ttext_1$(EXEEXT) \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test$(EXEEXT) \
@@ -1560,6 +1562,11 @@ flagstest_compress_debug_sections_none_SOURCES = \
flagstest_compress_debug_sections_none_OBJECTS = \
flagstest_compress_debug_sections_none.$(OBJEXT)
flagstest_compress_debug_sections_none_LDADD = $(LDADD)
+flagstest_compress_debug_sections_zstd_SOURCES = \
+ flagstest_compress_debug_sections_zstd.c
+flagstest_compress_debug_sections_zstd_OBJECTS = \
+ flagstest_compress_debug_sections_zstd.$(OBJEXT)
+flagstest_compress_debug_sections_zstd_LDADD = $(LDADD)
flagstest_o_specialfile_SOURCES = flagstest_o_specialfile.c
flagstest_o_specialfile_OBJECTS = flagstest_o_specialfile.$(OBJEXT)
flagstest_o_specialfile_LDADD = $(LDADD)
@@ -2306,6 +2313,7 @@ SOURCES = $(libgoldtest_a_SOURCES) $(aarch64_pr23870_SOURCES) \
flagstest_compress_debug_sections_gabi.c \
flagstest_compress_debug_sections_gnu.c \
flagstest_compress_debug_sections_none.c \
+ flagstest_compress_debug_sections_zstd.c \
flagstest_o_specialfile.c \
flagstest_o_specialfile_and_compress_debug_sections.c \
flagstest_o_ttext_1.c icf_virtual_function_folding_test.c \
@@ -3686,6 +3694,14 @@ exclude_libs_test$(EXEEXT): $(exclude_libs_test_OBJECTS) $(exclude_libs_test_DEP
@NATIVE_LINKER_FALSE@ @rm -f flagstest_compress_debug_sections_none$(EXEEXT)
@NATIVE_LINKER_FALSE@ $(AM_V_CCLD)$(LINK) $(flagstest_compress_debug_sections_none_OBJECTS) $(flagstest_compress_debug_sections_none_LDADD) $(LIBS)
+@GCC_FALSE@flagstest_compress_debug_sections_zstd$(EXEEXT): $(flagstest_compress_debug_sections_zstd_OBJECTS) $(flagstest_compress_debug_sections_zstd_DEPENDENCIES) $(EXTRA_flagstest_compress_debug_sections_zstd_DEPENDENCIES)
+@GCC_FALSE@ @rm -f flagstest_compress_debug_sections_zstd$(EXEEXT)
+@GCC_FALSE@ $(AM_V_CCLD)$(LINK) $(flagstest_compress_debug_sections_zstd_OBJECTS) $(flagstest_compress_debug_sections_zstd_LDADD) $(LIBS)
+
+@NATIVE_LINKER_FALSE@flagstest_compress_debug_sections_zstd$(EXEEXT): $(flagstest_compress_debug_sections_zstd_OBJECTS) $(flagstest_compress_debug_sections_zstd_DEPENDENCIES) $(EXTRA_flagstest_compress_debug_sections_zstd_DEPENDENCIES)
+@NATIVE_LINKER_FALSE@ @rm -f flagstest_compress_debug_sections_zstd$(EXEEXT)
+@NATIVE_LINKER_FALSE@ $(AM_V_CCLD)$(LINK) $(flagstest_compress_debug_sections_zstd_OBJECTS) $(flagstest_compress_debug_sections_zstd_LDADD) $(LIBS)
+
@GCC_FALSE@flagstest_o_specialfile$(EXEEXT): $(flagstest_o_specialfile_OBJECTS) $(flagstest_o_specialfile_DEPENDENCIES) $(EXTRA_flagstest_o_specialfile_DEPENDENCIES)
@GCC_FALSE@ @rm -f flagstest_o_specialfile$(EXEEXT)
@GCC_FALSE@ $(AM_V_CCLD)$(LINK) $(flagstest_o_specialfile_OBJECTS) $(flagstest_o_specialfile_LDADD) $(LIBS)
@@ -4802,6 +4818,7 @@ distclean-compile:
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/flagstest_compress_debug_sections_gabi.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/flagstest_compress_debug_sections_gnu.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/flagstest_compress_debug_sections_none.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/flagstest_compress_debug_sections_zstd.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/flagstest_o_specialfile.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/flagstest_o_specialfile_and_compress_debug_sections.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/flagstest_o_ttext_1.Po@am__quote@
@@ -7123,6 +7140,13 @@ flagstest_compress_debug_sections_gabi.log: flagstest_compress_debug_sections_ga
--log-file $$b.log --trs-file $$b.trs \
$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
"$$tst" $(AM_TESTS_FD_REDIRECT)
+flagstest_compress_debug_sections_zstd.log: flagstest_compress_debug_sections_zstd$(EXEEXT)
+ @p='flagstest_compress_debug_sections_zstd$(EXEEXT)'; \
+ b='flagstest_compress_debug_sections_zstd'; \
+ $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \
+ --log-file $$b.log --trs-file $$b.trs \
+ $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
+ "$$tst" $(AM_TESTS_FD_REDIRECT)
flagstest_o_specialfile_and_compress_debug_sections.log: flagstest_o_specialfile_and_compress_debug_sections$(EXEEXT)
@p='flagstest_o_specialfile_and_compress_debug_sections$(EXEEXT)'; \
b='flagstest_o_specialfile_and_compress_debug_sections'; \
@@ -8799,6 +8823,9 @@ uninstall-am:
@GCC_TRUE@@NATIVE_LINKER_TRUE@ cmp flagstest_compress_debug_sections_gabi.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ flagstest_compress_debug_sections_none.stdout > $@.tmp
@GCC_TRUE@@NATIVE_LINKER_TRUE@ mv -f $@.tmp $@
+@GCC_TRUE@@NATIVE_LINKER_TRUE@flagstest_compress_debug_sections_zstd: flagstest_debug.o gcctestdir/ld
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -o $@ $< -Wl,--compress-debug-sections=zstd
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ test -s $@
@GCC_TRUE@@NATIVE_LINKER_TRUE@flagstest_o_specialfile_and_compress_debug_sections: flagstest_debug.o \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ gcctestdir/ld
@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -o /dev/stdout $< -Wl,--compress-debug-sections=zlib 2>&1 | cat > $@