Skip to content

Commit c5fc31a

Browse files
AlekseyTsAlekseyTs
authored
Disallow complex forms of IndexerName attributes in extensions (#77781)
1 parent 39479f2 commit c5fc31a

File tree

6 files changed

+126
-20
lines changed

6 files changed

+126
-20
lines changed

‎src/Compilers/CSharp/Portable/Symbols/Source/QuickAttributeChecker.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ private static QuickAttributeChecker CreatePredefinedQuickAttributeChecker()
4848
var result = new QuickAttributeChecker();
4949
result.AddName(AttributeDescription.TypeIdentifierAttribute.Name, QuickAttributes.TypeIdentifier);
5050
result.AddName(AttributeDescription.TypeForwardedToAttribute.Name, QuickAttributes.TypeForwardedTo);
51+
result.AddName(AttributeDescription.IndexerNameAttribute.Name, QuickAttributes.IndexerName);
5152
result.AddName(AttributeDescription.AssemblyKeyNameAttribute.Name, QuickAttributes.AssemblyKeyName);
5253
result.AddName(AttributeDescription.AssemblyKeyFileAttribute.Name, QuickAttributes.AssemblyKeyFile);
5354
result.AddName(AttributeDescription.AssemblySignatureKeyAttribute.Name, QuickAttributes.AssemblySignatureKey);
@@ -144,9 +145,10 @@ internal enum QuickAttributes : byte
144145
None = 0,
145146
TypeIdentifier = 1 << 0,
146147
TypeForwardedTo = 1 << 1,
147-
AssemblyKeyName = 1 << 2,
148-
AssemblyKeyFile = 1 << 3,
149-
AssemblySignatureKey = 1 << 4,
148+
IndexerName = 1 << 2,
149+
AssemblyKeyName = 1 << 3,
150+
AssemblyKeyFile = 1 << 4,
151+
AssemblySignatureKey = 1 << 5,
150152
Last = AssemblySignatureKey,
151153
}
152154

@@ -171,6 +173,10 @@ public static QuickAttributes GetQuickAttributes(string name, bool inAttribute)
171173
{
172174
result |= QuickAttributes.TypeForwardedTo;
173175
}
176+
else if (matches(AttributeDescription.IndexerNameAttribute))
177+
{
178+
result |= QuickAttributes.IndexerName;
179+
}
174180
else if (matches(AttributeDescription.AssemblyKeyNameAttribute))
175181
{
176182
result |= QuickAttributes.AssemblyKeyName;

‎src/Compilers/CSharp/Portable/Symbols/Source/SourceAssemblySymbol.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1666,7 +1666,7 @@ internal CommonAssemblyWellKnownAttributeData GetSourceDecodedWellKnownAttribute
16661666
}
16671667

16681668
attributesBag = null;
1669-
Func<AttributeSyntax, bool> attributeMatches = attribute switch
1669+
Func<AttributeSyntax, Binder?, bool> attributeMatches = attribute switch
16701670
{
16711671
QuickAttributes.AssemblySignatureKey => isPossibleAssemblySignatureKeyAttribute,
16721672
QuickAttributes.AssemblyKeyName => isPossibleAssemblyKeyNameAttribute,
@@ -1678,19 +1678,19 @@ internal CommonAssemblyWellKnownAttributeData GetSourceDecodedWellKnownAttribute
16781678

16791679
return (CommonAssemblyWellKnownAttributeData?)attributesBag?.DecodedWellKnownAttributeData;
16801680

1681-
bool isPossibleAssemblySignatureKeyAttribute(AttributeSyntax node)
1681+
bool isPossibleAssemblySignatureKeyAttribute(AttributeSyntax node, Binder? rootBinderOpt)
16821682
{
16831683
QuickAttributeChecker checker = this.DeclaringCompilation.GetBinderFactory(node.SyntaxTree).GetBinder(node).QuickAttributeChecker;
16841684
return checker.IsPossibleMatch(node, QuickAttributes.AssemblySignatureKey);
16851685
}
16861686

1687-
bool isPossibleAssemblyKeyNameAttribute(AttributeSyntax node)
1687+
bool isPossibleAssemblyKeyNameAttribute(AttributeSyntax node, Binder? rootBinderOpt)
16881688
{
16891689
QuickAttributeChecker checker = this.DeclaringCompilation.GetBinderFactory(node.SyntaxTree).GetBinder(node).QuickAttributeChecker;
16901690
return checker.IsPossibleMatch(node, QuickAttributes.AssemblyKeyName);
16911691
}
16921692

1693-
bool isPossibleAssemblyKeyFileAttribute(AttributeSyntax node)
1693+
bool isPossibleAssemblyKeyFileAttribute(AttributeSyntax node, Binder? rootBinderOpt)
16941694
{
16951695
QuickAttributeChecker checker = this.DeclaringCompilation.GetBinderFactory(node.SyntaxTree).GetBinder(node).QuickAttributeChecker;
16961696
return checker.IsPossibleMatch(node, QuickAttributes.AssemblyKeyFile);
@@ -1756,7 +1756,7 @@ private static void AfterPossibleForwardedTypesAttributePartBound(AttributeSynta
17561756
Debug.Assert(removed);
17571757
}
17581758

1759-
private bool IsPossibleForwardedTypesAttribute(AttributeSyntax node)
1759+
private bool IsPossibleForwardedTypesAttribute(AttributeSyntax node, Binder rootBinderOpt)
17601760
{
17611761
QuickAttributeChecker checker =
17621762
this.DeclaringCompilation.GetBinderFactory(node.SyntaxTree).GetBinder(node).QuickAttributeChecker;

‎src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,11 @@ internal string SourceName
470470
// always use the real attribute bag of this symbol and modify LoadAndValidateAttributes to
471471
// handle partially filled bags.
472472
CustomAttributesBag<CSharpAttributeData>? temp = null;
473-
LoadAndValidateAttributes(OneOrMany.Create(indexerNameAttributeLists), ref temp, earlyDecodingOnly: true);
473+
Binder rootBinder = GetAttributeBinder(indexerNameAttributeLists, DeclaringCompilation);
474+
LoadAndValidateAttributes(
475+
OneOrMany.Create(indexerNameAttributeLists), ref temp, earlyDecodingOnly: true,
476+
binderOpt: rootBinder,
477+
attributeMatchesOpt: this.GetIsNewExtensionMember() ? isPossibleIndexerNameAttributeInExtension : isPossibleIndexerNameAttribute);
474478
if (temp != null)
475479
{
476480
Debug.Assert(temp.IsEarlyDecodedWellKnownAttributeDataComputed);
@@ -487,6 +491,24 @@ internal string SourceName
487491
}
488492

489493
return _lazySourceName;
494+
495+
static bool isPossibleIndexerNameAttribute(AttributeSyntax node, Binder? rootBinderOpt)
496+
{
497+
Debug.Assert(rootBinderOpt is not null);
498+
QuickAttributeChecker checker = rootBinderOpt.QuickAttributeChecker;
499+
return checker.IsPossibleMatch(node, QuickAttributes.IndexerName);
500+
}
501+
502+
static bool isPossibleIndexerNameAttributeInExtension(AttributeSyntax node, Binder? rootBinderOpt)
503+
{
504+
// PROTOTYPE: Temporarily limit binding to a string literal argument in order to avoid a binding cycle.
505+
if (node.ArgumentList?.Arguments is not [{ NameColon: null, NameEquals: null, Expression: LiteralExpressionSyntax { RawKind: (int)SyntaxKind.StringLiteralExpression } }])
506+
{
507+
return false;
508+
}
509+
510+
return isPossibleIndexerNameAttribute(node, rootBinderOpt);
511+
}
490512
}
491513
}
492514
#nullable disable
@@ -1696,6 +1718,12 @@ private void ValidateIndexerNameAttribute(CSharpAttributeData attribute, Attribu
16961718
{
16971719
diagnostics.Add(ErrorCode.ERR_BadArgumentToAttribute, node.ArgumentList.Arguments[0].Location, node.GetErrorDisplayName());
16981720
}
1721+
else if (this.GetIsNewExtensionMember() && SourceName != indexerName)
1722+
{
1723+
// PROTOTYPE: Report more descriptive error
1724+
// error CS8078: An expression is too long or complex to compile
1725+
diagnostics.Add(ErrorCode.ERR_InsufficientStack, node.ArgumentList.Arguments[0].Location);
1726+
}
16991727
}
17001728
}
17011729

