RFR: 8220095: Assertion failure when symlink (with different name) is used for lib/modules file
Ioi Lam
ioi.lam at oracle.com
Fri Mar 15 17:57:40 UTC 2019
On 3/15/19 10:35 AM, Jiangli Zhou wrote:
>
> On Thu, Mar 14, 2019 at 9:52 PM Ioi Lam <ioi.lam at oracle.com
> <mailto:ioi.lam at oracle.com>> wrote:
>
>
> On 3/14/19 7:40 PM, Ioi Lam wrote:
> > I think the code is actually very simple. It's used in only 2
> places:
> >
> > 1. For class loading log, print source=jrt:/java.base, etc, for
> those
> > classes loaded from the modules file by the boot class loader.
> >
> > 2. Decide that classpath_index = 0 during CDS dump, when the
> class is
> > loaded from the modules file.
> >
> > It's never intended to check classes that are not loaded by the
> boot
> > loader.
>
>
> The place in ClassLoader::record_result that performs the
> is_modules_image check does not take the notion of class loader. The
> underlying logic would become different with your change. And that
> leads to my other question that's related to your comments below about
> the source being 'jrt:'. In which cases a class have the modules path
> string as the source? And in which cases a class has the 'jrt:'
> protocol as the source? Can you please check with Lois and Harold on
> these different cases?
>
> The risk tolerance for backports is lower. A conserve fix without
> altering the underlying logic too much probably is preferred in that
> case.
OK let's continue discussion in the bug report.
The trouble with your modified is_modules_file() is it *does* change the
behavior. If lib/modules is a symlink, then calls to
is_modules_image("<java_home>/lib/modules") will now return false, where
it used to return true. So we can't just accept this change without
analyzing all the cases that is_modules_image() is called, including the
case you pointed above.
Thanks
- Ioi
> If a two step process addresses it better then it should be done.
> However, it doesn't seem to worth continuing to spam the others in the
> mailing list to resolve our different opinions. So I will reassign the
> bug to you and we can resolve this issue efficiently (the bug report
> for this issue is a more suitable place for all these technical
> discussions, it also helps tracking).
>
> Thanks,
> Jiangli
>
> >
> >
> > On 3/14/19 6:00 PM, Jiangli Zhou wrote:
> >>
> >> On Thu, Mar 14, 2019 at 11:01 AM Ioi Lam <ioi.lam at oracle.com
> <mailto:ioi.lam at oracle.com>
> >> <mailto:ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>>> wrote:
> >>
> >> Look at jvm_define_class_common in jvm.cpp. ClassFileStream
> can be
> >> stack allocated, without using the 'new' operator.
> >>
> >>
> http://closedjdk.us.oracle.com/jdk/closed-sandbox/open/file/5d095f4b2164/src/hotspot/share/prims/jvm.cpp#l944
> >>
> >>
> >> This could be solved by looking up the existing ClassPathEntry
> (and
> >> creating a new one if no existing one) from a hashtable.
> However, I
> >> realized the issue was it would still be relying on the
> comparison of
> >> the canonical path string (the 'name' argument) with the
> >> ClassPathEntry->name(). I also took a quick look (not a review) of
> >> your change. It appears it always assumes the class is not from
> the
> >> modules image in the jvm_define_class_common call path, since
> >> 'from_modules_image' defaults to false.
> >
> > If the name from_modules_image is not appropriate, how about
> > from_modules_image_by_boot_loader.
> >
> >> Application module classes can come from the runtime modules
> image,
> >> and the assumption can be problematic.
> >>
> I forgot to mention -- if the class come from the Java code via
> jvm_define_class_common(), the source is already set to
> "jrt://xxx.yyy".
> In this case, the source will never be the JAVA_HOME/lib/modules.
>
> You can experiment with
>
> javac -J-verbose -help
>
> Thanks
>
> - Ioi
>
>
>
> >> Pardon my ignorance, however I think this discussion does not
> need to
> >> be part of the code review of this bug fix and should the a
> follow up
> >> action. It deserves more investigations to cover all cases,
> including
> >> the edge cases that you have and those edge cases are not the ones
> >> this fix is trying to address.
> >>
> > The whole point of have a public review is to find a correct fix.
> >
> > If we can fix it correctly in 1 step with a simple fix, why
> should we
> > do it in 2 steps?
> >
> > We already have several people looking at this bug, and we have all
> > been confused by is_modules_image(). If we check in a partial
> fix now
> > by keeping the is_modules_image() API, we will all be confused
> again
> > when we fix it properly again.
> >
> > Fixing stuff in the JDK is not a light process, so 1 step is better
> > than 2.
> >
> > Thanks
> >
> > - Ioi
> >
> >
> >> Thanks,
> >> Jiangli
> >>
> >>
> >>
> >>
> >> Thanks
> >> - Ioi
> >>
> >> On 3/14/19 10:44 AM, Jiangli Zhou wrote:
> >>>
> >>> On Thu, Mar 14, 2019 at 10:26 AM Ioi Lam
> <ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>
> >>> <mailto:ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>>>
> wrote:
> >>>
> >>> What problem are you trying to solve by putting the
> >>> ClassPathEntry into ClassFileStream?
> >>>
> >>> ClassFileStream can be created without a ClassPathEntry.
> >>> E.g., when the classfile data comes from Java code.
> >>>
> >>>
> >>> Grep tells me that all the places creating a
> ClassFileStream with
> >>> a path string have known ClassPathEntry. Maybe I'm missing
> >>> something here? With ClassFileStream::_source becoming a
> >>> ClassPathEntry*, then all the is_modules_image(const
> char*) call
> >>> sites can use the ClassPathEntry::is_modules_image() API
> without
> >>> the path string comparison.
> >>>
> >>> Thanks,
> >>> Jiangli
> >>>
> >>>
> >>> On 3/14/19 9:54 AM, Jiangli Zhou wrote:
> >>>> I was going to propose eliminating the usage of
> >>>> is_modules_image(const char*) as well. I'm in the
> middle of
> >>>> refactoring ClassFileStream to take the 'source' as a *
> >>>> instead of a plain char*. I think that's a more elegant
> >>>> approach. However, this issue needs to be addressed
> in JDK
> >>>> 11 and 12 also. So a simpler fix should be applied
> first and
> >>>> backported to 11 & 12. The webrev.02 or webrev.03 can be
> >>>> used for the initial fix and backporting. Then we can be
> >>>> unblocked by this symlink issue.
> >>>>
> >>>> The elimination of is_modules_image(const char*) and
> >>>> refactoring work can be done with a different RFE. If you
> >>>> want to take over from there, that is ok with me.
> >>>>
> >>>> Thanks,
> >>>> Jiangli
> >>>>
> >>>> On Wed, Mar 13, 2019 at 11:46 PM Ioi Lam
> <ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>
> >>>> <mailto:ioi.lam at oracle.com
> <mailto:ioi.lam at oracle.com>>> wrote:
> >>>>
> >>>> I have a different idea ....
> >>>>
> >>>> I think we are all confused by is_modules_image(const
> >>>> char* name). It's a poorly designed interface -- it's
> >>>> not clear what it's intended for, and what its
> >>>> capabilities are (Can it handle
> case-insensitivity? Can
> >>>> it handle non-canonical names? Should it do a magic
> >>>> number check to see if the file is indeed a
> jimage file?).
> >>>>
> >>>> I think we should remove this function, otherwise it
> >>>> might be abused in the future and cause more
> problems.
> >>>>
> >>>> We should replace it with what we actually need.
> I.e.,
> >>>> check if a ClassFileStream was created from the
> jimage.
> >>>>
> >>>> I have written a simple fix and it passes all CDS
> tests,
> >>>> tier1/tier2, as well as Jiangli's new test
> >>>>
> >>>>
> http://cr.openjdk.java.net/~iklam/jdk13/8220095_remove_is_modules_image.v00/
> >>>>
> >>>> Any thoughts?
> >>>>
> >>>> Thanks
> >>>> - Ioi
> >>>>
> >>>>
> >>>>
> >>>> On 3/13/19 6:24 PM, Jiangli Zhou wrote:
> >>>>>
> >>>>>
> >>>>> On Wed, Mar 13, 2019 at 1:54 PM Ioi Lam
> >>>>> <ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>
> <mailto:ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>>> wrote:
> >>>>>
> >>>>>
> >>>>> On 3/13/2019 1:45 PM, Jiangli Zhou wrote:
> >>>>>> On Wed, Mar 13, 2019 at 12:21 PM Ioi Lam
> >>>>>> <ioi.lam at oracle.com
> <mailto:ioi.lam at oracle.com> <mailto:ioi.lam at oracle.com
> <mailto:ioi.lam at oracle.com>>>
> >>>>>> wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 3/13/2019 12:03 PM, Jiangli Zhou wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On Wed, Mar 13, 2019 at 11:53 AM Ioi Lam
> >>>>>>> <ioi.lam at oracle.com
> <mailto:ioi.lam at oracle.com>
> >>>>>>> <mailto:ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>>>
> wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 3/13/2019 11:41 AM, Jiangli
> Zhou wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Wed, Mar 13, 2019 at 11:03 AM Ioi Lam
> >>>>>>>> <ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>
> >>>>>>>> <mailto:ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>>>
> wrote:
> >>>>>>>>
> >>>>>>>> Hi Jiangli,
> >>>>>>>>
> >>>>>>>> Thanks for making the
> changes. This
> >>>>>>>> makes the code much simpler and
> >>>>>>>> fixes the original problem
> with the
> >>>>>>>> improper use of
> MODULES_IMAGE_NAME.
> >>>>>>>>
> >>>>>>>> [1] Naming and comments:
> >>>>>>>>
> >>>>>>>> modules_image_identity: how about
> >>>>>>>> modules_image_canonical_path, so we
> >>>>>>>> know what it actually is?
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> The modules_image_identity naming
> >>>>>>>> appears better as the canonical path is
> >>>>>>>> used as an identity here. It tells the
> >>>>>>>> intention of the variable.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> is_modules_image(const char*
> name)
> >>>>>>>> --> is_modules_image(const char*
> >>>>>>>> canonical_path)
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> I considered to change the name
> of the
> >>>>>>>> argument but decided against it. The
> >>>>>>>> sources of the argument can come from a
> >>>>>>>> ClassPathEntry name in some of the
> >>>>>>>> places. It creates confusion without
> >>>>>>>> also change the other existing code. A
> >>>>>>>> separate exercise can be used to clean
> >>>>>>>> up the existing code.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> class ClassPathImageEntry ... {
> >>>>>>>> + // returns canonical path of
> >>>>>>>> <JAVA_HOME>/lib/<MODULES_IMAGE_NAME>
> >>>>>>>> const char* name() const {
> return
> >>>>>>>> _name == NULL ? "" : _name; }
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> [2] 142 const char*
> >>>>>>>> ClassLoader::_modules_image_identity
> >>>>>>>> = MODULES_IMAGE_NAME;
> >>>>>>>>
> >>>>>>>> ... but your version of
> >>>>>>>> is_modules_image() assumes the
> >>>>>>>> uninitialized value is NULL.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Changed to initialize it to NULL.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> [3] For the following code:
> >>>>>>>>
> >>>>>>>> 452 static bool
> >>>>>>>> is_modules_image(const char*
> name) {
> >>>>>>>> 453 if
> >>>>>>>> (Arguments::has_jimage()) {
> >>>>>>>> 454
> assert(modules_image_identity()
> >>>>>>>> != NULL, "must be set");
> >>>>>>>> 455 return strcmp(name,
> >>>>>>>> modules_image_identity()) == 0;
> >>>>>>>> 456 }
> >>>>>>>> 457 return false;
> >>>>>>>> 458 }
> >>>>>>>>
> >>>>>>>> Instead of maintaining a copy
> of the
> >>>>>>>> jimage's name in
> >>>>>>>> _modules_image_identity, how
> about
> >>>>>>>>
> >>>>>>>> const char*
> modules_image_identity() {
> >>>>>>>> assert(get_jrt_entry() != NULL,
> >>>>>>>> "must be inited");
> >>>>>>>> return
> get_jrt_entry()->name();
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> Also, I think in most cases, the
> >>>>>>>> name is exactly the same
> pointer as
> >>>>>>>> get_jrt_entry()->name(), so line 455
> >>>>>>>> can be changed to
> >>>>>>>>
> >>>>>>>> const char* id =
> >>>>>>>> modules_image_identity();
> >>>>>>>> return name == id ||
> >>>>>>>> strcmp(name, id) == 0;
> >>>>>>>>
> >>>>>>>> Also, I think you should change
> >>>>>>>> ClassPathImageEntry::is_modules_image()
> >>>>>>>> to simply return true. Otherwise
> >>>>>>>> it's just comparing its name with
> >>>>>>>> its name .... If you do this,
> then
> >>>>>>>> you can add another assert to my
> >>>>>>>> version of
> modules_image_identity(),
> >>>>>>>> without worrying above infinite
> >>>>>>>> recursion:
> >>>>>>>>
> >>>>>>>> assert(get_jrt_entry()->is_modules_image(),
> >>>>>>>> "must be");
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> I'm hesitate to use ClassPathImageEntry
> >>>>>>>> name directly because it may not
> >>>>>>>> necessary to be full canonical path.
> >>>>>>>> Using a separate variable for modules
> >>>>>>>> identity seems to be more explicit and
> >>>>>>>> less error-prone in the future.
> >>>>>>>
> >>>>>>> But your code essentially just makes a
> >>>>>>> copy of get_jrt_entry()->name()?
> How does
> >>>>>>> that address your concern?
> >>>>>>>
> >>>>>>>
> >>>>>>> True, it makes an assumption about the
> >>>>>>> existing implementation, which might
> not be
> >>>>>>> future-proof. One solution is to change
> >>>>>>> create_class_path_entry() to give back the
> >>>>>>> canonical path explicitly.
> >>>>>>>
> >>>>>> Our problem is not strictly limited to
> >>>>>> canonical paths. All of the callers that
> >>>>>> expect a positive return from
> >>>>>> is_modules_image() get their input from
> >>>>>> get_jrt_entry()->name(). For example, see
> >>>>>> ClassLoader::load_class(). The caller wants to
> >>>>>> know "is the src, supplied in a
> >>>>>> ClassFileStream, the same as get_jrt_entry()".
> >>>>>>
> >>>>>> -> Just grep with
> 'is_modules_image[(][^)]'.
> >>>>>> There are very few callers.
> >>>>>>
> >>>>>> The problem is get_jrt_entry()->name()
> may be
> >>>>>> subject to transformation, so that it's not
> >>>>>> necessarily JAVA_HOME/lib/modules. In this
> >>>>>> particular bug, the transformation is
> pathname
> >>>>>> canonicalization. It's possible to use other
> >>>>>> transformations. E.g., change it to "?modules?".
> >>>>>>
> >>>>>> So the check we really want to do is:
> >>>>>> strcmp(name, get_jrt_entry()->name());
> >>>>>>
> >>>>>>
> >>>>>> There are mixed sources of the path strings
> when
> >>>>>> calling into is_modules_image(const char*). The
> >>>>>> ClassFileStream::_source is one and
> >>>>>> ClassPathEntry->name() is another case. The
> >>>>>> ClassFileStream::_source is the canonical path of
> >>>>>> the class file stream.
> >>>>>
> >>>>>
> >>>>> ClassLoader::load_class() {
> >>>>>
> >>>>> ...
> >>>>> stream = _jrt_entry->open_stream(file_name,
> >>>>> CHECK_NULL);
> >>>>> ...
> >>>>>
> >>>>> ClassFileStream*
> >>>>> ClassPathImageEntry::open_stream(const char* name,
> >>>>> TRAPS) {
> >>>>> ...
> >>>>> return new ClassFileStream((u1*)data,
> >>>>> (int)size,
> >>>>> _name,
> >>>>>
> >>>>> ClassFileStream::verify);
> >>>>>
> >>>>> So the ClassFileStream contains the same
> string as
> >>>>> in the _jrt_entry?
> >>>>>
> >>>>> Where do you see this "_source is the canonical
> >>>>> path of the class file stream" happening?
> >>>>>
> >>>>>
> >>>>> You are right, the ClassFileStream created for the
> >>>>> runtime image uses ClassPathImageEntry::_name as the
> >>>>> _source. So there is only a single source of the
> path
> >>>>> string, which is jrt_entry '_name'. Using that
> within
> >>>>> the check probably is ok then.
> >>>>>
> >>>>> Thanks,
> >>>>> Jiangli
> >>>>>
> >>>>>
> >>>>>
> >>>>>> If the ClassPathEntry->name() changes as you
> >>>>>> describe above (which is also the source of my
> >>>>>> hesitation), then using
> get_jrt_entry()->name() in
> >>>>>> the check would cause an issue. That's also the
> >>>>>> reason why I didn't think a simple strcmp was a
> >>>>>> good idea.
> >>>>>>
> >>>>>> If you are pushing to use
> get_jrt_entry()->name()
> >>>>>> and do a simple strcmp, then it needs
> to guarantee
> >>>>>> get_jrt_entry()->name() will always give the full
> >>>>>> canonical path. Not sure if a comment with a
> >>>>>> warning in ClassPathImageEntry class is
> sufficient?
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Jiangli
> >>>>>>
> >>>>>> Thanks
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>> Thanks,
> >>>>>>> Jiangli
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Thanks,
> >>>>>>>> Jiangli
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Thanks
> >>>>>>>> - Ioi
> >>>>>>>>
> >>>>>>>> On 3/13/19 10:22 AM, Jiangli
> Zhou
> >>>>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On Tue, Mar 12, 2019 at 4:42 PM
> >>>>>>>>> Jiangli Zhou
> >>>>>>>>> <jianglizhou at google.com <mailto:jianglizhou at google.com>
> >>>>>>>>> <mailto:jianglizhou at google.com
> <mailto:jianglizhou at google.com>>> wrote:
> >>>>>>>>>
> >>>>>>>>> One of my early experimental
> >>>>>>>>> revision actually simply
> used
> >>>>>>>>> strcmp to compare the
> full path
> >>>>>>>>> and to also recorded the
> full
> >>>>>>>>> path for the canonical
> modules
> >>>>>>>>> name.
> >>>>>>>>>
> >>>>>>>>> I decide to not go with that
> >>>>>>>>> because there is no
> guarantee
> >>>>>>>>> that the path given to
> >>>>>>>>> is_modules_image check is in
> >>>>>>>>> canonical form and it
> diverts
> >>>>>>>>> from the existing
> assumptions
> >>>>>>>>> about the usages of this API
> >>>>>>>>> (checking the file name
> only).
> >>>>>>>>> The "/dev/fd/0" example you
> >>>>>>>>> have is not a direct
> result of
> >>>>>>>>> the current fix but is an
> >>>>>>>>> inherent issue of the
> modules
> >>>>>>>>> check. Any file with the
> same
> >>>>>>>>> designated modules name
> would
> >>>>>>>>> pass the check. If we
> want to
> >>>>>>>>> safely use the full path
> >>>>>>>>> comparison, we need to
> closely
> >>>>>>>>> exam all the call sites and
> >>>>>>>>> make sure all the places use
> >>>>>>>>> canonical paths. I'll some
> >>>>>>>>> analysis on that.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> I've double checked all existing
> >>>>>>>>> call sites of
> >>>>>>>>> is_modules_image(const
> char*). No
> >>>>>>>>> extra processing is done (e.g.
> >>>>>>>>> extracting the file name) on the
> >>>>>>>>> path strings when passing to
> >>>>>>>>> is_modules_image. Using the full
> >>>>>>>>> string comparison appears to be
> >>>>>>>>> safe so I have no objection.
> I've
> >>>>>>>>> reverted my change to record the
> >>>>>>>>> complete canonical path as the
> >>>>>>>>> modules identity and changed
> to do
> >>>>>>>>> full path string comparison:
> >>>>>>>>>
> >>>>>>>>> http://cr.openjdk.java.net/~jiangli/8220095/webrev.02/
> >>>>>>>>>
> >>>>>>>>> Local testing looks good.
> I've also
> >>>>>>>>> submitted a submit-repo testing.
> >>>>>>>>> Please help rerun tier2,
> tier3 and
> >>>>>>>>> maybe some of the higher
> tier tests
> >>>>>>>>> due to the semantics change of
> >>>>>>>>> is_modules_image.
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>> Jiangli
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>> Jiangli
> >>>>>>>>>
> >>>>>>>>> On Tue, Mar 12, 2019 at
> 4:13 PM
> >>>>>>>>> Ioi Lam
> <ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>
> >>>>>>>>> <mailto:ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>>>
> wrote:
> >>>>>>>>>
> >>>>>>>>> This seems more
> complicated
> >>>>>>>>> that it needs to be.
> >>>>>>>>>
> >>>>>>>>> + The VM can have at
> most
> >>>>>>>>> one modules file.
> >>>>>>>>>
> >>>>>>>>> + The only reason the VM
> >>>>>>>>> calls
> is_modules_image(p)
> >>>>>>>>> is to check whether p, a
> >>>>>>>>> full path, is THE
> modules
> >>>>>>>>> file. In all cases, p is
> >>>>>>>>> actually derived
> from the
> >>>>>>>>> singleton
> >>>>>>>>> ClassPathImageEntry::name().
> >>>>>>>>> It's just that in some
> >>>>>>>>> cases, such as in the
> >>>>>>>>> logging code, we
> have lost
> >>>>>>>>> the pointer to the
> >>>>>>>>> ClassPathImageEntry, and
> >>>>>>>>> just have the full path.
> >>>>>>>>>
> >>>>>>>>> + On line 722 of
> >>>>>>>>>
> http://cr.openjdk.java.net/~jiangli/8220095/webrev.01/src/hotspot/share/classfile/classLoader.cpp.sdiff.html,
> >>>>>>>>> you already have a full
> >>>>>>>>> path of THE modules
> file.
> >>>>>>>>>
> >>>>>>>>> Why can't you just do a
> >>>>>>>>> strcmp of the full path?
> >>>>>>>>> That's much simpler, and
> >>>>>>>>> more importantly,
> correct.
> >>>>>>>>> Your version will return
> >>>>>>>>> true for "/dev/fd/0" for
> >>>>>>>>> the test case
> described in
> >>>>>>>>> the bug report. That
> >>>>>>>>> doesn't seem to be
> correct.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Thanks
> >>>>>>>>> - Ioi
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On 3/12/19 9:10 AM,
> Jiangli
> >>>>>>>>> Zhou wrote:
> >>>>>>>>>> Hi Ioi,
> >>>>>>>>>>
> >>>>>>>>>> Thanks for looking into
> >>>>>>>>>> this also.
> >>>>>>>>>>
> >>>>>>>>>> On Mon, Mar 11, 2019 at
> >>>>>>>>>> 8:19 PM Ioi Lam
> >>>>>>>>>> <ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>
> >>>>>>>>>> <mailto:ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>>>
> >>>>>>>>>> wrote:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> On 3/11/19 7:30 PM,
> >>>>>>>>>> Ioi Lam wrote:
> >>>>>>>>>> > I think this
> patch
> >>>>>>>>>> should be further
> >>>>>>>>>> improved.
> >>>>>>>>>> >
> >>>>>>>>>> > First of all, the
> >>>>>>>>>> original code is
> >>>>>>>>>> problematic:
> >>>>>>>>>> >
> >>>>>>>>>> > static bool
> >>>>>>>>>> is_modules_image(const
> >>>>>>>>>> char* name) {
> return
> >>>>>>>>>> >
> >>>>>>>>>> string_ends_with(name,
> >>>>>>>>>> MODULES_IMAGE_NAME); }
> >>>>>>>>>> >
> >>>>>>>>>> > So if we have
> a file
> >>>>>>>>>> called
> >>>>>>>>>> /foo/bar/xxxmodules,
> >>>>>>>>>> this function
> returns
> >>>>>>>>>> > true!
> >>>>>>>>>> >
> >>>>>>>>>>
> >>>>>>>>>> BTW, If you have a
> >>>>>>>>>> symlink
> >>>>>>>>>> JAVA_HOME/lib/modules
> >>>>>>>>>> -> JAVA_HOME/lib/r,
> >>>>>>>>>> then your new
> version
> >>>>>>>>>> of is_modules_image
> >>>>>>>>>> will return
> true for
> >>>>>>>>>> any jar
> >>>>>>>>>> file.
> >>>>>>>>>>
> >>>>>>>>>> See
> >>>>>>>>>>
> http://hg.openjdk.java.net/jdk/jdk/file/f984aca565c1/src/hotspot/share/oops/instanceKlass.cpp#l3370
> >>>>>>>>>>
> >>>>>>>>>> if
> >>>>>>>>>> (ClassLoader::is_modules_image(cfs->source()))
> >>>>>>>>>> {
> >>>>>>>>>> info_stream.print("
> >>>>>>>>>> source: jrt:/%s",
> >>>>>>>>>> module_name);
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>> This will cause all
> >>>>>>>>>> classes loaded from
> >>>>>>>>>> modular JAR
> files be
> >>>>>>>>>> incorrectly
> >>>>>>>>>> printed as "jrt://"
> >>>>>>>>>>
> >>>>>>>>>> Even if you
> also use
> >>>>>>>>>> the last "/" path
> >>>>>>>>>> separator, it's still
> >>>>>>>>>> not safe to
> >>>>>>>>>> use the
> filename alone
> >>>>>>>>>> to determine
> whether a
> >>>>>>>>>> file is indeed the
> >>>>>>>>>> modules
> >>>>>>>>>> image file.
> I.e., what
> >>>>>>>>>> if you have
> >>>>>>>>>> JAVA_HOME/lib/modules ->
> >>>>>>>>>> JAVA_HOME/lib/test.jar?
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> The current bug
> addresses
> >>>>>>>>>> the practical issue
> when
> >>>>>>>>>> the modules is a
> sym link
> >>>>>>>>>> to a unique content
> >>>>>>>>>> identifier. Using
> modules
> >>>>>>>>>> as a link to any
> arbitrary
> >>>>>>>>>> file is not
> addressed by
> >>>>>>>>>> this bug. It's in
> the same
> >>>>>>>>>> area but a
> different issue.
> >>>>>>>>>>
> >>>>>>>>>> If lib/modules
> >>>>>>>>>> ->lib/test.jar, or
> >>>>>>>>>> lib/modules -> ../
> (or any
> >>>>>>>>>> other arbitrary
> file/dir)
> >>>>>>>>>> is done, the VM fails
> >>>>>>>>>> differently. This
> is the
> >>>>>>>>>> same as if you replace
> >>>>>>>>>> libjvm.so with another
> >>>>>>>>>> random file.
> >>>>>>>>>>
> >>>>>>>>>> The canonical
> modules name
> >>>>>>>>>> is obtained from the
> >>>>>>>>>> actual modules image
> >>>>>>>>>> during
> >>>>>>>>>> setup_boot_search_path, so
> >>>>>>>>>> it's guaranteed to
> refer
> >>>>>>>>>> to the correct
> identifier.
> >>>>>>>>>> Checking the file name
> >>>>>>>>>> after '/' is safe.
> >>>>>>>>>>
> >>>>>>>>>> The substring issue
> that
> >>>>>>>>>> you pointed out has not
> >>>>>>>>>> been observed, as
> that's
> >>>>>>>>>> prevented by the naming
> >>>>>>>>>> method of the content
> >>>>>>>>>> identifiers. The
> substring
> >>>>>>>>>> issue however can be
> >>>>>>>>>> solved relatively
> >>>>>>>>>> inexpensively by making
> >>>>>>>>>> sure the check
> covers the
> >>>>>>>>>> entire file name
> after '/'.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Also, I think
> we can
> >>>>>>>>>> have a version
> that's
> >>>>>>>>>> simpler than what I
> >>>>>>>>>> proposed
> >>>>>>>>>> below:
> >>>>>>>>>>
> >>>>>>>>>> + We can have at
> >>>>>>>>>> most a single
> >>>>>>>>>> ClassPathImageEntry.
> >>>>>>>>>> + All calls to
> >>>>>>>>>> ClassLoader::is_modules_image()
> >>>>>>>>>> are made after the
> >>>>>>>>>> ClassPathImageEntry is
> >>>>>>>>>> created
> >>>>>>>>>>
> >>>>>>>>>> so:
> >>>>>>>>>>
> >>>>>>>>>> static bool
> >>>>>>>>>> is_modules_image(const
> >>>>>>>>>> char* name) {
> >>>>>>>>>> if
> >>>>>>>>>> (ClassPathImageEntry::singleton()
> >>>>>>>>>> != NULL) {
> >>>>>>>>>> return
> >>>>>>>>>> strcmp(name,
> >>>>>>>>>> ClassPathImageEntry::singleton()->name())
> >>>>>>>>>> == 0);
> >>>>>>>>>> } else {
> >>>>>>>>>> assert(_exploded_entries
> >>>>>>>>>> != NULL, "must be
> >>>>>>>>>> running with
> exploded
> >>>>>>>>>> modules");
> >>>>>>>>>> return false;
> >>>>>>>>>> }
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> The argument passed
> can be
> >>>>>>>>>> a path to the
> modules, so
> >>>>>>>>>> strcmp cannot be simply
> >>>>>>>>>> used for 'name'.
> >>>>>>>>>>
> >>>>>>>>>> static bool
> >>>>>>>>>> is_modules_image(const
> >>>>>>>>>> char* name) {
> >>>>>>>>>>
> >>>>>>>>>> if
> >>>>>>>>>> (Arguments::has_jimage()) {
> >>>>>>>>>>
> >>>>>>>>>> assert(_modules_image_name
> >>>>>>>>>> != NULL, "must be
> set");
> >>>>>>>>>>
> >>>>>>>>>> const char* p =
> >>>>>>>>>> skip_directories(name);
> >>>>>>>>>>
> >>>>>>>>>> return strcmp(p,
> >>>>>>>>>> _modules_image_name) == 0;
> >>>>>>>>>>
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>> return false;
> >>>>>>>>>>
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> I've revised the
> >>>>>>>>>> is_modules_image() to also
> >>>>>>>>>> cover the substring
> issue.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> This function
> should
> >>>>>>>>>> be changed to
> always
> >>>>>>>>>> return true
> >>>>>>>>>>
> >>>>>>>>>> bool
> >>>>>>>>>> ClassPathImageEntry::is_modules_image()
> >>>>>>>>>> const {
> >>>>>>>>>> - return
> >>>>>>>>>> ClassLoader::is_modules_image(name());
> >>>>>>>>>> + return true;
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Leaving
> >>>>>>>>>> ClassPathImageEntry::is_modules_image()
> >>>>>>>>>> without changing is
> safer,
> >>>>>>>>>> so it can make sure
> >>>>>>>>>> the ClassPathImageEntry
> >>>>>>>>>> name agrees with the
> >>>>>>>>>> canonical modules name.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> To make sure we
> don't
> >>>>>>>>>> call
> is_modules_image
> >>>>>>>>>> before
> >>>>>>>>>> ClassPathImageEntry
> >>>>>>>>>> is created, we
> could do
> >>>>>>>>>>
> >>>>>>>>>> static bool
> >>>>>>>>>> is_modules_image(const
> >>>>>>>>>> char* name) {
> >>>>>>>>>> DEBUG_ONLY(_is_modules_image_called
> >>>>>>>>>> = true;)
> >>>>>>>>>> ...
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> The
> >>>>>>>>>> 'assert(_modules_image_name
> >>>>>>>>>> != NULL, "must be
> set")'
> >>>>>>>>>> check makes sure
> >>>>>>>>>> is_modules_image(const
> >>>>>>>>>> char*) is not called
> >>>>>>>>>> before setup_boot_search_path.
> >>>>>>>>>> There is no need to add
> >>>>>>>>>> additional flag.
> >>>>>>>>>>
> >>>>>>>>>> Thanks,
> >>>>>>>>>> Jiangli
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> ClassPathImageEntry::ClassPathImageEntry()
> >>>>>>>>>> {
> >>>>>>>>>> assert(_singleton ==
> >>>>>>>>>> NULL, "...");
> >>>>>>>>>> assert(!_is_modules_image_called,
> >>>>>>>>>> "Don't call
> >>>>>>>>>> ClassLoader::is_modules_image()
> >>>>>>>>>> before
> >>>>>>>>>> ClassPathImageEntry is
> >>>>>>>>>> allocated");
> >>>>>>>>>> ...
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Thanks
> >>>>>>>>>> - Ioi
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> > It seems the
> patch
> >>>>>>>>>> simply makes
> the code
> >>>>>>>>>> more convoluted.
> >>>>>>>>>> Instead,
> >>>>>>>>>> > shouldn't we just
> >>>>>>>>>> open the file and
> >>>>>>>>>> check the magic
> number?
> >>>>>>>>>> >
> >>>>>>>>>> > If performance is
> >>>>>>>>>> important, we can use
> >>>>>>>>>> a cache, because we
> >>>>>>>>>> allow a
> >>>>>>>>>> > single module
> image:
> >>>>>>>>>> >
> >>>>>>>>>> > static bool
> >>>>>>>>>> is_modules_image(const
> >>>>>>>>>> char* name) {
> >>>>>>>>>> > static const
> char*
> >>>>>>>>>> singleton_module_name
> >>>>>>>>>> = NULL;
> >>>>>>>>>> >
> >>>>>>>>>> > if
> >>>>>>>>>> (singleton_module_name
> >>>>>>>>>> != NULL) {
> >>>>>>>>>> > return
> strcmp(name,
> >>>>>>>>>> singleton_module_name)
> >>>>>>>>>> == 0;
> >>>>>>>>>> > }
> >>>>>>>>>> > // open
> <name> and
> >>>>>>>>>> check magic number
> >>>>>>>>>> > if (good) {
> >>>>>>>>>> >
> >>>>>>>>>> singleton_module_name
> >>>>>>>>>> = os::strdup(name);
> >>>>>>>>>> > return true;
> >>>>>>>>>> > }
> >>>>>>>>>> > return false;
> >>>>>>>>>> > }
> >>>>>>>>>> >
> >>>>>>>>>> > Thanks
> >>>>>>>>>> > - Ioi
> >>>>>>>>>> >
> >>>>>>>>>> > On 3/11/19
> 10:51 AM,
> >>>>>>>>>> Jiangli Zhou wrote:
> >>>>>>>>>> >> On Mon, Mar 11,
> >>>>>>>>>> 2019 at 8:44 AM
> Calvin
> >>>>>>>>>> Cheung
> >>>>>>>>>> <calvin.cheung at oracle.com <mailto:calvin.cheung at oracle.com>
> >>>>>>>>>> <mailto:calvin.cheung at oracle.com
> <mailto:calvin.cheung at oracle.com>>>
> >>>>>>>>>> >> wrote:
> >>>>>>>>>> >>
> >>>>>>>>>> >>> Hi Jiangli,
> >>>>>>>>>> >>>
> >>>>>>>>>> >>> The updated webrev
> >>>>>>>>>> looks good.
> >>>>>>>>>> >>>
> >>>>>>>>>> >>> On 3/10/19, 6:41
> >>>>>>>>>> PM, Jiangli
> Zhou wrote:
> >>>>>>>>>> >>>> Here is the
> >>>>>>>>>> updated webrev:
> >>>>>>>>>> >>>>
> >>>>>>>>>> http://cr.openjdk.java.net/~jiangli/8220095/webrev.01/
> >>>>>>>>>> >>>>
> >>>>>>>>>> <http://cr.openjdk.java.net/%7Ejiangli/8220095/webrev.01/>.
> >>>>>>>>>> >>>>
> >>>>>>>>>> >>>> - Incorporated
> >>>>>>>>>> all suggestions
> except
> >>>>>>>>>> the test
> platforms for
> >>>>>>>>>> >>>>
> >>>>>>>>>> ModulesSymLink.java. I
> >>>>>>>>>> changed to requires
> >>>>>>>>>> linux, mac and
> solaris
> >>>>>>>>>> >>>> explicitly
> >>>>>>>>>> instead to
> avoid any
> >>>>>>>>>> potential issue with other
> >>>>>>>>>> >>>> non-mainline
> >>>>>>>>>> testing platforms.
> >>>>>>>>>> >>>> - Reverted the
> >>>>>>>>>> changes in
> >>>>>>>>>> sharedPathsMiscInfo.cpp.
> >>>>>>>>>> Further testing
> >>>>>>>>>> >>>> showed
> >>>>>>>>>> SharedPathsMiscInfo::check()
> >>>>>>>>>> were dealing the
> >>>>>>>>>> system boot path
> >>>>>>>>>> >>>> string set up by
> >>>>>>>>>> os::set_boot_path.
> >>>>>>>>>> Lois comments
> reminded
> >>>>>>>>>> me to
> >>>>>>>>>> >>>> double check with
> >>>>>>>>>> this. Thanks!
> >>>>>>>>>> >>>>
> >>>>>>>>>> >>>> Please help fire
> >>>>>>>>>> tier2 and tier3
> runs
> >>>>>>>>>> with mach5.
> >>>>>>>>>> >>> I ran tier2 and
> >>>>>>>>>> tier3 tests
> with your
> >>>>>>>>>> latest changes
> and saw
> >>>>>>>>>> no test
> >>>>>>>>>> >>> failures.
> >>>>>>>>>> >>>
> >>>>>>>>>> >> Great,
> thanks a lot!
> >>>>>>>>>> >>
> >>>>>>>>>> >>
> >>>>>>>>>> >>> BTW, I was unable
> >>>>>>>>>> to download the
> >>>>>>>>>> jdk.patch - got "403 -
> >>>>>>>>>> forbidden"
> >>>>>>>>>> >>> error.
> >>>>>>>>>> >>> Same error was
> >>>>>>>>>> seen with
> individual
> >>>>>>>>>> patch files.
> >>>>>>>>>> >>> I used wget to get
> >>>>>>>>>> all the raw
> files into
> >>>>>>>>>> my repo and then
> >>>>>>>>>> submitted
> >>>>>>>>>> >>> the
> >>>>>>>>>> >>> build and test job.
> >>>>>>>>>> >>> Could you check
> >>>>>>>>>> the file
> permissions?
> >>>>>>>>>> >>>
> >>>>>>>>>> >> Yes, that's the
> >>>>>>>>>> file permission
> issue.
> >>>>>>>>>> I fixed the
> permission
> >>>>>>>>>> for the
> >>>>>>>>>> >> jdk.patch.
> >>>>>>>>>> >>
> >>>>>>>>>> >> Thanks!
> >>>>>>>>>> >> Jiangli
> >>>>>>>>>> >>
> >>>>>>>>>> >>
> >>>>>>>>>> >>> thanks,
> >>>>>>>>>> >>> Calvin
> >>>>>>>>>> >>>> Thanks!
> >>>>>>>>>> >>>> Jiangli
> >>>>>>>>>> >>>>
> >>>>>>>>>> >>>> On Thu, Mar 7,
> >>>>>>>>>> 2019 at 5:11 AM
> Lois
> >>>>>>>>>> Foltan
> >>>>>>>>>> <lois.foltan at oracle.com <mailto:lois.foltan at oracle.com>
> >>>>>>>>>> <mailto:lois.foltan at oracle.com
> <mailto:lois.foltan at oracle.com>>
> >>>>>>>>>> >>>>
> >>>>>>>>>> <mailto:lois.foltan at oracle.com
> <mailto:lois.foltan at oracle.com>
> >>>>>>>>>> <mailto:lois.foltan at oracle.com
> <mailto:lois.foltan at oracle.com>>>>
> >>>>>>>>>> wrote:
> >>>>>>>>>> >>>>
> >>>>>>>>>> >>>> On 3/6/2019
> >>>>>>>>>> 6:20 PM,
> Jiangli Zhou
> >>>>>>>>>> wrote:
> >>>>>>>>>> >>>> > Please
> >>>>>>>>>> review the
> following
> >>>>>>>>>> fix for 8220095.
> >>>>>>>>>> >>>> >
> >>>>>>>>>> >>>> > webrev:
> >>>>>>>>>> http://cr.openjdk.java.net/~jiangli/8220095/webrev.00/
> >>>>>>>>>> >>>>
> >>>>>>>>>> <http://cr.openjdk.java.net/%7Ejiangli/8220095/webrev.00/>
> >>>>>>>>>> >>>> > bug:
> >>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8220095
> >>>>>>>>>> >>>> >
> >>>>>>>>>> >>>> > Symbolic
> >>>>>>>>>> links may be used
> >>>>>>>>>> commonly in
> some cloud
> >>>>>>>>>> environments.
> >>>>>>>>>> >>>> The target
> >>>>>>>>>> >>>> > files
> >>>>>>>>>> might have
> different
> >>>>>>>>>> names than the
> linked
> >>>>>>>>>> names. The VM
> >>>>>>>>>> >>>> crashes when
> >>>>>>>>>> >>>> >
> >>>>>>>>>> 'lib/modules' links to
> >>>>>>>>>> a target file
> with a
> >>>>>>>>>> different name, for
> >>>>>>>>>> >>>> example,
> >>>>>>>>>> >>>> >
> >>>>>>>>>> lib/modules -> lib/0.
> >>>>>>>>>> >>>> >
> >>>>>>>>>> >>>> > The usage
> >>>>>>>>>> of the hard-coded
> >>>>>>>>>> MODULES_IMAGE_NAME
> >>>>>>>>>> (used by
> >>>>>>>>>> >>>> >
> >>>>>>>>>> ClassLoader::is_modules_image(const
> >>>>>>>>>> char*)) can become
> >>>>>>>>>> >>>> problematic
> >>>>>>>>>> in this
> >>>>>>>>>> >>>> > case and
> >>>>>>>>>> leads to assertion
> >>>>>>>>>> failures and
> crashes
> >>>>>>>>>> in this
> >>>>>>>>>> >>>> case. The
> >>>>>>>>>> >>>> >
> >>>>>>>>>> is_modules_image()
> >>>>>>>>>> >>>> > checks
> >>>>>>>>>> always fail
> even the
> >>>>>>>>>> actual file is the
> >>>>>>>>>> runtime image
> >>>>>>>>>> >>>> because the
> >>>>>>>>>> >>>> > canonical
> >>>>>>>>>> paths (which
> might not
> >>>>>>>>>> end with 'modules')
> >>>>>>>>>> are passed
> >>>>>>>>>> >>>> in for the
> >>>>>>>>>> >>>> > check. The
> >>>>>>>>>> fix is to
> obtain the
> >>>>>>>>>> real name from the
> >>>>>>>>>> canonical
> >>>>>>>>>> >>>> path early
> >>>>>>>>>> >>>> > during
> >>>>>>>>>> initialization and use
> >>>>>>>>>> that for the
> >>>>>>>>>> is_modules_image()
> >>>>>>>>>> >>> check.
> >>>>>>>>>> >>>> >
> >>>>>>>>>> >>>> > Tested
> >>>>>>>>>> with submit repo
> >>>>>>>>>> testing, tier1,
> tier2,
> >>>>>>>>>> and hotspot
> >>>>>>>>>> >>>> runtime tests
> >>>>>>>>>> >>>> > locally.
> >>>>>>>>>> >>>> >
> >>>>>>>>>> >>>> > Thanks!
> >>>>>>>>>> >>>> > Jiangli
> >>>>>>>>>> >>>>
> >>>>>>>>>> >>>> Hi Jiangli,
> >>>>>>>>>> >>>>
> >>>>>>>>>> >>>> This change
> >>>>>>>>>> looks good! A
> couple
> >>>>>>>>>> of minor comments:
> >>>>>>>>>> >>>>
> >>>>>>>>>> >>>> -
> >>>>>>>>>> classLoader.hpp
> >>>>>>>>>> >>>> line
> >>>>>>>>>> #39 - I think you
> >>>>>>>>>> should expand
> on the
> >>>>>>>>>> comment and add
> >>>>>>>>>> >>>> why in
> >>>>>>>>>> >>>> the case of
> >>>>>>>>>> the canonical path
> >>>>>>>>>> name
> >>>>>>>>>> MODULES_IMAGE_NAME can
> >>>>>>>>>> not be
> >>>>>>>>>> >>>> used
> >>>>>>>>>> >>>> as-is but if
> >>>>>>>>>> one is dealing with
> >>>>>>>>>> the system boot
> path
> >>>>>>>>>> string set
> >>>>>>>>>> >>>> up by
> >>>>>>>>>> >>>>
> >>>>>>>>>> os::set_boot_path,
> >>>>>>>>>> than
> >>>>>>>>>> MODULES_IMAGE_NAME
> >>>>>>>>>> should be used.
> >>>>>>>>>> >>>>
> >>>>>>>>>> >>>> -
> >>>>>>>>>> classLoader.cpp
> >>>>>>>>>> >>>> line
> >>>>>>>>>> #702 - Can you
> add the
> >>>>>>>>>> same explanation
> >>>>>>>>>> comment as in
> >>>>>>>>>> >>>>
> >>>>>>>>>> classLoader.hpp ahead
> >>>>>>>>>> of the new
> >>>>>>>>>> >>>>
> >>>>>>>>>> ClassLoader::init_modules_image_name()
> >>>>>>>>>> >>>> method?
> >>>>>>>>>> That would be
> helpful.
> >>>>>>>>>> >>>> line
> >>>>>>>>>> #710 - ahead of
> >>>>>>>>>> setting
> >>>>>>>>>> _modules_image_name
> >>>>>>>>>> please
> >>>>>>>>>> >>>> consider
> >>>>>>>>>> >>>> adding an
> >>>>>>>>>> assert that p1's
> >>>>>>>>>> length is greater
> >>>>>>>>>> than 0?
> >>>>>>>>>> >>>>
> >>>>>>>>>>
> >>>>>>>>>> Maybe there should
> >>>>>>>>>> there be an
> error case
> >>>>>>>>>> >>>> test
> >>>>>>>>>> >>>> where
> >>>>>>>>>> "lib/modules -> lib/"
> >>>>>>>>>> >>>>
> >>>>>>>>>> >>>> Thanks,
> >>>>>>>>>> >>>> Lois
> >>>>>>>>>> >>>>
> >>>>>>>>>> >>>> -
> >>>>>>>>>> >>>>
> >>>>>>>>>> >
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>
> >>>
> >>
>
More information about the hotspot-runtime-dev
mailing list