fix: preserve hidden visibility when merging symbols and localize dynsym exports by PritamP20 · Pull Request #1700 · wild-linker/wild

PritamP20 · 2026-03-16T19:53:47Z

When a symbol has mixed visibility definitions across object files, the most restrictive visibility should win (hidden > protected > default). Wild was computing the merged visibility correctly but silently discarding it, so symbols that should be local were still leaking into dynsym.

Fixes #1693

Root cause

Two issues in handle_non_default_visibility:

The merged visibility was never passed in, so hidden and protected were treated identically
Only NON_INTERPOSABLE was set, never DOWNGRADE_TO_LOCAL — the flag that actually prevents export into dynsym

A third issue: data5 is declared hidden in one object but defined (with default visibility) in another. Since one is undefined and the other defined, they never appear in the same alternative-definition group, so the merged visibility path never ran.
This is fixed by tracking hidden undefined references during symbol loading and downgrading their definitions after resolution.

Before (Wild dynsym):           After (Wild dynsym):
  data1  ← wrong, should local    data2
  data2                           data3
  data3                           get_data1
  data4  ← wrong, should local    get_data5
  data5  ← wrong, should local
  get_data1
  get_data5

BFD dynsym (correct):
  data2, data3, get_data1, get_data5

davidlattimore

Thanks for working on this. I've benchmarked the change and unfortunately it incurs an 8% slowdown on linking librustc-driver and a 4% slowdown linking the Zed editor.

davidlattimore · 2026-03-16T23:23:31Z


    symbol_db.buckets = buckets;

