compilation failure with CFLAGS="fno-common" (multiple definition of `EC_PTHREAD_NULL')

[asarubbo]

You should be able to reproduce by yourself adding the mentioned flag

[sgeto]

Reminds me of a joke: An old woman goes to the doctor. She starts twisting her arm around and says: "Doctor, it hurts when I do this" And the doctor says: "Then don't do it."

Anyways, the error it probably thrown during linking, I'd suggest you export LDFLAGS=-Wl,--allow-multiple-definition before you start building or add LDFLAGS=--allow-multiple-definition to the makecommand (so make LDFLAGS=--allow-multiple-definition) and hold your breath...

This will make the linker use first definition. Not sure if this defeats the purpose of -fno-common though. Guess It'll show whether all the other definitions can be discarded.

[asarubbo]

@sgeto I didn't understand at all the joke you remembered, but, man gcc says:

Compiling with -fno-common is useful on targets for which it provides better performance, or if you wish to verify that the program will work on other systems that always treat uninitialized variable declarations this way.

From https://github.com/google/sanitizers/wiki/AddressSanitizer:

Q: Why didn't ASan report an obviously invalid memory access in my code? A: Another, C-only option is accesses to global common symbols which are not protected by Asan (you can use -fno-common to disable generation of common symbols and hopefully detect more bugs).

So, see the flag as something that makes a check. I don't need to export any LDFLAGS just for pass the compilation

[sgeto]

Hey sorry for the late reply. I suggested you to use the additional linker flag, because I assumed that you just wanted to get the damn thing to build. I can tell now that your intentions to compile with -fno-common are much more sophisticated. That would also explain why you didn't get the joke. It was a way of saying that if you simply don't use -fno-common you won't run into an error.

I guess that if you want that flag, you'll first need to find and get rid of all additional definitions of `EC_PTHREAD_NULL'. They (most likely) hold the exact same value, but can there can still only be one. Sorta like in "Highlander" (that's another joke)

[sgeto]

@asarubbo have you made peace with this?

I just remembered this issue and I wanted to share a few things: To recap, fno-common checks and errors if multiple definitions of global variables are encountered. Something which GCC usually allows. A more forgiving linker flag to use to analyse this "issue" would be --warn-common:

--warn-common
Warn when a common symbol is combined with another common symbol or with a symbol definition.
Unix linker allow this somewhat sloppy practice, but linker on some other operating systems do not.
This option allows you to find potential problems from combining global symbols.  Unfortunately,
some C libraries use this practice, so you may get some warnings about symbols in the libraries as
well as in your programs.

As expected, its output during linking shows that all symbols of `EC_PTHREAD_NULL' have the same size.

GCC by default (with -fcommon) basically puts all multiple definitions of `EC_PTHREAD_NULL' in a common block. Hence treats them as one. I guess that's acceptable since they are equal.

This behavior unfortunately isn't ISO C compliant. A compiler/linker other than GCC will complain... hard. It's kinda ugly too and it's hard to say right now whether and how much this is degrading ettercap's performance or is a cause for a bug.

So yeah, this should be taken care of. Even when it's not exactly a priority.

[joshuaulrich]

Note that -fno-common is the default in gcc 10.

From https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html:

-fcommon

In C code, this option controls the placement of global variables defined without an initializer, known as tentative definitions in the C standard. Tentative definitions are distinct from declarations of a variable with the extern keyword, which do not allocate storage.

The default is -fno-common, which specifies that the compiler places uninitialized global variables in the BSS section of the object file. This inhibits the merging of tentative definitions by the linker so you get a multiple-definition error if the same variable is accidentally defined in more than one compilation unit.

The -fcommon places uninitialized global variables in a common block. This allows the linker to resolve all tentative definitions of the same variable in different compilation units to the same object, or to a non-tentative definition. This behavior is inconsistent with C++, and on many targets implies a speed and code size penalty on global variable references. It is mainly useful to enable legacy code to link without errors.

[trofi]

https://gcc.gnu.org/PR85678 is a specific PR where the switch work is done in gcc.

[koeppea]

Thanks for the heads up. I'll make up a PR to explicitly include this in CFLAGS and make the code up to cope with the default switch shift.

[limburgher]

Hi! following this because ettercap now can't build in Fedora rawhide and I'm the package maintainer.

[koeppea]

Thanks for the hint, I'll raise priority on this...Thanks for reporting.

[koeppea]

@asarubbo , @limburgher , @trofi , @joshuaulrich Can you please try if pull request #999 fixes the issue?

[limburgher]

It does, thank you!

[koeppea]

Fixed with mergin pull request #1004