Skip to content

Commit b36a18d

Browse files
sivan-shanisivan-shani
authored
[AArch64][Build Attributes] Improve Parsing and Formatting (#126530)
- Removed assertion for duplicate values as adding them is valid. - Fix parsing: reject strings for unknown tags, allow any value for Tag_PAuth_Platform and Tag_PAuth_Schema. - Print tags by using numbers with comments to reduce compiler-assembler dependencies. - Parsing error messages now only point to the symbol (^) instead of printing it.
1 parent a4656bb commit b36a18d

24 files changed

+275
-219
lines changed

‎llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -485,10 +485,10 @@ void AArch64AsmPrinter::emitAttributes(unsigned Flags,
485485
AArch64BuildAttrs::SubsectionType::ULEB128);
486486
TS->emitAttribute(
487487
AArch64BuildAttrs::getVendorName(AArch64BuildAttrs::AEABI_PAUTHABI),
488-
AArch64BuildAttrs::TAG_PAUTH_PLATFORM, PAuthABIPlatform, "", false);
488+
AArch64BuildAttrs::TAG_PAUTH_PLATFORM, PAuthABIPlatform, "");
489489
TS->emitAttribute(
490490
AArch64BuildAttrs::getVendorName(AArch64BuildAttrs::AEABI_PAUTHABI),
491-
AArch64BuildAttrs::TAG_PAUTH_SCHEMA, PAuthABIVersion, "", false);
491+
AArch64BuildAttrs::TAG_PAUTH_SCHEMA, PAuthABIVersion, "");
492492
}
493493

494494
unsigned BTIValue = (Flags & AArch64BuildAttrs::Feature_BTI_Flag) ? 1 : 0;
@@ -502,13 +502,13 @@ void AArch64AsmPrinter::emitAttributes(unsigned Flags,
502502
AArch64BuildAttrs::SubsectionType::ULEB128);
503503
TS->emitAttribute(AArch64BuildAttrs::getVendorName(
504504
AArch64BuildAttrs::AEABI_FEATURE_AND_BITS),
505-
AArch64BuildAttrs::TAG_FEATURE_BTI, BTIValue, "", false);
505+
AArch64BuildAttrs::TAG_FEATURE_BTI, BTIValue, "");
506506
TS->emitAttribute(AArch64BuildAttrs::getVendorName(
507507
AArch64BuildAttrs::AEABI_FEATURE_AND_BITS),
508-
AArch64BuildAttrs::TAG_FEATURE_PAC, PACValue, "", false);
508+
AArch64BuildAttrs::TAG_FEATURE_PAC, PACValue, "");
509509
TS->emitAttribute(AArch64BuildAttrs::getVendorName(
510510
AArch64BuildAttrs::AEABI_FEATURE_AND_BITS),
511-
AArch64BuildAttrs::TAG_FEATURE_GCS, GCSValue, "", false);
511+
AArch64BuildAttrs::TAG_FEATURE_GCS, GCSValue, "");
512512
}
513513
}
514514