+    for name in &symbol_db.hidden_undefined_names {


Why does this need to be done from the main thread? Perhaps it could be done in parallel from each object or from each hash bucket? Better yet, could we avoid storing a collection of hidden undefined names at all and update the flags during the resolution phase? - i.e. when we resolve undefined symbols

yeah, Didn't think about it, so the hidden visibility merge cab be taken care during alternative resolution phase itself, reducing the main thread iteration

I think it could probably be done during resolution itself, which is after alternative symbol selection. Resolution is where undefined symbols are looked up. For performance reasons, we want to do just a single hash lookup of each symbol. For defined symbols this is done when building the symbol DB. For undefined symbols, it's during resolution. I suspect you can handle both definitions and undefined symbols in resolution though, since for defined symbols, we know what symbol ID they resolve to without having to do hashmap lookup.

davidlattimore · 2026-03-16T23:24:19Z

+                {
+                    let info = self.get_symbol_name_and_version(symbol, local_index)?;
+                    outputs
+                        .add_hidden_undefined_name(UnversionedSymbolName::prehashed(info.name()));


Why are we storing symbol names? Would SymbolIds work?

we can't store it in symbolid because if the symbol is hidden and undefined then there won't be any entry in symbol_definitions vec and can't find the definition ID so the same behaviour might repeat

davidlattimore · 2026-03-17T00:14:04Z

BTW, to what extent did you use LLMs/AI when writing the PR description? I don't know your usual writing style, but it has an unusual feel to it that reminds me of how LLMs write

PritamP20 · 2026-03-17T02:11:35Z

@davidlattimore So, I initially I write the PR description myself explaining the issue and the fix, and then use an llm to help polish the wording and formatting

davidlattimore · 2026-03-17T02:30:36Z

OK, thanks. Going forward, would you be able to just post what you write? Don't worry if it's not polished - if an LLM can understand what you wrote, then I'm sure we can too

PritamP20 · 2026-03-17T02:50:29Z

@davidlattimore Sure, noted from next time I will take care of it,

I just had a doubt while working on this the hidden visibility, handling was being skipped in the test files, so it was clear why wild's behaviour was wrong for the hidden merge. But the protected visibility seems to be skipped both in the tests and in resolve_alternative_symbol_definitions, Should that also be handled in this PR?

davidlattimore · 2026-03-17T02:58:11Z

It's good to keep PRs small and focused on one issue where possible. So I'd say fixing hidden symbols in this PR and protected in a separate PR would be best. But certainly good to think about how protected visibility might be handled and take that into consideration when deciding how to solve hidden. The exception, where I'd say it might be reasonable to fix both in the same PR would be if that actually made the PR simpler, but otherwise, probably keep them separate.

PritamP20 · 2026-03-19T16:41:27Z

@davidlattimore any review on this?

davidlattimore · 2026-03-19T20:29:55Z

+            if !local_symbol_attributes.default_visibility {
+                let visibility = resources.symbol_db.input_symbol_visibility(local_symbol_id);
+                if visibility != Visibility::Default {
+                    resources


I think in order to get acceptable performance, we need to not store into any SegQueues or Vecs. Would it work to get access to the AtomicPerSymbolFlags here and directly update it?

Yeah, we can do it I thought doing it sequentially would affect the efficiency but doing it in parallel can save a lot of time. Thanks for telling me about the atomic symbols, they do exactly what we're trying to do

davidlattimore · 2026-03-23T23:48:23Z

#1754 moved all test sources into separate subdirectories. The test files were mostly unchanged, so hopefully rebasing should only require moving your changes to the file in the new location.

PritamP20 · 2026-03-26T19:08:44Z

@davidlattimore I rebased the branch, git handled the rebase itself, so everything is in the right place

… exports

davidlattimore

Nice! I can confirm that the performance loss is gone, so this is very close to being able to be merged.

davidlattimore · 2026-03-26T21:14:23Z

+                            if visibility != Visibility::Default
+                                && let Some(def_id) = symbol_db.get_unversioned(&pre_hashed)
+                            {
+                                symbol_db::apply_visibility_to_definition(


If I comment out both calls to apply_visibility_to_definition, all the tests still pass. Do you think you can come up with an integration test that is fixed by this part of the change?

understood, so you want me to create a integration test, which tests part of the code is working or not, I will do that

Thanks. It looks like the new test successfully fails if this call is commented out, but commenting out the other call below still results in the tests passing. Do you think the test can be extended to cover that as well?

BTW, the failing test:

/usr/bin/ld.bfd: /__w/wild/wild/wild/tests/build/elf/x86_64/hidden-weak-archive-visibility/default/hidden-weak-archive-visibility.c.o: relocation R_X86_64_32 against hidden symbol `get_value' can not be used when making a shared object /usr/bin/ld.bfd: failed to set dynamic section sizes: bad value

I think might be fixed by adding CompArgs:-fPIC, although I haven't tried.

Thank you @davidlattimore , i did try extending the test for the second call but couldn’t make it fail when the second call is commented out i added that call just to be safe thinking the same symbol might show up twice in undefined_symbols but the first call already covers it, i will remove the 2nd call

What about having two objects with references to an undefined symbol, both weak references but only one hidden, then a configuration where the object containing the hidden symbol is first and a configuration where it's second?

damn, I didn't think in that way, Now i have created one more ref of get_value which is weak and protected which goes through the 2nd function call

davidlattimore · 2026-03-28T06:35:31Z

+//#Object:hidden-weak-archive-visibility-2.c
+//#NoDynSym:get_value
+//#ExpectDynSym:public_func
+//#Config:vacant:default


Could you put a blank line before each of the configs? That's how all our other tests are formatted since it makes it easier to see where they start

davidlattimore

Thanks for doing this :)

davidlattimore reviewed Mar 16, 2026

View reviewed changes

PritamP20 force-pushed the fix/visibility-merge-dynsym branch 4 times, most recently from a29c11c to 2f0f8e5 Compare March 18, 2026 15:10

davidlattimore reviewed Mar 19, 2026

View reviewed changes

PritamP20 force-pushed the fix/visibility-merge-dynsym branch 3 times, most recently from 096aacd to caa5ae7 Compare March 20, 2026 12:26

PritamP20 force-pushed the fix/visibility-merge-dynsym branch from caa5ae7 to 9b77a43 Compare March 26, 2026 19:08

fix: honor hidden visibility when merging symbols and localize dynsym…

21b6246

… exports

PritamP20 force-pushed the fix/visibility-merge-dynsym branch from 9b77a43 to 21b6246 Compare March 26, 2026 19:14

davidlattimore reviewed Mar 26, 2026

View reviewed changes

PritamP20 force-pushed the fix/visibility-merge-dynsym branch 2 times, most recently from 06fc429 to 59a2ac9 Compare March 28, 2026 05:34

davidlattimore reviewed Mar 28, 2026

View reviewed changes

added tests for testing apply_visibilty_to_definition

8312c01

PritamP20 force-pushed the fix/visibility-merge-dynsym branch from 59a2ac9 to 8312c01 Compare March 28, 2026 08:56

davidlattimore approved these changes Mar 28, 2026

View reviewed changes

davidlattimore merged commit 23e45ec into wild-linker:main Mar 28, 2026
24 checks passed


		symbol_db.buckets = buckets;

		for name in &symbol_db.hidden_undefined_names {

Uh oh!

Conversation

PritamP20 commented Mar 16, 2026

Root cause

Uh oh!

davidlattimore left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PritamP20 Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidlattimore commented Mar 17, 2026

Uh oh!

PritamP20 commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidlattimore commented Mar 17, 2026

Uh oh!

PritamP20 commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidlattimore commented Mar 17, 2026

Uh oh!

PritamP20 commented Mar 19, 2026

Uh oh!

davidlattimore Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidlattimore commented Mar 23, 2026

Uh oh!

PritamP20 commented Mar 26, 2026

Uh oh!

davidlattimore left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidlattimore left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

PritamP20 Mar 17, 2026 •

edited

Loading

PritamP20 commented Mar 17, 2026 •

edited

Loading

PritamP20 commented Mar 17, 2026 •

edited

Loading

davidlattimore Mar 19, 2026 •

edited

Loading