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

Fix #1424, api warning listInterfaces #1428

Conversation

paulober
Copy link

@paulober paulober commented Sep 8, 2022

By submitting this pull request, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your PR:

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

1

@yubiuser
Copy link
Member

yubiuser commented Sep 8, 2022

Pi-hole enforces DCO. Please sign-off your commit.

https://docs.pi-hole.net/guides/github/how-to-signoff/

@DL6ER DL6ER changed the base branch from master to development September 8, 2022 10:40
@DL6ER
Copy link
Member

DL6ER commented Sep 8, 2022

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

I changed the target branch for you.


Is this the only issue gcc-12 found? Does compilation succeed on your system on this branch? I'm just trying to avoid many bits-and-pieces pull requests.

@paulober
Copy link
Author

paulober commented Sep 8, 2022

I also got this error message on build with gcc 12.1.1 20220730:

[ 45%] Building C object src/dnsmasq/CMakeFiles/dnsmasq.dir/domain-match.c.o
/home/paul/FTL/src/args.c: In Funktion »cli_tick«:
/home/paul/FTL/src/args.c:413:37: Error: Function could be a candidate for attribute »pure« [-Werror=suggest-attribute=pure]
  413 | const char __attribute__ ((const)) *cli_tick(void)
      |                                     ^~~~~~~~
/home/paul/FTL/src/args.c: In Funktion »cli_cross«:
/home/paul/FTL/src/args.c:419:37: Error: Function could be a candidate for attribute »pure« [-Werror=suggest-attribute=pure]
  419 | const char __attribute__ ((const)) *cli_cross(void)
      |                                     ^~~~~~~~~
/home/paul/FTL/src/args.c: In Funktion »cli_info«:
/home/paul/FTL/src/args.c:425:37: Error: Function could be a candidate for attribute »pure« [-Werror=suggest-attribute=pure]
  425 | const char __attribute__ ((const)) *cli_info(void)
      |                                     ^~~~~~~~
/home/paul/FTL/src/args.c: In Funktion »cli_done«:
/home/paul/FTL/src/args.c:437:37: Error: Function could be a candidate for attribute »pure« [-Werror=suggest-attribute=pure]
  437 | const char __attribute__ ((const)) *cli_done(void)
      |                                     ^~~~~~~~
/home/paul/FTL/src/args.c: In Funktion »cli_bold«:
/home/paul/FTL/src/args.c:443:37: Error: Function could be a candidate for attribute »pure« [-Werror=suggest-attribute=pure]
  443 | const char __attribute__ ((const)) *cli_bold(void)
      |                                     ^~~~~~~~
/home/paul/FTL/src/args.c: In Funktion »cli_normal«:
/home/paul/FTL/src/args.c:449:37: Error: Function could be a candidate for attribute »pure« [-Werror=suggest-attribute=pure]
  449 | const char __attribute__ ((const)) *cli_normal(void)
      |                                     ^~~~~~~~~~
cc1: All warnings are treated as errors
make[2]: *** [src/CMakeFiles/FTL.dir/build.make:76: src/CMakeFiles/FTL.dir/args.c.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:237: src/CMakeFiles/FTL.dir/all] Error 2

@paulober
Copy link
Author

paulober commented Sep 8, 2022

I changed the target branch for you.

And thanks for changeing it

@paulober
Copy link
Author

paulober commented Sep 8, 2022

@paulober paulober force-pushed the fix-1424-api-warning-listInterfaces branch from 4023864 to 02fe348 Compare September 8, 2022 21:16
@DL6ER
Copy link
Member

DL6ER commented Sep 9, 2022

The GCC documentation clearly states:

The const attribute imposes greater restrictions on a function’s definition than the similar pure attribute.

Please try __attribute__ ((const,pure)) and see if the error goes away. This would be kind of an ugly workaround but it seems worth trying. I'm on a business trip and will only return tomorrow evening so I cannot do any real testing from my side right now

gcc 12.1.1 optimization
Signed-off-by: paulober <44974737+paulober@users.noreply.github.com>
@paulober paulober force-pushed the fix-1424-api-warning-listInterfaces branch from 02fe348 to bbe752e Compare September 9, 2022 08:57
@paulober
Copy link
Author

paulober commented Sep 9, 2022

Does not work:

[ 45%] Building C object src/dnsmasq/CMakeFiles/dnsmasq.dir/domain-match.c.o
/home/paul/FTL/src/tre-regex/xmalloc.c: In Funktion »xrealloc_impl«:
/home/paul/FTL/src/tre-regex/xmalloc.c:343:7: Warning: pointer "ptr" may be used after "realloc" [-Wuse-after-free]
  343 |       hash_table_del(xmalloc_table, ptr);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/paul/FTL/src/tre-regex/xmalloc.c:340:13: Anmerkung: »realloc« wird hier aufgerufen
  340 |   new_ptr = realloc(ptr, new_size);
      |             ^~~~~~~~~~~~~~~~~~~~~~
