[PATCHv4][PING][BZ #17833] _dl_close_worker() does not release inconsistent objects.

[H.J. Lu]

On Wed, May 27, 2015 at 6:12 AM, Yury Gribov <y.gribov@samsung.com> wrote:
> On 05/27/2015 04:04 PM, H.J. Lu wrote:
>>
>> On Wed, May 27, 2015 at 5:17 AM, Pavel Kopyl <p.kopyl@samsung.com> wrote:
>>>
>>>
>>>
>>> On 05/13/2015 02:40 PM, Pavel Kopyl wrote:
>>>>
>>>>
>>>>
>>>>
>>>> On 04/24/2015 02:47 PM, H.J. Lu wrote:
>>>>>
>>>>>
>>>>> On Fri, Apr 24, 2015 at 3:33 AM, Pavel Kopyl <p.kopyl@samsung.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 03/03/2015 12:13 PM, Pavel Kopyl wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 03/01/2015 10:17 PM, Mike Frysinger wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 27 Feb 2015 16:32, Pavel Kopyl wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/elf/tst-unique5lib.cc
>>>>>>>>> @@ -0,0 +1,13 @@
>>>>>>>>> +
>>>>>>>>
>>>>>>>>
>>>>>>>> i know existing tests are bad examples, but lets try and start
>>>>>>>> fixing
>>>>>>>> that.
>>>>>>>> namely, there should be a header here giving a quick overview of
>>>>>>>> what
>>>>>>>> it
>>>>>>>> is
>>>>>>>> exactly you're testing for, and a BZ reference.
>>>>>>>>
>>>>>>>>> +extern int not_exist ();
>>>>>>>>> +
>>>>>>>>> +inline int make_unique ()
>>>>>>>>> +{
>>>>>>>>> +  static int unique;
>>>>>>>>> +  return ++unique;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +int foo ()
>>>>>>>>> +{
>>>>>>>>> +  return make_unique () + not_exist ();
>>>>>>>>> +}
>>>>>>>>
>>>>>>>>
>>>>>>>> i don't know if this is just copy & pasting, but prototypes that do
>>>>>>>> not
>>>>>>>> intend
>>>>>>>> to take args should always be (void).
>>>>>>>> -mike
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Thanks for review, I fixed that in patch v3.
>>>>>>
>>>>>>
>>>>>> Ping.
>>>>>>
>>>>> Some comments:
>>>>>
>>>>> 1. The bug report is against STB_GNU_UNIQUE.  But I don't see
>>>>> STB_GNU_UNIQUE in
>>>>> testcase.  I can't tell if the original STB_GNU_UNIQUE bug is fixed.
>>>>> 2. Your testcase ignores dlopen error.  Why should it work at all?
>>>>> 3. Your testcase doesn't use test-skeleton.c.
>>>>>
>>>>
>>>> Thanks for review!
>>>>
>>>> 1. I added some comments in testcase to be more clear.
>>>>
>>>> 2. Actually the call below is expected to return NULL because of
>>>> undefined
>>>> symbol in the library.
>>>>
>>>> dlopen ("$ORIGIN/tst-unique5lib.so", RTLD_NOW);
>>>>
>>>> It's a goal of this testcase: I need some library with STB_GNU_UNIQUE
>>>> symbols that returns error when trying to load it.
>>>> In new patch version I especially point out to this fact by checking
>>>> return value.
>>>>
>>>> 3. It seems for me that template of the test-skeleton.c is not
>>>> appropriate
>>>> because this testcase has two related binary - library and executable.
>>>>
>>>>
>>>> -Pavel
>>>
>>>
>>> Ping.
>>>
>>
>> Since you changed DF_1_NODELETE, how did you test it?  Does your test
>> fail without the DF_1_NODELETE change?
>
>
> AFAIU DF_1_NODELETE change is necessary. Otherwise unique symbols force
> library to remain loaded in half-initialized state (with missing
> dependencies). This usecase is demonstrated by the testcase in patch.
>

But unique symbols != DF_1_NODELETE:

[hjl@gnu-tools-1 elf]$ readelf -sW tst-unique1mod1.so | grep UNIQUE
     9: 0000000000201020     4 OBJECT  UNIQUE DEFAULT   22 var
    53: 0000000000201020     4 OBJECT  UNIQUE DEFAULT   22 var
[hjl@gnu-tools-1 elf]$ readelf -d tst-unique1mod1.so

Dynamic section at offset 0xe40 contains 21 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000000c (INIT)               0x560
 0x000000000000000d (FINI)               0x6a0
 0x0000000000000019 (INIT_ARRAY)         0x200e28
 0x000000000000001b (INIT_ARRAYSZ)       8 (bytes)
 0x000000000000001a (FINI_ARRAY)         0x200e30
 0x000000000000001c (FINI_ARRAYSZ)       8 (bytes)
 0x0000000000000004 (HASH)               0x6f8
 0x000000006ffffef5 (GNU_HASH)           0x210
 0x0000000000000005 (STRTAB)             0x3a0
 0x0000000000000006 (SYMTAB)             0x250
 0x000000000000000a (STRSZ)              169 (bytes)
 0x000000000000000b (SYMENT)             24 (bytes)
 0x0000000000000007 (RELA)               0x488
 0x0000000000000008 (RELASZ)             216 (bytes)
 0x0000000000000009 (RELAENT)            24 (bytes)
 0x000000006ffffffe (VERNEED)            0x468
 0x000000006fffffff (VERNEEDNUM)         1
 0x000000006ffffff0 (VERSYM)             0x44a
 0x000000006ffffff9 (RELACOUNT)          3
 0x0000000000000000 (NULL)               0x0
[hjl@gnu-tools-1 elf]$

Does your test have the DF_1_NODELETE bit set?


-- 
H.J.