RFR(XXS): 8214125: [test] Fix comparison between pointer and integer in test_ptrQueueBufferAllocator.cpp

Thomas Stüfe thomas.stuefe at gmail.com
Wed Nov 21 07:38:57 UTC 2018


Hi Kim,

thanks for the thorough explanation!

As for improving GTEST_IS_NULL_LITERAL_ macro, if it is quick and easy
to do yes by all means :) But note that we had this problem on AIX, by
my count, exactly two times (8171225 and this one) and they had been
easy to spot and fix.

Cheers, Thomas

On Tue, Nov 20, 2018 at 7:24 PM Kim Barrett <kim.barrett at oracle.com> wrote:
>
> > On Nov 20, 2018, at 11:02 AM, Volker Simonis <volker.simonis at gmail.com> wrote:
> >
> > Hi,
> >
> > can I please have a review fort he following trivial gtest fix which
> > currently breaks the AIX build:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8214125/
> > https://bugs.openjdk.java.net/browse/JDK-8214125
> >
> > Change JDK-8213352 introduced an gtest which does a comparison between
> > an integer and a pointer which is forbidden in C++:
> >
> >    ASSERT_EQ(NULL, nodes[i]->next());
> >
> > I don't know why other compilers havn't complained about it, but XLC did.
> >
> > The fix for this is trivial - just cast NULL to the appropriate pointer type:
> >
> >    ASSERT_EQ((BufferNode*)NULL, nodes[i]->next());
> >
> >
> > Thank you and best regards,
> > Volker
>
> Change looks good, though I see it's already been pushed.
>
> Background information:
>
> The reason this works elsewhere, but not with the XLC compiler, is
> that there are two paths to this working, and neither is available on
> this platform.
>
> (1) Some platforms define NULL to be some special object (similar, or
> perhaps identical to C++11's nullptr).
>
> (2) There is a bit of code in the gtest framework to attempt to
> recognize NULL as the first argument in an _EQ check.
>
> XLC doesn't pass (1), since (if I recall correctly) it defines NULL to
> be 0 or 0L (see below).
>
> XLC doesn't pass (2) because the functioning implementation of that
> macro relies on UB (possibly passing a non-POD class object to an
> ellipsis function), which XLC doesn't like (not unreasonably), so the
> recognition via that macro is disabled.  See GTEST_IS_NULL_LITERAL_(x)
> and GTEST_ELLIPSIS_NEEDS_POD_.
>
> I looked at the implementation of GTEST_IS_NULL_LITERAL_ a while ago,
> and had an idea that might fix it, but I haven't had time to pursue
> it.  Given that we've run into exactly this problem multiple times,
> maybe something should be done (probably including upstreaming if a
> fix is produced).
>
> As to why NULL isn't just (void*)0, that's because the C++ standard
> says so.  Yeah, it's annoying that C++ and POSIX seem to explicitly
> disagree here.
>
> C++03 4.10
> A <it>null pointer constant</it> is an integral constant expression
> rvalue of integer type that evaluates to zero.
>
> C++03 18.1
> The macro NULL is an implementation-defined C++ null pointer constant
> [footnote] Possible definitions include 0 and 0L, but not (void*)0.
>
> C++14 4.10
> A <it>null pointer constant</it> is an integral literal with value
> zero or a prvalue of type std::nullptr_t.
>
> C++14 18.2
> The macro NULL is an implementation-defined C++ null pointer constant
> [footnote] Possible definitions include 0 and 0L, but not (void*)0.
>
>



More information about the hotspot-gc-dev mailing list