new warnings

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

new warnings

Mike Stump
/Volumes/SandBox/clang/clang/clang/unittests/VMCore/MetadataTest.cpp:  
In member function 'virtual  
void<unnamed>::MDNodeTest_Everything_Test::TestBody()':
/Volumes/SandBox/clang/clang/clang/unittests/VMCore/MetadataTest.cpp:
76: warning: dereferencing type-punned pointer will break strict-
aliasing rules

:-(
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: new warnings

Tanya Lattner-2

Mike,

Can you please just respond to the specific patch on llvm-commits instead
of emailing llvm-dev?

Thanks,
-Tanya

On Tue, 7 Apr 2009, Mike Stump wrote:

> /Volumes/SandBox/clang/clang/clang/unittests/VMCore/MetadataTest.cpp:
> In member function 'virtual
> void<unnamed>::MDNodeTest_Everything_Test::TestBody()':
> /Volumes/SandBox/clang/clang/clang/unittests/VMCore/MetadataTest.cpp:
> 76: warning: dereferencing type-punned pointer will break strict-
> aliasing rules
>
> :-(
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: new warnings

Mike Stump
On Apr 7, 2009, at 6:02 PM, Tanya M. Lattner wrote:
> Can you please just respond to the specific patch on llvm-commits  
> instead
> of emailing llvm-dev?

Don't happen to know which checkin caused it...
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: new warnings

Devang Patel

On Apr 7, 2009, at 6:22 PM, Mike Stump wrote:

> On Apr 7, 2009, at 6:02 PM, Tanya M. Lattner wrote:
>> Can you please just respond to the specific patch on llvm-commits
>> instead
>> of emailing llvm-dev?
>
> Don't happen to know which checkin caused it...

In that case email to llvm-commits list without a reference check-in  
OR email to llvmbugs list. A patch to fix these warning would be more  
appropriate for llvmdev list :)

-
Devang


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: new warnings

Nick Lewycky
In reply to this post by Mike Stump
Mike Stump wrote:
> On Apr 7, 2009, at 6:02 PM, Tanya M. Lattner wrote:
>> Can you please just respond to the specific patch on llvm-commits  
>> instead
>> of emailing llvm-dev?
>
> Don't happen to know which checkin caused it...

Given that there's only ever been one checkin to this file, it can't be
that hard. :)

But I'm not sure what it's complaining about. MDNode isa Constant, 'n1'
isa MDNode*, hence &n1 isa MDNode**. Casting that to Constant** breaks
strict-aliasing rules? Really?

Nick
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: new warnings

Jeffrey Yasskin
On Tue, Apr 7, 2009 at 8:31 PM, Nick Lewycky <[hidden email]> wrote:

> Mike Stump wrote:
>> On Apr 7, 2009, at 6:02 PM, Tanya M. Lattner wrote:
>>> Can you please just respond to the specific patch on llvm-commits
>>> instead
>>> of emailing llvm-dev?
>>
>> Don't happen to know which checkin caused it...
>
> Given that there's only ever been one checkin to this file, it can't be
> that hard. :)
>
> But I'm not sure what it's complaining about. MDNode isa Constant, 'n1'
> isa MDNode*, hence &n1 isa MDNode**. Casting that to Constant** breaks
> strict-aliasing rules? Really?

Yes, because MDNode* is a different type from Constant*. In the C++0X
draft, it's in section 3.10:

If a program attempts to access the stored value of an object through
an lvalue of other than one of the
following types the behavior is undefined

— the dynamic type of the object,
— a cv-qualified version of the dynamic type of the object,
— a type similar (as defined in 4.4) to the dynamic type of the object,
(This describes const/volatile qualification)
— a type that is the signed or unsigned type corresponding to the
dynamic type of the object,
— a type that is the signed or unsigned type corresponding to a
cv-qualified version of the dynamic type
of the object,
— an aggregate or union type that includes one of the aforementioned
types among its members (including,
recursively, a member of a subaggregate or contained union),
— a type that is a (possibly cv-qualified) base class type of the
dynamic type of the object,
— a char or unsigned char type.

Constant* is not a base class type of MDNode* even though Constant is
a base class of MDNode. The conversion would be unsafe even without
strict aliasing if you weren't actually going to Constant*const*.

You should instead write:

Constant *const tmp = n1;
MDNode *n2 = MDNode::get(&tmp, 1);

I'm not sure about LLVM's conventions, but I think it's a good idea to
avoid C-style cases in favor of the more specific (but longer) C++
ones. Then the compiler will tell you about dangerous casts by making
you use reinterpret_cast or const_cast to do them. And
reinterpret_cast is only safe when going to (or sometimes from) char*
or intptr_t.

Jeffrey

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: new warnings

John McCall-2
In reply to this post by Nick Lewycky
On Apr 7, 2009, at 8:31 PM, Nick Lewycky wrote:
> But I'm not sure what it's complaining about. MDNode isa Constant,  
> 'n1'
> isa MDNode*, hence &n1 isa MDNode**.  Casting that to Constant**
> breaks strict-aliasing rules? Really?

Yes.  Constant* is not the same type as MDNode*, ergo loading from an  
MDNode** as if it were a Constant** is an aliasing violation.  It's  
also just a bad idea for several practical reasons:
1.  C++ subtyping is coercive, i.e. it's not necessarily true that  
converting an MDNode* to a Constant* is a no-op.  It probably is in  
this case, of course, or the code just wouldn't work.
2.  Even if the coercion is a no-op, that cast can easily break the  
type system if you, say, assign an arbitrary Constant* to the  
Constant**.

John.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: new warnings

Nick Lewycky
In reply to this post by Jeffrey Yasskin
Jeffrey Yasskin wrote:

> On Tue, Apr 7, 2009 at 8:31 PM, Nick Lewycky <[hidden email]> wrote:
>> Mike Stump wrote:
>>> On Apr 7, 2009, at 6:02 PM, Tanya M. Lattner wrote:
>>>> Can you please just respond to the specific patch on llvm-commits
>>>> instead
>>>> of emailing llvm-dev?
>>> Don't happen to know which checkin caused it...
>> Given that there's only ever been one checkin to this file, it can't be
>> that hard. :)
>>
>> But I'm not sure what it's complaining about. MDNode isa Constant, 'n1'
>> isa MDNode*, hence &n1 isa MDNode**. Casting that to Constant** breaks
>> strict-aliasing rules? Really?
>
> Yes, because MDNode* is a different type from Constant*. In the C++0X
> draft, it's in section 3.10:
>
> If a program attempts to access the stored value of an object through
> an lvalue of other than one of the
> following types the behavior is undefined

Thanks. I knew about the 'Derived** is incompatible with Base**' rule
for containers and why you can't just downcast to pass to a callee (it
might try to insert a Derived2*) but for no good reason thought that
different rules applied to type aliasing.

> I'm not sure about LLVM's conventions, but I think it's a good idea to
> avoid C-style cases in favor of the more specific (but longer) C++
> ones. Then the compiler will tell you about dangerous casts by making
> you use reinterpret_cast or const_cast to do them. And
> reinterpret_cast is only safe when going to (or sometimes from) char*
> or intptr_t.

Yes, and in this case neither dynamic_cast nor static_cast would compile.

Sadly, I actually did think about it and wrongly conclude that what I
was doing was okay. Apparently I daydream new paragraphs in the C++
standard now. ;-)

Nick

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev