warnings in clang

Clang Frontend
1

Here are the warnings from a recent build of clang (-r59324):

DeclSerialization.cpp: In member function 'void clang::ScopedDecl::ReadInRec(llvm::Deserializer&, clang::ASTContext&)':
DeclSerialization.cpp:170: warning: dereferencing type-punned pointer will break strict-aliasing rules
DeclSerialization.cpp:171: warning: dereferencing type-punned pointer will break strict-aliasing rules
Expr.cpp: In member function 'virtual clang::StmtIterator clang::SizeOfAlignOfExpr::child_begin()':
Expr.cpp:1379: warning: dereferencing type-punned pointer will break strict-aliasing rules
Expr.cpp: In member function 'virtual clang::StmtIterator clang::SizeOfAlignOfExpr::child_end()':
Expr.cpp:1384: warning: dereferencing type-punned pointer will break strict-aliasing rules
ExprCXX.cpp: In member function 'virtual clang::StmtIterator clang::CXXTypeidExpr::child_begin()':
ExprCXX.cpp:31: warning: dereferencing type-punned pointer will break strict-aliasing rules
ExprCXX.cpp: In member function 'virtual clang::StmtIterator clang::CXXTypeidExpr::child_end()':
ExprCXX.cpp:34: warning: dereferencing type-punned pointer will break strict-aliasing rules

2

Mike Stump wrote:

Here are the warnings from a recent build of clang (-r59324):

DeclSerialization.cpp: In member function 'void clang::ScopedDecl::ReadInRec(llvm::Deserializer&, clang::ASTContext&)':
DeclSerialization.cpp:170: warning: dereferencing type-punned pointer will break strict-aliasing rules
DeclSerialization.cpp:171: warning: dereferencing type-punned pointer will break strict-aliasing rules
  
I'm the one to blame for these. Care to explain more about what is wrong and how to fix it ?
The offending line is:

    D.ReadUIntPtr(reinterpret_cast<uintptr_t&>(MDC->SemanticDC), SemaDCPtrID);

where 'MDC->SemanticDC' is a pointer (DeclContext*).

-Argiris

3

Argiris Kirtzidis wrote:

Mike Stump wrote:
  

Here are the warnings from a recent build of clang (-r59324):

DeclSerialization.cpp: In member function 'void clang::ScopedDecl::ReadInRec(llvm::Deserializer&, clang::ASTContext&)':
DeclSerialization.cpp:170: warning: dereferencing type-punned pointer will break strict-aliasing rules
DeclSerialization.cpp:171: warning: dereferencing type-punned pointer will break strict-aliasing rules
  
I'm the one to blame for these. Care to explain more about what is wrong and how to fix it ?
The offending line is:

    D.ReadUIntPtr(reinterpret_cast<uintptr_t&>(MDC->SemanticDC), SemaDCPtrID);

where 'MDC->SemanticDC' is a pointer (DeclContext*).
  

What's wrong with D.ReadPtr(MDC->SemanticDC, SemaDCPtrID)? That template function does exactly the same thing, but then it's LLVM core's problem, not yours.

Sebastian

4

Mike Stump wrote:

Here are the warnings from a recent build of clang (-r59324):

DeclSerialization.cpp: In member function 'void clang::ScopedDecl::ReadInRec(llvm::Deserializer&, clang::ASTContext&)':
DeclSerialization.cpp:170: warning: dereferencing type-punned pointer will break strict-aliasing rules
DeclSerialization.cpp:171: warning: dereferencing type-punned pointer will break strict-aliasing rules

I'm the one to blame for these. Care to explain more about what is wrong and how to fix it ?

Hum, depends upon your background. I'd assume you're new to the wording in the standards that says that you can only play with a type as the type (plus a few other obscure cases). If you change your code to always play with variables of type X, as type X, you can never get this error.

D.ReadUIntPtr(reinterpret_cast<uintptr_t&>(MDC->SemanticDC), SemaDCPtrID);

where 'MDC->SemanticDC' is a pointer (DeclContext*).

reinterpret_cast<uintptr_t&>? Never use reinterpret_cast.

   uintptr_t x;
   D.ReadUIntPtr(x, SemaDCPtrID);

will read a uintptr_t. This:

   MDC->SemanticDC = (DeclContext*)x;

will convert and store it. Another way, if you wanted, would be to have a templated reader that can read any pointer:

