[foreign] RFR 8220544: Jextract; library dependent header filtering

Jorn Vernee jbvernee at xs4all.nl
Thu Mar 14 19:53:53 UTC 2019


Sounds good!

As for the implementation; since the filtering requirement is platform 
specific, it seems better to implement findEntry as a platform specific 
function in java.base/<platform>/native/libjava/jni_util_md.c (e.g. [1]) 
?

We already have a function there for findEntryInProcess. We could add 
one for findEntry as well, and then filter on unix.

Jorn

[1] : 
http://hg.openjdk.java.net/panama/dev/file/b3831a541894/src/java.base/unix/native/libjava/jni_util_md.c

Maurizio Cimadamore schreef op 2019-03-14 20:37:
> This is my take on the issue (see attached patch).
> 
> Basically check path of library symbol against library path itself,
> and if they don't match do not return the symbol via Library::lookup.
> This should provide stable semantics across platforms.
> 
> Maurizio
> 
> On 14/03/2019 19:10, Maurizio Cimadamore wrote:
>> We do have soe support in the VM os class:
>> 
>> // Locate DLL/DSO. On success, full path of the library is copied to
>>   // buf, and offset is optionally set to be the distance between addr
>>   // and the library's base address. On failure, buf[0] is set to '\0'
>>   // and offset is set to -1 (if offset is non-NULL).
>>   static bool dll_address_to_library_name(address addr, char* buf,
>>                                           int buflen, int* offset);
>> 
>> 
>> trying to play with it and see what happens. Seems implemented for all 
>> OSs.
>> 
>> Maurizio
>> 
>> On 14/03/2019 18:58, Jorn Vernee wrote:
>>> Here's the Windows prototype I had for iterating the symbols in a 
>>> library: 
>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/itersyms/webrev.00
>>> 
>>> Jorn
>>> 
>>> Jorn Vernee schreef op 2019-03-14 19:50:
>>>> Maurizio Cimadamore schreef op 2019-03-14 19:32:
>>>>> So, I think that if we don't address this in some way, this work is
>>>>> not very effective on Linux/Mac, do yo agree?
>>>> 
>>>> Yes, this is unfortunate...
>>>> 
>>>>> As to how to address it, since there doesn't seem to be a way to
>>>>> perform a 'local' library search, what if we augmented the Symbol
>>>>> interface to also return a Path (of the shared library the symbol
>>>>> belongs to) ?
>>>>> 
>>>>> With that info we should be able to do some more filtering?
>>>>> 
>>>>> To get the path, on *NIX we can use dladdr. Not sure if Win has
>>>>> something similar.
>>>> 
>>>> I ran into dladdr as well, but that seems to be a glibc extension? 
>>>> On
>>>> Windows we have GetModuleFileName [1], so at least that's covered.
>>>> 
>>>> But, it might be preferable to try and implement the symbol table
>>>> crawl on the different operating systems, since that's more straight
>>>> towards what we want to achieve. I mean, if we're going the
>>>> non-portable route any ways... At least on Linux this seems to be
>>>> possible as well [2] I want to say that this should be possible to 
>>>> do
>>>> on most operating systems, as long as we can load the library file
>>>> directly into memory, and know the file format.
>>>> 
>>>> If we go that route I would need some help with the other platforms 
>>>> of course :)
>>>> 
>>>> Jorn
>>>> 
>>>> [1] :
>>>> https://docs.microsoft.com/en-us/windows/desktop/api/libloaderapi/nf-libloaderapi-getmodulefilenamea 
>>>> [2] :
>>>> https://stackoverflow.com/questions/26960377/how-to-programmatically-list-elf-shared-library-symbols
>>>>> Maurizio
>>>>> 
>>>>> On 14/03/2019 17:41, Maurizio Cimadamore wrote:
>>>>>> 
>>>>>> On 14/03/2019 17:36, Jorn Vernee wrote:
>>>>>>> I think you got it
>>>>>>> 
>>>>>>> With dlsym it seems to be recursive: [1] "The search performed by 
>>>>>>> dlsym() is breadth first through the dependency tree of these 
>>>>>>> libraries."
>>>>>>> 
>>>>>>> But, GetProcAdress on Windows [2] looks at the export table of 
>>>>>>> the library you give it, so AFAICT this is non-recursive.
>>>>>> 
>>>>>> Yep - found the same, you beat me to it :-)
>>>>>> 
>>>>>> Maurizio
>>>>>> 
>>>>>>> 
>>>>>>> Jorn
>>>>>>> 
>>>>>>> [1] : https://linux.die.net/man/3/dlsym
>>>>>>> [2] : 
>>>>>>> https://docs.microsoft.com/en-us/windows/desktop/api/libloaderapi/nf-libloaderapi-getprocaddress
>>>>>>> 
>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 18:25:
>>>>>>>> Nope - with this:
>>>>>>>> 
>>>>>>>> $ cat foo.h
>>>>>>>> #include <string.h>
>>>>>>>> 
>>>>>>>> void foo(int f);
>>>>>>>> 
>>>>>>>> $ cat foo.c
>>>>>>>> #include "foo.h"
>>>>>>>> 
>>>>>>>> void foo(int f) { }
>>>>>>>> 
>>>>>>>> then,
>>>>>>>> 
>>>>>>>> $  gcc -shared -o libfoo.so foo.c
>>>>>>>> 
>>>>>>>> $ jextract -L . -l foo -d classes foo.h
>>>>>>>> 
>>>>>>>> 
>>>>>>>> And then
>>>>>>>> 
>>>>>>>> classes
>>>>>>>> ├── clang_support
>>>>>>>> │   ├── stddef.class
>>>>>>>> │   ├── stddef_h.class
>>>>>>>> │   └── stddef$size_t.class
>>>>>>>> ├── foo.class
>>>>>>>> ├── foo_h.class
>>>>>>>> ├── META-INF
>>>>>>>> │   └── jextract.properties
>>>>>>>> └── usr
>>>>>>>>     └── include
>>>>>>>>         ├── string.class
>>>>>>>>         ├── string_h.class
>>>>>>>>         ├── xlocale.class
>>>>>>>>         ├── xlocale_h.class
>>>>>>>>         ├── xlocale.class
>>>>>>>>         ├── xlocale.class
>>>>>>>>         └── xlocale.class
>>>>>>>> 
>>>>>>>> 4 directories, 13 files
>>>>>>>> 
>>>>>>>> 
>>>>>>>> I wonder if some of the library lookup functions we have on 
>>>>>>>> linux
>>>>>>>> (dlopen and co.) are doing a 'recursive' search into dependent 
>>>>>>>> libs as
>>>>>>>> well, which might be disabled on Win.
>>>>>>>> 
>>>>>>>> 
>>>>>>>> Maurizio
>>>>>>>> 
>>>>>>>> 
>>>>>>>> On 14/03/2019 16:38, Maurizio Cimadamore wrote:
>>>>>>>>> Ah ok
>>>>>>>>> 
>>>>>>>>> will give that another try
>>>>>>>>> 
>>>>>>>>> Maurizio
>>>>>>>>> 
>>>>>>>>> On 14/03/2019 16:16, Jorn Vernee wrote:
>>>>>>>>>> Sorry, that's with the v3 patch 
>>>>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.03/ 
>>>>>>>>>> I reverted back after the concerns raised about changing the 
>>>>>>>>>> implementation of TypedefTree::def You're right in that this 
>>>>>>>>>> incorrectly includes some trees.
>>>>>>>>>> 
>>>>>>>>>> Sorry,
>>>>>>>>>> Jorn
>>>>>>>>>> 
>>>>>>>>>> Jorn Vernee schreef op 2019-03-14 17:08:
>>>>>>>>>>> This produces test.class and test_h.class as expected, but 
>>>>>>>>>>> nothing else...
>>>>>>>>>>> 
>>>>>>>>>>> Used command is: `jextract -L . -l test test.h`
>>>>>>>>>>> 
>>>>>>>>>>> You would get stuff from string.h when the -l is omitted.
>>>>>>>>>>> 
>>>>>>>>>>> Jorn
>>>>>>>>>>> 
>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 17:00:
>>>>>>>>>>>> I think you can reproduce simply by having an header like 
>>>>>>>>>>>> this:
>>>>>>>>>>>> 
>>>>>>>>>>>> #include <string.h>
>>>>>>>>>>>> 
>>>>>>>>>>>> void foo(int i);
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>> if you extract that you would expect one symbol, but in 
>>>>>>>>>>>> reality you
>>>>>>>>>>>> get also all the stuff from string.h.
>>>>>>>>>>>> 
>>>>>>>>>>>> Maurizio
>>>>>>>>>>>> 
>>>>>>>>>>>> On 14/03/2019 15:49, Jorn Vernee wrote:
>>>>>>>>>>>>> The Windows jextract artifact for the OpenGL sample is a 
>>>>>>>>>>>>> different beast, because it relies on the standard Windows 
>>>>>>>>>>>>> OpenGL libraries. So, seeing some system headers is to be 
>>>>>>>>>>>>> expected :) In the case of python I'm also seeing some 
>>>>>>>>>>>>> system headers, because python depends on a hand full of 
>>>>>>>>>>>>> corecrt types, so that's also correct. But, neither one 
>>>>>>>>>>>>> seems to be referencing imaxabs, so I'm not sure what's 
>>>>>>>>>>>>> going on...
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I'll try to see if I can find a repro example on Windows to 
>>>>>>>>>>>>> see about a fix. I'm not sure I see how the SymbolFilter 
>>>>>>>>>>>>> could get to the VM libraries? Though I remember seeing a 
>>>>>>>>>>>>> similar case in the past...
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 15:55:
>>>>>>>>>>>>>> I think I got at the bottom of this. I tried debugging an 
>>>>>>>>>>>>>> extraction
>>>>>>>>>>>>>> of opengl, and it eventually ends up adding inttypes.h to 
>>>>>>>>>>>>>> the rootset
>>>>>>>>>>>>>> because of this symbol:
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> imaxabs
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> The library filter doesn't work here, because this is a 
>>>>>>>>>>>>>> function
>>>>>>>>>>>>>> defined in the set of default libraries that comes with 
>>>>>>>>>>>>>> the VM, so
>>>>>>>>>>>>>> jextract 'thinks' this symbol is genuinely part of one of 
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> libraries specified in the command line - except is not.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> I tried an hacky fix which checks for absence of symbol in
>>>>>>>>>>>>>> Libraries.getDefaultLibrary - and that seems to give the 
>>>>>>>>>>>>>> expected
>>>>>>>>>>>>>> results - but I leave this to your consideration.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> On 14/03/2019 14:27, Maurizio Cimadamore wrote:
>>>>>>>>>>>>>>> Hi Jorn,
>>>>>>>>>>>>>>> I gave this a try with our OpenGL and Python examples. 
>>>>>>>>>>>>>>> The extraction process works fine, and the generated jars 
>>>>>>>>>>>>>>> are a bit smaller - but I still see a lot of 
>>>>>>>>>>>>>>> system-related headers. Python has still lots of them, 
>>>>>>>>>>>>>>> OpenGL less so, but even in the GL case, I think it's 
>>>>>>>>>>>>>>> interesting, because I can't see references to types 
>>>>>>>>>>>>>>> mentioned in these headers (nor in the resulting 
>>>>>>>>>>>>>>> classfiles). Do you know, on top of your head, as to why 
>>>>>>>>>>>>>>> this could be happening?
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> On 14/03/2019 12:45, Jorn Vernee wrote:
>>>>>>>>>>>>>>>> Updated webrev: 
>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.04/ 
>>>>>>>>>>>>>>>> Changed Treemaker implementation to recurse to the 
>>>>>>>>>>>>>>>> 'root' type when creating TypdefTree::def, and using 
>>>>>>>>>>>>>>>> that in DependencyFinder.
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> Jorn Vernee schreef op 2019-03-14 12:49:
>>>>>>>>>>>>>>>>> Oh ok, sorry. I'll try that out.
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 12:43:
>>>>>>>>>>>>>>>>>> On 14/03/2019 11:40, Jorn Vernee wrote:
>>>>>>>>>>>>>>>>>>> I see what you mean.
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Well, `struct BAR` has a definition cursor, but 
>>>>>>>>>>>>>>>>>>> `struct BAR*` does not have a definition/declaration, 
>>>>>>>>>>>>>>>>>>> it's a type that's automatically derived from `struct 
>>>>>>>>>>>>>>>>>>> BAR`. So for `struct BAR*` there is no good value for 
>>>>>>>>>>>>>>>>>>> TypedefTree::def.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> I know that there's no good value for struct BAR* - I 
>>>>>>>>>>>>>>>>>> was wondering if
>>>>>>>>>>>>>>>>>> the parser should detect the pointer, and recurse 
>>>>>>>>>>>>>>>>>> down, then store
>>>>>>>>>>>>>>>>>> 'struct BAR' into 'def'.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> TypedefTree::def seems to be a helper method mostly 
>>>>>>>>>>>>>>>>>>> targeted at inline type definitions (looking at 
>>>>>>>>>>>>>>>>>>> TypedefHandler [1]).
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> I think for my use case using 
>>>>>>>>>>>>>>>>>>> TypedefTree.type().canonicalType() is good enough, 
>>>>>>>>>>>>>>>>>>> but I guess we could query that once in the parser 
>>>>>>>>>>>>>>>>>>> and then store it in a field in TypedefTree?
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> [1] : 
>>>>>>>>>>>>>>>>>>> http://hg.openjdk.java.net/panama/dev/file/43ed53f40957/src/jdk.jextract/share/classes/com/sun/tools/jextract/TypedefHandler.java#l107
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 11:57:
>>>>>>>>>>>>>>>>>>>> On 14/03/2019 00:23, Jorn Vernee wrote:
>>>>>>>>>>>>>>>>>>>>> Sorry, bad example. The actual failure case is:
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>      struct BAR { int x; }
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>      typedef struct BAR* FOO;
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> The TreeMaker code looks at the canonical type of 
>>>>>>>>>>>>>>>>>>>>> the typedef `struct BAR*` which is neither a type 
>>>>>>>>>>>>>>>>>>>>> definition or declaration, so TypedefTree::def is 
>>>>>>>>>>>>>>>>>>>>> empty. But, in the case of dependecy analysis, we 
>>>>>>>>>>>>>>>>>>>>> still want to inspect that type, get the pointee 
>>>>>>>>>>>>>>>>>>>>> type and add it to the root set.
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> Right - and I think what I'm claiming is that this 
>>>>>>>>>>>>>>>>>>>> is probably some
>>>>>>>>>>>>>>>>>>>> kind of a parser bug? That is, if Typedef::def (we 
>>>>>>>>>>>>>>>>>>>> should probably
>>>>>>>>>>>>>>>>>>>> find another name, 'def' is not good enough IMHO) is 
>>>>>>>>>>>>>>>>>>>> set correctly for
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> typedef struct BAR FOO;
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> it should also be set for
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> typedef struct BAR* FOO;
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> That is, I claim that from the typedef tree you 
>>>>>>>>>>>>>>>>>>>> should always see a
>>>>>>>>>>>>>>>>>>>> referenced type declaration w/o using the clang API.
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> Jorn Vernee schreef op 2019-03-14 01:14:
>>>>>>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 01:04:
>>>>>>>>>>>>>>>>>>>>>>> So, if I understand correctly, the issue is that 
>>>>>>>>>>>>>>>>>>>>>>> if you have:
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> typedef struct { int x; } FOO;
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> you want the struct to be added to the root set. 
>>>>>>>>>>>>>>>>>>>>>>> Which make sense.
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> Now, with respect to getUnderlyingType - I'm a 
>>>>>>>>>>>>>>>>>>>>>>> bit at odds; it seems
>>>>>>>>>>>>>>>>>>>>>>> to me that the jextract tree should have the 
>>>>>>>>>>>>>>>>>>>>>>> right info, under
>>>>>>>>>>>>>>>>>>>>>>> TypedefTree::def, at least that is set by the 
>>>>>>>>>>>>>>>>>>>>>>> parser for
>>>>>>>>>>>>>>>>>>>>>>> declarations/definitions.
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> That exact case is covered by TypedefTree::def, 
>>>>>>>>>>>>>>>>>>>>>> but it doesn't cover a
>>>>>>>>>>>>>>>>>>>>>> case like this:
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>     struct BAR { int x; }
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>     typedef struct BAR FOO;
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> i.e. TypedefTree::def is only set when the typedef 
>>>>>>>>>>>>>>>>>>>>>> contains an inline
>>>>>>>>>>>>>>>>>>>>>> definition or a prototype. But, if the type is 
>>>>>>>>>>>>>>>>>>>>>> just referenced,
>>>>>>>>>>>>>>>>>>>>>> TypedefTree::def is an empty Optional.
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> Wouldn't visiting TypedefTree::def (if present) 
>>>>>>>>>>>>>>>>>>>>>>> be enough (and more in
>>>>>>>>>>>>>>>>>>>>>>> sync with the rest of jextract) ? After all, 
>>>>>>>>>>>>>>>>>>>>>>> that's the kind of thing
>>>>>>>>>>>>>>>>>>>>>>> you wanna be sure not to miss by the dependency 
>>>>>>>>>>>>>>>>>>>>>>> analysis -
>>>>>>>>>>>>>>>>>>>>>>> getUnderlyingType seems a tad too noisy?
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> This is what I had before, but it fell short in 
>>>>>>>>>>>>>>>>>>>>>> some cases.
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> In other words, let's say that there's a typedef 
>>>>>>>>>>>>>>>>>>>>>>> in an header of a
>>>>>>>>>>>>>>>>>>>>>>> library you want to jextract which is related, 
>>>>>>>>>>>>>>>>>>>>>>> via a chain of other 42
>>>>>>>>>>>>>>>>>>>>>>> typedefs to something in some obscure system 
>>>>>>>>>>>>>>>>>>>>>>> library. Are you sure you
>>>>>>>>>>>>>>>>>>>>>>> want the resulting artifact to have all the 42 
>>>>>>>>>>>>>>>>>>>>>>> intermediate typedefs?
>>>>>>>>>>>>>>>>>>>>>>> After all, the library you wanna extract has just 
>>>>>>>>>>>>>>>>>>>>>>> defined 'its
>>>>>>>>>>>>>>>>>>>>>>> version' of the name for that particular 
>>>>>>>>>>>>>>>>>>>>>>> construct... so it's very
>>>>>>>>>>>>>>>>>>>>>>> likely that users of that library will prefer 
>>>>>>>>>>>>>>>>>>>>>>> that name to the other
>>>>>>>>>>>>>>>>>>>>>>> 42 ones?
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> 42 intermediate typedefs seems a bit extreme...
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> But, I think you have a point. Putting the typedef 
>>>>>>>>>>>>>>>>>>>>>> and the canonical
>>>>>>>>>>>>>>>>>>>>>> type into the root set is probably enough.
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> On 13/03/2019 23:31, Jorn Vernee wrote:
>>>>>>>>>>>>>>>>>>>>>>>> Update webrev: 
>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.02/ 
>>>>>>>>>>>>>>>>>>>>>>>> This fixes an issue where typedef underlying 
>>>>>>>>>>>>>>>>>>>>>>>> types were not being added to the root set.
>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>> It turns out I was using the wrong API to get 
>>>>>>>>>>>>>>>>>>>>>>>> the underlying type, and the needed API didn't 
>>>>>>>>>>>>>>>>>>>>>>>> have a binding yet :)
>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>> The changes with respect to the previous webrev;
>>>>>>>>>>>>>>>>>>>>>>>>   * added Cursor.getTypedefDeclUnderlyingType() 
>>>>>>>>>>>>>>>>>>>>>>>> binding
>>>>>>>>>>>>>>>>>>>>>>>>   * added TypdefTree.underlyingType() method
>>>>>>>>>>>>>>>>>>>>>>>>   * changed DependencyFinder to use this method 
>>>>>>>>>>>>>>>>>>>>>>>> instead of typeDefinition()
>>>>>>>>>>>>>>>>>>>>>>>>   * added test case for the failure case (also 
>>>>>>>>>>>>>>>>>>>>>>>> simplified test class to only run jextract once 
>>>>>>>>>>>>>>>>>>>>>>>> and reuse the result)
>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-13 22:43:
>>>>>>>>>>>>>>>>>>>>>>>>> On 13/03/2019 21:36, Jorn Vernee wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>> Good questions. When no filter patterns are 
>>>>>>>>>>>>>>>>>>>>>>>>>>> specified that is the case
>>>>>>>>>>>>>>>>>>>>>>>>>>> everything is outputted, but when filter 
>>>>>>>>>>>>>>>>>>>>>>>>>>> patterns are specified
>>>>>>>>>>>>>>>>>>>>>>>>>>> headers are only added to the root set if 
>>>>>>>>>>>>>>>>>>>>>>>>>>> they contain a symbol that
>>>>>>>>>>>>>>>>>>>>>>>>>>> passes the filter.
>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>> FWIW, basically headers with symbols that pass 
>>>>>>>>>>>>>>>>>>>>>>>>>> both types of filter are added to the root 
>>>>>>>>>>>>>>>>>>>>>>>>>> set. There's 1.) Filtereing based on whether a 
>>>>>>>>>>>>>>>>>>>>>>>>>> symbol is found in a library, and 2.) based on 
>>>>>>>>>>>>>>>>>>>>>>>>>> the used filter patterns (--include-symbols & 
>>>>>>>>>>>>>>>>>>>>>>>>>> --exclude-symbols). I thought it would be 
>>>>>>>>>>>>>>>>>>>>>>>>>> better to exclude a header from the root set 
>>>>>>>>>>>>>>>>>>>>>>>>>> if all the symbols in it were filtered out by 
>>>>>>>>>>>>>>>>>>>>>>>>>> the filter patterns.
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> Yep - I think that sounds sensible
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn Vernee schreef op 2019-03-13 22:30:
>>>>>>>>>>>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-13 
>>>>>>>>>>>>>>>>>>>>>>>>>>> 22:08:
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Jorn,
>>>>>>>>>>>>>>>>>>>>>>>>>>>> this looks very good indeed. I'd like to 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> play with it a bit before we
>>>>>>>>>>>>>>>>>>>>>>>>>>>> go ahead with this, if that's ok.
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>> Please do!
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Also, one question: if the user does not 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> specify "-l" what happens? I
>>>>>>>>>>>>>>>>>>>>>>>>>>>> guess the root set will then contain all the 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> headers and we just get
>>>>>>>>>>>>>>>>>>>>>>>>>>>> back the full output (which would be ok) ?
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>> Good questions. When no filter patterns are 
>>>>>>>>>>>>>>>>>>>>>>>>>>> specified that is the case
>>>>>>>>>>>>>>>>>>>>>>>>>>> everything is outputted, but when filter 
>>>>>>>>>>>>>>>>>>>>>>>>>>> patterns are specified
>>>>>>>>>>>>>>>>>>>>>>>>>>> headers are only added to the root set if 
>>>>>>>>>>>>>>>>>>>>>>>>>>> they contain a symbol that
>>>>>>>>>>>>>>>>>>>>>>>>>>> passes the filter.
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>> Maybe we should just fall back to 
>>>>>>>>>>>>>>>>>>>>>>>>>>> 'include-everything' when no "-l"
>>>>>>>>>>>>>>>>>>>>>>>>>>> options are specified, regardless of filter?
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 13/03/2019 18:06, Jorn Vernee wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Updated webrev: 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.01/ 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Fixed the bug, also added tests & ran all 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the examples again to try and find other 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bugs (nothing found).
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn Vernee schreef op 2019-03-13 17:50:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Discovered a bug in this after posting :(
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DependencyFinder also needs to look for 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> pointee and array element type
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> cursors (which it currently doesn't). I've 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> tested a working fix, but
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> will also add some tests for the 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> particular cases. Updated webrev
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> coming soon™
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn Vernee schreef op 2019-03-13 17:21:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> After some discussion:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> https://mail.openjdk.java.net/pipermail/panama-dev/2019-March/004828.html 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I've implemented the discussed approach 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to header & declaration filtering.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bug: 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8220544 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Webrev: 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.00/ 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> There are a few new implementation 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> classes:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * PattertnFilter; a helper class that 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> encapsulates the Pattern based
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> filtering code previously found in 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> SymbolFilter. This code had to be
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> reused.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * Filters; a Context sub-component 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> holding PatternFilters for
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> different types of declarations.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * DependencyFinder; finds dependencies 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> of the root set of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> declarations found in the root headers.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * ElementFilter; filters out 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> non-library symbol elements based on
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> whether they appear in a root header or 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> are required by something
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> therein.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * RootSet; a helper class for managing 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> root headers + required element Trees.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I also encountered some crashes when 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> debug printing Trees, which I've
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> fixed/worked around accordingly.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> FWIW, I tried this out with the Windows 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> registry API and it's working
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> very nicely! Going from well over 100 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> headers to under 10. Some room
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> for improvement still exists; some empty 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> headers and static forwarders
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> could still be omitted, but imho that 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> should be handled by a separate
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> patch.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn


More information about the panama-dev mailing list