Re: [PATCH] BZ#20292: Remove redundant statement in _dl_addr_inside_object (refactor and unit test).

[Carlos O'Donell <carlos at redhat dot com>]

On 07/04/2016 01:12 PM, Carlos O'Donell wrote:
> The following patch refactors two copies of _dl_addr_inside_object
> into dl-addr-obj.c, and removes the redundant comparison detected by
> static analysis. The remaining statement:
> "reladdr - l->l_phdr[n].p_vaddr < l->l_phdr[n].p_memsz"
> is always sufficient to detect if the unsigned address is inside the
> load segment of the link map.
> 
> The copy in dl-addr-obj.c is then subjected to a dynamically linked
> PIE test where the test driver is linked directly with the built *.os
> version of the file and the operations exercised.
> 
> Note that the refactoring creates an additional object file
> elf/rtld-dl-addr-obj.os needed by the dynamic linker, and an
> elf/dl-addr-obj.os needed by libc. We test the latter for conformance
> with the expected behaviours.
> 
> The goal behind the unit test is _not_ to find bugs, but to exercise
> the expected behavior of the function. Integration tests are where we
> should focus our bug finding. Instead the tests here serve to make it
> precisely clear what the functions should or should not do. I plan to
> follow the same tactic with the ELF sorting functions and dependency
> sorting functions (not for this release cycle though).
> 
> I'll check this fix in before the end of the week if nobody objects.
> 
> 2016-07-04  Carlos O'Donell  <carlos@redhat.com>
> 
> 	[BZ #20292]
> 	* elf/Makefile (routines): Add dl-addr-obj.
> 	[ifeq (yesyes,$(have-fpie)$(build-shared))] (tests): Add
> 	tst-_dl_addr_inside_object.
> 	[ifeq (yesyes,$(have-fpie)$(build-shared))] (tests-pie): Likewise.
> 	[ifeq (yesyes,$(have-fpie)$(build-shared))]
> 	($(objpfx)tst-_dl_addr_inside_object): Add $(objpfx)dl-addr-obj.os.
> 	[ifeq (yesyes,$(have-fpie)$(build-shared))]
> 	(CFLAGS-tst-_dl_addr_inside_object.c): Add $(PIE-ccflag).
> 	* elf/dl-addr.c: Remove _dl_addr_inside_object function. 
> 	* elf/dl-open.c: Likewise.
> 	* elf/dl-addr-obj.c: New file.
> 	* elf/tst-_dl_addr_inside_object.c: New file.

Checked in v2 with a few more comments and one extra unit test.

-- 
Cheers,
Carlos.