‎llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7874,8 +7874,7 @@ bool AArch64AsmParser::parseDirectiveAeabiSubSectionHeader(SMLoc L) {
78747874
IsOptional = AArch64BuildAttrs::getOptionalID(Optionality);
78757875
if (AArch64BuildAttrs::OPTIONAL_NOT_FOUND == IsOptional) {
78767876
Error(Parser.getTok().getLoc(),
7877-
AArch64BuildAttrs::getSubsectionOptionalUnknownError() + ": " +
7878-
Optionality);
7877+
AArch64BuildAttrs::getSubsectionOptionalUnknownError());
78797878
return true;
78807879
}
78817880
if (SubsectionExists) {
@@ -7923,7 +7922,7 @@ bool AArch64AsmParser::parseDirectiveAeabiSubSectionHeader(SMLoc L) {
79237922
Type = AArch64BuildAttrs::getTypeID(Name);
79247923
if (AArch64BuildAttrs::TYPE_NOT_FOUND == Type) {
79257924
Error(Parser.getTok().getLoc(),
7926-
AArch64BuildAttrs::getSubsectionTypeUnknownError() + ": " + Name);
7925+
AArch64BuildAttrs::getSubsectionTypeUnknownError());
79277926
return true;
79287927
}
79297928
if (SubsectionExists) {
@@ -7952,6 +7951,7 @@ bool AArch64AsmParser::parseDirectiveAeabiSubSectionHeader(SMLoc L) {
79527951
}
79537952
}
79547953
Parser.Lex();
7954+
79557955
// Parsing finished, check for trailing tokens.
79567956
if (Parser.getTok().isNot(llvm::AsmToken::EndOfStatement)) {
79577957
Error(Parser.getTok().getLoc(), "unexpected token for AArch64 build "
@@ -7990,14 +7990,18 @@ bool AArch64AsmParser::parseDirectiveAeabiAArch64Attr(SMLoc L) {
79907990

79917991
StringRef TagStr = "";
79927992
unsigned Tag;
7993-
if (Parser.getTok().is(AsmToken::Identifier)) {
7993+
if (Parser.getTok().is(AsmToken::Integer)) {
7994+
Tag = getTok().getIntVal();
7995+
} else if (Parser.getTok().is(AsmToken::Identifier)) {
79947996
TagStr = Parser.getTok().getIdentifier();
79957997
switch (ActiveSubsectionID) {
7996-
default:
7997-
assert(0 && "Subsection name error");
7998-
break;
79997998
case AArch64BuildAttrs::VENDOR_UNKNOWN:
8000-
// Private subsection, accept any tag.
7999+
// Tag was provided as an unrecognized string instead of an unsigned
8000+
// integer
8001+
Error(Parser.getTok().getLoc(), "unrecognized Tag: '" + TagStr +
8002+
"' \nExcept for public subsections, "
8003+
"tags have to be an unsigned int.");
8004+
return true;
80018005
break;
80028006
case AArch64BuildAttrs::AEABI_PAUTHABI:
80038007
Tag = AArch64BuildAttrs::getPauthABITagsID(TagStr);
@@ -8018,8 +8022,6 @@ bool AArch64AsmParser::parseDirectiveAeabiAArch64Attr(SMLoc L) {
80188022
}
80198023
break;
80208024
}
8021-
} else if (Parser.getTok().is(AsmToken::Integer)) {
8022-
Tag = getTok().getIntVal();
80238025
} else {
80248026
Error(Parser.getTok().getLoc(), "AArch64 build attributes tag not found");
80258027
return true;
@@ -8063,10 +8065,9 @@ bool AArch64AsmParser::parseDirectiveAeabiAArch64Attr(SMLoc L) {
80638065
Error(Parser.getTok().getLoc(), "AArch64 build attributes value not found");
80648066
return true;
80658067
}
8066-
// Check for possible unaccepted values for known tags (AEABI_PAUTHABI,
8067-
// AEABI_FEATURE_AND_BITS)
8068-
if (!(ActiveSubsectionID == AArch64BuildAttrs::VENDOR_UNKNOWN) &&
8069-
TagStr != "") { // TagStr was a recognized string
8068+
// Check for possible unaccepted values for known tags
8069+
// (AEABI_FEATURE_AND_BITS)
8070+
if (ActiveSubsectionID == AArch64BuildAttrs::AEABI_FEATURE_AND_BITS) {
80708071
if (0 != ValueInt && 1 != ValueInt) {
80718072
Error(Parser.getTok().getLoc(),
80728073
"unknown AArch64 build attributes Value for Tag '" + TagStr +
@@ -8075,7 +8076,8 @@ bool AArch64AsmParser::parseDirectiveAeabiAArch64Attr(SMLoc L) {
80758076
}
80768077
}
80778078
Parser.Lex();
8078-
// Parsing finished, check for trailing tokens.
8079+
8080+
// Parsing finished. Check for trailing tokens.
80798081
if (Parser.getTok().isNot(llvm::AsmToken::EndOfStatement)) {
80808082
Error(Parser.getTok().getLoc(),
80818083
"unexpected token for AArch64 build attributes tag and value "
@@ -8084,13 +8086,11 @@ bool AArch64AsmParser::parseDirectiveAeabiAArch64Attr(SMLoc L) {
80848086
}
80858087

80868088
if (unsigned(-1) != ValueInt) {
8087-
getTargetStreamer().emitAttribute(ActiveSubsectionName, Tag, ValueInt, "",
8088-
false);
8089+
getTargetStreamer().emitAttribute(ActiveSubsectionName, Tag, ValueInt, "");
80898090
}
8090-
80918091
if ("" != ValueStr) {
80928092
getTargetStreamer().emitAttribute(ActiveSubsectionName, Tag, unsigned(-1),
8093-
ValueStr, false);
8093+
ValueStr);
80948094
}
80958095
return false;
80968096
}

‎llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp

Lines changed: 17 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ class AArch64TargetAsmStreamer : public AArch64TargetStreamer {
152152
}
153153

154154
void emitAttribute(StringRef VendorName, unsigned Tag, unsigned Value,
155-
std::string String, bool Override) override {
155+
std::string String) override {
156156

157157
// AArch64 build attributes for assembly attribute form:
158158
// .aeabi_attribute tag, value
@@ -164,19 +164,15 @@ class AArch64TargetAsmStreamer : public AArch64TargetStreamer {
164164
unsigned VendorID = AArch64BuildAttrs::getVendorID(VendorName);
165165

166166
switch (VendorID) {
167-
default:
168-
assert(0 && "Subsection name error");
169-
break;
170167
case AArch64BuildAttrs::VENDOR_UNKNOWN:
171168
if (unsigned(-1) != Value) {
172169
OS << "\t.aeabi_attribute" << "\t" << Tag << ", " << Value;
173-
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "",
174-
Override);
170+
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "");
175171
}
176172
if ("" != String) {
177173
OS << "\t.aeabi_attribute" << "\t" << Tag << ", " << String;
178174
AArch64TargetStreamer::emitAttribute(VendorName, Tag, unsigned(-1),
179-
String, Override);
175+
String);
180176
}
181177
break;
182178
// Note: AEABI_FEATURE_AND_BITS takes only unsigned values
@@ -186,16 +182,14 @@ class AArch64TargetAsmStreamer : public AArch64TargetStreamer {
186182
OS << "\t.aeabi_attribute" << "\t" << Tag << ", " << Value;
187183
// Keep the data structure consistent with the case of ELF emission
188184
// (important for llvm-mc asm parsing)
189-
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "",
190-
Override);
185+
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "");
191186
break;
192187
case AArch64BuildAttrs::TAG_FEATURE_BTI:
193188
case AArch64BuildAttrs::TAG_FEATURE_GCS:
194189
case AArch64BuildAttrs::TAG_FEATURE_PAC:
195-
OS << "\t.aeabi_attribute" << "\t"
196-
<< AArch64BuildAttrs::getFeatureAndBitsTagsStr(Tag) << ", " << Value;
197-
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "",
198-
Override);
190+
OS << "\t.aeabi_attribute" << "\t" << Tag << ", " << Value << "\t// "
191+
<< AArch64BuildAttrs::getFeatureAndBitsTagsStr(Tag);
192+
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "");
199193
break;
200194
}
201195
break;
@@ -206,15 +200,13 @@ class AArch64TargetAsmStreamer : public AArch64TargetStreamer {
206200
OS << "\t.aeabi_attribute" << "\t" << Tag << ", " << Value;
207201
// Keep the data structure consistent with the case of ELF emission
208202
// (important for llvm-mc asm parsing)
209-
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "",
210-
Override);
203+
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "");
211204
break;
212205
case AArch64BuildAttrs::TAG_PAUTH_PLATFORM:
213206
case AArch64BuildAttrs::TAG_PAUTH_SCHEMA:
214-
OS << "\t.aeabi_attribute" << "\t"
215-
<< AArch64BuildAttrs::getPauthABITagsStr(Tag) << ", " << Value;
216-
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "",
217-
Override);
207+
OS << "\t.aeabi_attribute" << "\t" << Tag << ", " << Value << "\t// "
208+
<< AArch64BuildAttrs::getPauthABITagsStr(Tag);
209+
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "");
218210
break;
219211
}
220212
break;
@@ -241,8 +233,8 @@ class AArch64TargetAsmStreamer : public AArch64TargetStreamer {
241233
StringRef ParameterStr = getTypeStr(ParameterType);
242234

243235
switch (SubsectionID) {
244-
default: {
245-
// Treated as a private subsection
236+
case AArch64BuildAttrs::VENDOR_UNKNOWN: {
237+
// Private subsection
246238
break;
247239
}
248240
case AArch64BuildAttrs::AEABI_PAUTHABI: {
@@ -431,13 +423,12 @@ void AArch64TargetELFStreamer::emitAtributesSubsection(
431423
}
432424

433425
void AArch64TargetELFStreamer::emitAttribute(StringRef VendorName, unsigned Tag,
434-
unsigned Value, std::string String,
435-
bool Override) {
426+
unsigned Value,
427+
std::string String) {
436428
if (unsigned(-1) != Value)
437-
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "", Override);
429+
AArch64TargetStreamer::emitAttribute(VendorName, Tag, Value, "");
438430
if ("" != String)
439-
AArch64TargetStreamer::emitAttribute(VendorName, Tag, unsigned(-1), String,
440-
Override);
431+
AArch64TargetStreamer::emitAttribute(VendorName, Tag, unsigned(-1), String);
441432
}
442433

443434
void AArch64TargetELFStreamer::emitInst(uint32_t Inst) {

‎llvm/lib/Target/AArch64/MCTargetDesc/AArch64TargetStreamer.cpp

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "llvm/BinaryFormat/ELF.h"
1616
#include "llvm/MC/ConstantPools.h"
1717
#include "llvm/MC/MCContext.h"
18+
#include "llvm/MC/MCELFStreamer.h"
1819
#include "llvm/MC/MCSection.h"
1920
#include "llvm/MC/MCSectionELF.h"
2021
#include "llvm/MC/MCSubtargetInfo.h"
@@ -193,8 +194,7 @@ AArch64TargetStreamer::getAtributesSubsectionByName(StringRef Name) {
193194
}
194195

195196
void AArch64TargetStreamer::emitAttribute(StringRef VendorName, unsigned Tag,
196-
unsigned Value, std::string String,
197-
bool Override) {
197+
unsigned Value, std::string String) {
198198

199199
if (unsigned(-1) == Value && "" == String) {
200200
assert(0 && "Arguments error");
@@ -214,22 +214,14 @@ void AArch64TargetStreamer::emitAttribute(StringRef VendorName, unsigned Tag,
214214
return;
215215
}
216216
for (MCELFStreamer::AttributeItem &Item : SubSection.Content) {
217+
// Tag already exists
217218
if (Item.Tag == Tag) {
218-
if (!Override) {
219-
if ((unsigned(-1) != Value && Item.IntValue != Value) ||
220-
("" != String && Item.StringValue != String)) {
221-
assert(0 &&
222-
"Can not add AArch64 build attribute: An attribute with "
223-
"the same tag and a different value already exists");
224-
return;
225-
} else {
226-
// Case Item.IntValue == Value, no need to emit twice
227-
assert(0 &&
228-
"AArch64 build attribute: An attribute with the same tag "
229-
"and a same value already exists");
230-
return;
231-
}
232-
}
219+
Item.Type = unsigned(-1) != Value
220+
? MCELFStreamer::AttributeItem::NumericAttribute
221+
: MCELFStreamer::AttributeItem::TextAttribute;
222+
Item.IntValue = unsigned(-1) != Value ? Value : unsigned(-1);
223+
Item.StringValue = unsigned(-1) != Value ? "" : String;
224+
return;
233225
}
234226
}
235227
if (unsigned(-1) != Value)

‎llvm/lib/Target/AArch64/MCTargetDesc/AArch64TargetStreamer.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ class AArch64TargetStreamer : public MCTargetStreamer {
100100
AArch64BuildAttrs::SubsectionOptional IsOptional,
101101
AArch64BuildAttrs::SubsectionType ParameterType);
102102
virtual void emitAttribute(StringRef VendorName, unsigned Tag, unsigned Value,
103-
std::string String, bool Override);
103+
std::string String);
104104
void activateAtributesSubsection(StringRef VendorName);
105105
std::unique_ptr<MCELFStreamer::AttributeSubSection>
106106
getActiveAtributesSubsection();
@@ -127,7 +127,7 @@ class AArch64TargetELFStreamer : public AArch64TargetStreamer {
127127
StringRef VendorName, AArch64BuildAttrs::SubsectionOptional IsOptional,
128128
AArch64BuildAttrs::SubsectionType ParameterType) override;
129129
void emitAttribute(StringRef VendorName, unsigned Tag, unsigned Value,
130-
std::string String, bool Override = false) override;
130+
std::string String) override;
131131
void emitInst(uint32_t Inst) override;
132132
void emitDirectiveVariantPCS(MCSymbol *Symbol) override;
133133
void finish() override;

‎llvm/test/CodeGen/AArch64/aarch64-build-attributes-all.ll renamed to ‎llvm/test/CodeGen/AArch64/build-attributes-all.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
; RUN: llc %s -filetype=obj -o - | llvm-readelf --hex-dump=.ARM.attributes - | FileCheck %s --check-prefix=ELF
33

44
; ASM: .aeabi_subsection aeabi_feature_and_bits, optional, uleb128
5-
; ASM-NEXT: .aeabi_attribute Tag_Feature_BTI, 1
6-
; ASM-NEXT: .aeabi_attribute Tag_Feature_PAC, 1
7-
; ASM-NEXT: .aeabi_attribute Tag_Feature_GCS, 1
5+
; ASM-NEXT: .aeabi_attribute 0, 1 // Tag_Feature_BTI
6+
; ASM-NEXT: .aeabi_attribute 1, 1 // Tag_Feature_PAC
7+
; ASM-NEXT: .aeabi_attribute 2, 1 // Tag_Feature_GCS
88

99
; ELF: Hex dump of section '.ARM.attributes':
1010
; ELF-NEXT: 0x00000000 41230000 00616561 62695f66 65617475 A#...aeabi_featu

‎llvm/test/CodeGen/AArch64/aarch64-build-attributes-bti.ll renamed to ‎llvm/test/CodeGen/AArch64/build-attributes-bti.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
; RUN: llc %s -filetype=obj -o - | llvm-readelf --hex-dump=.ARM.attributes - | FileCheck %s --check-prefix=ELF
33

44
; ASM: .aeabi_subsection aeabi_feature_and_bits, optional, uleb128
5-
; ASM-NEXT: .aeabi_attribute Tag_Feature_BTI, 1
6-
; ASM-NEXT: .aeabi_attribute Tag_Feature_PAC, 0
7-
; ASM-NEXT: .aeabi_attribute Tag_Feature_GCS, 0
5+
; ASM-NEXT: .aeabi_attribute 0, 1 // Tag_Feature_BTI
6+
; ASM-NEXT: .aeabi_attribute 1, 0 // Tag_Feature_PAC
7+
; ASM-NEXT: .aeabi_attribute 2, 0 // Tag_Feature_GCS
88

99
; ELF: Hex dump of section '.ARM.attributes':
1010
; ELF-NEXT: 0x00000000 41230000 00616561 62695f66 65617475 A#...aeabi_featu

‎llvm/test/CodeGen/AArch64/aarch64-build-attributes-gcs.ll renamed to ‎llvm/test/CodeGen/AArch64/build-attributes-gcs.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
; RUN: llc %s -filetype=obj -o - | llvm-readelf --hex-dump=.ARM.attributes - | FileCheck %s --check-prefix=ELF
33

44
; ASM: .aeabi_subsection aeabi_feature_and_bits, optional, uleb128
5-
; ASM-NEXT: .aeabi_attribute Tag_Feature_BTI, 0
6-
; ASM-NEXT: .aeabi_attribute Tag_Feature_PAC, 0
7-
; ASM-NEXT: .aeabi_attribute Tag_Feature_GCS, 1
5+
; ASM-NEXT: .aeabi_attribute 0, 0 // Tag_Feature_BTI
6+
; ASM-NEXT: .aeabi_attribute 1, 0 // Tag_Feature_PAC
7+
; ASM-NEXT: .aeabi_attribute 2, 1 // Tag_Feature_GCS
88

99
; ELF: Hex dump of section '.ARM.attributes':
1010
; ELF-NEXT: 0x00000000 41230000 00616561 62695f66 65617475 A#...aeabi_featu

‎llvm/test/CodeGen/AArch64/aarch64-build-attributes-pac.ll renamed to ‎llvm/test/CodeGen/AArch64/build-attributes-pac.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
; RUN: llc %s -filetype=obj -o - | llvm-readelf --hex-dump=.ARM.attributes - | FileCheck %s --check-prefix=ELF
33

44
; ASM: .aeabi_subsection aeabi_feature_and_bits, optional, uleb128
5-
; ASM-NEXT: .aeabi_attribute Tag_Feature_BTI, 0
6-
; ASM-NEXT: .aeabi_attribute Tag_Feature_PAC, 1
7-
; ASM-NEXT: .aeabi_attribute Tag_Feature_GCS, 0
5+
; ASM-NEXT: .aeabi_attribute 0, 0 // Tag_Feature_BTI
6+
; ASM-NEXT: .aeabi_attribute 1, 1 // Tag_Feature_PAC
7+
; ASM-NEXT: .aeabi_attribute 2, 0 // Tag_Feature_GCS
88

99
; ELF: Hex dump of section '.ARM.attributes':
1010
; ELF-NEXT: 0x00000000 41230000 00616561 62695f66 65617475 A#...aeabi_featu

‎llvm/test/CodeGen/AArch64/aarch64-build-attributes-pauthabi.ll renamed to ‎llvm/test/CodeGen/AArch64/build-attributes-pauthabi.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
; RUN: llc %s -filetype=obj -o - | llvm-readelf --hex-dump=.ARM.attributes - | FileCheck %s --check-prefix=ELF
33

44
; ASM: .aeabi_subsection aeabi_pauthabi, required, uleb128
5-
; ASM-NEXT: .aeabi_attribute Tag_PAuth_Platform, 2
6-
; ASM-NEXT: .aeabi_attribute Tag_PAuth_Schema, 31
5+
; ASM-NEXT: .aeabi_attribute 1, 2 // Tag_PAuth_Platform
6+
; ASM-NEXT: .aeabi_attribute 2, 31 // Tag_PAuth_Schema
77

88
; ELF: Hex dump of section '.ARM.attributes':
99
; ELF-NEXT: 0x00000000 41190000 00616561 62695f70 61757468 A....aeabi_pauth

0 commit comments

Comments
 (0)