‎src/Compilers/CSharp/Portable/Symbols/Symbol_Attributes.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ internal bool LoadAndValidateAttributes(
302302
AttributeLocation symbolPart = AttributeLocation.None,
303303
bool earlyDecodingOnly = false,
304304
Binder? binderOpt = null,
305-
Func<AttributeSyntax, bool>? attributeMatchesOpt = null,
305+
Func<AttributeSyntax, Binder?, bool>? attributeMatchesOpt = null,
306306
Action<AttributeSyntax>? beforeAttributePartBound = null,
307307
Action<AttributeSyntax>? afterAttributePartBound = null)
308308
{
@@ -586,7 +586,7 @@ private ImmutableArray<AttributeSyntax> GetAttributesToBind(
586586
AttributeLocation symbolPart,
587587
BindingDiagnosticBag diagnostics,
588588
CSharpCompilation compilation,
589-
Func<AttributeSyntax, bool> attributeMatchesOpt,
589+
Func<AttributeSyntax, Binder, bool> attributeMatchesOpt,
590590
Binder rootBinderOpt,
591591
out ImmutableArray<Binder> binders)
592592
{
@@ -624,7 +624,7 @@ private ImmutableArray<AttributeSyntax> GetAttributesToBind(
624624
{
625625
foreach (var attribute in attributesToBind)
626626
{
627-
if (attributeMatchesOpt(attribute))
627+
if (attributeMatchesOpt(attribute, rootBinderOpt))
628628
{
629629
syntaxBuilder.Add(attribute);
630630
attributesToBindCount++;
@@ -666,7 +666,7 @@ protected virtual bool ShouldBindAttributes(AttributeListSyntax attributeDeclara
666666
}
667667

668668
#nullable enable
669-
private Binder GetAttributeBinder(SyntaxList<AttributeListSyntax> attributeDeclarationSyntaxList, CSharpCompilation compilation, Binder? rootBinder = null)
669+
protected Binder GetAttributeBinder(SyntaxList<AttributeListSyntax> attributeDeclarationSyntaxList, CSharpCompilation compilation, Binder? rootBinder = null)
670670
{
671671
var binder = rootBinder ?? compilation.GetBinderFactory(attributeDeclarationSyntaxList.Node!.SyntaxTree).GetBinder(attributeDeclarationSyntaxList.Node);
672672
binder = new ContextualAttributeBinder(binder, this);

‎src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22837,15 +22837,15 @@ public MyAttr(int p) {}
2283722837
);
2283822838
}
2283922839

22840-
[Fact(Skip = "Cycle")] // PROTOTYPE: There is a cycle due to the attribute
22840+
[Fact]
2284122841
public void ReceiverParameterScope_07_InAttribute()
2284222842
{
2284322843
var src = @"
2284422844
static class Extensions
2284522845
{
2284622846
extension(int p)
2284722847
{
22848-
[System.Runtime.CompilerServices.IndexerName(nameof(p))]
22848+
[MyAttr(nameof(p))]
2284922849
int this[int y]
2285022850
{
2285122851
get
@@ -22855,6 +22855,11 @@ int this[int y]
2285522855
}
2285622856
}
2285722857
}
22858+
22859+
class MyAttr : System.Attribute
22860+
{
22861+
public MyAttr(string p) {}
22862+
}
2285822863
";
2285922864
var comp = CreateCompilation(src);
2286022865

@@ -22987,13 +22992,13 @@ string M3()
2298722992
);
2298822993
}
2298922994

22990-
[Fact(Skip = "Cycle")] // PROTOTYPE: There is a cycle due to the attribute
22995+
[Fact]
2299122996
public void CycleInAttribute_01()
2299222997
{
2299322998
var src = @"
2299422999
static class Extensions
2299523000
{
22996-
static const string Str = ""val""
23001+
const string Str = ""val"";
2299723002
extension(string p)
2299823003
{
2299923004
[System.Runtime.CompilerServices.IndexerName(Str)]
@@ -23009,7 +23014,12 @@ int this[int y]
2300923014
";
2301023015
var comp = CreateCompilation(src);
2301123016

23012-
comp.VerifyEmitDiagnostics();
23017+
// PROTOTYPE: We do not allow complex forms of IndexerName attribute due to a possible binding cycle
23018+
comp.VerifyEmitDiagnostics(
23019+
// (7,54): error CS8078: An expression is too long or complex to compile
23020+
// [System.Runtime.CompilerServices.IndexerName(Str)]
23021+
Diagnostic(ErrorCode.ERR_InsufficientStack, "Str").WithLocation(7, 54)
23022+
);
2301323023
}
2301423024

2301523025
[Fact]

‎src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5107,7 +5107,37 @@ partial struct S1
51075107
""";
51085108

51095109
var comp = CreateCompilation(source);
5110-
comp.VerifyEmitDiagnostics();
5110+
CompileAndVerify(comp).VerifyDiagnostics().VerifyTypeIL("S1",
5111+
"""
5112+
.class private sequential ansi sealed beforefieldinit S1
5113+
extends [mscorlib]System.ValueType
5114+
{
5115+
.custom instance void [mscorlib]System.Reflection.DefaultMemberAttribute::.ctor(string) = (
5116+
01 00 06 4d 79 4e 61 6d 65 00 00
5117+
)
5118+
.pack 0
5119+
.size 1
5120+
// Methods
5121+
.method public hidebysig specialname
5122+
instance int32 get_MyName (
5123+
int32 x
5124+
) cil managed
5125+
{
5126+
// Method begins at RVA 0x2067
5127+
// Code size 2 (0x2)
5128+
.maxstack 8
5129+
IL_0000: ldarg.1
5130+
IL_0001: ret
5131+
} // end of method S1::get_MyName
5132+
// Properties
5133+
.property instance int32 MyName(
5134+
int32 x
5135+
)
5136+
{
5137+
.get instance int32 S1::get_MyName(int32)
5138+
}
5139+
} // end of class S1
5140+
""".Replace("[mscorlib]", ExecutionConditionUtil.IsMonoOrCoreClr ? "[netstandard]" : "[mscorlib]"));
51115141
}
51125142

51135143
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/76842")]
@@ -5126,7 +5156,39 @@ partial struct S1
51265156
""";
51275157

51285158
var comp = CreateCompilation(source);
5129-
comp.VerifyEmitDiagnostics();
5159+
5160+
// Note, the indexer name in metadata is "Item", expected "MyName"
5161+
CompileAndVerify(comp).VerifyDiagnostics().VerifyTypeIL("S1",
5162+
"""
5163+
.class private sequential ansi sealed beforefieldinit S1
5164+
extends [mscorlib]System.ValueType
5165+
{
5166+
.custom instance void [mscorlib]System.Reflection.DefaultMemberAttribute::.ctor(string) = (
5167+
01 00 04 49 74 65 6d 00 00
5168+
)
5169+
.pack 0
5170+
.size 1
5171+
// Methods
5172+
.method public hidebysig specialname
5173+
instance int32 get_Item (
5174+
int32 x
5175+
) cil managed
5176+
{
5177+
// Method begins at RVA 0x2067
5178+
// Code size 2 (0x2)
5179+
.maxstack 8
5180+
IL_0000: ldarg.1
5181+
IL_0001: ret
5182+
} // end of method S1::get_Item
5183+
// Properties
5184+
.property instance int32 Item(
5185+
int32 x
5186+
)
5187+
{
5188+
.get instance int32 S1::get_Item(int32)
5189+
}
5190+
} // end of class S1
5191+
""".Replace("[mscorlib]", ExecutionConditionUtil.IsMonoOrCoreClr ? "[netstandard]" : "[mscorlib]"));
51305192
}
51315193
}
51325194
}

0 commit comments

Comments
 (0)