[Bug c++/83271] New: const variable previously declared "extern" results in "weak declaration must be public" error

2017-12-04 Thread alexey.salmin at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83271

Bug ID: 83271
   Summary: const variable previously declared "extern" results in
"weak declaration must be public" error
   Product: gcc
   Version: 8.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: alexey.salmin at gmail dot com
  Target Milestone: ---

The following code works well and produces a global read-only symbol:

extern const int x; // typically comes from a header file
const int x = 0;

But if you add a weak attribute, it fails:

extern const int x;
const int __attribute__((weak)) x = 0;

error: weak declaration of ‘x’ must be public

Even though "const" implies internal linkage in C++, the standard explicitly
makes an exception for names that were previously declared "extern" (see C++17
6.5 "Program and linkage"). The definition in question is already "public", it
should work without the "extern" specifier.

I've encountered this issue with g++ 6.4.0 20171026 (Debian 6.4.0-9) on
x86_64-linux-gnu. It seems to be reproducible with all g++ versions available
on godbolt.org (from 4.4 to 8.0 trunk).

[Bug c++/83271] const variable previously declared "extern" results in "weak declaration must be public" error

2017-12-04 Thread alexey.salmin at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83271

--- Comment #2 from Alexey Salmin  ---
(In reply to Jakub Jelinek from comment #1)
> I must say I fail to see usefulness of adding the attribute to the
> definition rather than declaration though.

Here's my case. There's a const bool flag in a static library that should be
reliably available before the main() entry point, therefore the static
initialization is used (see C++11 3.6.2.2). By default the flag is set to true
but it's overridden to false in a custom build.

flag.h:
extern const bool flag;

enabled.cpp:
#include "flag.h"
const bool flag __attribute__((weak)) = true;

disabled.cpp:
#include "flag.h"
const bool flag = false; // strong override

Library behaves differently depending on whether the "disabled.o" was provided
to the linker or not, while the "enabled.o" is always ar-ed into the static
library. I include the same header file in both cpp files to make sure both
definitions correspond to the same declaration.

Let me know if there's a better way to achieve this behavior. The one I can
think of involves a macro and two different builds of the library but it's not
welcome in our build system. Another option would be to keep both "enabled.o"
and "disabled.o" out of the static library and don't use weak symbols at all,
but that breaks the compatibility.

[Bug c++/83271] const variable previously declared "extern" results in "weak declaration must be public" error

2017-12-04 Thread alexey.salmin at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83271

--- Comment #4 from Alexey Salmin  ---
(In reply to Jonathan Wakely from comment #3)
> At least there's a simple workaround (adding 'extern' to the definition
> where the attribute).

That's what we do, so this is really a minor bug. Still, I decided to report it
for a few reasons:
1) Current g++ behavior looks inconsistent
2) clang++ and icc handle this case correctly
3) I'm used to a practice where declarations has the "extern" specifier while
definitions don't. This matters when you rely on the zero-initialization which
I normally don't do, but this pattern is somehow stuck in my head

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

2018-01-12 Thread alexey.salmin at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

Alexey Salmin  changed:

   What|Removed |Added

 CC||alexey.salmin at gmail dot com

--- Comment #36 from Alexey Salmin  ---
FYI a test case that actually triggers SIGSEGV on x86 with gcc 7.2:
https://godbolt.org/g/kPw6NJ

Compiler emits a movaps instruction which is alignment-sensitive. A warning
would be helpful.

struct pair_t {
char c;
__int128_t i;
} __attribute__((packed));

struct pair_t p = {0, 1};
__int128_t *addr = &p.i;

int main() {
*addr = ~(__int128_t)0;
return (p.i != 1) ? 0 : 1;
}

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

2018-01-16 Thread alexey.salmin at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #39 from Alexey Salmin  ---
Thank you, this patch works for me.

Gives a warning in the attached test case, but still allows to take an address
of a packed struct members when the packed attribute is preserved in the
pointer.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

2018-01-17 Thread alexey.salmin at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #42 from Alexey Salmin  ---
Sorry for being unclear.

When I need a pointer to an unaligned type I wrap it in a struct. E.g. a fix
for SIGSEGV from comment#36 would look like this:

struct pair_t {
char c;
__int128_t i;
} __attribute__((packed));

typedef struct unaligned_int128_t_ {
__int128_t value;
} __attribute__((packed)) unaligned_int128_t;

struct pair_t p = {0, 1};
unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i);

int main() {
addr->value = ~(__int128_t)0;
return (p.i != 1) ? 0 : 1;
}

I was trying to say that I'm glad that the "address-of-packed-member" warning
isn't triggered by this code. It still relies on the "address of packed member"
but in a safe way.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

2018-01-18 Thread alexey.salmin at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #46 from Alexey Salmin  ---
Tested the latest patch, behavior looks very reasonable even in tricky cases.

1) No warning, gcd(12, 8) == 4
struct tuple_t {
char c[12];
__int128_t i;
} __attribute__((packed, aligned(8)));

typedef struct unaligned_int128_t_ {
__int128_t value;
} __attribute__((packed, aligned(4))) unaligned_int128_t;

struct tuple_t p = {0, 1};
unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i);

2) Gives a warning, gcd(10, 8) < 4
struct tuple_t {
char c[10];
__int128_t i;
} __attribute__((packed, aligned(8)));

typedef struct unaligned_int128_t_ {
__int128_t value;
} __attribute__((packed, aligned(4))) unaligned_int128_t;

struct tuple_t p = {0, 1};
unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i);