Macos linking warning due to visibility

GiudGiud commented 3 months ago

Bug Description

We get these warnings for classes using templated members (especially the variables)

ld: warning: direct access in function 'AdvectiveFluxAux::AdvectiveFluxAux(InputParameters const&)' from file '/Users/giudgl/projects/moose/framework/build/unity_src/.libs/auxkernels_Unity.aarch64-apple-darwin20.0.0.devel.o' to global weak symbol 'typeinfo for MooseVariableFV<double>' from file '/Users/giudgl/projects/moose/framework/build/unity_src/.libs/variables_Unity.aarch64-apple-darwin20.0.0.devel.o' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'AdvectiveFluxAux::AdvectiveFluxAux(InputParameters const&)' from file '/Users/giudgl/projects/moose/framework/build/unity_src/.libs/auxkernels_Unity.aarch64-apple-darwin20.0.0.devel.o' to global weak symbol 'typeinfo for MooseVariableFV<double>' from file '/Users/giudgl/projects/moose/framework/build/unity_src/.libs/variables_Unity.aarch64-apple-darwin20.0.0.devel.o' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'AdvectiveFluxAux::AdvectiveFluxAux(InputParameters const&)' from file '/Users/giudgl/projects/moose/framework/build/unity_src/.libs/auxkernels_Unity.aarch64-apple-darwin20.0.0.devel.o' to global weak symbol 'typeinfo for MooseVariableField<double>' from file '/Users/giudgl/projects/moose/framework/build/unity_src/.libs/variables_Unity.aarch64-apple-darwin20.0.0.devel.o' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

I looked into it and I think the templates are compiled in several translation unit, and the typeinfo attribute is inlined in one of them and not the other. Then because we pass this flag: -fvisibility-inlines-hidden, this leads to an inconsistent visibility

A few things I have looked into:

-fvisibility=hidden definitely makes everything hidden and consistent. But it prevents linking downstream code unless we go and make everything we need public (not hidden)
-linkonce-templates not widely available but could help?
attributes (type_visibility notably) on the variables. It does not seem to work so far

@cticenhour looked into this for M1 support too. I dont know if there is an issue dedicated to this problem

Steps to Reproduce

Compile then link on mac

Impact

Distraction. It's 400 lines of warning, that on a smaller screen takes 2000 lines on terminal

cticenhour commented 3 months ago

I thought I had made an issue, but cannot find it. Thanks @GiudGiud. Assigning this to myself since I still plan to work on it.

cticenhour commented 3 months ago

A couple contextual notes:

I have tried applying -fvisibility=hidden and -fvisibility-inlines-hidden. Mostly at the level of our package environment variables, but on suspicion that PETSc is overriding some of these (they are), I have added them directly into the PETSc build process. Some of these common warnings are coming out of Eigen (hence the PETSc-level troubleshooting). But this was ineffective.
-linkonce-templates is not something I have explored.

After speaking with @permcody about this a while back, I'm currently exploring whether how we do explicit template instantiation in our combined non-AD/AD classes may be contributing here.

Probably forgetting some details of things I tried a few months ago....

GiudGiud commented 3 months ago

So I think the explicit template instantiations is the right lead for most of the errors

If you look at the errors, the majority of them come from templates that are strictly in moose

GiudGiud commented 3 months ago

@nmnobre I see you have unrelated commits linking to this issue. If they go to a PR please amend them first :)

nmnobre commented 3 months ago

Yep, that was a mistake. I’ve amended them and that PR has already been merged, sorry.

idaholab / moose