Skip to content

lld: no error for referenced symbols defined in sections discarded by linker script #58891

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

Closed
bevin-hansson opened this issue Nov 9, 2022 · 3 comments
Labels

Comments

@bevin-hansson
Copy link
Contributor

bevin-hansson commented Nov 9, 2022

lld has some very dubious behavior when dealing with symbols that would have been included in the binary, but weren't due to their section being discarded by a linker script.
I've constructed the following example; four objects, with one referencing the three others:
a.c:

void foo();
void bar();
void baz();

int main() {
  foo(); bar(); baz();
}

b.c, c.c, d.c:

void foo|bar|baz() { }

Linking these files normally with main as an entry point works fine:

$ lld -flavor ld a.o b.o c.o d.o -e main -Map=m
$

m:
...
          2011f4           2011f4       3b     1 .text
          2011f4           2011f4       29     1         a.o:(.text)
          2011f4           2011f4       29     1                 main
          20121d           20121d        6     1         b.o:(.text)
          20121d           20121d        6     1                 foo
          201223           201223        6     1         c.o:(.text)
          201223           201223        6     1                 bar
          201229           201229        6     1         d.o:(.text)
          201229           201229        6     1                 baz

But if we add a linker script that discards the .text section of d:

SECTIONS {
  /DISCARD/ : {
    d.o(.text)
  }
}

The link still succeeds, even though the binary no longer contains baz:

$ lld -flavor ld a.o b.o c.o d.o -e main -Map=m script
$

m:
              7c               7c       35     1 .text
              7c               7c       29     1         a.o:(.text)
              7c               7c       29     1                 main
              a5               a5        6     1         b.o:(.text)
              a5               a5        6     1                 foo
              ab               ab        6     1         c.o:(.text)
              ab               ab        6     1                 bar

Should it be like this? The relocation to baz in main will contain an invalid value, since baz's section was never assigned an offset. I would expect an error about referenced symbols in discarded sections, but it doesn't appear. The binary is simply silently broken.

Regular ld is also silent, but it keeps baz even though its section was /DISCARD/:

Symbol table '.symtab' contains 15 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
     1: 00000000004000b0     0 SECTION LOCAL  DEFAULT    1 
     2: 00000000004000f0     0 SECTION LOCAL  DEFAULT    2 
     3: 0000000000000000     0 SECTION LOCAL  DEFAULT    3 
     4: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS a.c
     5: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS b.c
     6: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS c.c
     7: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS d.c
     8: 00000000004000e5     6 FUNC    GLOBAL DEFAULT    1 baz
     9: 0000000000601000     0 NOTYPE  GLOBAL DEFAULT    2 __bss_start
    10: 00000000004000b0    41 FUNC    GLOBAL DEFAULT    1 main
    11: 00000000004000d9     6 FUNC    GLOBAL DEFAULT    1 foo
    12: 0000000000601000     0 NOTYPE  GLOBAL DEFAULT    2 _edata
    13: 0000000000601000     0 NOTYPE  GLOBAL DEFAULT    2 _end
    14: 00000000004000df     6 FUNC    GLOBAL DEFAULT    1 bar
@llvmbot
Copy link
Member

llvmbot commented Nov 9, 2022

@llvm/issue-subscribers-lld-elf

@EugeneZelenko EugeneZelenko removed the lld label Nov 9, 2022
@smithp35
Copy link
Collaborator

smithp35 commented Nov 9, 2022

I think the current behaviour isn't right. As you point out I think there are two logical positions, either error if there is a relocation to a discarded section or don't discard the section.

My personal preference is to error as I would not want my DISCARDs from being silently undone, but I can see the argument for keeping if compatibility with ld.bfd is required.

@bevin-hansson
Copy link
Contributor Author

The temporary solution I opted for was to do something like this after processing the linker script:

  for (Symbol *sym : symtab.getSymbols())
    if (Defined *d = dyn_cast<Defined>(sym))
      if (d->section && d->section->partition == 0)
        Undefined(nullptr, sym->getName(), sym->binding, sym->stOther,
                  sym->type).overwrite(*sym);

This seemed to be similar to what is done in demoteSharedAndLazySymbols to "undefine" the symbols.

This doesn't give the "relocation refers to a symbol in a discarded section" that I would expect though, just "undefined symbol", since

  • I couldn't figure out how to get the section index of the defined symbol, and
  • The section with that index wouldn't be InputSection::discarded anyway, because the linker script processing doesn't touch the file's section list when discarding a section

I'm not confident enough in my lld knowledge to know if this is the/a correct solution.

bevin-hansson added a commit to bevin-hansson/llvm-project that referenced this issue Oct 16, 2023
After linker script processing, it may be the case that sections
were discarded via /DISCARD/, but relocations to symbols in those
sections remain. Currently, such symbols are still considered
Defined even though their section is not live and will not be
placed anywhere. This results in a silently broken binary.

This patch goes through the symbols after placement and changes
them from Defined to Undefined if their section is no longer
live at that point. During relocation processing, we will catch
such undefined symbols and report an error.

See llvm#58891.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants