Fix for bug 21725: wrong results with union and strict-aliasing

[jeroen.dobbelaere]

Summary

Generate path-aware tbaa for unions. This fixes an aliasing issue described in bug 21725.

[jeroen.dobbelaere]

Further investigation shows that llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp is not ready to handle struct types where multiple members have the same (0) offset. The clang fix only makes sense once that issue is resolved.

[hfinkel]

In D8056#141899, @jeroen.dobbelaere wrote:

Further investigation shows that llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp is not ready to handle struct types where multiple members have the same (0) offset. The clang fix only makes sense once that issue is resolved.

Can you work on this? If not (or even if you can), can you file a bug report with an example?

[yinma]

Hello, I am wondering if the current llvm/clang's tbaa supports union or not?

lib/CodeGen/CGExpr.cpp

   if (rec->isUnion()) {                                                                                
     // For unions, there is no pointer adjustment.                                                     
     assert(!type->isReferenceType() && "union has reference member");                                  
-    // TODO: handle path-aware TBAA for union.                                                         
-    TBAAPath = false;                                                                                  
   } else {                                                                                             
     // For structs, we GEP to the field that the record layout suggests.                               
     unsigned idx = CGM.getTypes().getCGRecordLayout(rec).getLLVMFieldNo(field);                        

lib/CodeGen/CodeGenTBAA.cpp

       return false;                                                                                    
     // RD can be struct, union, class, interface or enum.                                              
     // For now, we only handle struct and class.                                                       
-    if (RD->isStruct() || RD->isClass())                                                               
+    if (RD->isStruct() || RD->isClass() || RD->isUnion())                                              
       return true;                                                                                     
   }                                                                                                    
   return false;                                                                                        

test/CodeGen/tbaa-union.cpp

