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

Build failure with GCC 15 (incompatible pointer types) #5857

Open
orlitzky opened this issue Dec 4, 2024 · 13 comments
Open

Build failure with GCC 15 (incompatible pointer types) #5857

orlitzky opened this issue Dec 4, 2024 · 13 comments

Comments

@orlitzky
Copy link
Contributor

orlitzky commented Dec 4, 2024

Original report: https://bugs.gentoo.org/944036

GCC 15 has -Werror=incompatible-pointer-types and std=gnu23 by default, and this is causing some issues with the function pointers that GAP passes around. For example,

src/bool.c: In function 'InitKernel':
src/bool.c:332:22: error: passing argument 1 of 'InitHandlerFunc' from incompatible pointer type [-Wincompatible-pointer-types]
  332 |     InitHandlerFunc( ReturnTrue1, "src/bool.c:ReturnTrue1" );
      |                      ^~~~~~~~~~~
      |                      |
      |                      struct OpaqueBag * (*)(struct OpaqueBag *, struct OpaqueBag *)
In file included from src/bool.c:20:
src/calls.h:416:30: note: expected 'ObjFunc' {aka 'struct OpaqueBag * (*)(void)'} but argument is of type 'struct OpaqueBag * (*)(struct OpaqueBag *, struct OpaqueBag *)'
  416 | void InitHandlerFunc(ObjFunc hdlr, const Char * cookie);

(and many others like it).

After investigating the same issue in cvec, it looks like the issue can be traced back to the ObjFunc typedef. In C23, foo() means that foo takes zero arguments, so a cast is needed if you want to pass in a function that takes arguments in place of such a foo().

@fingolfin
Copy link
Member

Great. This will break basically all packages with kernel extensions. Gotta love how C committee replace the C language by a new language but retains the old name and shoves it down everyone's throat.

@fingolfin
Copy link
Member

I have access to neither GCC 14 and 15 and won't go out of my way to obtain it and work on this. But if anyone would like to submit reasonable patches to address this (likely by adding a gazillion type casts), I'll happily review and merge it.

Perhaps it would also work to change the first argument of InitHandlerFunc to take a void * (loosing expressiveness at the altar of "innovation", but so be it).

But there are a bunch of other functions taking an ObjFunc, e.g. SET_HDLR_FUNC, NewFunction, NewOperation

@fingolfin
Copy link
Member

(To be clear my anger is directed against the C committee, not against @orlitzky -- thank you for taking the time to report this, @orlitzky !)

@james-d-mitchell
Copy link
Contributor

Isn't one potential fix to just explicitly state the C standard that GAP is using? I.e. instead of using the default std=gnu23 (which I agree with @fingolfin is nuts), we explicitly state std=gnu99 or something instead?

@fingolfin
Copy link
Member

Not all compiler support that. One thing C and C++ standardization efforts completely dropped the ball is build systems. Which to me is the number 1 issue in terms of portability these days.

Of course we can try to handle it in configure... or just pretend only gcc and clang exist...

It is also a transitive issue, as we also need to specify this in GAP packages with a kernel extension. We could just try to pass the required -std option on, but a kext might be built using a different compiler than GAP... urgh

@orlitzky
Copy link
Contributor Author

orlitzky commented Dec 5, 2024

-std doesn't have to work everywhere, only on compilers that might default to C23 in the near future, so it could be a decent band-aid. The ./configure script can ensure that the compiler at least accepts the flag.

I should mention that foo(...) is supposed to be the C23 replacement for foo() in older versions, but I tried hacking the ObjFunc definition and it didn't magically fix everything so I gave up. I have too many other things to do at the moment (like fixing the 600+ instances of this same problem in symmetrica...) but if there's one typedef that works in C23 and one that works everywhere else, then we could put them behind an #if ( __STDC_VERSION__ >= 202300L ).

@fingolfin
Copy link
Member

Thanks for the pointers @orlitzky . Of course it shouldn't fall on you to deal with this, I understand you have other things on your plate. I am mostly grateful that we got a report, I much prefer this over applying patches without telling us!

@dimpase
Copy link
Member

dimpase commented Dec 5, 2024

I have access to neither GCC 14 and 15 and won't go out of my way to obtain it and work on this.

