Reland "[Clang][CodeGen][ObjC]: Fix CoreFoundation on ELF with `-fconstant-cfstrings`"
authorKristina Brooks <kristina@nym.hush.com>
Tue, 25 Sep 2018 22:27:40 +0000 (22:27 +0000)
committerKristina Brooks <kristina@nym.hush.com>
Tue, 25 Sep 2018 22:27:40 +0000 (22:27 +0000)
Relanding rL342883 with more fragmented tests to test ELF-specific
section emission separately from broad-scope CFString tests. Now this
tests the following separately

1). CoreFoundation builds and linkage for ELF while building it.
2). CFString ELF section emission outside CF in assembly output.
3). Broad scope `cfstring3.c` tests which cover all object formats at
    bitcode level and assembly level (including ELF).

This fixes non-bridged CoreFoundation builds on ELF targets
that use -fconstant-cfstrings. The original changes from differential
for a similar patch to PE/COFF (https://reviews.llvm.org/D44491) did not
check for an edge case where the global could be a constant which surfaced
as an issue when building for ELF because of different linkage semantics.

This patch addresses several issues with crashes related to CF builds on ELF
as well as improves data layout by ensuring string literals that back
the actual CFConstStrings end up in .rodata in line with Mach-O.

Change itself tested with CoreFoundation on Linux x86_64 but should be valid
for BSD-like systems as well that use ELF as the native object format.

Differential Revision: https://reviews.llvm.org/D52344

llvm-svn: 343038

clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/cfstring-elf-cfbuild-x86_64.c [new file with mode: 0644]
clang/test/CodeGen/cfstring-elf-sections-x86_64.c [new file with mode: 0644]
clang/test/CodeGen/cfstring3.c [moved from clang/test/CodeGen/CFStrings.c with 89% similarity]

index 85fed52..baafa98 100644 (file)
@@ -4109,37 +4109,48 @@ CodeGenModule::GetAddrOfConstantCFString(const StringLiteral *Literal) {
 
   llvm::Constant *Zero = llvm::Constant::getNullValue(Int32Ty);
   llvm::Constant *Zeros[] = { Zero, Zero };
-
+  
   // If we don't already have it, get __CFConstantStringClassReference.
   if (!CFConstantStringClassRef) {
     llvm::Type *Ty = getTypes().ConvertType(getContext().IntTy);
     Ty = llvm::ArrayType::get(Ty, 0);
-    llvm::GlobalValue *GV = cast<llvm::GlobalValue>(
-        CreateRuntimeVariable(Ty, "__CFConstantStringClassReference"));
-
-    if (getTriple().isOSBinFormatCOFF()) {
-      IdentifierInfo &II = getContext().Idents.get(GV->getName());
-      TranslationUnitDecl *TUDecl = getContext().getTranslationUnitDecl();
-      DeclContext *DC = TranslationUnitDecl::castToDeclContext(TUDecl);
-
-      const VarDecl *VD = nullptr;
-      for (const auto &Result : DC->lookup(&II))
-        if ((VD = dyn_cast<VarDecl>(Result)))
-          break;
-
-      if (!VD || !VD->hasAttr<DLLExportAttr>()) {
-        GV->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
-        GV->setLinkage(llvm::GlobalValue::ExternalLinkage);
-      } else {
-        GV->setDLLStorageClass(llvm::GlobalValue::DLLExportStorageClass);
-        GV->setLinkage(llvm::GlobalValue::ExternalLinkage);
+    llvm::Constant *C =
+        CreateRuntimeVariable(Ty, "__CFConstantStringClassReference");
+    
+    if (getTriple().isOSBinFormatELF() || getTriple().isOSBinFormatCOFF()) {
+      llvm::GlobalValue *GV = nullptr;
+      
+      if ((GV = dyn_cast<llvm::GlobalValue>(C))) {
+        IdentifierInfo &II = getContext().Idents.get(GV->getName());
+        TranslationUnitDecl *TUDecl = getContext().getTranslationUnitDecl();
+        DeclContext *DC = TranslationUnitDecl::castToDeclContext(TUDecl);
+
+        const VarDecl *VD = nullptr;
+        for (const auto &Result : DC->lookup(&II))
+          if ((VD = dyn_cast<VarDecl>(Result)))
+            break;
+          
+        if (getTriple().isOSBinFormatELF()) {
+          if (!VD)
+            GV->setLinkage(llvm::GlobalValue::ExternalLinkage);
+        }
+        else {
+          if (!VD || !VD->hasAttr<DLLExportAttr>()) {
+            GV->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
+            GV->setLinkage(llvm::GlobalValue::ExternalLinkage);
+          } else {
+            GV->setDLLStorageClass(llvm::GlobalValue::DLLExportStorageClass);
+            GV->setLinkage(llvm::GlobalValue::ExternalLinkage);
+          }
+        }
+        
+        setDSOLocal(GV);
       }
     }
-    setDSOLocal(GV);
-
+  
     // Decay array -> ptr
     CFConstantStringClassRef =
-        llvm::ConstantExpr::getGetElementPtr(Ty, GV, Zeros);
+        llvm::ConstantExpr::getGetElementPtr(Ty, C, Zeros);
   }
 
   QualType CFTy = getContext().getCFConstantStringType();
@@ -4185,7 +4196,11 @@ CodeGenModule::GetAddrOfConstantCFString(const StringLiteral *Literal) {
   if (getTriple().isOSBinFormatMachO())
     GV->setSection(isUTF16 ? "__TEXT,__ustring"
                            : "__TEXT,__cstring,cstring_literals");
-
+  // Make sure the literal ends up in .rodata to allow for safe ICF and for
+  // the static linker to adjust permissions to read-only later on.
+  else if (getTriple().isOSBinFormatELF())
+    GV->setSection(".rodata");
+  
   // String.
   llvm::Constant *Str =
       llvm::ConstantExpr::getGetElementPtr(GV->getValueType(), GV, Zeros);
diff --git a/clang/test/CodeGen/cfstring-elf-cfbuild-x86_64.c b/clang/test/CodeGen/cfstring-elf-cfbuild-x86_64.c
new file mode 100644 (file)
index 0000000..8b90d7c
--- /dev/null
@@ -0,0 +1,36 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple x86_64-elf -DCF_BUILDING_CF -DDECL -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-IN-CF-DECL
+// RUN: %clang_cc1 -triple x86_64-elf -DCF_BUILDING_CF -DDEFN -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-IN-CF-DEFN
+// RUN: %clang_cc1 -triple x86_64-elf -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF
+// RUN: %clang_cc1 -triple x86_64-elf -DEXTERN -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-EXTERN
+
+// RUN: %clang_cc1 -Os -triple x86_64-elf -DCF_BUILDING_CF -DDECL -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-IN-CF-DECL
+// RUN: %clang_cc1 -Os -triple x86_64-elf -DCF_BUILDING_CF -DDEFN -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-IN-CF-DEFN
+// RUN: %clang_cc1 -Os -triple x86_64-elf -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF
+// RUN: %clang_cc1 -Os -triple x86_64-elf -DEXTERN -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-EXTERN
+
+
+#if defined(CF_BUILDING_CF)
+#if defined(DECL)
+extern long __CFConstantStringClassReference[];
+#elif defined(DEFN)
+long __CFConstantStringClassReference[32];
+#endif
+#else
+#if defined(EXTERN)
+extern long __CFConstantStringClassReference[];
+#else
+long __CFConstantStringClassReference[];
+#endif
+#endif
+
+typedef struct __CFString *CFStringRef;
+const CFStringRef string = (CFStringRef)__builtin___CFStringMakeConstantString("string");
+
+
+// CHECK-CF-IN-CF-DECL: @__CFConstantStringClassReference = external global [0 x i32]
+// CHECK-CF-IN-CF-DEFN: @__CFConstantStringClassReference = common global [32 x i64] zeroinitializer, align 16
+// CHECK-CF: @__CFConstantStringClassReference = common global [1 x i64] zeroinitializer, align 8
+// CHECK-CF-EXTERN: @__CFConstantStringClassReference = external global [0 x i32]
+// CHECK-CF-EXTERN: @.str = private unnamed_addr constant [7 x i8] c"string\00", section ".rodata", align 1
diff --git a/clang/test/CodeGen/cfstring-elf-sections-x86_64.c b/clang/test/CodeGen/cfstring-elf-sections-x86_64.c
new file mode 100644 (file)
index 0000000..439113a
--- /dev/null
@@ -0,0 +1,23 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple x86_64-elf -S %s -o - | FileCheck %s -check-prefix CHECK-ELF-DATA-SECTION
+
+typedef struct __CFString *CFStringRef;
+const CFStringRef one = (CFStringRef)__builtin___CFStringMakeConstantString("one");
+const CFStringRef two = (CFStringRef)__builtin___CFStringMakeConstantString("\xef\xbf\xbd\x74\xef\xbf\xbd\x77\xef\xbf\xbd\x6f");
+
+// CHECK-ELF-DATA-SECTION: .type .L.str,@object
+// CHECK-ELF-DATA-SECTION: .section .rodata,"a",@progbits
+// CHECK-ELF-DATA-SECTION: .L.str:
+// CHECK-ELF-DATA-SECTION: .asciz "one"
+
+// CHECK-ELF-DATA-SECTION: .type .L.str.1,@object
+// CHECK-ELF-DATA-SECTION: .section .rodata,"a",@progbits
+// CHECK-ELF-DATA-SECTION: .L.str.1:
+// CHECK-ELF-DATA-SECTION: .short 65533
+// CHECK-ELF-DATA-SECTION: .short 116
+// CHECK-ELF-DATA-SECTION: .short 65533
+// CHECK-ELF-DATA-SECTION: .short 119
+// CHECK-ELF-DATA-SECTION: .short 65533
+// CHECK-ELF-DATA-SECTION: .short 111
+// CHECK-ELF-DATA-SECTION: .short 0
similarity index 89%
rename from clang/test/CodeGen/CFStrings.c
rename to clang/test/CodeGen/cfstring3.c
index 4edb5ff..48714f3 100644 (file)
@@ -7,7 +7,6 @@
 // RUN: %clang_cc1 -triple armv7-elf -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-ELF -check-prefix CHECK-ELF32
 // RUN: %clang_cc1 -triple i686-elf -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-ELF -check-prefix CHECK-ELF32
 // RUN: %clang_cc1 -triple x86_64-elf -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-ELF -check-prefix CHECK-ELF64
-// RUN: %clang_cc1 -triple armv7-elf -S %s -o - | FileCheck %s -check-prefix CHECK-ELF-DATA-SECTION
 
 // RUN: %clang_cc1 -triple armv7-macho -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-MACHO -check-prefix CHECK-MACHO32
 // RUN: %clang_cc1 -triple i386-apple-macosx -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-MACHO -check-prefix CHECK-MACHO32
@@ -22,7 +21,7 @@ const CFStringRef one = (CFStringRef)__builtin___CFStringMakeConstantString("one
 const CFStringRef two = (CFStringRef)__builtin___CFStringMakeConstantString("\xef\xbf\xbd\x74\xef\xbf\xbd\x77\xef\xbf\xbd\x6f");
 
 // CHECK-COFF: @.str = private unnamed_addr constant [4 x i8] c"one\00", align 1
-// CHECK-ELF: @.str = private unnamed_addr constant [4 x i8] c"one\00", align 1
+// CHECK-ELF: @.str = private unnamed_addr constant [4 x i8] c"one\00", section ".rodata", align 1
 // CHECK-MACHO: @.str = private unnamed_addr constant [4 x i8] c"one\00", section "__TEXT,__cstring,cstring_literals", align 1
 
 // CHECK-COFF: @_unnamed_cfstring_ = private global %struct.__NSConstantString_tag { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @__CFConstantStringClassReference, i32 0, i32 0), i32 1992, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i32 0, i32 0), i32 3 }, section "cfstring", align {{[48]}}
@@ -32,7 +31,7 @@ const CFStringRef two = (CFStringRef)__builtin___CFStringMakeConstantString("\xe
 // CHECK-MACHO64: @_unnamed_cfstring_ = private global %struct.__NSConstantString_tag { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @__CFConstantStringClassReference, i32 0, i32 0), i32 1992, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i32 0, i32 0), i64 3 }, section "__DATA,__cfstring", align 8
 
 // CHECK-COFF: @.str.1 = private unnamed_addr constant [7 x i16] [i16 -3, i16 116, i16 -3, i16 119, i16 -3, i16 111, i16 0], align 2
-// CHECK-ELF: @.str.1 = private unnamed_addr constant [7 x i16] [i16 -3, i16 116, i16 -3, i16 119, i16 -3, i16 111, i16 0], align 2
+// CHECK-ELF: @.str.1 = private unnamed_addr constant [7 x i16] [i16 -3, i16 116, i16 -3, i16 119, i16 -3, i16 111, i16 0], section ".rodata", align 2
 // CHECK-MACHO: @.str.1 = private unnamed_addr constant [7 x i16] [i16 -3, i16 116, i16 -3, i16 119, i16 -3, i16 111, i16 0], section "__TEXT,__ustring", align 2
 
 // CHECK-COFF: @_unnamed_cfstring_.2 = private global %struct.__NSConstantString_tag { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @__CFConstantStringClassReference, i32 0, i32 0), i32 2000, i8* bitcast ([7 x i16]* @.str.1 to i8*), i32 6 }, section "cfstring", align {{[48]}}
@@ -41,19 +40,6 @@ const CFStringRef two = (CFStringRef)__builtin___CFStringMakeConstantString("\xe
 // CHECK-MACHO32: @_unnamed_cfstring_.2 = private global %struct.__NSConstantString_tag { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @__CFConstantStringClassReference, i32 0, i32 0), i32 2000, i8* bitcast ([7 x i16]* @.str.1 to i8*), i32 6 }, section "__DATA,__cfstring", align 4
 // CHECK-MACHO64: @_unnamed_cfstring_.2 = private global %struct.__NSConstantString_tag { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @__CFConstantStringClassReference, i32 0, i32 0), i32 2000, i8* bitcast ([7 x i16]* @.str.1 to i8*), i64 6 }, section "__DATA,__cfstring", align 8
 
-// CHECK-ELF-DATA-SECTION: .section .rodata.str1.1
-// CHECK-ELF-DATA-SECTION: .asciz "one"
-
-// CHECK-ELF-DATA-SECTION: .section .rodata.str2.2
-// CHECK-ELF-DATA-SECTION: .short 65533
-// CHECK-ELF-DATA-SECTION: .short 116
-// CHECK-ELF-DATA-SECTION: .short 65533
-// CHECK-ELF-DATA-SECTION: .short 119
-// CHECK-ELF-DATA-SECTION: .short 65533
-// CHECK-ELF-DATA-SECTION: .short 111
-// CHECK-ELF-DATA-SECTION: .short 0
-
 // CHECK-ASM-COFF: .section cfstring,"dw"
-// CHECK-ASM-ELF: .section cfstring,"aw"
+// CHECK-ASM-ELF: .section cfstring,"aw",@progbits
 // CHECK-ASM-MACHO: .section __DATA,__cfstring
-