Hi Andrei, On Mon, 2021-06-28 at 18:26 -0700, Andrei Homescu wrote: > The mkl_memory_patched.o object inside the libmkl_core.a library from > the Intel Math Kernel Library version 2018.2.199 has this section > with an alignment of 4096 and offset of 0xb68: > [ 2] .data PROGBITS 0000000000000000 000b68 011000 00 WA 0 0 4096 > > Reading this file with libelf and trying to write it back to disk triggers > the following sequence of events: > 1) code in elf_getdata.c clamps d_align for this section's data buffer > to the section's offset > 2) code in elf32_updatenull.c checks if the alignment is a power of two > and incorrectly returns an error > > This commit fixes this corner case by increasing the alignment to the > next power of two after the clamping, so the check passes. > > A test that reproduces this bug using strip is also included.
Thanks for this perfect patch, including and excellent explanation of the issue and a testcase. I have only a minor suggestion: > --- /dev/null > +++ b/tests/run-strip-largealign.sh > @@ -0,0 +1,34 @@ > +#! /bin/sh > +# Copyright (C) 2011 TODO. Should we make this Copyright (C) 2021 Andrei Homescu <a...@immunant.com> or Copyright (C) 2021 Immunant, Inc. > +# This file is part of elfutils. > +# > +# This file is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published > by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# elfutils is distributed in the hope that it will be useful, but > +# WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see < > http://www.gnu.org/licenses/>. > +# > + > +. $srcdir/test-subr.sh > + > +# = testfile-largealign.S = > +# section .data > +# align 4096 > +# dd 0x12345678 > +# > +# nasm -f elf64 -o testfile-largealign.o testfile-largealign.S > + > +infile=testfile-largealign.o > +outfile=$infile.stripped > + > +testfiles $infile > +tempfiles $outfile > + > +testrun ${abs_top_builddir}/src/strip -o $outfile $infile The testcase already fails before the patch and succeeds after, but it would be nice to double check the output with elflint just in case. Shall we add: testrun ${abs_top_builddir}/src/elflint --gnu $outfile Thanks, Mark