https://formulae.brew.sh/formula/gcc#default gives you gcc 14.
(or https://ports.macports.org/port/gcc14/details/)

@ChrisJefferson
Copy link
Contributor

While it's solving the problem by just turning off all checking, I installed gcc-14, observed the failuers, then changed common.h:168 (definition of ObjFunc) to just typedef void* ObjFunc;. Seemed to fix building GAP, and packages -- but I'm not 100% sure I built all the packages with gcc-14 and all the right flags (some might still be using the system compiler), so if it's not hard to do, could you check if that fixes the issue @orlitzky ?

Note some packages still fail, as they define variables called bool, but that's a fix for another day.

@orlitzky
Copy link
Contributor Author

orlitzky commented Dec 9, 2024

Using typedef void* ObjFunc; actually works pretty well. Semigroups still fails to build because the C++ bindings need an update, but otherwise the C packages that I tried all compiled without fuss.

I also did a few more experiments with (...) in C23. Clang 19 also fails to build GAP if I declare an ObjFunc to take a variable number of arguments, but it mainly seems concerned about passing in e.g. an ObjFunc_1ARGS where an ObjFunc is expected. I don't know why that would be an error if it wasn't one before C23, though. If I disable the incompatible pointer types error while building GAP, then the other packages like cvec do build against the header with typedef Obj (* ObjFunc) (...); in it.

GCC on the other hand is not so lucky. Building (say) cvec against the installed GAP (where ObjFunc is declared with a variable number of args) with gcc-15 still produces a bunch of

src/cvec.c:4234:5: error: initialization of 'struct OpaqueBag * (*)()' from incompatible pointer type 'struct OpaqueBag * (*)(struct OpaqueBag *)' [-Wincompatible-pointer-types]

But what's strange to me is the type in the error message. There are no ellipses in the first type, which makes me wonder if GCC is still thinking that ObjFunc takes no arguments for some reason? I'm using what is now a slightly older snapshot of gcc-15, but I'll try to upgrade and see if anything has changed. If not I may poke some people about this error.

@orlitzky
Copy link
Contributor Author

Here is a minimal example that fails with both clang and gcc in c23 mode:

#if ( __STDC_VERSION__ < 202300L )
// Works in C17 and earlier
typedef int (* IntFunc)();
#else
// Should work in C23?
typedef int (* IntFunc)(...);
#endif

int run_it(IntFunc f) {
  return f(22);
}

int random_number(int x) {
  return 6;
}

int main(int argc, char** argv) {
  return run_it(random_number);
}

@dimpase
Copy link
Member

dimpase commented Dec 12, 2024

The syntax of ISO C requires at least one fixed argument before the ‘…’. To make the example above work with clang 18, one needs to change two declarations/definitions:

typedef int (* IntFunc)(int, ...);

int random_number(int x, ...) {

@orlitzky
Copy link
Contributor Author

That should have changed with C23 though. Here is the old text:

If the list terminates with an ellipsis (, ...), no information about the number or types of the parameters after the comma is supplied.

And the new:

If the list terminates with an ellipsis (...), no information about the number or types of the parameters after the comma is supplied.

The paragraphs about compatible parameter lists have changed, too, though. In C17:

For two function types to be compatible, both shall specify compatible return types. Moreover, the parameter type lists, if both are present, shall agree in the number of parameters and in use of the ellipsis terminator; corresponding parameters shall have compatible types. If one type has a parameter type list and the other type is specified by a function declarator that is not part of a function definition and that contains an empty identifier list, the parameter list shall not have an ellipsis terminator and the type of each parameter shall be compatible with the type that results from the application of the default argument promotions. If one type has a parameter type list and the other type is specified by a function definition that contains a (possibly empty) identifier list, both shall agree in the number of parameters, and the type of each prototype parameter shall be compatible with the type that results from the application of the default argument promotions to the type of the corresponding identifier. (In the determination of type compatibility and of a composite type, each parameter declared with function or array type is taken as having the adjusted type and each parameter declared with qualified type is taken as having the unqualified version of its declared type.)

And C23:

For two function types to be compatible, both shall specify compatible return types. Moreover, the parameter type lists shall agree in the number of parameters and in use of the final ellipsis; corresponding parameters shall have compatible types. In the determination of type compatibility and of a composite type, each parameter declared with function or array type is taken as having the adjusted type and each parameter declared with qualified type is taken as having the unqualified version of its declared type.

So maybe it comes down to the precise meaning of that word salad, or the interpretation of "agree."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants