[lld] [LLD] [COFF] Don't preserve unnecessary __imp_ prefixed symbols (PR #72989)

[Martin Storsjö via llvm-commits]

https://github.com/mstorsjo created https://github.com/llvm/llvm-project/pull/72989

This redoes the fix from 3ab6209a3f93bdbeec8e9b9fcc00a9a4980915ff differently, without the unwanted effect of preserving unnecessary `__imp_` prefixed symbols.

If the referencing object is a regular object, the `__imp_` symbol will have `isUsedInRegularObj` set on it from that already. If the referencing object is an LTO object, we set `isUsedInRegularObj` for any symbol starting with `__imp_`.

If the object file defining the `__imp_` symbol is a regular object, the `isUsedInRegularObj` flag has no effect. If it is an LTO object, it causes the symbol to be preserved.

>From 446b13d87a964e0943f6e2259c5d983fbe79b5eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Mon, 20 Nov 2023 00:28:15 +0200
Subject: [PATCH] [LLD] [COFF] Don't preserve unnecessary __imp_ prefixed
 symbols

This redoes the fix from 3ab6209a3f93bdbeec8e9b9fcc00a9a4980915ff
differently, without the unwanted effect of preserving unnecessary
__imp_ prefixed symbols.

If the referencing object is a regular object, the __imp_ symbol
will have isUsedInRegularObj set on it from that already. If the
referencing object is an LTO object, we set isUsedInRegularObj for
any symbol starting with __imp_.

If the object file defining the __imp_ symbol is a regular object,
the isUsedInRegularObj flag has no effect. If it is an LTO
object, it causes the symbol to be preserved.
---
 lld/COFF/InputFiles.cpp         | 19 +++++++++++++------
 lld/test/COFF/lto-imp-prefix.ll |  8 ++------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 132a3ccfaffbb8f..dd2e1419bb10a60 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -1042,6 +1042,19 @@ void BitcodeFile::parse() {
       sym = ctx.symtab.addUndefined(symName, this, false);
       if (objSym.isWeak())
         sym->deferUndefined = true;
+      // If one LTO object file references (i.e. has an undefined reference to)
+      // a symbol with an __imp_ prefix, the LTO compilation itself sees it
+      // as unprefixed but with a dllimport attribute instead, and doesn't
+      // understand the relation to a concrete IR symbol with the __imp_ prefix.
+      //
+      // For such cases, mark the symbol as used in a regular object (i.e. the
+      // symbol must be retained) so that the linker can associate the
+      // references in the end. If the symbol is defined in an import library
+      // or in a regular object file, this has no effect, but if it is defined
+      // in another LTO object file, this makes sure it is kept, to fulfill
+      // the reference when linking the output of the LTO compilation.
+      if (symName.starts_with("__imp_"))
+        sym->isUsedInRegularObj = true;
     } else if (objSym.isCommon()) {
       sym = ctx.symtab.addCommon(this, symName, objSym.getCommonSize());
     } else if (objSym.isWeak() && objSym.isIndirect()) {
@@ -1063,12 +1076,6 @@ void BitcodeFile::parse() {
     } else {
       sym = ctx.symtab.addRegular(this, symName, nullptr, fakeSC, 0,
                                   objSym.isWeak());
-      // Model all symbols with the __imp_ prefix as having external
-      // references. If one LTO object defines a __imp_<foo> symbol, and
-      // another LTO object refers to <foo> with dllimport, make sure the
-      // __imp_ symbol is kept.
-      if (symName.starts_with("__imp_"))
-        sym->isUsedInRegularObj = true;
     }
     symbols.push_back(sym);
     if (objSym.isUsed())
diff --git a/lld/test/COFF/lto-imp-prefix.ll b/lld/test/COFF/lto-imp-prefix.ll
index 56a7c48cc9d168a..d617a6ef5810785 100644
--- a/lld/test/COFF/lto-imp-prefix.ll
+++ b/lld/test/COFF/lto-imp-prefix.ll
@@ -8,13 +8,9 @@
 
 ; RUN: lld-link /entry:entry %t.main.obj %t.other1.obj /out:%t1.exe /subsystem:console /debug:symtab
 
-;; The current implementation for handling __imp_ symbols retains all of them.
-;; Observe that this currently produces __imp_unusedFunc even if nothing
-;; references unusedFunc in any form.
-
+;; Check that we don't retain __imp_ prefixed symbols we don't need.
 ; RUN: llvm-nm %t1.exe | FileCheck %s
-
-; CHECK: __imp_unusedFunc
+; CHECK-NOT: __imp_unusedFunc
 
 ; RUN: lld-link /entry:entry %t.main.obj %t.other2.obj /out:%t2.exe /subsystem:console