This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCHv4][PING][BZ #17833] _dl_close_worker() does not release inconsistent objects.
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Yury Gribov <y dot gribov at samsung dot com>
- Cc: Pavel Kopyl <p dot kopyl at samsung dot com>, GNU C Library <libc-alpha at sourceware dot org>, "Carlos O'Donell" <carlos at redhat dot com>, Roland McGrath <roland at hack dot frob dot com>, Viacheslav Garbuzov <v dot garbuzov at samsung dot com>
- Date: Wed, 27 May 2015 06:23:41 -0700
- Subject: Re: [PATCHv4][PING][BZ #17833] _dl_close_worker() does not release inconsistent objects.
- Authentication-results: sourceware.org; auth=none
- References: <54BD4F65 dot 2090108 at samsung dot com> <54BEF851 dot 70902 at redhat dot com> <54DBC3CB dot 5080703 at samsung dot com> <54F071DB dot 9040106 at samsung dot com> <20150301191710 dot GB19363 at vapier> <54F57B52 dot 6080202 at samsung dot com> <553A1BEE dot 6070705 at samsung dot com> <CAMe9rOqiyuyNAtJDZGbfs+0kk0j16-Vowp5f0z_x2zfsd76fMQ at mail dot gmail dot com> <5553382F dot 3020906 at samsung dot com> <5565B5E5 dot 7060101 at samsung dot com> <CAMe9rOr8yDpnHtRFbL3M56Sx5FWX-FVqEstnwsgtW6H+khvziQ at mail dot gmail dot com> <5565C2A8 dot 60306 at samsung dot com>
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.