[ARM] Stop the LLVM ELF streamer uppercasing NTBS data in build attributes.

[• chatur01]

Summary

For example,

$ llvm-mc -triple armv7-elf -filetype obj -o test.o
.eabi_attribute  Tag_compatibility, 136, "Foo Corp"
$ llvm-readobj -arm-attributes test.o 
...
      Attribute {
        Tag: 136
        TagName: compatibility
        Value: FOO CORP
...

I originally discovered this behaviour was suspect when Tim Northover commented on D6319, where he said

"What's doing the capitalisation here? It seems highly suspect (not mentioned in the ABI as far as I can tell, and gets into sticky locale questions)."

ARMTargetELFStreamer::finishAttributeSection unconditionally uppercases the string-valued data for build attributes, I'm not sure why.

The git logs don't shed any light on the matter for me. There was a vague reference to gas compatibility, but as of gas 2.23.2.20140731, this is not the observed behavior, the capitalization is preserved. I haven't checked older versions.

All I have managed to find is a mail discussing the behavior of an old codesourcery toolchain which uppercased .cpu values in object files, but not assembly files,

http://comments.gmane.org/gmane.comp.compilers.llvm.cvs/87133

The following commit by Jason Kim implemented that behavior,

http://llvm.org/viewvc/llvm-project?view=revision&revision=125025

and Jason mentions this again with reference to another patch proposal that this might be expected behavior,

http://thread.gmane.org/gmane.comp.compilers.llvm.cvs/88234/focus=88261

I'm CC'ing Jason to this issue because maybe he has some background I'm not aware of on this topic. (Not that .cpu is a build-attribute, it's just all I could find!)

I wonder if the build attribute values are being upper-cased because of the precedent set for .cpu, but that's just wild conjecture. Recent GNU tools camel case .cpu names into the object file, do we captialise for compatibility with old versions of ld perhaps?

The ARM ABI document does not mention anything about captialisation conventions, so the existing behaviour doesn't appear technically incorrect, but there are a few reasons why I think no text transformations should be applied unconditionally,

• It's less work
• Some vendors may legitimately have case-sensitive checks for these attributes which would fail on LLVM generated object files.
• Locale questions? (Tim's comment).

This investigation was mostly spurred on by Tim's comments. I'm not sure myself whether this patch is the right thing for LLVM to do, I'm submitting it primarily to help frame my question.

Thank you for your time,
Charlie.

[rengolin]

Hi Charlie,

I'm not sure why it was like this, but I agree that we should not capitalise. Maybe Tim could comment about his view before we can finally commit this, as I want to hear his concerns.

cheers,
--renato

[t.p.northover]

I'm not sure why it was like this, but I agree that we should not capitalise. Maybe Tim could comment about his view before we can finally commit this, as I want to hear his concerns.

Yep, I agree fully. I was just hanging back to give Jason a chance to
comment, in case we were missing something. Perhaps give it another
few days in case he's up to his eyes in other stuff. Otherwise assume
he's got nothing to say.

Cheers.

Tim.

[• chatur01]

Thanks for looking Renato & Tim. I'm going to commit this tomorrow as I haven't heard from Jason or another concerned party.

Kind regards,
Charlie.

[• chatur01]

Committed as r222882.

Thanks,
Charlie.

[jasonk]

Sorry for the delay

lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp

       Streamer.EmitULEB128IntValue(item.IntValue);
       break;
     case AttributeItem::TextAttribute:
-      Streamer.EmitBytes(item.StringValue.upper());
+      Streamer.EmitBytes(item.StringValue);
       Streamer.EmitIntValue(0, 1); // '\0'
       break;
     case AttributeItem::NumericAndTextAttributes:
       Streamer.EmitULEB128IntValue(item.IntValue);
-      Streamer.EmitBytes(item.StringValue.upper());
+      Streamer.EmitBytes(item.StringValue);
       Streamer.EmitIntValue(0, 1); // '\0'
       break;
     }

test/CodeGen/ARM/build-attributes-encoding.s

 // CHECK-NEXT:     EntrySize: 0
 // CHECK-NEXT:     SectionData (
 // CHECK-NEXT:       0000: 41460000 00616561 62690001 3C000000
-// CHECK-NEXT:       0010: 05434F52 5445582D 41380006 0A074108
+// CHECK-NEXT:       0010: 05636F72 7465782D 61380006 0A074108
 // CHECK-NEXT:       0020: 0109020A 030C0214 01150117 01180119
 // CHECK-NEXT:       0030: 011B001C 0124012A 012C0244 036EA001
 // CHECK-NEXT:       0040: 81013100 FA0101

