thr3ads.net - llvm dev - [LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules [Mar 2015]

If this information is useful, please help other people find it:
Share via:

Mark Millard

2015-Mar-15 00:36 UTC

[LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules

When trying to build the 11.0-CURRENT clang 3.5 on powerpc64 I ran into a
violation of C++ accessibility rules (for private) that stopped the compile. So
not the usual defect category. (This was a bootstrapping procedure as
powerpc/powerpc64 FreeBSD world’s clang has an odd status and getting from 3.4
under 10.1-STABLE to 3.5 on 11.0-CURRENT is not automatic.)

Given the language rules and difficulty interpreting them I figured an open
discussion area might be the better place to go until/unless someone from llvm
agrees with the information. I'm not sure what priority being non-standard
has for points other compilers have trouble with for the code.

I have looked on the web and Revision 232289 of IntrusiveRefCntPtr.h still has
the same code structure for the issue.


The problem...

FreeBSD 11.0-CURRENT's contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
has...

  template <typename T>
  class IntrusiveRefCntPtr {
    T* Obj;

  public:
  ...
    template <class X>
    IntrusiveRefCntPtr(IntrusiveRefCntPtr<X>&& S) : Obj(S.get()) {
      S.Obj = 0;
    }
  ...
  }

To first illustrate a (partial) but-simpler-to-follow example use that would
show the problem with the above:

using Ta = ...;
using Tb = ...;// Not the same type, more than just a name change.

// Note that private members of IntrusiveRefCntPtr<Ta>
// are not (should not be) accessible to
// IntrusiveRefCntPtr<Tb> methods (and vice-versa).

IntrusiveRefCntPtr<Ta> a{}

IntrusiveRefCntPtr<Tb> b{a};

// We then would have a usage where an example of:

IntrusiveRefCntPtr<Tb>::IntrusiveRefCntPtr

is then trying to access an example of

IntrusiveRefCntPtr<Ta>'s Obj private member.

It would take a friend relationship to be established to allow the cross-type
access to Obj.


The code in contrib/llvm/tools/clang/lib/Frontend/ChainedIncludesSource.cpp has
such a use and so makes an instance of the violation of the language rules in
the actual code.

The function clang::createChainedIncludesSourceIt uses classes... 

class ChainedIncludesSource : public ExternalSemaSource
where...
class ExternalSemaSource : public ExternalASTSource
where...
class ExternalASTSource : public RefCountedBase<ExternalASTSource>
where...
template <class Derived> class RefCountedBase;

and it uses both of the following types...

IntrusiveRefCntPtr<ExternalSemaSource>
and...
IntrusiveRefCntPtr<ChainedIncludesSource>

In fact IntrusiveRefCntPtr<ChainedIncludesSource> is the return-expresison
type for the following routine that has return type
IntrusiveRefCntPtr<ExternalSemaSource>...

IntrusiveRefCntPtr<ExternalSemaSource> clang::createChainedIncludesSource(
    CompilerInstance &CI, IntrusiveRefCntPtr<ExternalSemaSource>
&Reader) {
...
  IntrusiveRefCntPtr<ChainedIncludesSource> source(new
ChainedIncludesSource());
...
  return source;
}

==Mark Millard
markmi at dsl-only.net

David Blaikie

2015-Mar-15 00:55 UTC

head link

[LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules

I assume there's a bit more to it than this - otherwise we would've
discovered it earlier? Which compiler is rejecting this code? Do you have a
reduced example of something that clang accepts and this other compiler
rejects?

On Sat, Mar 14, 2015 at 5:36 PM, Mark Millard <markmi at dsl-only.net>
wrote:
> When trying to build the 11.0-CURRENT clang 3.5 on powerpc64 I ran into a
> violation of C++ accessibility rules (for private) that stopped the
> compile. So not the usual defect category. (This was a bootstrapping
> procedure as powerpc/powerpc64 FreeBSD world’s clang has an odd status and
> getting from 3.4 under 10.1-STABLE to 3.5 on 11.0-CURRENT is not
automatic.)
>
> Given the language rules and difficulty interpreting them I figured an
> open discussion area might be the better place to go until/unless someone
> from llvm agrees with the information. I'm not sure what priority being
> non-standard has for points other compilers have trouble with for the code.
>
> I have looked on the web and Revision 232289 of IntrusiveRefCntPtr.h still
> has the same code structure for the issue.
>
>
> The problem...
>
> FreeBSD 11.0-CURRENT's
contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
> has...
>
>   template <typename T>
>   class IntrusiveRefCntPtr {
>     T* Obj;
>
>   public:
>   ...
>     template <class X>
>     IntrusiveRefCntPtr(IntrusiveRefCntPtr<X>&& S) :
Obj(S.get()) {
>       S.Obj = 0;
>     }
>   ...
>   }
>
> To first illustrate a (partial) but-simpler-to-follow example use that
> would show the problem with the above:
>
> using Ta = ...;
> using Tb = ...;// Not the same type, more than just a name change.
>
> // Note that private members of IntrusiveRefCntPtr<Ta>
> // are not (should not be) accessible to
> // IntrusiveRefCntPtr<Tb> methods (and vice-versa).
>
> IntrusiveRefCntPtr<Ta> a{}
>
> IntrusiveRefCntPtr<Tb> b{a};
>
> // We then would have a usage where an example of:
>
> IntrusiveRefCntPtr<Tb>::IntrusiveRefCntPtr
>
> is then trying to access an example of
>
> IntrusiveRefCntPtr<Ta>'s Obj private member.
>
> It would take a friend relationship to be established to allow the
> cross-type access to Obj.
>
>
> The code in
> contrib/llvm/tools/clang/lib/Frontend/ChainedIncludesSource.cpp has such a
> use and so makes an instance of the violation of the language rules in the
> actual code.
>
> The function clang::createChainedIncludesSourceIt uses classes...
>
> class ChainedIncludesSource : public ExternalSemaSource
> where...
> class ExternalSemaSource : public ExternalASTSource
> where...
> class ExternalASTSource : public RefCountedBase<ExternalASTSource>
> where...
> template <class Derived> class RefCountedBase;
>
> and it uses both of the following types...
>
> IntrusiveRefCntPtr<ExternalSemaSource>
> and...
> IntrusiveRefCntPtr<ChainedIncludesSource>
>
> In fact IntrusiveRefCntPtr<ChainedIncludesSource> is the
return-expresison
> type for the following routine that has return type
> IntrusiveRefCntPtr<ExternalSemaSource>...
>
> IntrusiveRefCntPtr<ExternalSemaSource>
clang::createChainedIncludesSource(
>     CompilerInstance &CI, IntrusiveRefCntPtr<ExternalSemaSource>
&Reader) {
> ...
>   IntrusiveRefCntPtr<ChainedIncludesSource> source(new
> ChainedIncludesSource());
> ...
>   return source;
> }
>
> ==> Mark Millard
> markmi at dsl-only.net
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150314/d4fd36a1/attachment.html>

Tim Northover

2015-Mar-15 01:25 UTC

head link

[LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules

On 14 March 2015 at 17:55, David Blaikie <dblaikie at gmail.com>
wrote:> I assume there's a bit more to it than this - otherwise we would've
> discovered it earlier? Which compiler is rejecting this code? Do you have a
> reduced example of something that clang accepts and this other compiler
> rejects?
Probably RVO related. The problematic constructor (and it does seem
pointless) is only invoked if an actual copy is needed. Clang takes
advantage of RVO even at -O0 and only ever calls
llvm::IntrusiveRefCntPtr<clang::ExternalSemaSource>::IntrusiveRefCntPtr(clang::ExternalSemaSource*).

If so, I suspect LLVM does need to be changed to work on compilers
without RVO too (the standard says implementations *may* perform that
optimisation, not that they must).

I'd be interested to know what this other compiler was, too.

Tim.

Jonathan Roelofs

2015-Mar-15 01:34 UTC

head link

[LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules

On 3/14/15 6:36 PM, Mark Millard wrote:> When trying to build the 11.0-CURRENT clang 3.5 on powerpc64 I ran into a
violation of C++ accessibility rules (for private) that stopped the compile. So
not the usual defect category. (This was a bootstrapping procedure as
powerpc/powerpc64 FreeBSD world’s clang has an odd status and getting from 3.4
under 10.1-STABLE to 3.5 on 11.0-CURRENT is not automatic.)
>
> Given the language rules and difficulty interpreting them I figured an open
discussion area might be the better place to go until/unless someone from llvm
agrees with the information. I'm not sure what priority being non-standard
has for points other compilers have trouble with for the code.
>
> I have looked on the web and Revision 232289 of IntrusiveRefCntPtr.h still
has the same code structure for the issue.
>
>
> The problem...
>
> FreeBSD 11.0-CURRENT's
contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h has...
>
>    template <typename T>
>    class IntrusiveRefCntPtr {
>      T* Obj;
>
>    public:
>    ...
>      template <class X>
>      IntrusiveRefCntPtr(IntrusiveRefCntPtr<X>&& S) :
Obj(S.get()) {
>        S.Obj = 0;
>      }
>    ...
>    }
>
> To first illustrate a (partial) but-simpler-to-follow example use that
would show the problem with the above:
>
> using Ta = ...;
> using Tb = ...;// Not the same type, more than just a name change.
>
> // Note that private members of IntrusiveRefCntPtr<Ta>
> // are not (should not be) accessible to
> // IntrusiveRefCntPtr<Tb> methods (and vice-versa).
>
> IntrusiveRefCntPtr<Ta> a{}
>
> IntrusiveRefCntPtr<Tb> b{a};
>
> // We then would have a usage where an example of:
>
> IntrusiveRefCntPtr<Tb>::IntrusiveRefCntPtr
>
> is then trying to access an example of
>
> IntrusiveRefCntPtr<Ta>'s Obj private member.
>
> It would take a friend relationship to be established to allow the
cross-type access to Obj.
Doesn't the:

     template <typename X>
     friend class IntrusiveRefCntPtr;

here: 
https://github.com/llvm-mirror/llvm/blob/master/include/llvm/ADT/IntrusiveRefCntPtr.h#L202
take care of that?


Jon>
>
> The code in contrib/llvm/tools/clang/lib/Frontend/ChainedIncludesSource.cpp
has such a use and so makes an instance of the violation of the language rules
in the actual code.
>
> The function clang::createChainedIncludesSourceIt uses classes...
>
> class ChainedIncludesSource : public ExternalSemaSource
> where...
> class ExternalSemaSource : public ExternalASTSource
> where...
> class ExternalASTSource : public RefCountedBase<ExternalASTSource>
> where...
> template <class Derived> class RefCountedBase;
>
> and it uses both of the following types...
>
> IntrusiveRefCntPtr<ExternalSemaSource>
> and...
> IntrusiveRefCntPtr<ChainedIncludesSource>
>
> In fact IntrusiveRefCntPtr<ChainedIncludesSource> is the
return-expresison type for the following routine that has return type
IntrusiveRefCntPtr<ExternalSemaSource>...
>
> IntrusiveRefCntPtr<ExternalSemaSource>
clang::createChainedIncludesSource(
>      CompilerInstance &CI, IntrusiveRefCntPtr<ExternalSemaSource>
&Reader) {
> ...
>    IntrusiveRefCntPtr<ChainedIncludesSource> source(new
ChainedIncludesSource());
> ...
>    return source;
> }
>
> ==> Mark Millard
> markmi at dsl-only.net
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded

Mark Millard

2015-Mar-15 01:45 UTC

head link

[LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules

Johnathan Roelofs is correct when he wrote:
> Doesn't the:
> 
>    template <typename X>
>    friend class IntrusiveRefCntPtr;
> 
> here:
https://github.com/llvm-mirror/llvm/blob/master/include/llvm/ADT/IntrusiveRefCntPtr.h#L202
take care of that?

I missed that difference from the FreeBSD source when I looked on the web.

And it looks like it has been in place for some time.

Sorry for the noise.

==Mark Millard
markmi at dsl-only.net

On 2015-Mar-14, at 05:55 PM, David Blaikie <dblaikie at gmail.com> wrote:

I assume there's a bit more to it than this - otherwise we would've
discovered it earlier? Which compiler is rejecting this code? Do you have a
reduced example of something that clang accepts and this other compiler rejects?

On Sat, Mar 14, 2015 at 5:36 PM, Mark Millard <markmi at dsl-only.net>
wrote:
When trying to build the 11.0-CURRENT clang 3.5 on powerpc64 I ran into a
violation of C++ accessibility rules (for private) that stopped the compile. So
not the usual defect category. (This was a bootstrapping procedure as
powerpc/powerpc64 FreeBSD world’s clang has an odd status and getting from 3.4
under 10.1-STABLE to 3.5 on 11.0-CURRENT is not automatic.)

Given the language rules and difficulty interpreting them I figured an open
discussion area might be the better place to go until/unless someone from llvm
agrees with the information. I'm not sure what priority being non-standard
has for points other compilers have trouble with for the code.

I have looked on the web and Revision 232289 of IntrusiveRefCntPtr.h still has
the same code structure for the issue.

The problem...

FreeBSD 11.0-CURRENT's contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
has...

  template <typename T>
  class IntrusiveRefCntPtr {
    T* Obj;

  public:
  ...
    template <class X>
    IntrusiveRefCntPtr(IntrusiveRefCntPtr<X>&& S) : Obj(S.get()) {
      S.Obj = 0;
    }
  ...
  }

To first illustrate a (partial) but-simpler-to-follow example use that would
show the problem with the above:

using Ta = ...;
using Tb = ...;// Not the same type, more than just a name change.

// Note that private members of IntrusiveRefCntPtr<Ta>
// are not (should not be) accessible to
// IntrusiveRefCntPtr<Tb> methods (and vice-versa).

IntrusiveRefCntPtr<Ta> a{}

IntrusiveRefCntPtr<Tb> b{a};

// We then would have a usage where an example of:

IntrusiveRefCntPtr<Tb>::IntrusiveRefCntPtr

is then trying to access an example of

IntrusiveRefCntPtr<Ta>'s Obj private member.

It would take a friend relationship to be established to allow the cross-type
access to Obj.

The code in contrib/llvm/tools/clang/lib/Frontend/ChainedIncludesSource.cpp has
such a use and so makes an instance of the violation of the language rules in
the actual code.

The function clang::createChainedIncludesSourceIt uses classes...

class ChainedIncludesSource : public ExternalSemaSource
where...
class ExternalSemaSource : public ExternalASTSource
where...
class ExternalASTSource : public RefCountedBase<ExternalASTSource>
where...
template <class Derived> class RefCountedBase;

and it uses both of the following types...

IntrusiveRefCntPtr<ExternalSemaSource>
and...
IntrusiveRefCntPtr<ChainedIncludesSource>

In fact IntrusiveRefCntPtr<ChainedIncludesSource> is the return-expresison
type for the following routine that has return type
IntrusiveRefCntPtr<ExternalSemaSource>...

IntrusiveRefCntPtr<ExternalSemaSource> clang::createChainedIncludesSource(
    CompilerInstance &CI, IntrusiveRefCntPtr<ExternalSemaSource>
&Reader) {
...
  IntrusiveRefCntPtr<ChainedIncludesSource> source(new
ChainedIncludesSource());
...
  return source;
}

==Mark Millard
markmi at dsl-only.net

_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

llvm dev - Mar 2015 - [LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules

[LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules

[LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules

[LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules

[LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules

[LLVMdev] FreeBSD's 11.0-CURRENT contrib/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h's IntrusiveRefCntPtr and its use violates C++ privacy rules