NEED REVIEWER RFR(S): JDK-8038392 Generating prelink cache breaks JAVA 'jinfo' utility normal behavior

[serguei.spitsyn at oracle.com]

Dmitry,

Thank you for the explanations!
The fix looks good pending the simplification below.
I'm Ok with your testing approach.

Thanks,
Serguei

On 6/12/14 4:23 AM, Dmitry Samersoff wrote:
> Serguei,
>
> Thank you for the review, please see below.
>
> On 2014-06-12 14:49, serguei.spitsyn at oracle.com wrote:
>> Dmitry,
>>
>> Thank you for the details!
>>
>> I have a couple of comments so far.
>>
>> +    if (word[5][0] == '[') {
>> +        // not a shared library entry. ignore.
>> +      if (strncmp(word[5],"[stack",6) == 0) {
>> +        continue;
>> +      }
>> +      if (strncmp(word[5],"[heap]",6) == 0) {
>> +        continue;
>> +      }
>> +
>> +      // SA don't handle VDSO
>> +      if (strncmp(word[5],"[vdso]",6) == 0) {
>> +        continue;
>> +      }
>> +      if (strncmp(word[5],"[vsyscall]",6) == 0) {
>> +        continue;
>> +      }
>> +    }
>>
>> I'd suggest to simplify the fragment above to something like thus:
>>
>> +    // SA does not handle the lines with patterns:
>> +    //   "[stack]", "[heap]", "[vdso]", "[vsyscall]", etc.
>> +    if (word[5][0] == '[') {
>> +      continue; // not a shared library entry, ignore
>> +    }
> I'll change it.
>
>> This fragment does not look correct:
>>
>> +    if (nwords > 6) {
>> +      // prelink altered mapfile when the program is running.
>> +      // Entries like one below have to be skipped
>> +      //  /lib64/libc-2.15.so (deleted)
>> +      // SO name in entries like one below have to be stripped.
>> +      //  /lib64/libpthread-2.15.so.#prelink#.EECVts
>> +      char *s = strstr(word[5],".#prelink#");
>> +      if (s == NULL) {
>> +        // No prelink keyword. skip deleted library
>> +        print_debug("skip shared object %s deleted by prelink\n", word[5]);
>> +        continue;
>> +      }
>> +
>> +      // Fall through
>> +      print_debug("rectifing shared object name %s changed by prelink\n", word[5]);
>> +      *s = 0;
>> +    }
>
>    We always have word "(deleted)" at the end (see fragment of map below).
>
>   But if the word "(deleted)" relates to original DSO we should skip the
> entry but if it relates to tmp file created by prelink we should accept
> this mapping - it's a new place for original library.
>
>
> Actually map entry looks like:
>
> 7f490b190000-7f490b1a8000 r-xp 00000000 08:01 350
>   /lib64/libpthread-2.15.so.#prelink#.EECVts (deleted)
>
> 7f490b1a8000-7f490b3a7000 ---p 00018000 08:01 350
>   /lib64/libpthread-2.15.so.#prelink#.EECVts (deleted)
>
> 7f490b3a7000-7f490b3a8000 r--p 00017000 08:01 350
>   /lib64/libpthread-2.15.so.#prelink#.EECVts (deleted)
>
> 7f490b3a8000-7f490b3a9000 rw-p 00018000 08:01
> 350                        /lib64/libpthread-2.15.so.#prelink#.EECVts
> (deleted)
>
> 7f490b3a9000-7f490b3ad000 rw-p 00000000 00:00 0
> 7f490b3ad000-7f490b3cf000 r-xp 00000000 08:01
>
> 48013                      /lib64/ld-2.15.so (deleted)
>
>
>> The line with the pattern "(deleted)" has 7 words, but the line like:
>>      <addr>  /lib64/libpthread-2.15.so.#prelink#.EECVts
>>
>> has only 6 words, and so, your code will not process it properly.
>>
>> I'd expect something like this:
>>
>> +    if (nwords > 6) {
>> +      // prelink altered mapfile when the program is running.
>> +      // Entries like one below have to be skipped:
>> +      //  /lib64/libc-2.15.so (deleted)
>> +       print_debug("skip shared object %s deleted by prelink\n", word[5]);
>> +       continue;
>> +    } else {
>> +      char *s = strstr(word[5],".#prelink#");
>> +      if (s != NULL) {
>> +        // There is the prelink keyword
>> +        print_debug("rectifying shared object name %s changed by prelink\n", word[5]);
>> +        *s = 0;
>> +      }
>> +    }
>>
>>
>> Is it hard to add a unit test for this issue?
> It's not possible to test prelink during nightly as it affects entire OS
> (actually it's a bad idea to run prelink while java program is running).
>
>
> -Dmitry
>
>
>
>> Thanks,
>> Serguei
>>
>> On 6/12/14 3:10 AM, Dmitry Samersoff wrote:
>>> Serguei,
>>>
>>> *The problem:*
>>>
>>> If someone run prelink utility while Java program is running DSO's
>>> mappings in it's /proc/<pid>/maps become messy.
>>>
>>> After prelink it contains entry like
>>>
>>> <addr> /lib64/libc-2.15.so (deleted)
>>> <addr>  /lib64/libpthread-2.15.so.#prelink#.EECVts
>>>
>>> instead of normal
>>>
>>> <addr> /lib64/libc-2.15.so
>>>
>>> Here is the letter from Carlos, describing the problem in details:
>>> https://bugs.openjdk.java.net/browse/JDK-8038392
>>>
>>> *The fix:*
>>>
>>> The fix allows SA to handle situation above.
>>>
>>> Also I fix longstanding issue - maps file parser in SA doesn't skip
>>> [stack*, [heap] etc entry and attempts to open it as a DSO
>>>
>>> *Testing:*
>>>
>>> I tested it manually with a different of prelink options (no prelink,
>>> prelink all, prelink some libraries only)
>>>
>>> -Dmitry
>>>
>>>
>>> On 2014-06-12 13:50, serguei.spitsyn at oracle.com wrote:
>>>> Hi Dmitry,
>>>>
>>>> There is no description of the fix.
>>>> Could you, please, provide one?
>>>> What did you basically wanted to achieve?
>>>> Also, how did the fix been tested?
>>>> It seems, a unit test would be nice to have.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>> On 6/5/14 12:08 AM, Dmitry Samersoff wrote:
>>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8038392/webrev.02/
>>>>>
>>>>> On 2014-05-20 17:47, Dmitry Samersoff wrote:
>>>>>> On 2014-04-07 16:56, Dmitry Samersoff wrote:
>>>>>>> Hi Everybody,
>>>>>>>
>>>>>>> Please review the patch
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8038392/webrev.02/
>>>>>>>
>>>>>>> it's based on the patch contributed by Carlos Santos (see CR) and all
>>>>>>> credentials belong to him.
>>>>>>>
>>>>>>> -Dmitry
>>>>>>>
>