Browse Source

Fix #2372: report a field's address loads on their own

Loading a field's address was counted as both a read and a write, so a
use that only reads - calling a method on a value-type field emits
ldflda and writes nothing - was listed under "Assigned By".

An address load says only that something needed a reference to the
field. What happens through that reference is up to the consumer, and
the IL scan here does not look at it, so the load is neither a read nor
a write and gets a category of its own. Special-casing the methods known
not to mutate their receiver would answer this one screenshot and leave
every ref argument still guessing.

Assisted-by: Claude:claude-opus-5:Claude Code
fix/2372-address-taken-by
Siegfried Pammer 1 week ago
parent
commit
7572b57eee
  1. 48
      ICSharpCode.ILSpyX/Analyzers/Builtin/FieldAccessAnalyzer.cs
  2. 98
      ILSpy.Tests/Analyzers/Library/FieldAccessAnalyzerTests.cs
  3. 36
      ILSpy.Tests/Analyzers/Library/TestCases/MainAssembly.cs

48
ICSharpCode.ILSpyX/Analyzers/Builtin/FieldAccessAnalyzer.cs

@ -33,42 +33,61 @@ using ILOpCode = System.Reflection.Metadata.ILOpCode;
namespace ICSharpCode.ILSpyX.Analyzers.Builtin namespace ICSharpCode.ILSpyX.Analyzers.Builtin
{ {
/// <summary> /// <summary>
/// Finds methods where this field is read. /// Finds methods where this field is written.
/// </summary> /// </summary>
[ExportAnalyzer(Header = "Assigned By", Order = 20)] [ExportAnalyzer(Header = "Assigned By", Order = 20)]
[Shared] [Shared]
class AssignedByFieldAccessAnalyzer : FieldAccessAnalyzer class AssignedByFieldAccessAnalyzer : FieldAccessAnalyzer
{ {
public AssignedByFieldAccessAnalyzer() : base(true) { } public AssignedByFieldAccessAnalyzer() : base(FieldAccessKind.Write) { }
} }
/// <summary> /// <summary>
/// Finds methods where this field is written. /// Finds methods where this field is read.
/// </summary> /// </summary>
[ExportAnalyzer(Header = "Read By", Order = 10)] [ExportAnalyzer(Header = "Read By", Order = 10)]
[Shared] [Shared]
class ReadByFieldAccessAnalyzer : FieldAccessAnalyzer class ReadByFieldAccessAnalyzer : FieldAccessAnalyzer
{ {
public ReadByFieldAccessAnalyzer() : base(false) { } public ReadByFieldAccessAnalyzer() : base(FieldAccessKind.Read) { }
}
/// <summary>
/// Finds methods that load this field's address.
/// </summary>
[ExportAnalyzer(Header = "Address Taken By", Order = 30)]
[Shared]
class AddressTakenByFieldAccessAnalyzer : FieldAccessAnalyzer
{
public AddressTakenByFieldAccessAnalyzer() : base(FieldAccessKind.AddressOf) { }
}
enum FieldAccessKind
{
Read,
Write,
AddressOf
} }
/// <summary> /// <summary>
/// Finds methods where this field is read or written. /// Finds methods that access this field in one particular way.
/// </summary> /// </summary>
class FieldAccessAnalyzer : IAnalyzer class FieldAccessAnalyzer : IAnalyzer
{ {
const GetMemberOptions Options = GetMemberOptions.IgnoreInheritedMembers | GetMemberOptions.ReturnMemberDefinitions; const GetMemberOptions Options = GetMemberOptions.IgnoreInheritedMembers | GetMemberOptions.ReturnMemberDefinitions;
readonly bool showWrites; // true: show writes; false: show read access readonly FieldAccessKind kind;
public FieldAccessAnalyzer(bool showWrites) public FieldAccessAnalyzer(FieldAccessKind kind)
{ {
this.showWrites = showWrites; this.kind = kind;
} }
public bool Show(ISymbol? symbol) public bool Show(ISymbol? symbol)
{ {
return symbol is IField field && (!showWrites || !field.IsConst); // A constant is inlined at every use: there is nothing to assign to and no address
// to take.
return symbol is IField field && (kind == FieldAccessKind.Read || !field.IsConst);
} }
public IEnumerable<ISymbol> Analyze(ISymbol analyzedSymbol, AnalyzerContext context) public IEnumerable<ISymbol> Analyze(ISymbol analyzedSymbol, AnalyzerContext context)
@ -201,13 +220,18 @@ namespace ICSharpCode.ILSpyX.Analyzers.Builtin
{ {
case ILOpCode.Ldfld: case ILOpCode.Ldfld:
case ILOpCode.Ldsfld: case ILOpCode.Ldsfld:
return !showWrites; return kind == FieldAccessKind.Read;
case ILOpCode.Stfld: case ILOpCode.Stfld:
case ILOpCode.Stsfld: case ILOpCode.Stsfld:
return showWrites; return kind == FieldAccessKind.Write;
case ILOpCode.Ldflda: case ILOpCode.Ldflda:
case ILOpCode.Ldsflda: case ILOpCode.Ldsflda:
return true; // always show address-loading // An address load says only that something needed a reference to the field.
// What happens through that reference is decided by the consumer - calling a
// method on a value-type field reads it, passing it as a ref argument may
// write it - and the IL scan here does not look at the consumer, so it is
// neither a read nor a write.
return kind == FieldAccessKind.AddressOf;
default: default:
return false; return false;
} }

98
ILSpy.Tests/Analyzers/Library/FieldAccessAnalyzerTests.cs

@ -0,0 +1,98 @@
// Copyright (c) 2026 Siegfried Pammer
//
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
// software and associated documentation files (the "Software"), to deal in the Software
// without restriction, including without limitation the rights to use, copy, modify, merge,
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
// to whom the Software is furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in all copies or
// substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
using System.Linq;
using AwesomeAssertions;
using ICSharpCode.Decompiler.TypeSystem;
using ICSharpCode.ILSpy.Languages;
using ICSharpCode.ILSpyX;
using ICSharpCode.ILSpyX.Analyzers;
using ICSharpCode.ILSpyX.Analyzers.Builtin;
using NUnit.Framework;
namespace ICSharpCode.ILSpy.Tests.Analyzers.Library;
/// <summary>
/// Loading a field's address says only that something needed a reference to it, not what was
/// done through that reference: `flag.ToString()` on a value-type field emits ldflda and writes
/// nothing. Counting it as an assignment put read-only uses under "Assigned By" (issue #2372),
/// so it is reported on its own instead.
/// </summary>
[TestFixture]
public class FieldAccessAnalyzerTests
{
AssemblyList assemblyList = null!;
CSharpLanguage language = null!;
ITypeDefinition typeDefinition = null!;
[OneTimeSetUp]
public void Setup()
{
assemblyList = new AssemblyList();
var testAssembly = assemblyList.OpenAssembly(typeof(FieldAccessAnalyzerTests).Assembly.Location);
assemblyList.OpenAssembly(typeof(void).Assembly.Location);
language = new CSharpLanguage();
typeDefinition = testAssembly.GetTypeSystemOrNull()!
.FindType(typeof(TestCases.Main.FieldAccess))
.GetDefinition()!;
}
string[] Analyze(IAnalyzer analyzer, string fieldName)
{
var context = new AnalyzerContext { AssemblyList = assemblyList, Language = language };
var field = typeDefinition.Fields.Single(f => f.Name == fieldName);
return analyzer.Analyze(field, context).OfType<IEntity>().Select(e => e.Name).ToArray();
}
[TestCase("instanceFlag", "ReadsInstanceFlagByAddress")]
[TestCase("staticFlag", "ReadsStaticFlagByAddress")]
public void An_Address_Load_Is_Not_An_Assignment(string fieldName, string addressUser)
{
Analyze(new AssignedByFieldAccessAnalyzer(), fieldName)
.Should().NotContain(addressUser, "taking the address is not a write");
}
[TestCase("instanceFlag", "ReadsInstanceFlagByAddress")]
[TestCase("staticFlag", "ReadsStaticFlagByAddress")]
public void An_Address_Load_Is_Not_A_Read_Either(string fieldName, string addressUser)
{
Analyze(new ReadByFieldAccessAnalyzer(), fieldName)
.Should().NotContain(addressUser, "what the address is used for is not known here");
}
[TestCase("instanceFlag", "ReadsInstanceFlagByAddress")]
[TestCase("staticFlag", "ReadsStaticFlagByAddress")]
public void An_Address_Load_Is_Reported_On_Its_Own(string fieldName, string addressUser)
{
Analyze(new AddressTakenByFieldAccessAnalyzer(), fieldName)
.Should().Contain(addressUser);
}
[Test]
public void Plain_Reads_And_Writes_Are_Unaffected()
{
Analyze(new ReadByFieldAccessAnalyzer(), "instanceFlag").Should().Contain("ReadsInstanceFlag");
Analyze(new AssignedByFieldAccessAnalyzer(), "instanceFlag").Should().Contain("WritesInstanceFlag");
Analyze(new AssignedByFieldAccessAnalyzer(), "staticFlag").Should().Contain("WritesStaticFlag");
Analyze(new AddressTakenByFieldAccessAnalyzer(), "instanceFlag")
.Should().NotContain("WritesInstanceFlag", "a plain stfld takes no address");
}
}

36
ILSpy.Tests/Analyzers/Library/TestCases/MainAssembly.cs

@ -33,4 +33,40 @@ namespace ICSharpCode.ILSpy.Tests.Analyzers.Library.TestCases.Main
return int.Parse("1234"); return int.Parse("1234");
} }
} }
// Fixture for the field-access analysers. A field of a value type reached through a
// method call is loaded by address (ldflda/ldsflda), which says nothing about whether
// the call writes to it - issue #2372.
class FieldAccess
{
public bool instanceFlag;
public static bool staticFlag;
public string ReadsInstanceFlagByAddress()
{
// callvirt Boolean::ToString(ldflda instanceFlag)
return instanceFlag.ToString();
}
public static string ReadsStaticFlagByAddress()
{
// call Boolean::ToString(ldsflda staticFlag)
return staticFlag.ToString();
}
public bool ReadsInstanceFlag()
{
return instanceFlag;
}
public void WritesInstanceFlag()
{
instanceFlag = true;
}
public static void WritesStaticFlag()
{
staticFlag = true;
}
}
} }

Loading…
Cancel
Save