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

visibility warnings with ARM GCC 7.3 #1973

Closed
alexezeder opened this issue Nov 2, 2020 · 6 comments
Closed

visibility warnings with ARM GCC 7.3 #1973

alexezeder opened this issue Nov 2, 2020 · 6 comments

Comments

@alexezeder
Copy link
Contributor

There is a weird warning related to the visibility of some classes of {fmt} on GCC 6.4 and 7.3 on ARM. These -Wattributes warnings produced with -fvisibility=hidden only.

For example, there is no warning for:

#include "fmt/format.h"

int main(int argc, char** argv) {
    auto message = fmt::format(FMT_STRING("{}"), argv[0]);
}

But if I want to use literals then there is a warning telling that visibility of FMT_COMPILE_STRING struct (defined inside the FMT_STRING's lambda) is greater than the visibility of its base fmt::compile_string struct:
https://godbolt.org/z/vfGanM

Same warning, simplified code:
https://godbolt.org/z/Pr8xKz

#include "fmt/format.h"

template <typename... Args>
std::string func(Args&&... args) {
    return format(FMT_STRING("{}"), std::forward<Args>(args)...);
}

int main() {
    auto m = func(42);
}

ARM GCC 7.3 output:

In file included from <source>:1:0:
<source>: In instantiation of 'struct func(Args&& ...)::<lambda()> [with Args = {int}]::FMT_COMPILE_STRING':
<source>:5:19:   required from 'func(Args&& ...)::<lambda()> [with Args = {int}]'
<source>:5:19:   required from 'struct func(Args&& ...) [with Args = {int}; std::__cxx11::string = std::__cxx11::basic_string<char>]::<lambda()>'
<source>:5:19:   required from 'std::__cxx11::string func(Args&& ...) [with Args = {int}; std::__cxx11::string = std::__cxx11::basic_string<char>]'
<source>:9:21:   required from here
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/format.h:3061:12: warning: 'func(Args&& ...)::<lambda()> [with Args = {int}]::FMT_COMPILE_STRING' declared with greater visibility than the type of its field 'func(Args&& ...)::<lambda()> [with Args = {int}]::FMT_COMPILE_STRING::<anonymous>' [-Wattributes]
     struct FMT_COMPILE_STRING : base {                            \
            ^
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/format.h:3081:23: note: in expansion of macro 'FMT_STRING_IMPL'
 #define FMT_STRING(s) FMT_STRING_IMPL(s, fmt::compile_string)
                       ^~~~~~~~~~~~~~~
<source>:5:19: note: in expansion of macro 'FMT_STRING'
     return format(FMT_STRING("{}"), std::forward<Args>(args)...);
                   ^~~~~~~~~~
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/format.h:3061:12: warning: 'func(Args&& ...)::<lambda()> [with Args = {int}]::FMT_COMPILE_STRING' declared with greater visibility than its base 'fmt::v7::compile_string' [-Wattributes]
     struct FMT_COMPILE_STRING : base {                            \
            ^
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/format.h:3081:23: note: in expansion of macro 'FMT_STRING_IMPL'
 #define FMT_STRING(s) FMT_STRING_IMPL(s, fmt::compile_string)
                       ^~~~~~~~~~~~~~~
<source>:5:19: note: in expansion of macro 'FMT_STRING'
     return format(FMT_STRING("{}"), std::forward<Args>(args)...);
                   ^~~~~~~~~~

And no warning without template in a simplified version:
https://godbolt.org/z/4TsdWx

Looks more like it is a GCC bug, also it's specific to the ARM version because x86-64 GCC 7.3 (and down to 5.3 at least) does not produce any warning. So the question is: should it be handled somehow in {fmt}?

@alexezeder
Copy link
Contributor Author

alexezeder commented Nov 2, 2020

Forgot to mention that this problem is not new, it's not related to the latest {fmt} release, for instance simplified code example with {fmt} v5.0.0 also produce this warning:
https://godbolt.org/z/b9d1az

@YarikTH
Copy link

YarikTH commented Nov 2, 2020

My dirty patch to work around it:

--- a/lib/fmt/fmt-7.0.3/include/fmt/core.h
+++ b/lib/fmt/fmt-7.0.3/include/fmt/core.h
@@ -479,7 +479,12 @@ inline basic_string_view<Char> to_string_view(detail::std_string_view<Char> s) {
 
 // A base class for compile-time strings. It is defined in the fmt namespace to
 // make formatting functions visible via ADL, e.g. format(FMT_STRING("{}"), 42).
-struct compile_string {};
+struct
+// Work around '***::FMT_COMPILE_STRING' declared with greater visibility than its base 'fmt::v7::compile_string' [-Werror=attributes]
+#if __GNUC__ >= 4
+__attribute__ ((visibility ("default")))
+#endif
+    compile_string {};
 
 template <typename S>
 struct is_compile_string : std::is_base_of<compile_string, S> {};

But can't say it's a proper solution. And I don't want to patch every {fmt} version we use this way.

@alexezeder
Copy link
Contributor Author

My dirty patch to work around it:
+#if __GNUC__ >= 4

it's not correct, for example GCC 5.4, 6.3, 8.2, 9.2 do not produce this warning

@YarikTH
Copy link

YarikTH commented Nov 3, 2020

My dirty patch to work around it:
+#if __GNUC__ >= 4

it's not correct, for example GCC 5.4, 6.3, 8.2, 9.2 do not produce this warning

It just checks if "visibility" attribute available in GCC. I thought that even warning is not produced, there is no harm in explicitly making visibility "default" for this struct. But this assumption may be false.

@madscientist
Copy link

I don't think that's a good idea. It means that when people build shared libraries and link a static library version of fmt, this will become part of the public interface. If they are trying to carefully control the public symbols of their shared libraries by setting hidden on the compile line, this is a problem.

I don't think the fmt library should enforce any specific visibility in the code.

@vitaut
Copy link
Contributor

vitaut commented Nov 3, 2020

Closing as the fact that the warning is gone in newer gcc versions indicates that it's a compiler bug. That said a PR with a workaround would be welcome. The easiest workaround is to mark FMT_COMPILE_STRING with __attribute__ ((visibility ("hidden"))).

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

4 participants