golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.16k stars 17.56k forks source link

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

Open philtay opened 4 years ago

philtay commented 4 years ago

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 commented 4 years ago

/cc @ianlancetaylor

ianlancetaylor commented 4 years ago

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

philtay commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

Thanks for the pointers.

philtay commented 4 years ago

/cc @FiloSottile

philtay commented 4 years ago

@ianlancetaylor I'd like to help with this issue. Can you point me in the right direction? Thanks.

ianlancetaylor commented 4 years ago

At this point I don't know what the right direction is. Changes in this area have a history of breaking existing code.

philtay commented 4 years ago

What about a cgo directive? Something like:

// #cgo include <foo.h>

ianlancetaylor commented 4 years ago

A #cgo directive seems like kind of a last resort. I would hate to have to write the documentation that explains when you should write #include and when you should write #cgo include. While cgo is not particularly simple, our goal as to always be to make it as simple as possible, and distinguishing #include and #cgo include cannot be described as simple.

odeke-em commented 4 years ago

Quite a tricky change to get into this cycle, thus I shall move it to Go1.16.

aclements commented 3 years ago

Moving to Unplanned, since currently we don't have a plan to address this.

lollipopman commented 1 day ago

Though not a full solution to this request, you can use -D_POSIX_C_SOURCE=200112L in cgo's CFLAGS to set the _POSIX_C_SOURCE version desired and override the version default in `