Biohazrd icon indicating copy to clipboard operation
Biohazrd copied to clipboard

Assert failure: Clang FunctionProtoType outside of PointerType

Open PathogenDavid opened this issue 5 years ago • 4 comments

In 1c9621867db4efcbce62ba0aa2960a5b6494ff26 an issue with function pointers being translated as double pointers because Clang represents a function pointer as a Pointertype with an PointeeType of FunctionProtoType. (Where FunctionProtoType is a function type, not a function pointer type.)

The fix was to special-case a PointerType to a FunctionProtoType during type reduction. The old FunctionProtoType branch was retained with an assert added:

https://github.com/InfectedLibraries/Biohazrd/blob/fcca2db7651dfbdd512ce04564e2c891e1505b35/Biohazrd.Transformation/Common/TypeReductionTransformation.cs#L75

I ran into this today while testing Biohazrd with the Windows headers as a smoke test. In particular, this occurred with the declaration of RpcObjectSetInqFn at rpcdce.h:841.

Turns out this situation is possible because you can have a PointerType -> TypedefType -> FunctionProtoType with the following uncommon (in my experience) syntax:

typedef void (function_t)(int);
void Test(function_t* function);

The typical syntax I usually see is:

typedef void (*function_pointer_t)(int);
void Test(function_pointer_t function);

My first instinct was we should switch back to translating everything as PointerTypeReference -> FunctionPointerTypeReference and add an implicit post-pass into TypeReductionTransformation that removes the unnecessary level of indirection, but as I wrote some new tests to cover existing function pointer behavior, I realized this approach runs into issues when the typedef isn't removed. (What type does the typedef have?

As such we might need to change FunctionPointerTypeDeference to FunctionTypereference instead.

PathogenDavid avatar Dec 14 '20 20:12 PathogenDavid

Here's a skeleton test to test this issue is resolved. The typedef check needs to be added once a decision is made there.

        [Fact]
        [RelatedIssue("https://github.com/InfectedLibraries/Biohazrd/issues/115")]
        public void FunctionPointerTest3()
        {
            TranslatedLibrary library = CreateLibrary
            (@"
typedef void (function_t)(int);
void Test(function_t* function);
"
            );

            // Reduce with the typedef
            {
                TranslatedLibrary reduced = new TypeReductionTransformation().Transform(library);
                TranslatedTypedef typedef = reduced.FindDeclaration<TranslatedTypedef>("function_t");
                //TODO
                //AssertVoidIntFunctionPointerType(typedef.UnderlyingType);
            }

            // Reduce without the typedef
            {
                TranslatedLibrary withoutTypedef = new RemoveRemainingTypedefsTransformation().Transform(library);
                withoutTypedef = new TypeReductionTransformation().Transform(withoutTypedef);

                TranslatedParameter parameter = withoutTypedef.FindDeclaration<TranslatedFunction>("Test").FindDeclaration<TranslatedParameter>("function");
                AssertVoidIntFunctionPointerType(parameter.Type);
            }
        }

PathogenDavid avatar Dec 14 '20 20:12 PathogenDavid

After talking with a friend about this a bit, I discovered you can use function typedefs like this function pointers without the *:

typedef void (function_t)(int x);
static function_t lol;

void Test(function_t function);
void TestPtr(function_t* function);

void Example(int x);

void UseTest()
{
    Test(Example);
}

void UseTestPtr()
{
    TestPtr(Example);
}

This compiles with no issues on Godbolt with Clang 11 and results in the following assembly:

UseTest(): # @UseTest()
  mov edi, offset Example(int)
  jmp Test(void (*)(int)) # TAILCALL
UseTestPtr(): # @UseTestPtr()
  mov edi, offset Example(int)
  jmp TestPtr(void (*)(int)) # TAILCALL

This means we potentially need to handle this FunctionTypeReference as a function pointer in C# output emit (if we go that route), or we need to ensure they're always wrapped in a PointerTypeReference.

I think I need to read the C/C++ spec a bit to understand what these types are good for. Especially since you can't really use them for variables:

void UseTest2()
{
    // <source>:120:16: error: illegal initializer (only variables can be initialized)
    function_t e = Example;
    Test(e);
}

PathogenDavid avatar Dec 14 '20 21:12 PathogenDavid

This issue is fixed, opened https://github.com/InfectedLibraries/Biohazrd/issues/126 for the investigating other situations where these are valid. (Which may be more important now that there's no assert to alert us to the fact that this situation has occurred.)

PathogenDavid avatar Dec 25 '20 14:12 PathogenDavid

This seemingly regressed at some point or there is an edge case which is not being covered.

For example, in PortAudio the following declaration is translated incorrectly:

typedef void PaStreamFinishedCallback( void *userData );

PaError Pa_SetStreamFinishedCallback( PaStream *stream, PaStreamFinishedCallback* streamFinishedCallback );

The second parameter is incorrectly translated as delegate* unmanaged[Cdecl]<void*, void>* streamFinishedCallback.

This applies to all function pointers across the library.

PathogenDavid avatar Oct 24 '24 23:10 PathogenDavid