abcminiuser / lufa

LUFA - the Lightweight USB Framework for AVRs.
http://www.lufa-lib.org
1.04k stars 325 forks source link

Compilation fails with latest gcc 8.1 #130

Closed NicoHood closed 6 years ago

NicoHood commented 6 years ago

I get 2 kind of errors:

In file included from ../..//lib/USB_CDC_SERIAL/../USB//../LUFA/Drivers/USB/Class/Device/CDCClassDevice.c:38:
../..//lib/USB_CDC_SERIAL/../USB//../LUFA/Drivers/USB/Class/Device/CDCClassDevice.h:373:5: error: 'const' attribute on function returning 'void' [-Werror=attributes]
     void CDC_Device_Event_Stub(void) ATTR_CONST;
     ^~~~
../..//lib/USB_CDC_SERIAL/../USB//../LUFA/Drivers/USB/Class/Device/CDCClassDevice.h:375:10: error: 'EVENT_CDC_Device_LineEncodingChanged' alias between functions of incompatible types 'void(USB_ClassInfo_CDC_Device_t * const)' {aka 'void(struct <anonymous> * const)'} and 'void(void)' [-Werror=attribute-alias]
     void EVENT_CDC_Device_LineEncodingChanged(USB_ClassInfo_CDC_Device_t* const CDCInterfaceInfo)
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../..//lib/USB_CDC_SERIAL/../USB//../LUFA/Drivers/USB/Class/Device/CDCClassDevice.c:364:6: note: aliased declaration here
 void CDC_Device_Event_Stub(void)
      ^~~~~~~~~~~~~~~~~~~~~

The first one can be fixed by removing the const attribute. I am not sure what side effects it has.

The second one can be fixed by adding the parameters to the aliased function. Problem with that is, that for example those 3 functions link tot he same aliased function, but have different parameters:

void CDC_Device_Event_Stub(void);

void EVENT_CDC_Device_LineEncodingChanged(USB_ClassInfo_CDC_Device_t* const CDCInterfaceInfo)
                                          ATTR_WEAK ATTR_NON_NULL_PTR_ARG(1) ATTR_ALIAS(CDC_Device_Event_Stub);
void EVENT_CDC_Device_ControLineStateChanged(USB_ClassInfo_CDC_Device_t* const CDCInterfaceInfo)
                                             ATTR_WEAK ATTR_NON_NULL_PTR_ARG(1) ATTR_ALIAS(CDC_Device_Event_Stub);
void EVENT_CDC_Device_BreakSent(USB_ClassInfo_CDC_Device_t* const CDCInterfaceInfo,
                                const uint8_t Duration) ATTR_WEAK ATTR_NON_NULL_PTR_ARG(1)
                                ATTR_ALIAS(CDC_Device_Event_Stub);

I do not know how to properly fix it.

$ avr-gcc --version
avr-gcc (GCC) 8.1.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
abcminiuser commented 6 years ago

The ATTR_CONST is a mistake, and should be removed - easy fix, as you say.

Interesting (and good!) that newer GCC versions can validate the signatures of aliased functions. Does changing the signature to just void CDC_Device_Event_Stub(); help? In C, that should make the function accept any arguments.

What OS are you using to get access to such a new toolchain?

NicoHood commented 6 years ago

If I leave out the void it says:

../..//lib/USB_CDC_SERIAL/../USB//../LUFA/Drivers/USB/Class/Device/AudioClassDevice.h:380:5: error: function declaration isn't a prototype [-Werror=strict-prototypes]
     void Audio_Device_Event_Stub();

I am using ArchLinux.

Edit: Workaround: -Wno-attribute-alias from https://lkml.org/lkml/2018/6/11/675 Fix: https://github.com/abcminiuser/lufa/commit/129503c74d572bdbd4b049323f80ec8e0cccbeac

I recommend to not use those pragmas. If there is a better fix, please go for it. I have no idea sadly :(

abcminiuser commented 6 years ago

Fixed in d8e0d67caefa312970787645180ec5db2eff92cb and d8e0d67caefa312970787645180ec5db2eff92cb. I went with just adding a second stub function (that calls the first in case someone ever wants to override the default behaviour). Ugly, but better than disabling an (otherwise) useful warning.

NicoHood commented 6 years ago

You linked two identical commits. Does this create more overhead? :S

abcminiuser commented 6 years ago

Oops, I meant d8e0d67caefa312970787645180ec5db2eff92cb and a08a02481ba9e68933ad0b89483f5328767df9f3.

It's possible, depending on how clever the compiler and linker is, that this would result in an extra JMP instruction being emitted if a stubbed function is compiled in and not redirected, if the compiler can't figure out the extra symbol is superfluous.