[InstallAPI] Add support for aliased exports #88750
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cyndyishida
added
skip-precommit-approval
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Cyndy Ishida (cyndyishida) ChangesApple's ld supports alias_lists, described as To handle this for installapi-produced TBD files, pass along the same input and account for it in verification. Patch is 24.16 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/88750.diff 10 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td b/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td
index 396bff0146a373..91a40cd589b385 100644
--- a/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td
+++ b/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td
@@ -24,6 +24,7 @@ def err_no_matching_target : Error<"no matching target found for target variant
def err_unsupported_vendor : Error<"vendor '%0' is not supported: '%1'">;
def err_unsupported_environment : Error<"environment '%0' is not supported: '%1'">;
def err_unsupported_os : Error<"os '%0' is not supported: '%1'">;
+def err_cannot_read_alias_list : Error<"could not read alias list '%0': %1">;
} // end of command line category.
let CategoryName = "Verification" in {
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index e24626913add76..8c38acb72362c7 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1505,6 +1505,7 @@ def end_no_unused_arguments : Flag<["--"], "end-no-unused-arguments">,
def interface_stub_version_EQ : JoinedOrSeparate<["-"], "interface-stub-version=">,
Visibility<[ClangOption, CC1Option]>;
def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
+def alias_list : Separate<["-"], "alias_list">, Flags<[LinkerInput]>;
def extract_api : Flag<["-"], "extract-api">,
Visibility<[ClangOption, CC1Option]>, Group<Action_Group>,
HelpText<"Extract API information">;
diff --git a/clang/include/clang/InstallAPI/DylibVerifier.h b/clang/include/clang/InstallAPI/DylibVerifier.h
index 31de212fc423a5..f1a949d8f99394 100644
--- a/clang/include/clang/InstallAPI/DylibVerifier.h
+++ b/clang/include/clang/InstallAPI/DylibVerifier.h
@@ -78,10 +78,12 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
DylibVerifier() = default;
DylibVerifier(llvm::MachO::Records &&Dylib, ReexportedInterfaces &&Reexports,
- DiagnosticsEngine *Diag, VerificationMode Mode, bool Zippered,
- bool Demangle, StringRef DSYMPath)
- : Dylib(std::move(Dylib)), Reexports(std::move(Reexports)), Mode(Mode),
- Zippered(Zippered), Demangle(Demangle), DSYMPath(DSYMPath),
+ AliasMap Aliases, DiagnosticsEngine *Diag,
+ VerificationMode Mode, bool Zippered, bool Demangle,
+ StringRef DSYMPath)
+ : Dylib(std::move(Dylib)), Reexports(std::move(Reexports)),
+ Aliases(std::move(Aliases)), Mode(Mode), Zippered(Zippered),
+ Demangle(Demangle), DSYMPath(DSYMPath),
Exports(std::make_unique<SymbolSet>()), Ctx(VerifierContext{Diag}) {}
Result verify(GlobalRecord *R, const FrontendAttrs *FA);
@@ -104,7 +106,7 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
void setTarget(const Target &T);
/// Release ownership over exports.
- std::unique_ptr<SymbolSet> getExports() { return std::move(Exports); }
+ std::unique_ptr<SymbolSet> getExports();
/// Get result of verification.
Result getState() const { return Ctx.FrontendState; }
@@ -189,6 +191,9 @@ class DylibVerifier : llvm::MachO::RecordVisitor {
// Reexported interfaces apart of the library.
ReexportedInterfaces Reexports;
+ // Symbol aliases.
+ AliasMap Aliases;
+
// Controls what class of violations to report.
VerificationMode Mode = VerificationMode::Invalid;
diff --git a/clang/include/clang/InstallAPI/MachO.h b/clang/include/clang/InstallAPI/MachO.h
index 854399f54ba6c8..9da91a62e23311 100644
--- a/clang/include/clang/InstallAPI/MachO.h
+++ b/clang/include/clang/InstallAPI/MachO.h
@@ -23,6 +23,7 @@
#include "llvm/TextAPI/TextAPIWriter.h"
#include "llvm/TextAPI/Utils.h"
+using AliasMap = llvm::MachO::AliasMap;
using Architecture = llvm::MachO::Architecture;
using ArchitectureSet = llvm::MachO::ArchitectureSet;
using SymbolFlags = llvm::MachO::SymbolFlags;
diff --git a/clang/lib/InstallAPI/DylibVerifier.cpp b/clang/lib/InstallAPI/DylibVerifier.cpp
index 84d9b5892e88da..27b28e59105983 100644
--- a/clang/lib/InstallAPI/DylibVerifier.cpp
+++ b/clang/lib/InstallAPI/DylibVerifier.cpp
@@ -674,6 +674,11 @@ void DylibVerifier::visitSymbolInDylib(const Record &R, SymbolContext &SymCtx) {
return;
}
+ if (Aliases.count({SymbolName.str(), SymCtx.Kind})) {
+ updateState(Result::Valid);
+ return;
+ }
+
// All checks at this point classify as some kind of violation.
// The different verification modes dictate whether they are reported to the
// user.
@@ -973,5 +978,24 @@ bool DylibVerifier::verifyBinaryAttrs(const ArrayRef<Target> ProvidedTargets,
return true;
}
+std::unique_ptr<SymbolSet> DylibVerifier::getExports() {
+ for (const auto &[Alias, Base] : Aliases) {
+ TargetList Targets;
+ SymbolFlags Flags = SymbolFlags::None;
+ if (const Symbol *Sym = Exports->findSymbol(Base.second, Base.first)) {
+ Flags = Sym->getFlags();
+ Targets = {Sym->targets().begin(), Sym->targets().end()};
+ }
+
+ Record R(Alias.first, RecordLinkage::Exported, Flags);
+ SymbolContext SymCtx;
+ SymCtx.SymbolName = Alias.first;
+ SymCtx.Kind = Alias.second;
+ addSymbol(&R, SymCtx, std::move(Targets));
+ }
+
+ return std::move(Exports);
+}
+
} // namespace installapi
} // namespace clang
diff --git a/clang/test/InstallAPI/alias_list.test b/clang/test/InstallAPI/alias_list.test
new file mode 100644
index 00000000000000..3e12221e088c4b
--- /dev/null
+++ b/clang/test/InstallAPI/alias_list.test
@@ -0,0 +1,461 @@
+; RUN: rm -rf %t
+; RUN: split-file %s %t
+; RUN: sed -e "s|DSTROOT|%/t|g" %t/inputs.json.in > %t/inputs.json
+; RUN: yaml2obj %t/AliasList.yaml -o %t/Frameworks/AliasList.framework/AliasList
+
+; RUN: clang-installapi --target=x86_64-apple-macos13 \
+; RUN: -alias_list %t/aliases.txt \
+; RUN: -install_name /System/Library/Frameworks/AliasList.framework/Versions/A/AliasList \
+; RUN: -current_version 1 -compatibility_version 1 \
+; RUN: -F%t/Frameworks -ObjC %t/inputs.json --verify-mode=Pedantic \
+; RUN: --verify-against=%t/Frameworks/AliasList.framework/AliasList \
+; RUN: -o %t/AliasList.tbd 2>&1 | FileCheck -allow-empty %s \
+; RUN: --implicit-check-not=error --implicit-check-not=warning
+; RUN: llvm-readtapi -compare %t/expected.tbd %t/AliasList.tbd
+
+// Check error handling.
+; RUN: not clang-installapi --target=x86_64-apple-macos13 \
+; RUN: -alias_list %t/invalid.txt \
+; RUN: -install_name /System/Library/Frameworks/AliasList.framework/Versions/A/AliasList \
+; RUN: -current_version 1 -compatibility_version 1 \
+; RUN: -F%t/Frameworks -ObjC %t/inputs.json --verify-mode=Pedantic \
+; RUN: --verify-against=%t/Frameworks/AliasList.framework/AliasList \
+; RUN: -o %t/AliasList.tbd 2>&1 | FileCheck -allow-empty %s \
+; RUN: --check-prefix=INVALID
+
+; INVALID: error: could not read alias list {{.*}} missing alias for: _hidden
+
+;--- Frameworks/AliasList.framework/Headers/AliasList.h
+// simple alias from one symbol to another.
+extern int simple_symbol;
+extern int alias_symbol;
+
+// This symbol comes from the alias file.
+extern int exported_symbol;
+
+// This symbol was moved here and has several special hide symbols in the alias
+// file.
+extern int moved_here_symbol;
+
+// This alias is public, whereas the source is private.
+extern int public_symbol;
+
+;--- Frameworks/AliasList.framework/PrivateHeaders/AliasList_Private.h
+// This is a private symbol that has a public alias.
+extern int private_symbol;
+
+;--- aliases.txt
+# comment
+_simple_symbol _alias_symbol
+# test multiple space characters separated symbol and alias
+_hidden_symbol _exported_symbol # test inline comment with spaces
+# test tab character separated symbol and alias
+_moved_here_symbol $ld$hide$os10.4$_moved_here_symbol# test inline comment without spaces
+# test trailing space character
+_moved_here_symbol $ld$hide$os10.5$_moved_here_symbol
+# test trailing tab character
+_moved_here_symbol $ld$hide$os10.6$_moved_here_symbol
+_private_symbol _public_symbol
+
+;--- invalid.txt
+# comment
+_simple_symbol _alias_symbol
+_hidden # no matching
+
+;--- expected.tbd
+{
+ "main_library": {
+ "exported_symbols": [
+ {
+ "data": {
+ "global": [
+ "_exported_symbol", "_simple_symbol", "_moved_here_symbol",
+ "$ld$hide$os10.6$_moved_here_symbol", "$ld$hide$os10.4$_moved_here_symbol",
+ "$ld$hide$os10.5$_moved_here_symbol", "_public_symbol",
+ "_private_symbol", "_alias_symbol"
+ ]
+ }
+ }
+ ],
+ "flags": [
+ {
+ "attributes": [
+ "not_app_extension_safe"
+ ]
+ }
+ ],
+ "install_names": [
+ {
+ "name": "/System/Library/Frameworks/AliasList.framework/Versions/A/AliasList"
+ }
+ ],
+ "target_info": [
+ {
+ "min_deployment": "13",
+ "target": "x86_64-macos"
+ }
+ ]
+ },
+ "tapi_tbd_version": 5
+}
+
+;--- AliasList.yaml
+--- !mach-o
+FileHeader:
+ magic: 0xFEEDFACF
+ cputype: 0x1000007
+ cpusubtype: 0x3
+ filetype: 0x6
+ ncmds: 13
+ sizeofcmds: 920
+ flags: 0x100085
+ reserved: 0x0
+LoadCommands:
+ - cmd: LC_SEGMENT_64
+ cmdsize: 152
+ segname: __TEXT
+ vmaddr: 0
+ vmsize: 4096
+ fileoff: 0
+ filesize: 4096
+ maxprot: 5
+ initprot: 5
+ nsects: 1
+ flags: 0
+ Sections:
+ - sectname: __text
+ segname: __TEXT
+ addr: 0xBB8
+ size: 0
+ offset: 0xBB8
+ align: 0
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x80000000
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ content: ''
+ - cmd: LC_SEGMENT_64
+ cmdsize: 152
+ segname: __DATA_CONST
+ vmaddr: 4096
+ vmsize: 4096
+ fileoff: 4096
+ filesize: 4096
+ maxprot: 3
+ initprot: 3
+ nsects: 1
+ flags: 16
+ Sections:
+ - sectname: __objc_imageinfo
+ segname: __DATA_CONST
+ addr: 0x1000
+ size: 8
+ offset: 0x1000
+ align: 0
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x0
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ content: '0000000040000000'
+ - cmd: LC_SEGMENT_64
+ cmdsize: 152
+ segname: __DATA
+ vmaddr: 8192
+ vmsize: 4096
+ fileoff: 8192
+ filesize: 0
+ maxprot: 3
+ initprot: 3
+ nsects: 1
+ flags: 0
+ Sections:
+ - sectname: __common
+ segname: __DATA
+ addr: 0x2000
+ size: 40
+ offset: 0x0
+ align: 2
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x1
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ - cmd: LC_SEGMENT_64
+ cmdsize: 72
+ segname: __LINKEDIT
+ vmaddr: 12288
+ vmsize: 672
+ fileoff: 8192
+ filesize: 672
+ maxprot: 1
+ initprot: 1
+ nsects: 0
+ flags: 0
+ - cmd: LC_DYLD_INFO_ONLY
+ cmdsize: 48
+ rebase_off: 0
+ rebase_size: 0
+ bind_off: 0
+ bind_size: 0
+ weak_bind_off: 0
+ weak_bind_size: 0
+ lazy_bind_off: 0
+ lazy_bind_size: 0
+ export_off: 8192
+ export_size: 248
+ - cmd: LC_SYMTAB
+ cmdsize: 24
+ symoff: 8448
+ nsyms: 11
+ stroff: 8624
+ strsize: 240
+ - cmd: LC_DYSYMTAB
+ cmdsize: 80
+ ilocalsym: 0
+ nlocalsym: 1
+ iextdefsym: 1
+ nextdefsym: 9
+ iundefsym: 10
+ nundefsym: 1
+ tocoff: 0
+ ntoc: 0
+ modtaboff: 0
+ nmodtab: 0
+ extrefsymoff: 0
+ nextrefsyms: 0
+ indirectsymoff: 0
+ nindirectsyms: 0
+ extreloff: 0
+ nextrel: 0
+ locreloff: 0
+ nlocrel: 0
+ - cmd: LC_ID_DYLIB
+ cmdsize: 96
+ dylib:
+ name: 24
+ timestamp: 0
+ current_version: 65536
+ compatibility_version: 65536
+ Content: '/System/Library/Frameworks/AliasList.framework/Versions/A/AliasList'
+ ZeroPadBytes: 5
+ - cmd: LC_UUID
+ cmdsize: 24
+ uuid: 4C4C4468-5555-3144-A123-B0FDB87F9813
+ - cmd: LC_BUILD_VERSION
+ cmdsize: 32
+ platform: 1
+ minos: 851968
+ sdk: 983040
+ ntools: 1
+ Tools:
+ - tool: 4
+ version: 1245184
+ - cmd: LC_LOAD_DYLIB
+ cmdsize: 56
+ dylib:
+ name: 24
+ timestamp: 0
+ current_version: 88539136
+ compatibility_version: 65536
+ Content: '/usr/lib/libSystem.B.dylib'
+ ZeroPadBytes: 6
+ - cmd: LC_FUNCTION_STARTS
+ cmdsize: 16
+ dataoff: 8440
+ datasize: 8
+ - cmd: LC_DATA_IN_CODE
+ cmdsize: 16
+ dataoff: 8448
+ datasize: 0
+LinkEditData:
+ ExportTrie:
+ TerminalSize: 0
+ NodeOffset: 0
+ Name: ''
+ Flags: 0x0
+ Address: 0x0
+ Other: 0x0
+ ImportName: ''
+ Children:
+ - TerminalSize: 0
+ NodeOffset: 21
+ Name: '$ld$hide$os10.'
+ Flags: 0x0
+ Address: 0x0
+ Other: 0x0
+ ImportName: ''
+ Children:
+ - TerminalSize: 3
+ NodeOffset: 89
+ Name: '4$_moved_here_symbol'
+ Flags: 0x0
+ Address: 0x2000
+ Other: 0x0
+ ImportName: ''
+ - TerminalSize: 3
+ NodeOffset: 94
+ Name: '6$_moved_here_symbol'
+ Flags: 0x0
+ Address: 0x2008
+ Other: 0x0
+ ImportName: ''
+ - TerminalSize: 3
+ NodeOffset: 99
+ Name: '5$_moved_here_symbol'
+ Flags: 0x0
+ Address: 0x2004
+ Other: 0x0
+ ImportName: ''
+ - TerminalSize: 0
+ NodeOffset: 104
+ Name: _
+ Flags: 0x0
+ Address: 0x0
+ Other: 0x0
+ ImportName: ''
+ Children:
+ - TerminalSize: 3
+ NodeOffset: 179
+ Name: alias_symbol
+ Flags: 0x0
+ Address: 0x2024
+ Other: 0x0
+ ImportName: ''
+ - TerminalSize: 0
+ NodeOffset: 184
+ Name: p
+ Flags: 0x0
+ Address: 0x0
+ Other: 0x0
+ ImportName: ''
+ Children:
+ - TerminalSize: 3
+ NodeOffset: 217
+ Name: ublic_symbol
+ Flags: 0x0
+ Address: 0x2020
+ Other: 0x0
+ ImportName: ''
+ - TerminalSize: 3
+ NodeOffset: 222
+ Name: rivate_symbol
+ Flags: 0x0
+ Address: 0x2018
+ Other: 0x0
+ ImportName: ''
+ - TerminalSize: 3
+ NodeOffset: 227
+ Name: simple_symbol
+ Flags: 0x0
+ Address: 0x200C
+ Other: 0x0
+ ImportName: ''
+ - TerminalSize: 3
+ NodeOffset: 232
+ Name: moved_here_symbol
+ Flags: 0x0
+ Address: 0x2014
+ Other: 0x0
+ ImportName: ''
+ - TerminalSize: 3
+ NodeOffset: 237
+ Name: exported_symbol
+ Flags: 0x0
+ Address: 0x201C
+ Other: 0x0
+ ImportName: ''
+ NameList:
+ - n_strx: 122
+ n_type: 0x1E
+ n_sect: 3
+ n_desc: 0
+ n_value: 8208
+ - n_strx: 2
+ n_type: 0xF
+ n_sect: 3
+ n_desc: 0
+ n_value: 8192
+ - n_strx: 37
+ n_type: 0xF
+ n_sect: 3
+ n_desc: 0
+ n_value: 8196
+ - n_strx: 72
+ n_type: 0xF
+ n_sect: 3
+ n_desc: 0
+ n_value: 8200
+ - n_strx: 107
+ n_type: 0xF
+ n_sect: 3
+ n_desc: 0
+ n_value: 8204
+ - n_strx: 137
+ n_type: 0xF
+ n_sect: 3
+ n_desc: 0
+ n_value: 8212
+ - n_strx: 156
+ n_type: 0xF
+ n_sect: 3
+ n_desc: 0
+ n_value: 8216
+ - n_strx: 172
+ n_type: 0xF
+ n_sect: 3
+ n_desc: 0
+ n_value: 8220
+ - n_strx: 189
+ n_type: 0xF
+ n_sect: 3
+ n_desc: 0
+ n_value: 8224
+ - n_strx: 204
+ n_type: 0xF
+ n_sect: 3
+ n_desc: 0
+ n_value: 8228
+ - n_strx: 218
+ n_type: 0x1
+ n_sect: 0
+ n_desc: 256
+ n_value: 0
+ StringTable:
+ - ' '
+ - '$ld$hide$os10.4$_moved_here_symbol'
+ - '$ld$hide$os10.5$_moved_here_symbol'
+ - '$ld$hide$os10.6$_moved_here_symbol'
+ - _simple_symbol
+ - _hidden_symbol
+ - _moved_here_symbol
+ - _private_symbol
+ - _exported_symbol
+ - _public_symbol
+ - _alias_symbol
+ - dyld_stub_binder
+ - ''
+ - ''
+ - ''
+ - ''
+ - ''
+...
+
+;--- inputs.json.in
+{
+ "headers": [
+ {
+ "path" : "DSTROOT/Frameworks/AliasList.framework/Headers/AliasList.h",
+ "type" : "public"
+ },
+ {
+ "path" : "DSTROOT/Frameworks/AliasList.framework/PrivateHeaders/AliasList_Private.h",
+ "type" : "private"
+ }
+ ],
+ "version": "3"
+}
diff --git a/clang/tools/clang-installapi/Options.cpp b/clang/tools/clang-installapi/Options.cpp
index 3dc61476ce09d9..e79e1f67f8731a 100644
--- a/clang/tools/clang-installapi/Options.cpp
+++ b/clang/tools/clang-installapi/Options.cpp
@@ -259,7 +259,10 @@ bool Options::processLinkerOptions(InputArgList &Args) {
if (auto *Arg = Args.getLastArg(drv::OPT_umbrella))
LinkerOpts.ParentUmbrella = Arg->getValue();
- LinkerOpts.IsDylib = Args.hasArg(drv::OPT_dynamiclib);
+ for (auto *Arg : Args.filtered(drv::OPT_alias_list)) {
+ LinkerOpts.AliasLists.emplace_back(Arg->getValue());
+ Arg->claim();
+ }
LinkerOpts.AppExtensionSafe = Args.hasFlag(
drv::OPT_fapplication_extension, drv::OPT_fno_application_extension,
@@ -684,6 +687,25 @@ InstallAPIContext Options::createContext() {
return Ctx;
Ctx.Reexports = Reexports;
+ // Collect symbols from alias lists.
+ AliasMap Aliases;
+ llvm::SmallVector<std::unique_ptr<MemoryBuffer>, 2> AliasBuffers;
+ for (const StringRef ListPath : LinkerOpts.AliasLists) {
+ auto Buffer = FM->getBufferForFile(ListPath);
+ if (auto Err = Buffer.getError()) {
+ Diags->Report(diag::err_cannot_open_file) << ListPath << Err.message();
+ return C...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
cyndyishida
force-pushed
the
eng/PR-AliasList
branch
from
3170f89
to
d7a4e72
Compare
Apple's ld supports alias_lists, described as
```
-alias_list filename
The specified filename contains a list of aliases. The symbol name and its alias are on one
line, separated by whitespace. Lines starting with # are ignored.
```
To handle this for installapi-produced TBD files, pass along the same
input and account for it in verification.
cyndyishida
force-pushed
the
eng/PR-AliasList
branch
from
d7a4e72
to
b2b54ad
Compare
ributzka
reviewed
Apr 15, 2024
Co-authored-by: Juergen Ributzka <juergen@ributzka.de>
|
ping, is there anything else needed to land this patch? |
zixu-w
reviewed
Apr 17, 2024
zixu-w
approved these changes
Apr 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Apple's ld supports alias_lists, described as
To handle this for installapi-produced TBD files, pass along the same input and account for it in verification.