[feature request] Allow set custom attributes on inner method

[moonheart]

The problem:

When calling extern method, it may throw System.AccessViolationException, which can not be catched and will crashes the process.

To catch this exception (.NET Framework only), the caller must set two attributes on current method: HandleProcessCorruptedStateExceptionsAttribute and SecurityCriticalAttribute.

But the weaver did not copy these attributes to the generated innner method, and causes the process crash.

Possible solution

1. Always copy these two attributes to inner method if exist This is the easist but not the best, because it takes extra steps to check attributes.
2. Allow set custom attributes in user advices. Users can set attributes based on their logic.

[picrap]

The attributes are still there, I don’t understand why this happens.

[moonheart]

csproj file:

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <OutputType>Exe</OutputType>
        <TargetFramework>net462</TargetFramework>
        <ImplicitUsings>enable</ImplicitUsings>
        <Nullable>enable</Nullable>
        <LangVersion>latest</LangVersion>
    </PropertyGroup>

    <ItemGroup>
        <PackageReference Include="MrAdvice" Version="2.15.0" />
    </ItemGroup>
</Project>

Program.cs:

using System.Runtime.ExceptionServices;
using System.Security;
using ArxOne.MrAdvice.Advice;

internal class Program
{
    public static void Main(string[] args)
    {
        TestMethod();
    }

    [Test]
    [HandleProcessCorruptedStateExceptions]
    [SecurityCritical]
    private static void TestMethod()
    {
        Console.WriteLine("May throw System.AccessViolationException here.");
    }
}

public class TestAttribute : Attribute, IMethodAdvice
{
    public void Advise(MethodAdviceContext context)
    {
        context.Proceed();
    }
}

Generateed Code (decompiled):

using ArxOne.MrAdvice;
using ArxOne.MrAdvice.Annotation;
using System;
using System.Runtime.ExceptionServices;
using System.Runtime.InteropServices;
using System.Security;

#nullable enable
internal class Program
{
  public static void Main(string[] args) => Program.TestMethod();

  [Test]
  [HandleProcessCorruptedStateExceptions]
  [SecurityCritical]
  private static void TestMethod()
  {
    // ISSUE: method reference
    // ISSUE: method reference
    // ISSUE: method reference
    ⚡Invocation.ProceedAspect(__methodref (Program.TestMethod), __methodref (Program.TestMethod′), __methodref (Program.TestMethod″));
  }

  [ExecutionPoint]
  private static void TestMethod′()
  {
    Console.WriteLine("May throw System.AccessViolationException here.");
  }

  private static 
  #nullable disable
  object TestMethod″([In] object obj0, [In] object[] obj1)
  {
    Program.TestMethod′();
    return (object) null;
  }
}

As you can see, the generated method TestMethod′ is not marked with [SecurityCritical] and [HandleProcessCorruptedStateExceptions] attributes.

[picrap]

Sorry for huge delay.

I’ve been thinking to this problem and I don’t know how to solve this. Let me rephrase your needs :

• currently, MrAdvice weaves its advices inside the method, so the method attributes are left outside.
• your needs are the opposite, as you want the advice to be applied outside.

So how to solve this ?

• changing global behavior is a bad idea
• having a mixed behavior would actually require two weavings : one inside (as it is now), one outside, as you request.

Adding custom attributes beside the [ExecutionPoint] at build could be an idea. Do you think you could go with that ?

[moonheart]

In most cases, the current solution works well. However, the HandleProcessCorruptedStateExceptionsAttribute and SecurityCriticalAttribute are special cases. Therefore, I believe we should only copy these two attributes in AspectWeaver.WeaveAdvices. There is no need to copy all attributes besides [ExecutionPoint], as this could introduce breaking changes.