Re: [Cython] Fwd: Question about how best require compiler options for C sources
On Fri, Apr 8, 2016 at 5:49 PM, Nathaniel Smith wrote: > Can you give a tiny concrete example? My questions are basic enough that I > feel like I'm missing something fundamental :-) Yes, I think you might be missing something, but I'm not sure exactly where. In the issue I provided a tiny example which you can see here: https://gist.github.com/embray/12a67edb82b213217e31f408007898e6 The C code generated by this example currently does not compile on Windows, because of how Cython uses DL_IMPORT incorrectly. Regardless of what it *does* do, Cython should not be using the DL_IMPORT macro at all actually since that no longer even exists in Python. > My first question is why you even need this, since AFAIK there are no cases > where it is correct to have a cython module dllexporting symbols that appear > in header files. This is what cimport is for, right? I don't think this has anything to do with cimport. Could you explain what you mean? > My second question is why you would want to do this via the command line, > when compiling the dll means that you are compiling some cython-generated > .c, which means that you can put the #define directly in the source code, > no? Not necessarily. As you can see in my example the file fooutil.c is hand-written C code that was not generated by Cython, but which uses a function in the Cython-generated code. It includes "foo.h". In principle you're right--the hand-written C code could set the proper #defines but it would have to do so *before* including "foo.h". It's not very obvious that this would be needed. Most other code I've seen that addresses this issue--including cpython itself--do so by passing an appropriate define to the compiler via the command-line, and that seems the clearest to me. Best, Erik > On Apr 8, 2016 5:35 AM, "Erik Bray" wrote: >> >> Hi all, >> >> I'd like to call attention to an issue I've been looking into for the >> past couple days: >> >> https://github.com/cython/cython/pull/360 >> >> To summarize the discussion, when building DLLs for Windows, functions >> that should be exported by that DLL must be marked >> __declspec(dllexport) in their declaration. However, when using the >> same header file in a project that links to that DLL the same function >> must be declared __declspec(dllimport). >> >> It's common practice to have a macro for this, whose value is >> controlled by whether or not a macro (often called something like >> "DLL_EXPORT"). When compiling the DLL we would define -DDLL_EXPORT to >> output __declspec(dllexport). Otherwise it outputs >> __declspec(dllimport). >> >> Cython currently handles this with such a macro called DL_IMPORT which >> comes from Python. However, this macro was deprecated some time ago, >> and is removed in current Python 3 versions. So Cython must replace >> it with its own. >> >> I'm working on a patch for this--to reduce confusion the macro is >> named specifically for the Cython module it's associated with. For >> example, for a Cython module named "foo.bar" there are two macros >> DLL_EXPORT__foo__bar and EXPORT__foo__bar. If the latter is defined >> then the former outputs dllexport, otherwise it outputs dllimport. >> I've attached the patch in progress. >> >> I'm open to comment on this, but where I'm stuck now is that in order >> for the "foo.bar" module to be compiled correctly it needs >> EXPORT__foo__bar to be defined at compile time. And it's not clear to >> me what the best way is for the Cython compiler to pass down options >> that are passed to the C/C++ compiler. In general the best way would >> be to attach this to the define_macros attribute of the associated >> distutils/setuptools Extension object and let the distutils compiler >> class generate the right compiler options. But it's not clear to me >> from Cython's internals where the best place to do that would be. >> >> Thanks, >> Erik >> >> ___ >> cython-devel mailing list >> cython-devel@python.org >> https://mail.python.org/mailman/listinfo/cython-devel >> > > ___ > cython-devel mailing list > cython-devel@python.org > https://mail.python.org/mailman/listinfo/cython-devel > ___ cython-devel mailing list cython-devel@python.org https://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] Fwd: Question about how best require compiler options for C sources
On Apr 11, 2016 04:18, "Erik Bray" wrote: > > On Fri, Apr 8, 2016 at 5:49 PM, Nathaniel Smith wrote: > > Can you give a tiny concrete example? My questions are basic enough that I > > feel like I'm missing something fundamental :-) > > Yes, I think you might be missing something, but I'm not sure exactly > where. In the issue I provided a tiny example which you can see here: > > https://gist.github.com/embray/12a67edb82b213217e31f408007898e6 > > The C code generated by this example currently does not compile on > Windows, because of how Cython uses DL_IMPORT incorrectly. Regardless > of what it *does* do, Cython should not be using the DL_IMPORT macro > at all actually since that no longer even exists in Python. Sure. For that example, the correct thing to do is to *not* export the function. Backing up, to make sure we're on the same page: There are three levels of symbol visibility in C: file-internal, shared library internal (different .c files that make up the library can see it, but users of the shared library can't see it), and shared library exported (everyone can see it; can also carry other consequences, e.g. on Linux then internal calls will become noticeably slower, and it becomes possible for weird symbol interposition issues to occur). So the rule of thumb is to make everything as private as you can get away with. Making this more interesting: - vanilla C only includes 2 ways to mark symbol visibility, which is not enough to make a 3-way distinction. Hence the need for an extra attribute thingummy. - everyone agrees that symbols marked 'static' should be file-internal, but different platforms disagree about what should happen if the extra attribute thingummy is missing. So on Windows, the convention is: 'static' -> file internal no marking -> shared library internal 'dllexport' -> public And on Linux it's: 'static' -> file internal 'visibility (hidden)' -> shared library internal no marking -> public It's generally agreed that Linux got this wrong and that you should always use '-fvisibility=hidden' to switch it to the windows style, but cython doesn't control compiler options and thus should probably generate code that works correctly regardless of such compiler settings. Fortunately, Linux does provide some markings to explicitly make things public: you can mark a symbol 'visibility (default)' (which means public), or you can use the dllexport syntax, just like Windows, because gcc is helpful like that. OTOH, windows is annoying because of this dllimport thing that started this whole thread: on other systems just marking symbols as extern is enough to handle both shared-object-internal and shared-library-exported symbols, and the linker will sort it out. On Windows, you have to explicitly distinguish between these. (And annoyingly, if you accidentally leave out the dllimport making on functions then it will use some fallback hack that works but silently degrades performance; on other symbols it just doesn't work, and ditto for if you use it when it isn't needed.) So final conclusion: for non-static symbols, cython should first decide whether they are supposed to be shared-library-internal or actually exported from the shared library. For shared-library-internal symbols: their definition should be marked 'visibility(hidden)' on Linux, and unmarked on Windows. This is easy using some preprocessor gunk. (Or maybe simplest is: marked that everywhere except if using msvc, because I think everyone else will understand 'visibility (hidden)' even if it's a no op.) Their declaration in the header file should just be 'extern' everywhere. For shared-library-exported symbols: I am dubious about whether cython should even support these at all. But if it does, then the definitions should be marked 'dllexport' (no macro trickery needed, because everyone understands this syntax), and their declaration in the header file needs some extra hack that is the subject of this thread. Now, back to your example: Here the caller and callee are both compiled into the same shared library, so you don't want dllexport/dllimport at all, you just want a shared-library-internal symbol, which as we see is much easier. NumPy also ran into some problems with this in our experiments with using cython internally. Our temporary solution was to use the preprocessor to monkeypatch DL_IMPORT into expanding to the appropriate shared-library-internal thing :-). > > My first question is why you even need this, since AFAIK there are no cases > > where it is correct to have a cython module dllexporting symbols that appear > > in header files. This is what cimport is for, right? > > I don't think this has anything to do with cimport. Could you explain > what you mean? We only need to solve the dllimport issue if we want to support shared-library-exported symbols from cython extensions. This is only useful if you have different extensions that are directly linking to each other using the platform linker. But this simply
Re: [Cython] Fwd: Question about how best require compiler options for C sources
On Mon, Apr 11, 2016 at 2:51 PM, Nathaniel Smith wrote: > Now, back to your example: Here the caller and callee are both compiled into > the same shared library, so you don't want dllexport/dllimport at all, you > just want a shared-library-internal symbol, which as we see is much easier. Sorry, I'll respond more to your (helpfully detailed and precise) message in a second. But something I wanted to point out is that my example is incomplete and I should have arranged a more complete example. In this case I really do want the symbol "hello" to be exported by the DLL, as well as be understood between translation units making up the same library. A more complete example would have shown a separate library which links with "foo" and uses the "hello" function in it. Yes, it's arguable that exporting anything more than then initmodule function from a Python extension module is not best practice, but the possibility should not be ruled out either. So I think later on you hit correctly on the deeper problem, which is that Cython currently doesn't have a great way to distinguish between intra-library visibility and *inter*-library visibility. And if both are needed then some DL_IMPORT-like macro is needed that sets the visibility for a symbol correctly depending on the context in which it's being used. (And yes, this is not a problem on *nix, but it is on Windows due to the way __declspec(dllimport) causes name mangling :( Thanks, Erik ___ cython-devel mailing list cython-devel@python.org https://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] Fwd: Question about how best require compiler options for C sources
On Mon, Apr 11, 2016 at 2:51 PM, Nathaniel Smith wrote: > On Apr 11, 2016 04:18, "Erik Bray" wrote: >> >> On Fri, Apr 8, 2016 at 5:49 PM, Nathaniel Smith wrote: >> > Can you give a tiny concrete example? My questions are basic enough that >> > I >> > feel like I'm missing something fundamental :-) >> >> Yes, I think you might be missing something, but I'm not sure exactly >> where. In the issue I provided a tiny example which you can see here: >> >> https://gist.github.com/embray/12a67edb82b213217e31f408007898e6 >> >> The C code generated by this example currently does not compile on >> Windows, because of how Cython uses DL_IMPORT incorrectly. Regardless >> of what it *does* do, Cython should not be using the DL_IMPORT macro >> at all actually since that no longer even exists in Python. > > Sure. > > For that example, the correct thing to do is to *not* export the function. So as I wrote in my previous message, my example was incomplete. But indeed if the intent was *only* to share the declaration between TUs of the same library then I agree the *most* correct thing would be to not use dllexport. Unfortunately there isn't currently a way in Cython to make this distinction. > Backing up, to make sure we're on the same page: Yep, I agree with your analysis that follows, and it's helpful to have all the cases laid out in one place, so thanks for that! There are three levels of > symbol visibility in C: file-internal, shared library internal (different .c > files that make up the library can see it, but users of the shared library > can't see it), and shared library exported (everyone can see it; can also > carry other consequences, e.g. on Linux then internal calls will become > noticeably slower, and it becomes possible for weird symbol interposition > issues to occur). So the rule of thumb is to make everything as private as > you can get away with. > > Making this more interesting: > - vanilla C only includes 2 ways to mark symbol visibility, which is not > enough to make a 3-way distinction. Hence the need for an extra attribute > thingummy. > - everyone agrees that symbols marked 'static' should be file-internal, but > different platforms disagree about what should happen if the extra attribute > thingummy is missing. > > So on Windows, the convention is: > 'static' -> file internal > no marking -> shared library internal > 'dllexport' -> public > > And on Linux it's: > 'static' -> file internal > 'visibility (hidden)' -> shared library internal > no marking -> public > > It's generally agreed that Linux got this wrong and that you should always > use '-fvisibility=hidden' to switch it to the windows style, but cython > doesn't control compiler options and thus should probably generate code that > works correctly regardless of such compiler settings. Fortunately, Linux > does provide some markings to explicitly make things public: you can mark a > symbol 'visibility (default)' (which means public), or you can use the > dllexport syntax, just like Windows, because gcc is helpful like that. > > OTOH, windows is annoying because of this dllimport thing that started this > whole thread: on other systems just marking symbols as extern is enough to > handle both shared-object-internal and shared-library-exported symbols, and > the linker will sort it out. On Windows, you have to explicitly distinguish > between these. (And annoyingly, if you accidentally leave out the dllimport > making on functions then it will use some fallback hack that works but > silently degrades performance; on other symbols it just doesn't work, and > ditto for if you use it when it isn't needed.) Yep. Agreed with all of the above. > So final conclusion: for non-static symbols, cython should first decide > whether they are supposed to be shared-library-internal or actually exported > from the shared library. > > For shared-library-internal symbols: their definition should be marked > 'visibility(hidden)' on Linux, and unmarked on Windows. This is easy using > some preprocessor gunk. (Or maybe simplest is: marked that everywhere except > if using msvc, because I think everyone else will understand 'visibility > (hidden)' even if it's a no op.) Their declaration in the header file should > just be 'extern' everywhere. > > For shared-library-exported symbols: I am dubious about whether cython > should even support these at all. But if it does, then the definitions > should be marked 'dllexport' (no macro trickery needed, because everyone > understands this syntax), and their declaration in the header file needs > some extra hack that is the subject of this thread. I understand your doubts here, but for the purpose of discussion let's just assume that it should be supported? :) And yes, the issue that the header file needs some hacky context-dependent declarations. A possible alternative, which in fact is exactly the work-around I'm using right now, would be for Cython to generate two headers. In the case of my example the
Re: [Cython] Fwd: Question about how best require compiler options for C sources
On Apr 11, 2016 06:23, "Erik Bray" wrote:
>
> On Mon, Apr 11, 2016 at 2:51 PM, Nathaniel Smith wrote:
> > Now, back to your example: Here the caller and callee are both compiled
into
> > the same shared library, so you don't want dllexport/dllimport at all,
you
> > just want a shared-library-internal symbol, which as we see is much
easier.
>
> Sorry, I'll respond more to your (helpfully detailed and precise)
> message in a second. But something I wanted to point out is that my
> example is incomplete and I should have arranged a more complete
> example. In this case I really do want the symbol "hello" to be
> exported by the DLL, as well as be understood between translation
> units making up the same library. A more complete example would have
> shown a separate library which links with "foo" and uses the "hello"
> function in it.
>
> Yes, it's arguable that exporting anything more than then initmodule
> function from a Python extension module is not best practice, but the
> possibility should not be ruled out either.
>
> So I think later on you hit correctly on the deeper problem, which is
> that Cython currently doesn't have a great way to distinguish between
> intra-library visibility and *inter*-library visibility.
>
> And if both are needed then some DL_IMPORT-like macro is needed that
> sets the visibility for a symbol correctly depending on the context in
> which it's being used. (And yes, this is not a problem on *nix, but it
> is on Windows due to the way __declspec(dllimport) causes name
> mangling :(
This is highly tangential to the main conversation, but FYI and in the
general interests of demystifying this stuff: the reason for all this
rigmarole on Windows is that dllimport doesn't just cause name mangling, it
causes *type* mangling. The way windows symbol relocations work, is that
you can only import pointers. So code like
__declspec(dllimport) extern void f(...);
__declspec(dllimport) extern int myvalue;
is syntactic sugar for something like:
// pointer that will be filled in by loader
void (*__imp_f)(...);
// desugar direct calls into indirect calls
#define f(...) (*__imp_f)(...)
// similar
int *__imp_myint;
#define myint (*__imp_myint)
...except that (a) instead of using the preprocessor to perform the
substitution, it happens in the compiler frontend, so it can correctly
follow scoping rules and things that the preprocessor can't, and (b) the
linker will also automagically generate a function like
void f(...) {
return (*__imp_f)(...);
}
So there shouldn't still be any mentions of 'f' in the resulting file --
they should all be replaced by mentions of (*__imp_f) -- but just in case
we missed any your code will still work, albeit at the cost of some icache
pollution and an extra indirect jump at every call.
Note in particular that this __imp_ nonsense *isn't* an approximation on my
part, there really will be a shared-library-internal symbol called
__imp_whatever whose value is a pointer that gets filled in my the loader.
(Except possibly I got my underscores wrong -- on phone so too lazy to
check.) You could literally write the code I wrote above with the #define's
and it would actually work on windows...
-n
___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] Fwd: Question about how best require compiler options for C sources
On 2016-04-11 15:23, Erik Bray wrote: In this case I really do want the symbol "hello" to be exported by the DLL, as well as be understood between translation units making up the same library. Are you really sure that you want this? I doubt that this is supported on OS X: on OS X, there are two kinds of files: shared libraries (with .dylib extension) and loadable modules or bundles (usually with .so extension but officially with .bundle extension). C extensions for Python are compiled as a loadable module and I don't think you can link against these. See http://stackoverflow.com/questions/2339679/what-are-the-differences-between-so-and-dylib-on-osx Some details in the above might be wrong, but I remember running into this issue in some early version of cysignals. Jeroen. ___ cython-devel mailing list cython-devel@python.org https://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] Fwd: Question about how best require compiler options for C sources
On Mon, Apr 11, 2016 at 7:46 AM Erik Bray wrote: > On Mon, Apr 11, 2016 at 2:51 PM, Nathaniel Smith wrote: > > On Apr 11, 2016 04:18, "Erik Bray" wrote: > >> > >> On Fri, Apr 8, 2016 at 5:49 PM, Nathaniel Smith wrote: > >> > Can you give a tiny concrete example? My questions are basic enough > that > >> > I > >> > feel like I'm missing something fundamental :-) > >> > >> Yes, I think you might be missing something, but I'm not sure exactly > >> where. In the issue I provided a tiny example which you can see here: > >> > >> https://gist.github.com/embray/12a67edb82b213217e31f408007898e6 > >> > >> The C code generated by this example currently does not compile on > >> Windows, because of how Cython uses DL_IMPORT incorrectly. Regardless > >> of what it *does* do, Cython should not be using the DL_IMPORT macro > >> at all actually since that no longer even exists in Python. > > > > Sure. > > > > For that example, the correct thing to do is to *not* export the > function. > > So as I wrote in my previous message, my example was incomplete. But > indeed if the intent was *only* to share the declaration between TUs > of the same library then I agree the *most* correct thing would be to > not use dllexport. Unfortunately there isn't currently a way in Cython > to make this distinction. > > > Backing up, to make sure we're on the same page: > > Yep, I agree with your analysis that follows, and it's helpful to have > all the cases laid out in one place, so thanks for that! > > There are three levels of > > symbol visibility in C: file-internal, shared library internal > (different .c > > files that make up the library can see it, but users of the shared > library > > can't see it), and shared library exported (everyone can see it; can also > > carry other consequences, e.g. on Linux then internal calls will become > > noticeably slower, and it becomes possible for weird symbol interposition > > issues to occur). So the rule of thumb is to make everything as private > as > > you can get away with. > > > > Making this more interesting: > > - vanilla C only includes 2 ways to mark symbol visibility, which is not > > enough to make a 3-way distinction. Hence the need for an extra attribute > > thingummy. > > - everyone agrees that symbols marked 'static' should be file-internal, > but > > different platforms disagree about what should happen if the extra > attribute > > thingummy is missing. > > > > So on Windows, the convention is: > > 'static' -> file internal > > no marking -> shared library internal > > 'dllexport' -> public > > > > And on Linux it's: > > 'static' -> file internal > > 'visibility (hidden)' -> shared library internal > > no marking -> public > > > > It's generally agreed that Linux got this wrong and that you should > always > > use '-fvisibility=hidden' to switch it to the windows style, but cython > > doesn't control compiler options and thus should probably generate code > that > > works correctly regardless of such compiler settings. Fortunately, Linux > > does provide some markings to explicitly make things public: you can > mark a > > symbol 'visibility (default)' (which means public), or you can use the > > dllexport syntax, just like Windows, because gcc is helpful like that. > > > > OTOH, windows is annoying because of this dllimport thing that started > this > > whole thread: on other systems just marking symbols as extern is enough > to > > handle both shared-object-internal and shared-library-exported symbols, > and > > the linker will sort it out. On Windows, you have to explicitly > distinguish > > between these. (And annoyingly, if you accidentally leave out the > dllimport > > making on functions then it will use some fallback hack that works but > > silently degrades performance; on other symbols it just doesn't work, and > > ditto for if you use it when it isn't needed.) > > Yep. Agreed with all of the above. > > > So final conclusion: for non-static symbols, cython should first decide > > whether they are supposed to be shared-library-internal or actually > exported > > from the shared library. > > > > For shared-library-internal symbols: their definition should be marked > > 'visibility(hidden)' on Linux, and unmarked on Windows. This is easy > using > > some preprocessor gunk. (Or maybe simplest is: marked that everywhere > except > > if using msvc, because I think everyone else will understand 'visibility > > (hidden)' even if it's a no op.) Their declaration in the header file > should > > just be 'extern' everywhere. > > > > For shared-library-exported symbols: I am dubious about whether cython > > should even support these at all. But if it does, then the definitions > > should be marked 'dllexport' (no macro trickery needed, because everyone > > understands this syntax), and their declaration in the header file needs > > some extra hack that is the subject of this thread. > > I understand your doubts here, but for the purpose of discussion let's > just assume that i
Re: [Cython] Fwd: Question about how best require compiler options for C sources
On Mon, Apr 11, 2016 at 11:50 AM Ian Henriksen < insertinterestingnameh...@gmail.com> wrote: > To answer the original question about define macros, it appears that the > canonical > way to pass preprocessor defines through distutils is to use the > define_macros > keyword when constructing your Extension class. You should also be able to > do > this within a Cython source file by including a directive like: > > # distutils: define_macros = MY_DEFINE, MY_DEFINE_2=2 > > Unfortunately, it looks like there's a bug in that that's making it so > that these > macros are undef'ed rather than being defined, so, for now, just pass the > appropriate flags to your Extension object. > > Small update on this, it looks like the issue with undef/define only applies when a define is specified this way without a value, so I either haven't gotten the syntax quite right, or that's not supported yet. Specifying an actual value for the macro works fine. Best, Ian Henriksen ___ cython-devel mailing list cython-devel@python.org https://mail.python.org/mailman/listinfo/cython-devel
