Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/cgo: inject preamble before other include directives #35315

Open
philtay opened this issue Nov 2, 2019 · 10 comments

Comments

@philtay
Copy link

@philtay philtay commented Nov 2, 2019

What version of Go are you using (go version)?

go version go1.13.3 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you see?

The cgo generated file _cgo_export.c includes stdlib.h before anything else (see here). It precludes the possibility to use some "include first" headers without errors or warnings, such as Python.

From the Python docs:

Since Python may define some pre-processor definitions which affect the standard headers on some systems, you must include Python.h before any standard headers are included.

Cgo should allow to inject a C preamble before any other include directive.

@smasher164 smasher164 changed the title _cgo_export.c and include directives cmd/cgo: inject preamble before other include directives Nov 3, 2019
@smasher164

This comment has been minimized.

Copy link
Member

@smasher164 smasher164 commented Nov 3, 2019

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 5, 2019

Can you show an example where it would be appropriate for cgo code to include Python.h?

@philtay

This comment has been minimized.

Copy link
Author

@philtay philtay commented Nov 5, 2019

Sure. Basically just before any generated include directive. E.g.:

Actual _cgo_export.c:

/* Code generated by cmd/cgo; DO NOT EDIT. */

#include <stdlib.h>
#include "_cgo_export.h"

#pragma GCC diagnostic ignored "-Wunknown-pragmas"
...

Desired _cgo_export.c:

/* Code generated by cmd/cgo; DO NOT EDIT. */

#include <Python.h>
#include <stdlib.h>
#include "_cgo_export.h"

#pragma GCC diagnostic ignored "-Wunknown-pragmas"
...

The same goes for any other generated file which may contain Go exported functions and/or their declaration.

@philtay

This comment has been minimized.

Copy link
Author

@philtay philtay commented Nov 5, 2019

A possible solution could be to remove #include <stdlib.h> keeping only #include "_cgo_export.h". Then in _cgo_export.h the preamble from import "C" comments should be injected before any other include (i.e. stdlib.h and stddef.h).

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 5, 2019

@philtay Thanks. I understand the change you want to make. What I'm hoping to see is an example of actual Go code that requires that change. Not the output of cgo, but the code that a user would write that requires this change.

@philtay

This comment has been minimized.

Copy link
Author

@philtay philtay commented Nov 8, 2019

@ianlancetaylor Ah, ok. Please find below a self contained example:

main.go

package main

/*
#cgo pkg-config: python3
#cgo CFLAGS: -std=c99
#include <Python.h>
*/
import "C"

//export foobar
func foobar() *C.PyObject {
	return nil
}

func main() {}

main.c

#include <Python.h>
#include "_cgo_export.h"

PyObject *
foobar_wrap(PyObject *self, PyObject *Py_UNUSED(ignored))
{
    return foobar();
}

Run main.go to get an error like this one:

In file included from /usr/include/python3.7m/Python.h:8,
                 from main.go:6,
                 from _cgo_export.c:4:
/usr/include/python3.7m/pyconfig.h:1522: warning: "_POSIX_C_SOURCE" redefined
 1522 | #define _POSIX_C_SOURCE 200809L
      | 
In file included from /usr/include/bits/libc-header-start.h:33,
                 from /usr/include/stdlib.h:25,
                 from _cgo_export.c:3:
/usr/include/features.h:295: note: this is the location of the previous definition
  295 | # define _POSIX_C_SOURCE 199506L
      | 
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 8, 2019

Thanks for the example.

I'm sorry to be so unclear, but that is still not what I am looking for. I'm trying to understand in what cases someone would want to write Go code that uses cgo that does a #include <Python.h>. What is the actual Go code where this case arises?

Looking at your example, why would someone write Go code that returns a *PyObject? That seems like a memory leak or dangling pointer waiting to happen. Why not instead write C code that calls Go code to do some operation and then do the Python conversion in C? That seems much more likely to get memory issues right across the three languages.

@philtay

This comment has been minimized.

Copy link
Author

@philtay philtay commented Nov 8, 2019

@ianlancetaylor Two full blown examples in the wild:

My code is just a minimal reproducible example.

I think the author of this blog post is on your team:

https://blog.filippo.io/building-python-modules-with-go-1-5/

It explains why someone would want to write cgo code that does a #include <Python.h>.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 8, 2019

Thanks for the pointers.

@ianlancetaylor ianlancetaylor added this to the Go1.15 milestone Nov 8, 2019
@philtay

This comment has been minimized.

Copy link
Author

@philtay philtay commented Nov 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.