+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - -O1 %s | FileCheck %s                   
+//                                                                                                     
+// Check that we generate !tbaa.struct metadata for union copies - when they are large enough          
+union A {                                                                                              
+  short s;                                                                                             
+  int i;                                                                                               
+  char c;                                                                                              
+  int j[4];                                                                                            
+};                                                                                                     
+                                                                                                       
+void copy(union A *a, union A *b) {                                                                    
+  *a = *b;                                                                                             
+}                                                                                                      
+                                                                                                       
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %{{.*}}, i8* %{{.*}}, i64 16, i32 4, i1 false), !tbaa.struct [[UNION_A:!.*]]
+                                                                                                                               
+union B {                                                                                                                      
+  char c1;                                                                                                                     
+  union A a;                                                                                                                   
+  int ii;                                                                                                                      
+};                                                                                                                             
+                                                                                                                               
+void copy2(union B *a, union B *b) {                                                                                           
+  *a = *b;                                                                                                                     
+}                                                                                                                              
+                                                                                                                               
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %{{.*}}, i8* %{{.*}}, i64 16, i32 4, i1 false), !tbaa.struct [[UNION_B:!.*]]
+                                                                                                                               
+typedef _Complex int T2;                                                                                                       
+typedef _Complex char T5;                                                                                                      
+typedef _Complex int T7;                                                                                                       
+typedef struct T4 { T5 field0; T7 field1; } T4;                                                                                
+typedef union T1 { T2 field0; T4 field1; } T1;                                                                                 
+                                                                                                                               
+void copy3 (T1 *a, T1 *b) {                                                                                                    
+  *a = *b;                                                                                                                     
+}                                                                                                                              
+                                                                                                                               
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %{{.*}}, i8* %{{.*}}, i64 12, i32 4, i1 false), !tbaa.struct [[UNION_T1:!.*]]
+                                                                                                                                
+                                                                                                                                
+union U0 {                                                                                                                      
+  short s;                                                                                                                      
+  int i;                                                                                                                        
+};                                                                                                                              
+                                                                                                                                
+U0 g_u0;                                                                                                                        
+U0* g_pu0=&g_u0;                                                                                                                
+                                                                                                                                
+int same_union_0() {                                                                                                            
+  *g_pu0=g_u0;                                                                                                                  
+  return g_u0.i;                                                                                                                
+}                                                                                                                               
+                                                                                                                                
+// CHECK:  load i32, i32* getelementptr inbounds (%{{.*}}* @g_u0, i64 0, i32 0), align 4, !tbaa [[TAG_UNION_U0__I:!.*]]         
+                                                                                                                                
+int same_union_1() {                                                                                                            
+  g_u0.s=123;                                                                                                                   
+  g_u0.i=456;                                                                                                                   
+  return g_u0.i;                                                                                                                
+}                                                                                                                               
+// CHECK:    store i32 456, i32* getelementptr inbounds (%{{.*}}* @g_u0, i64 0, i32 0), align 4, !tbaa [[TAG_UNION_U0__I]]      
+                                                                                                                                
+int same_union_2() {                                                                                                            
+  g_u0.i=456;                                                                                                                   
+  g_u0.s=123;                                                                                                                   
+  return g_u0.i;                                                                                                                
+}                                                                                                                               
+// CHECK:  store i32 456, i32* getelementptr inbounds (%{{.*}}* @g_u0, i64 0, i32 0), align 4, !tbaa [[TAG_UNION_U0__I]]        
+// CHECK-NEXT:  store i16 123, i16* bitcast (%{{.*}}* @g_u0 to i16*), align 4, !tbaa [[TAG_UNION_U0__S:!.*]]                    
+// CHECK-NEXT:  load i32, i32* getelementptr inbounds (%{{.*}}* @g_u0, i64 0, i32 0), align 4, !tbaa [[TAG_UNION_U0__I]]        
+                                                                                                                                
+                                                                                                                                
+int aliasing_access_0() {                                                                                                       
+  g_u0.s=123;    // access 'short' member of U0                                                                                 
+  g_pu0->i=456;  // access 'int' member of U0                                                                                   
+                 // Both accesses of a U0 -> reordering NOT allowed.                                                            
+                                                                                                                                
+  return g_u0.i; // should return 456                                                                                           
+}                                                                                                                               
+// CHECK:       store i16 123, i16* bitcast (%{{.*}}* @g_u0 to i16*), align 4, !tbaa [[TAG_UNION_U0__S]]                        
+// CHECK-NEXT:  load i32*, i32** bitcast (%{{.*}} @g_pu0 to i32**), align 8, !tbaa [[TAG_POINTER:!.*]]                          
+// CHECK-NEXT:  store i32 456, i32* %{{.*}}, align 4, !tbaa [[TAG_UNION_U0__I]]                                                 
+// CHECK-NEXT:  load i32, i32* getelementptr inbounds (%{{.*}}* @g_u0, i64 0, i32 0), align 4, !tbaa [[TAG_UNION_U0__I]]        
+                                                                                                                                
+int aliasing_access_1() {                                                                                                       
+  int* pi=&g_pu0->i;                                                                                                            
+                                                                                                                                
+  g_u0.s=123;   // access 'short' member of U0                                                                                  
+  *pi=456;      // access 'int'                                                                                                 
+                // There exists an 'int' member of U0 -> reordering NOT allowed.                                                
+                                                                                                                                
+  return g_u0.i; // should return 456                                                                                           
+}                                                                                                                               
+                                                                                                                                
+// CHECK:       load i32*, i32** bitcast (%{{.*}}* @g_pu0 to i32**), align 8, !tbaa [[TAG_POINTER]]                             
+// CHECK-NEXT:  store i16 123, i16* bitcast (%{{.*}}* @g_u0 to i16*), align 4, !tbaa [[TAG_UNION_U0__S]]                        
+// CHECK-NEXT:  store i32 456, i32* %{{.*}}, align 4, !tbaa [[TAG_INT:!.*]]                                                     
+// CHECK-NEXT:  load i32, i32* getelementptr inbounds (%{{.*}}* @g_u0, i64 0, i32 0), align 4, !tbaa [[TAG_UNION_U0__I]]        
+                                                                                                                                
+                                                                                                                                
+int aliasing_access_2() {                                                                                                       
+  short* ps=&g_u0.s;                                                                                                            
+  int* pi=&g_pu0->i;                                                                                                            
+                                                                                                                                
+  *ps=123; // access 'short'                                                                                                    
+  *pi=456; // access 'int'                                                                                                      
+           // no relationship between 'short' and 'int' -> reordering allowed !                                                 
+                                                                                                                                
+  return g_u0.i; // anything can happen                                                                                         
+}                                                                                                                               
+
+int aliasing_access_2b() {
+  short& ps=g_u0.s;
+  int* pi=&g_pu0->i;
+
+  ps=123; // access 'short'
+  *pi=456; // access 'int'
+           // no relationship between 'short' and 'int' -> reordering allowed !
+
+  return g_u0.i; // anything can happen
+}
+
+// CHECK:  store i16 123, i16* bitcast (%{{.*}}* @g_u0 to i16*), align 4, !tbaa [[TAG_SHORT:!.*]]
+// CHECK:  store i32 456, i32* %{{.*}}, align 4, !tbaa [[TAG_INT]]
+// CHECK:  load i32, i32* getelementptr inbounds (%{{.*}}* @g_u0, i64 0, i32 0), align 4, !tbaa [[TAG_UNION_U0__I]]
+
+int aliasing_access_3(short* ps, int* pi) {
+  *ps=123; // access 'short'
+  *pi=456; // access 'int'
+           // no relationship between 'short' and 'int' -> reordering allowed !
+
+  return g_u0.i; // anything can happen
+}
+
+// CHECK:  store i16 123, i16* %ps, align 2, !tbaa [[TAG_SHORT]]
+// CHECK:  store i32 456, i32* %pi, align 4, !tbaa [[TAG_INT]]
+// CHECK:  load i32, i32* getelementptr inbounds (%{{.*}}* @g_u0, i64 0, i32 0), align 4, !tbaa [[TAG_UNION_U0__I]]
+
+void aliasing_access_4(short* ps, int* pi) {
+  *ps=123; // access 'short'
+  *pi=456; // access 'int'
+           // no relationship between 'short' and 'int' -> reordering allowed !
+}
+
+// CHECK:  store i16 123, i16* %ps, align 2, !tbaa [[TAG_SHORT]]
+// CHECK:  store i32 456, i32* %pi, align 4, !tbaa [[TAG_INT]]
+
+
+
+// (offset, size) = (0,8) char; (0,2) char; (4,8) char
+// CHECK: [[UNION_A]] = !{i64 0, i64 2, [[TAG_SHORT]], i64 0, i64 4, [[TAG_INT]], i64 0, i64 1, [[TAG_CHAR:!.*]], i64 0, i64 16, [[TAG_CHAR]]}
+// CHECK: [[TAG_SHORT]] = !{[[SHORT:!.*]], [[SHORT]], i64 0}
+// CHECK: [[SHORT]] = !{!"short", [[CHAR:!.*]],
+// CHECK: [[CHAR]] = !{!"omnipotent char", !{{.*}}}
+// CHECK: [[TAG_INT]] = !{[[INT:!.*]], [[INT]], i64 0}
+// CHECK: [[INT]] = !{!"int", [[CHAR]]
+// CHECK: [[TAG_CHAR]] = !{[[CHAR]], [[CHAR]], i64 0}
+// CHECK: [[UNION_B]] = !{i64 0, i64 1, [[TAG_CHAR]], i64 0, i64 2, [[TAG_SHORT]], i64 0, i64 4, [[TAG_INT]], i64 0, i64 1, [[TAG_CHAR]], i64 0, i64 16, [[TAG_CHAR]], i64 0, i64 4, [[TAG_INT]]}
+// CHECK: [[UNION_T1]] = !{i64 0, i64 8, [[TAG_CHAR]], i64 0, i64 2, [[TAG_CHAR]], i64 4, i64 8, [[TAG_CHAR]]}
+// CHECK: [[TAG_POINTER]] = !{[[POINTER:!.*]], [[POINTER]], i64 0}
+// CHECK: [[POINTER]] = !{!"any pointer", [[CHAR]], i64 0}
+// CHECK: [[TAG_UNION_U0__I]] = !{[[UNION_U0:!.*]], [[INT]], i64 0}
+// CHECK: [[UNION_U0]] = !{!"_ZTS2U0", [[SHORT]], i64 0, [[INT]], i64 0}
+// CHECK: [[TAG_UNION_U0__S]] = !{[[UNION_U0]], [[SHORT]], i64 0}