template <T&>
myread(T t) {
   read (fd, (char*)&t, sizeof (t));

this works, because read can read into any type.

5

Sebastian Redl wrote:

What's wrong with D.ReadPtr(MDC->SemanticDC, SemaDCPtrID)?

Whoa, why on earth did I think there wasn't a ReadPtr version that also accepted a SerializedPtrID ?
Thanks a lot Sebastian!

-Argiris

6

Mike Stump wrote:

Mike Stump wrote:

Here are the warnings from a recent build of clang (-r59324):

DeclSerialization.cpp: In member function 'void clang::ScopedDecl::ReadInRec(llvm::Deserializer&, clang::ASTContext&)':
DeclSerialization.cpp:170: warning: dereferencing type-punned pointer will break strict-aliasing rules
DeclSerialization.cpp:171: warning: dereferencing type-punned pointer will break strict-aliasing rules

I'm the one to blame for these. Care to explain more about what is wrong and how to fix it ?

Hum, depends upon your background. I'd assume you're new to the wording in the standards that says that you can only play with a type as the type (plus a few other obscure cases). If you change your code to always play with variables of type X, as type X, you can never get this error.

D.ReadUIntPtr(reinterpret_cast<uintptr_t&>(MDC->SemanticDC), SemaDCPtrID);

where 'MDC->SemanticDC' is a pointer (DeclContext*).

reinterpret_cast<uintptr_t&>? Never use reinterpret_cast.

Here's a similar case, doesn't gcc has a problem with this ?

  template <typename T>
  void ReadPtr(T*& PtrRef, const SerializedPtrID& PtrID,
               bool AllowBackpatch = true) {
    ReadUIntPtr(reinterpret_cast<uintptr_t&>(PtrRef), PtrID, AllowBackpatch);
  }

7

Mike Stump wrote:

8

Argiris Kirtzidis wrote:

Here's a similar case, doesn't gcc has a problem with this ?

  template <typename T>
  void ReadPtr(T*& PtrRef, const SerializedPtrID& PtrID,
               bool AllowBackpatch = true) {
    ReadUIntPtr(reinterpret_cast<uintptr_t&>(PtrRef), PtrID, AllowBackpatch);
  }
  

It's a trick to avoid GCC's warning. Because the function takes a reference and passes a reference on, the type-punned "pointer" (reference, actually) is never "dereferenced" (accessed). The actual ReadUIntPtr accesses it, but by then GCC has forgotten the original type.

Sebastian

9

As I read that, it sounds like someone is praying and hoping and violating the language standard. Don't do that. We don't trick gcc into not complaining, if you do that, you _will_ lose. One _must_ write code that is type safe. That isn't a trick. I reviewed lib/Bitcode/Reader/Deserialize.cpp Deserializer::ReadUIntPtr. :frowning: I don't see anything that is type safe about this code.

:frowning:

It is really easy to fix, for example:

PtrRef = 0;

becomes:

   uintptr_t x = 0;
   memcpy (&PtrRef, x, sizeof());

suddenly, perfectly type safe, and, not an instruction larger, not a single clock slower to boot.

Welcome to C.

10

Mike Stump wrote:

As I read that, it sounds like someone is praying and hoping and violating the language standard. Don't do that. We don't trick gcc into not complaining, if you do that, you _will_ lose. One _must_ write code that is type safe. That isn't a trick. I reviewed lib/Bitcode/Reader/Deserialize.cpp Deserializer::ReadUIntPtr. :frowning: I don't see anything that is type safe about this code.

Huh? So, it isn't a trick and indeed legal? Or it's not type safe and illegal?

I said that it's a trick to silence GCC's warning, not that it is actually legal.

Sebastian

11

If by it, you mean playing with the addresses of memory, yes, that is legal. You can play all you want with the address, no harm will come of it. That _part_ isn't a trick and is valid.

If you mean, can you then use those addresses after playing with them. Yes, provided that the type of access is the same. In the case of ReadUIntPtr, it is wrong, the type is different, as I have said.

12

Sebastian Redl wrote:

Argiris Kirtzidis wrote:

Here's a similar case, doesn't gcc has a problem with this ?

  template <typename T>
  void ReadPtr(T*& PtrRef, const SerializedPtrID& PtrID,
               bool AllowBackpatch = true) {
    ReadUIntPtr(reinterpret_cast<uintptr_t&>(PtrRef), PtrID, AllowBackpatch);
  }
  

It's a trick to avoid GCC's warning. Because the function takes a reference and passes a reference on, the type-punned "pointer" (reference, actually) is never "dereferenced" (accessed). The actual ReadUIntPtr accesses it, but by then GCC has forgotten the original type.

What's the difference with

   D.ReadUIntPtr(reinterpret_cast<uintptr_t&>(MDC->SemanticDC), SemaDCPtrID);

The type-punned pointer is never dereferenced either.

13

* Argiris Kirtzidis:

What's the difference with

   D.ReadUIntPtr(reinterpret_cast<uintptr_t&>(MDC->SemanticDC),
SemaDCPtrID);

The type-punned pointer is never dereferenced either.

You write an object of pointer type as an uintptr_t, which is not
allowed. Read access would be fine, though.

14

* Mike Stump:

It is really easy to fix, for example:

PtrRef = 0;

becomes:

   uintptr_t x = 0;
   memcpy (&PtrRef, x, sizeof());

suddenly, perfectly type safe, and, not an instruction larger, not a
single clock slower to boot.

Unless you compile with -fno-builtins because that makes other code
run quite a bit faster.

15

Mike,

Thanks for reviewing the deserializer code. If you are up for it, it would be really helpful if you submitted a patch that addresses your concerns. That way our dialogue can certain around your patch.

Ted

16

Odd, sounds like a very broken compiler if you have to compile clang with that.