Win32 / OLE issues

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Thu Oct 28 21:06:41 UTC 2021


Thanks for the detailed report, I think I know where to look, and sorry 
for the late reply.

This is a minimal reproducer which shows all the issues:

```
typedef int*(*foo)(int*);
```

Extracting this reveals two problems:

* the type of the functional interface is wrong - e.g. 
(MemoryAddress)MemoryAddress, instead of (MemoryAddress)Addressable
* as you noted, the arguments (and the return types) are missing some 
casts (from MemoryAddress to Addressable and back) which doesn't sit 
well with invokeExact

I've filed this:

https://bugs.openjdk.java.net/browse/JDK-8276136

Thanks for your patience.

Maurizio



On 28/10/2021 21:05, Duncan Gittins wrote:
> Apologies for the long email. I've refactored / inlined my Windows OLE code
> to a single class test case showing the bug in jextract.
>
> All this code sample does is load a Window COM object by the string version
> of its CLSID GUID, and retrieves a requested interface IID GUID (plus any
> other IIDs specified. This ought to work for any COM / IID interface, but
> it defaults to CLSID_ShellLink with IID_IShellLinkW / IID_Persist COM
> interface lookup using only the IUnknown callbacks - QueryInterface /
> AddRef / Release. Perhaps you could refactor into a test case for jextract
> in future Windows builds.
>
> The error is:
>
> java.lang.AssertionError: should not reach here
>          at
> duncan.win.ole.IUnknownVtbl$Release.lambda$ofAddress$0(IUnknownVtbl.java:123)
>          at duncan.panama.test.TestGuid$JUnknown.Release(Unknown Source)
>          at duncan.panama.test.TestGuid.main(Unknown Source)
>          at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
> Method)
>          at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:76)
>          at
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:51)
>          at java.base/java.lang.reflect.Method.invoke(Method.java:569)
>          at duncan.launch.Launch.main(Unknown Source)
>          at duncan.launch.Launch.run(Unknown Source)
>          at duncan.launch.Launch.main(Unknown Source)
> Caused by: java.lang.invoke.WrongMethodTypeException: expected
> (NativeSymbol,Addressable)int but found (NativeSymbol,MemoryAddress)int
>          at
> java.base/java.lang.invoke.Invokers.newWrongMethodTypeException(Invokers.java:523)
>          at
> java.base/java.lang.invoke.Invokers.checkExactType(Invokers.java:532)
>          at
> duncan.win.ole.IUnknownVtbl$Release.lambda$ofAddress$0(IUnknownVtbl.java:121)
>
> Class is below
>
> Kind regards
>
> Duncan
>
> import static duncan.win.ole.Ole32_h.S_OK;
> import static jdk.incubator.foreign.ValueLayout.ADDRESS;
> import static jdk.incubator.foreign.ValueLayout.JAVA_BYTE;
>
> import java.nio.charset.StandardCharsets;
> import java.util.Arrays;
> import java.util.HexFormat;
> import java.util.concurrent.TimeUnit;
> import java.util.function.Supplier;
>
> import org.junit.jupiter.api.Test;
> import org.junit.jupiter.api.condition.EnabledOnOs;
> import org.junit.jupiter.api.condition.OS;
>
> import duncan.win.ole.GUID;
> import duncan.win.ole.IUnknown;
> import duncan.win.ole.IUnknownVtbl;
> import duncan.win.ole.Ole32_h;
>
> import jdk.incubator.foreign.MemoryAddress;
> import jdk.incubator.foreign.MemorySegment;
> import jdk.incubator.foreign.ResourceScope;
> import jdk.incubator.foreign.SegmentAllocator;
>
> /**
>   * Test app to run through Windows COM IUnknown API calls.
>   * A JUNIT test runs main() - easily removed
>   *
>   * Usage:
>   java ...  TestGuid GUID_CLSID GUID_IID+
>   *
>   * GUIDs are defined in Windows format eg.
> "{00021401-0000-0000-C000-000000000046}"
>   *
>   * Note: have kept to Windows naming conventions when calling Windows
> definitions.
>   *
>   * This instantiates the class with GUID_CLSID, and returns the requested
> IID interface.
>   * <p>If additional IIDs are provided these are requested
>   * <p>This requires jextract where files are:
>   * Ole32.h:
>   *      #include <objbase.h>
>   *
>   * jextract -source -lole32 -t duncan.win.ole -d duncan.win\src Ole32.h
>   */
> public class TestGuid
> {
>      // Junit testcase
>      @EnabledOnOs(OS.WINDOWS)
>      @Test void coverageTestGuid() throws Exception
>      {
>          TestGuid.main();
>      }
>
>      // This uses slimmed down version of my JUnknown, inlined here
>      // along with all the dependent calls on other my other win32 classes
>      // Not called IUnknown to avoid name classes with jextract generated
> code.
>      static class JUnknown
>      {
>          protected final MemoryAddress comObj;
>
>          private final IUnknownVtbl.QueryInterface QueryInterface;
>          private final IUnknownVtbl.AddRef         AddRef;
>          private final IUnknownVtbl.Release        Release;
>
>          /**
>           * Derived classes can use this after chaining to the protected
> constructor
>           */
>          protected final MemorySegment vtable;
>          protected final SegmentAllocator allocator;
>
>          /**
>           * Derived classes must not use this constructor as it
>           * only reserves only a small memory segment for the VTABLE
>           */
>          public JUnknown(ResourceScope scope, SegmentAllocator allocator,
> MemoryAddress comObj)
>          {
>              this(scope, allocator, comObj,
> IUnknownVtbl.ofAddress(vtable(scope, comObj), scope));
>          }
>
>          /**
>           * This version ensures a larger vTABLE can be read:
>           * A constructor for a derived class - say IShellLink
>           * would need to extract its own callbacks from vtable like this:
>           * <code>
>              public class JShellLink extends JUnknown { ...
>
>                  public JShellLink(ResourceScope scope, SegmentAllocator
> allocator, MemoryAddress comObj)
>                  {
>                      super(scope, allocator, comObj,
> IShellLinkWVtbl.ofAddress(vtable(scope, comObj), scope));
>                      GetPath = IShellLinkWVtbl.GetPath(vtable, scope);
>                      SetPath = IShellLinkWVtbl.SetPath(vtable, scope);
>                      SetArguments = IShellLinkWVtbl.SetArguments(vtable,
> scope);
>                      GetArguments = IShellLinkWVtbl.GetArguments(vtable,
> scope);
>                      GetDescription = IShellLinkWVtbl.GetDescription(vtable,
> scope);
>                      GetIDList      = IShellLinkWVtbl.GetIDList(vtable,
> scope);
>                  }
>              ...
>              }
>           * </code>
>           * @param scope
>           * @param allocator
>           * @param comObj
>           * @param vtable as provided by a call to
> Ole32_h.IxxxxxxVtbl.ofAddressRestricted(vtableA(comObj))
>           */
>          protected JUnknown(ResourceScope scope, SegmentAllocator allocator,
> MemoryAddress comObj, MemorySegment vtable)
>          {
>              this.allocator = allocator;
>              this.comObj = comObj;
>              this.vtable = vtable;
>
>              QueryInterface = IUnknownVtbl.QueryInterface(vtable, scope);
>              AddRef = IUnknownVtbl.AddRef(vtable, scope);
>              Release = IUnknownVtbl.Release(vtable, scope);
>          }
>
>          public String toString()
>          {
>              return
> getClass().getSimpleName()+"["+toHex(comObj.toRawLongValue())+"]";
>          }
>
>          /**
>           * Reads the vTABLE for this COM object, returning the address
>           * which must be dereferenced by the outermost derived class with
> the jextract
>           * call specific for the derived classes VTABLE:
>           *      IClassName.ofAddressRestricted(vtable(comObj));
>           */
>          public static MemoryAddress vtable(ResourceScope scope,
> MemoryAddress comObj)
>          {
>              // Only need to retrieve a pointer at index 0 so size of one
> pointer is OK:
>              MemorySegment memseg = IUnknown.ofAddress(comObj, scope);
>
>              // The address of QueryInterface is the first entry in the
> vtable:
>              MemoryAddress pVT = IUnknown.lpVtbl$get(memseg);
>
>              // This is only the address of the vTable, see
> IClassName.ofAddressRestricted(pVT);
>              return pVT;
>          }
>
>          public int AddRef()
>          {
>              int refcount = AddRef.apply(comObj);
>              System.out.println(this+".AddRef => "+refcount);
>
>              return refcount;
>          }
>
>          /**
>           * IUnknown::Release method (unknwn.h)
>           *
> https://docs.microsoft.com/en-us/windows/win32/api/unknwn/nf-unknwn-iunknown-release
>           * ULONG Release()
>           */
>          public int Release()
>          {
>              int refcount = Release.apply(comObj);
>              System.out.println(this+".Release => "+refcount);
>
>              return refcount;
>          }
>
>          /**
>           *
> https://docs.microsoft.com/en-us/windows/win32/api/unknwn/nf-unknwn-iunknown-queryinterface(refiid_void)
>           * IUnknown::QueryInterface(REFIID,void) method (unknwn.h)
>           * HRESULT IUnknown::QueryInterface(REFIID iid, void** ppv);
>           * <P>
>           * https://osm.hpi.de/LV/Components04/VL5/MSDN/DrGUI-on-COM
>           */
>          public MemoryAddress QueryInterface(MemorySegment riid)
>          {
>              System.out.println(this+".QueryInterface
> riid="+toHex(riid.toArray(JAVA_BYTE)));
>
>              // This is allocated each time in case more than one call made
> to QueryInterface
>              MemorySegment queryRes = allocator.allocate(ADDRESS,
> MemoryAddress.NULL);
>
>              int hRes = QueryInterface.apply(comObj, riid.address(),
> queryRes.address());
>
>              MemoryAddress address = queryRes.get(ADDRESS, 0);
>              check0("QueryInterface", hRes);
>              System.out.println(" => IID="+toHex(address.toRawLongValue()));
>
>              return address;
>          }
>      }
>
>      public static MemorySegment toWideString(String s, SegmentAllocator
> allocator)
>      {
>          return allocator.allocateArray(JAVA_BYTE,
> (s+"\0").getBytes(StandardCharsets.UTF_16LE));
>      }
>      public static void check0(String func, int hRes)
>      {
>          checkThat(hRes == S_OK(), () -> "Error: "+func+" NOT OK, result was
> "+hRes+" / 0x"+Integer.toHexString(hRes));
>          System.out.println(func+" OK result => "+hRes);
>      }
>
>      public static void checkThat(boolean condition, Supplier<String> error)
>      {
>          if (!condition)
>          {
>              String message = error.get();
>              System.err.println(message);
>              throw new RuntimeException(message);
>          }
>      }
>
>      public static MemorySegment CLSIDFromString(SegmentAllocator allocator,
> String guid)
>      {
>          System.out.println("CLSIDFromString clsid="+guid);
>          // Falls through to do the actual conversion:
>          MemorySegment pGUID = GUID.allocate(allocator);
>
>          MemorySegment wide = toWideString(guid, allocator);
>
>          int hRes = Ole32_h.CLSIDFromString(wide, pGUID);
>          check0("CLSIDFromString", hRes);
>          System.out.println(" => GUID="+toHex(pGUID.toArray(JAVA_BYTE)));
>
>          return pGUID;
>      }
>      public static String toHex(long value)
>      {
>          return "0x"+Long.toHexString(value).toUpperCase();
>      }
>      private static String toHex(byte[] arr) {
>          HexFormat hex = HexFormat.ofDelimiter(",
> ").withPrefix("0x").withUpperCase();
>          return "new byte["+arr.length+"] {"+hex.formatHex(arr)+"}";
>      }
>      public static MemoryAddress CoCreateInstance(SegmentAllocator
> allocator, MemorySegment rclsid, int dwClsContext, MemorySegment riid)
>      {
>          MemoryAddress pUnkOuter = MemoryAddress.NULL;
>          MemorySegment ptrComObj = allocator.allocate(ADDRESS,
> MemoryAddress.NULL);
>          // https://www.purebasic.fr/english/viewtopic.php?f=13&t=45583
>
>          System.out.println("CoCreateInstance
> rclsid="+toHex(rclsid.toArray(JAVA_BYTE)));
>          System.out.println("CoCreateInstance
> pUnkOuter="+pUnkOuter.address());
>          System.out.println("CoCreateInstance dwClsContext="+dwClsContext);
>          System.out.println("CoCreateInstance
> riid="+toHex(riid.toArray(JAVA_BYTE)));
>
>          int hRes = Ole32_h.CoCreateInstance(rclsid, pUnkOuter,
> dwClsContext, riid, ptrComObj);
>          check0("CoCreateInstance", hRes);
>          MemoryAddress comObj = ptrComObj.get(ADDRESS, 0);
>          System.out.println("CoCreateInstance =>
> address="+toHex(comObj.toRawLongValue()));
>
>          return comObj;
>      }
>      public static void checkResult(String func, int hRes, int ... okcodes)
>      {
>          for (int ok : okcodes)
>          {
>              if (ok == hRes)
>              {
>                  System.out.println(func+" OK result => "+hRes+"
> ok="+Arrays.toString(okcodes));
>                  return;
>              }
>          }
>          checkThat(false, () -> "Error: "+func+" NOT OK, result code
> "+hRes+" / 0x"+Integer.toHexString(hRes)+" ok="+Arrays.toString(okcodes));
>      }
>      public static AutoCloseable CoInitialize()
>      {
>          int hRes = Ole32_h.CoInitialize(MemoryAddress.NULL);
>          checkResult("CoInitialize", hRes, Ole32_h.S_OK(),
> Ole32_h.S_FALSE());
>
>          return Ole32_h::CoUninitialize;
>      }
>
>      public static void main(String ... args) throws Exception
>      {
>          final long t0 = System.nanoTime();
>
>          // Default run instantiates COM object CLSID_ShellLink and asks for
> its IShellLinkW and IID_Persist interfaces
>          String[] defaults = {
>                  /*GUID_CLSID_ShellLink_str*/
> "{00021401-0000-0000-C000-000000000046}",
>                  /*GUID_IID_IShellLinkW_str*/
> "{000214F9-0000-0000-C000-000000000046}",
>                  /*GUID_IID_Persist_str*/
>   "{0000010B-0000-0000-C000-000000000046}",
>          };
>
>          if (args.length < 2) args = defaults;
>
>          String clsidStr = args[0];
>          String iidStr   = args[1];
>          System.out.println("lookup GUID CLSID "+clsidStr+" IID "+iidStr);
>
>          try(ResourceScope scope = ResourceScope.newConfinedScope())
>          {
>              SegmentAllocator allocator =
> SegmentAllocator.newNativeArena(scope);
>
>              // Lookup GUIDs for class and interfaces
>              MemorySegment clsid  = CLSIDFromString(allocator, clsidStr);
>              MemorySegment iid    = CLSIDFromString(allocator, iidStr);
>
>              // Setup COM:
>              try(var autoC = CoInitialize())
>              {
>                  // Get a pointer to the COM instance.
>                  MemoryAddress comObj = CoCreateInstance(allocator, clsid,
> Ole32_h.CLSCTX_INPROC_SERVER(), iid);
>
>                  // JUnknown ignores the complete vtable, only initialises
> callbacks for IUnknown interface
>                  JUnknown iUnknown = new JUnknown(scope, allocator, comObj);
>                  try
>                  {
>                      // test add/release on this iUnknown
>                      int ar = iUnknown.AddRef();
>                      int re = iUnknown.Release();
>                      checkResult("AddRef/Release()", ar, re+1);
>
>                      // Access other interfaces via the first one:
>                      for (int ii = 2; ii < args.length; ii++)
>                      {
>                          // Use Query interface on another interface from
> the CLSID
>                          MemorySegment iidextra = CLSIDFromString(allocator,
> args[ii]);
>
>                          // Query CLS for this other interface definition
>                          MemoryAddress iOtherUnknown =
> iUnknown.QueryInterface(iidextra);
>                          JUnknown iOther = new JUnknown(scope, allocator,
> iOtherUnknown);
>                          try
>                          {
>                              // test add/release on this other iUnknown (NB
> same instance but with different vtable)
>                              int add = iOther.AddRef();
>                              checkResult("AddRef()", add, 3);
>                              int rel = iOther.Release();
>                              checkResult("Release()", rel, 2);
>                          }
>                          finally
>                          {
>                              // Signal end of use of iOther
>                              // Note that the ref count includes the value
> on iUnknown
>                              int refC = iOther.Release();
>                              checkResult("Release()", refC, 1);
>                          }
>                      }
>                  }
>                  finally
>                  {
>                      // Signal end of use of iUnknown
>                      int refC = iUnknown.Release();
>                      // Note that the ref count should now be zero
>                      checkResult("Release()", refC, 0);
>                  }
>              }
>              final long now = System.nanoTime();
>              System.out.println("Ended
> ms="+TimeUnit.NANOSECONDS.toMillis(now - t0)+" "+clsidStr);
>          }
>      }
> }
>
> On Mon, 27 Sept 2021 at 16:45, Duncan Gittins <duncan.gittins at gmail.com>
> wrote:
>
>> The jextract parameters which generate the broken Windows OLE API code I
>> outlined below is:
>>
>>      jextract -source -lole32 -t duncan.win.ole -d source\duncan.win\java
>> headers\Ole32.h
>>
>> where headers\Ole32.h contains:
>>
>>      #include <objbase.h>
>>
>> A temporary workaround for this jextract issue is to edit
>> FunctionalInterfaceBuilder.java / emitFunctionalFactoryForPointer() to
>> insert the (Addressable) casts for MemoryAddress parameters:
>>
>> ---
>> a/src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/FunctionalInterfaceBuilder.java
>> +++
>> b/src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/FunctionalInterfaceBuilder.java
>> @@ -123,7 +123,7 @@ public class FunctionalInterfaceBuilder extends
>> ClassSourceBuilder {
>>               append(mhConstant.accessExpression() +
>> ".invokeExact((Addressable)addr");
>>               if (fiType.parameterCount() > 0) {
>>                   String params = IntStream.range(0,
>> fiType.parameterCount())
>> -                        .mapToObj(i -> "x" + i)
>> +                        .mapToObj(i ->
>> (fiType.parameterType(i).getName().endsWith(".MemoryAddress") ?
>> "(Addressable)":"")+"x" + i)
>>                           .collect(Collectors.joining(", "));
>>                   append(", " + params);
>>               }
>>
>> this fixes the ofAddress callbacks, for example see IPersistFileVtbl.java:
>>
>>     public interface Load {
>>          ....
>>          static Load ofAddress(MemoryAddress addr) {
>>              return (jdk.incubator.foreign.MemoryAddress x0,
>> jdk.incubator.foreign.MemoryAddress x1, int x2) -> {
>>                  try {
>> // WAS:          return
>> (int)IPersistFileVtbl.Load$MH.invokeExact((Addressable)addr, x0, x1, x2);
>>                        return
>> (int)IPersistFileVtbl.Load$MH.invokeExact((Addressable)addr,
>> (Addressable)x0, (Addressable)x1, x2);
>>                  } catch (Throwable ex$) {
>>                      throw new AssertionError("should not reach here", ex$);
>>                  }
>>              };
>>          }
>>
>> Kind regards
>>
>> Duncan
>>
>>
>> On Fri, 24 Sept 2021 at 16:36, Duncan Gittins <duncan.gittins at gmail.com>
>> wrote:
>>
>>> I've pulled latest panama-foreign which has the changes outlined in
>>>
>>>      https://inside.java/2021/09/16/finalizing-the-foreign-apis/
>>>
>>> I've a few problems to resolve to match up, one is related to jextract
>>> which generates interfaces for Windows OLE APIs.  The invokeExact params
>>> are missing some (Addressable) casts (I think?) eg this is IUnknown Release:
>>>
>>>      public interface Release {
>>>
>>>          int apply(jdk.incubator.foreign.MemoryAddress x0);
>>>          static CLinker.UpcallStub allocate(Release fi) {
>>>              return RuntimeHelper.upcallStub(Release.class, fi,
>>> IUnknownVtbl.Release$FUNC, "(Ljdk/incubator/foreign/MemoryAddress;)I");
>>>          }
>>>          static CLinker.UpcallStub allocate(Release fi, ResourceScope
>>> scope) {
>>>              return RuntimeHelper.upcallStub(Release.class, fi,
>>> IUnknownVtbl.Release$FUNC, "(Ljdk/incubator/foreign/MemoryAddress;)I",
>>> scope);
>>>          }
>>>          static Release ofAddress(MemoryAddress addr) {
>>>              return (jdk.incubator.foreign.MemoryAddress x0) -> {
>>>                  try {
>>>                      return
>>> (int)IUnknownVtbl.Release$MH.invokeExact((Addressable)addr, x0);
>>>                  } catch (Throwable ex$) {
>>>                      throw new AssertionError("should not reach here",
>>> ex$);
>>>                  }
>>>              };
>>>          }
>>>      }
>>>
>>> Stack traces show
>>>
>>>       Caused by: java.lang.invoke.WrongMethodTypeException: expected
>>> (Addressable,Addressable)int but found (Addressable,MemoryAddress)int
>>>
>>> Kind regards
>>>
>>> Duncan
>>>
>>>
>>>


More information about the panama-dev mailing list