[PATCH] Setting a symbol's type to function has no effect if its type is gnu_indirect_function.

[Peter Collingbourne]

  Use a more systematic approach.

http://llvm-reviews.chandlerc.com/D607

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D607?vs=1473&id=1579#toc

Files:
  lib/MC/MCELFStreamer.cpp
  test/MC/ELF/type.s

Index: lib/MC/MCELFStreamer.cpp
===================================================================
--- lib/MC/MCELFStreamer.cpp
+++ lib/MC/MCELFStreamer.cpp
@@ -126,6 +126,26 @@
   Alias->setVariableValue(Value);
 }
 
+// When GNU as encounters more than one .type declaration for an object it seems
+// to use a mechanism similar to the one below to decide which type is actually
+// used in the object file.  The greater of T1 and T2 is selected based on the
+// following ordering:
+//  STT_NOTYPE < STT_OBJECT < STT_FUNC < STT_GNU_IFUNC < STT_TLS < anything else
+// If neither T1 < T2 nor T2 < T1 according to this ordering, use T2 (the user
+// provided type).
+static unsigned CombineSymbolTypes(unsigned T1, unsigned T2) {
+  unsigned TypeOrdering[] = {ELF::STT_NOTYPE, ELF::STT_OBJECT, ELF::STT_FUNC,
+                             ELF::STT_GNU_IFUNC, ELF::STT_TLS};
+  for (unsigned i = 0; i != sizeof(TypeOrdering)/sizeof(TypeOrdering[0]); ++i) {
+    if (T1 == TypeOrdering[i])
+      return T2;
+    if (T2 == TypeOrdering[i])
+      return T1;
+  }
+
+  return T2;
+}
+
 void MCELFStreamer::EmitSymbolAttribute(MCSymbol *Symbol,
                                           MCSymbolAttr Attribute) {
   // Indirect symbols are handled differently, to match how 'as' handles
@@ -187,27 +207,34 @@
     break;
 
   case MCSA_ELF_TypeFunction:
-    MCELF::SetType(SD, ELF::STT_FUNC);
+    MCELF::SetType(SD, CombineSymbolTypes(MCELF::GetType(SD),
+                                          ELF::STT_FUNC));
     break;
 
   case MCSA_ELF_TypeIndFunction:
-    MCELF::SetType(SD, ELF::STT_GNU_IFUNC);
+    MCELF::SetType(SD, CombineSymbolTypes(MCELF::GetType(SD),
+                                          ELF::STT_GNU_IFUNC));
     break;
 
   case MCSA_ELF_TypeObject:
-    MCELF::SetType(SD, ELF::STT_OBJECT);
+    MCELF::SetType(SD, CombineSymbolTypes(MCELF::GetType(SD),
+                                          ELF::STT_OBJECT));
     break;
 
   case MCSA_ELF_TypeTLS:
-    MCELF::SetType(SD, ELF::STT_TLS);
+    MCELF::SetType(SD, CombineSymbolTypes(MCELF::GetType(SD),
+                                          ELF::STT_TLS));
     break;
 
   case MCSA_ELF_TypeCommon:
-    MCELF::SetType(SD, ELF::STT_COMMON);
+    // TODO: Emit these as a common symbol.
+    MCELF::SetType(SD, CombineSymbolTypes(MCELF::GetType(SD),
+                                          ELF::STT_OBJECT));
     break;
 
   case MCSA_ELF_TypeNoType:
-    MCELF::SetType(SD, ELF::STT_NOTYPE);
+    MCELF::SetType(SD, CombineSymbolTypes(MCELF::GetType(SD),
+                                          ELF::STT_NOTYPE));
     break;
 
   case MCSA_Protected:
Index: test/MC/ELF/type.s
===================================================================
--- test/MC/ELF/type.s
+++ test/MC/ELF/type.s
@@ -12,35 +12,70 @@
 // Test that gnu_unique_object is accepted.
         .type zed, at gnu_unique_object
 
+obj:
+        .global obj
+        .type obj, at object
+        .type obj, at notype
+
+func:
+        .global func
+        .type func, at function
+        .type func, at object
+
 ifunc:
         .global ifunc
         .type ifunc, at gnu_indirect_function
+        .type ifunc, at function
+
+tls:
+        .global tls
+        .type tls, at tls_object
+        .type tls, at gnu_indirect_function
 
-// CHECK:      # Symbol 4
-// CHECK-NEXT: (('st_name', 0x00000005) # 'bar'
+// CHECK:      (('st_name', {{.*}}) # 'bar'
 // CHECK-NEXT:  ('st_bind', 0x1)
 // CHECK-NEXT:  ('st_type', 0x1)
 // CHECK-NEXT:  ('st_other', 0x00)
 // CHECK-NEXT:  ('st_shndx', 0x0001)
 // CHECK-NEXT:  ('st_value', 0x0000000000000000)
 // CHECK-NEXT:  ('st_size', 0x0000000000000000)
-// CHECK-NEXT: ),
-// CHECK-NEXT: # Symbol 5
-// CHECK-NEXT: (('st_name', 0x00000001) # 'foo'
+
+// CHECK:      (('st_name', {{.*}}) # 'foo'
+// CHECK-NEXT:  ('st_bind', 0x1)
+// CHECK-NEXT:  ('st_type', 0x2)
+// CHECK-NEXT:  ('st_other', 0x00)
+// CHECK-NEXT:  ('st_shndx', 0x0001)
+// CHECK-NEXT:  ('st_value', 0x0000000000000000)
+// CHECK-NEXT:  ('st_size', 0x0000000000000000)
+
+// CHECK:      (('st_name', {{.*}}) # 'func'
 // CHECK-NEXT:  ('st_bind', 0x1)
 // CHECK-NEXT:  ('st_type', 0x2)
 // CHECK-NEXT:  ('st_other', 0x00)
 // CHECK-NEXT:  ('st_shndx', 0x0001)
 // CHECK-NEXT:  ('st_value', 0x0000000000000000)
 // CHECK-NEXT:  ('st_size', 0x0000000000000000)
-// CHECK-NEXT: ),
-// CHECK-NEXT: # Symbol 6
-// CHECK-NEXT: (('st_name', 0x00000009) # 'ifunc'
+
+// CHECK:      (('st_name', {{.*}}) # 'ifunc'
 // CHECK-NEXT:  ('st_bind', 0x1)
 // CHECK-NEXT:  ('st_type', 0xa)
 // CHECK-NEXT:  ('st_other', 0x00)
 // CHECK-NEXT:  ('st_shndx', 0x0001)
 // CHECK-NEXT:  ('st_value', 0x0000000000000000)
 // CHECK-NEXT:  ('st_size', 0x0000000000000000)
-// CHECK-NEXT: ),
 
+// CHECK:      (('st_name', {{.*}}) # 'obj'
+// CHECK-NEXT:  ('st_bind', 0x1)
+// CHECK-NEXT:  ('st_type', 0x1)
+// CHECK-NEXT:  ('st_other', 0x00)
+// CHECK-NEXT:  ('st_shndx', 0x0001)
+// CHECK-NEXT:  ('st_value', 0x0000000000000000)
+// CHECK-NEXT:  ('st_size', 0x0000000000000000)
+
+// CHECK:      (('st_name', {{.*}}) # 'tls'
+// CHECK-NEXT:  ('st_bind', 0x1)
+// CHECK-NEXT:  ('st_type', 0x6)
+// CHECK-NEXT:  ('st_other', 0x00)
+// CHECK-NEXT:  ('st_shndx', 0x0001)
+// CHECK-NEXT:  ('st_value', 0x0000000000000000)
+// CHECK-NEXT:  ('st_size', 0x0000000000000000)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D607.2.patch
Type: text/x-patch
Size: 5294 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130409/53315a68/attachment.bin>