test/MC/ARM/directive-arch-iwmmxt.s

 @ CHECK-ATTR: FileAttributes {
 @ CHECK-ATTR:   Attribute {
 @ CHECK-ATTR:     TagName: CPU_name
-@ CHECK-ATTR:     Value: IWMMXT
+@ CHECK-ATTR:     Value: iwmmxt
 @ CHECK-ATTR:   }
 @ CHECK-ATTR:   Attribute {
 @ CHECK-ATTR:     TagName: CPU_arch

test/MC/ARM/directive-arch-iwmmxt2.s

 @ CHECK-ATTR: FileAttributes {
 @ CHECK-ATTR:   Attribute {
 @ CHECK-ATTR:     TagName: CPU_name
-@ CHECK-ATTR:     Value: IWMMXT2
+@ CHECK-ATTR:     Value: iwmmxt2
 @ CHECK-ATTR:   }
 @ CHECK-ATTR:   Attribute {
 @ CHECK-ATTR:     TagName: CPU_arch

test/MC/ARM/directive-cpu.s

 @ CHECK: 10000000
 
   .cpu  cortex-a8
-@ CHECK: 05
-@ CHECK: 434F52 5445582D 413800
+@ CHECK: 05636F72 7465782D 613800
 
 @ CHECK: )

test/MC/ARM/directive-eabi_attribute-overwrite.s

 
   .syntax unified
   .thumb
-
+@ FIXME: The next directive is not correct, Tag_compatibility isn't getting parsed correctly.        
   .eabi_attribute Tag_compatibility, 1
   .eabi_attribute Tag_compatibility, 1, "aeabi"
 
 @ CHECK-ATTR: FileAttributes {
 @ CHECK-ATTR:   Attribute {
-@ CHECK-ATTR:     Value: 1, AEABI
+@ CHECK-ATTR:     Value: 1, aeabi
 @ CHECK-ATTR:     TagName: compatibility
 @ CHECK-ATTR:     Description: AEABI Conformant
 @ CHECK-ATTR:   }

test/MC/ARM/directive-eabi_attribute.s

 @ CHECK: .eabi_attribute 4, "Cortex-A9"
 @ CHECK-OBJ:        Tag: 4
 @ CHECK-OBJ-NEXT:   TagName: CPU_raw_name
-@ CHECK-OBJ-NEXT:   Value: CORTEX-A9
+@ CHECK-OBJ-NEXT:   Value: Cortex-A9
   .eabi_attribute Tag_CPU_name, "cortex-a9"
 @ CHECK: .cpu cortex-a9
 @ CHECK-OBJ:        Tag: 5
 @ CHECK-OBJ-NEXT:   TagName: CPU_name
-@ CHECK-OBJ-NEXT:   Value: CORTEX-A9
+@ CHECK-OBJ-NEXT:   Value: cortex-a9
   .eabi_attribute Tag_CPU_arch, 10
 @ CHECK: .eabi_attribute 6, 10
 @ CHECK-OBJ:        Tag: 6

   .eabi_attribute Tag_compatibility, 1, "aeabi"
 @ CHECK: .eabi_attribute 32, 1, "aeabi"
 @ CHECK-OBJ:        Tag: 32
-@ CHECK-OBJ-NEXT:   Value: 1, AEABI
+@ CHECK-OBJ-NEXT:   Value: 1, aeabi
 @ CHECK-OBJ-NEXT:   TagName: compatibility
 @ CHECK-OBJ-NEXT:   Description: AEABI Conformant
   .eabi_attribute Tag_CPU_unaligned_access, 0

 @ CHECK: .eabi_attribute 65, "gnu"
 @ CHECK-OBJ:        Tag: 65
 @ CHECK-OBJ-NEXT:   TagName: also_compatible_with
-@ CHECK-OBJ-NEXT:   Value: GNU
+@ CHECK-OBJ-NEXT:   Value: gnu
   .eabi_attribute Tag_T2EE_use, 0
 @ CHECK: .eabi_attribute 66, 0
 @ CHECK-OBJ:        Tag: 66

test/tools/llvm-readobj/ARM/attributes.s

 @ CHECK:       Attribute {
 @ CHECK:         Tag: 4
 @ CHECK:         TagName: CPU_raw_name
-@ CHECK:         Value: CORTEX-A9
+@ CHECK:         Value: Cortex-A9
 @ CHECK:       }
 @ CHECK:       Attribute {
 @ CHECK:         Tag: 5
 @ CHECK:         TagName: CPU_name
-@ CHECK:         Value: CORTEX-A9
+@ CHECK:         Value: cortex-a9
 @ CHECK:       }
 @ CHECK:       Attribute {
 @ CHECK:         Tag: 6

 @ CHECK:       }
 @ CHECK:       Attribute {
 @ CHECK:         Tag: 32
-@ CHECK:         Value: 1, AEABI
+@ CHECK:         Value: 1, aeabi
 @ CHECK:         TagName: compatibility
 @ CHECK:         Description: AEABI Conformant
 @ CHECK:       }

 @ CHECK:       Attribute {
 @ CHECK:         Tag: 65
 @ CHECK:         TagName: also_compatible_with
-@ CHECK:         Value: GNU
+@ CHECK:         Value: gnu
 @ CHECK:       }
 @ CHECK:       Attribute {
 @ CHECK:         Tag: 66