[ 45%] Built target tre-regex
[ 45%] Building C object src/CMakeFiles/FTL.dir/args.c.o
/home/paul/FTL/src/args.c:414:1: Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  414 | {
      | ^
In Datei, eingebunden von /home/paul/FTL/src/args.c:17:
/home/paul/FTL/src/args.h:19:13: Note: previous declaration here
   19 | const char *cli_tick(void) __attribute__ ((const));
      |             ^~~~~~~~
/home/paul/FTL/src/args.c:414:1: Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  414 | {
      | ^
/home/paul/FTL/src/args.h:19:13: Note: previous declaration here
   19 | const char *cli_tick(void) __attribute__ ((const));
      |             ^~~~~~~~
/home/paul/FTL/src/args.c:420:1: Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  420 | {
      | ^
/home/paul/FTL/src/args.h:20:13: Note: previous declaration here
   20 | const char *cli_cross(void) __attribute__ ((const));
      |             ^~~~~~~~~
/home/paul/FTL/src/args.c:420:1: Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  420 | {
      | ^
/home/paul/FTL/src/args.h:20:13: Note: previous declaration here
   20 | const char *cli_cross(void) __attribute__ ((const));
      |             ^~~~~~~~~
/home/paul/FTL/src/args.c:426:1: Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  426 | {
      | ^
/home/paul/FTL/src/args.h:21:13: Note: previous declaration here
   21 | const char *cli_info(void) __attribute__ ((const));
      |             ^~~~~~~~
/home/paul/FTL/src/args.c:426:1: Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  426 | {
      | ^
/home/paul/FTL/src/args.h:21:13: Note: previous declaration here
   21 | const char *cli_info(void) __attribute__ ((const));
      |             ^~~~~~~~
/home/paul/FTL/src/args.c:438:1: Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  438 | {
      | ^
/home/paul/FTL/src/args.h:23:13: Note: previous declaration here
   23 | const char *cli_done(void) __attribute__ ((const));
      |             ^~~~~~~~
/home/paul/FTL/src/args.c:438:1: Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  438 | {
      | ^
/home/paul/FTL/src/args.h:23:13: Note: previous declaration here
   23 | const char *cli_done(void) __attribute__ ((const));
      |             ^~~~~~~~
/home/paul/FTL/src/args.c:444:1: Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  444 | {
      | ^
/home/paul/FTL/src/args.h:24:13: Note: previous declaration here
   24 | const char *cli_bold(void) __attribute__ ((const));
      |             ^~~~~~~~
/home/paul/FTL/src/args.c:444:1: Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  444 | {
      | ^
/home/paul/FTL/src/args.h:24:13: Note: previous declaration here
   24 | const char *cli_bold(void) __attribute__ ((const));
      |             ^~~~~~~~
/home/paul/FTL/src/args.c:450:1:Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  450 | {
      | ^
/home/paul/FTL/src/args.h:25:13: Note: previous declaration here
   25 | const char *cli_normal(void) __attribute__ ((const));
      |             ^~~~~~~~~~
/home/paul/FTL/src/args.c:450:1: Error: attribute "pure" conflicts with attribute "const"; ignored [-Werror=attributes]
  450 | {
      | ^
/home/paul/FTL/src/args.h:25:13: Note: previous declaration here
   25 | const char *cli_normal(void) __attribute__ ((const));
      |             ^~~~~~~~~~
[ 46%] Building C object src/lua/CMakeFiles/lua.dir/lfunc.c.o
/home/paul/FTL/src/args.c: In Funktion »cli_tick«:
/home/paul/FTL/src/args.c:413:43: Error: function could be a candidate for attribute "pure [-Werror=suggest-attribute=pure]
  413 | const char __attribute__ ((const, pure)) *cli_tick(void)
      |                                           ^~~~~~~~
/home/paul/FTL/src/args.c: In Funktion »cli_cross«:
/home/paul/FTL/src/args.c:419:43: Error: function could be a candidate for attribute "pure [-Werror=suggest-attribute=pure]
  419 | const char __attribute__ ((const, pure)) *cli_cross(void)
      |                                           ^~~~~~~~~
/home/paul/FTL/src/args.c: In Funktion »cli_info«:
/home/paul/FTL/src/args.c:425:43: Error: function could be a candidate for attribute "pure [-Werror=suggest-attribute=pure]
  425 | const char __attribute__ ((const, pure)) *cli_info(void)
      |                                           ^~~~~~~~
/home/paul/FTL/src/args.c: In Funktion »cli_done«:
/home/paul/FTL/src/args.c:437:43: Error: function could be a candidate for attribute "pure [-Werror=suggest-attribute=pure]
  437 | const char __attribute__ ((const, pure)) *cli_done(void)
      |                                           ^~~~~~~~
/home/paul/FTL/src/args.c: In Funktion »cli_bold«:
/home/paul/FTL/src/args.c:443:43: Error: function could be a candidate for attribute "pure [-Werror=suggest-attribute=pure]
  443 | const char __attribute__ ((const, pure)) *cli_bold(void)
      |                                           ^~~~~~~~
/home/paul/FTL/src/args.c: In Funktion »cli_normal«:
/home/paul/FTL/src/args.c:449:43: Error: function could be a candidate for attribute "pure [-Werror=suggest-attribute=pure]
  449 | const char __attribute__ ((const, pure)) *cli_normal(void)
      |                                           ^~~~~~~~~~
cc1: Alle Warnungen werden als Fehler behandelt
make[2]: *** [src/CMakeFiles/FTL.dir/build.make:76: src/CMakeFiles/FTL.dir/args.c.o] Fehler 1
make[1]: *** [CMakeFiles/Makefile2:237: src/CMakeFiles/FTL.dir/all] Fehler 2
make[1]: *** Es wird auf noch nicht beendete Prozesse gewartet....
[ 46%] Building C object src/lua/CMakeFiles/lua.dir/lgc.c.o

@DL6ER
Copy link
Member

DL6ER commented Sep 16, 2022

It somewhat looks rather odd what you are seeing there. How about doing what it suggests?

function could be a candidate for attribute "pure

so something like

-const char __attribute__ ((const)) *cli_tick(void)
+const char __attribute__ ((pure)) *cli_tick(void)

?

Having said that, I should also say that this would make it worse from a performance optimization point as const is more strict that pure, since functions attributed like this are not allowed to read global memory and purely return the same value all the time (hence, the name const).

@paulober
Copy link
Author

How about doing what it suggests?

Does still not work... And does also throw errors like:

/home/paul/FTL/src/args.h:24:13: Note: previous declaration here
   24 | const char *cli_bold(void) __attribute__ ((const));
      | ^~~~~~~~
/home/paul/FTL/src/args.c:450:1: Error: attribute "pure" conflicts with attribute "const";

@DL6ER
Copy link
Member

DL6ER commented Sep 18, 2022

Sure, you also need to change the attribute there (declaration), not only where you did it now (definition). I'd do it myself (probably using an ArchLinux docker container, but I don't have much time right now and also no knowledge about the commands needed on ArchLinux to set up a system suitable for compilation. I you'd provide what is needed (like which packages need to get installed (and how)), I can try taking a look myself. Otherwise, I'm also happy to assist you further if you want to do this yourself.

@paulober
Copy link
Author

Sure, you also need to change the attribute there (declaration), not only where you did it now (definition). [...] I'm also happy to assist you further if you want to do this yourself.

I just launched my virtual box and push commit as soon as possible. Thanks for the advice

@paulober
Copy link
Author

Ok now that i also changed the declaration i suggests const instead of pure:

/home/paul/FTL/src/args.c:443:36: Error: function could be a candidate for attribute »const« [-Werror=suggest-attribute=const]
  443 | const char __attribute__ ((pure)) *cli_bold(void)

Maybe it would be bettwer if we just remove the Werror for const and pure suggestions in the compiler?

@DL6ER
Copy link
Member

DL6ER commented Sep 18, 2022

So this looks very much like an unfixable gcc bug in ArchLinux (suggesting pure when we have const, suggesting const when we have pure). As far as I remember, Arch's whole philosophy is tailored around "bleeding edge" and this may be one of the problems that are kind of foreseeable with such an approach. Hopefully, this will be resolved soon - have you checked if there is already a bug report for this? If not, it is surely worthwhile to open one.

Even when we'd relax FTLs philosophy to enforce zero compile warnings for the code, we'd still have an unresolvable warning which just feels wrong in every direction I can think of.

@paulober
Copy link
Author

paulober commented Sep 18, 2022

@DL6ER Yes that's true, the version i used is GCC 12.2.0 which has been released on 19th August 2022 so very bleeding edge.
And i found a bug report which seems to be the one we have: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105676

It says "retargeting bugs to GCC 12.3."...

@DL6ER
Copy link
Member

DL6ER commented Sep 24, 2022

I've just seen your edit, this doesn't look good. So I guess this becomes a "won't fix" until GCC 12.3 then?

@DL6ER
Copy link
Member

DL6ER commented Nov 6, 2022

@paulober This has been addressed in #1465 - would be nice if you could confirm the current development branch builds fine on your system. I'm going to close this PR now.

@DL6ER DL6ER closed this Nov 6, 2022
@paulober
Copy link
Author

paulober commented Nov 6, 2022

@paulober This has been addressed in #1465 - would be nice if you could confirm the current development branch builds fine on your system. I'm going to close this PR now.

I replied in the linked pr. But why haven't you just commited to this pr?

@DL6ER
Copy link
Member

DL6ER commented Nov 6, 2022

Because I had to fix this myself in a different context and just cheery-picked my commit from over there. It wasn't compatible with your branch due to quite a few conflicts and resolving them didn't look like a fun thing to do when I already had a commit that applied cleanly on the development branch

@paulober paulober deleted the fix-1424-api-warning-listInterfaces branch November 6, 2022 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants