[lld] f499b93 - Revert "Revert "Revert "Revert "Revert "Revert "[lld-macho] Implement -dependency_info (partially - more opcodes needed)""""""
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 30 14:25:31 PDT 2021
It'd be great if you could include the full commit/recommit summary &
probably cleanup the commit title to remove all the "revert revert revert"
maybe something like "Recommit <blah>" then include the original full
description of the patch (the detailed multi-paragraph description, looks
like it cited a bug link, provided examples, etc) then a list of the hash
of each commit and each revert, summarizing what was addressed/changed in
each one.
Helps make it easy to figure out what's significant in this change, what
the history of this change was/what the risks are when it's being
recommitted, etc.
On Tue, Mar 23, 2021 at 11:51 AM Vy Nguyen via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
>
> Author: Vy Nguyen
> Date: 2021-03-23T14:51:05-04:00
> New Revision: f499b932bfc45b3a3543f7a49dd90af6deff5d86
>
> URL:
> https://github.com/llvm/llvm-project/commit/f499b932bfc45b3a3543f7a49dd90af6deff5d86
> DIFF:
> https://github.com/llvm/llvm-project/commit/f499b932bfc45b3a3543f7a49dd90af6deff5d86.diff
>
> LOG: Revert "Revert "Revert "Revert "Revert "Revert "[lld-macho] Implement
> -dependency_info (partially - more opcodes needed)""""""
>
> This reverts commit 4876ba5b2d6a1264ec73e5cf3fcad083f6927d19.
>
> Third-attemp relanding D98559, new change:
> - explicitly cast enum to underlying type to avoid ambiguity (workaround
> to clang's bug).
>
> Added:
> lld/test/MachO/Inputs/DependencyDump.py
> lld/test/MachO/dependency-info.s
>
> Modified:
> lld/MachO/Driver.cpp
> lld/MachO/Driver.h
> lld/MachO/DriverUtils.cpp
> lld/MachO/Options.td
>
> Removed:
>
>
>
>
> ################################################################################
> diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
> index 341ddaf870a6..93592f4f3a84 100644
> --- a/lld/MachO/Driver.cpp
> +++ b/lld/MachO/Driver.cpp
> @@ -54,7 +54,8 @@ using namespace llvm::sys;
> using namespace lld;
> using namespace lld::macho;
>
> -Configuration *lld::macho::config;
> +Configuration *macho::config;
> +DependencyTracker *macho::depTracker;
>
> static HeaderFileType getOutputType(const InputArgList &args) {
> // TODO: -r, -dylinker, -preload...
> @@ -84,6 +85,8 @@ findAlongPathsWithExtensions(StringRef name,
> ArrayRef<StringRef> extensions) {
> Twine location = base + ext;
> if (fs::exists(location))
> return location.str();
> + else
> + depTracker->logFileNotFound(location);
> }
> }
> return {};
> @@ -815,6 +818,9 @@ bool macho::link(ArrayRef<const char *> argsArr, bool
> canExitEarly,
> symtab = make<SymbolTable>();
> target = createTargetInfo(args);
>
> + depTracker =
> + make<DependencyTracker>(args.getLastArgValue(OPT_dependency_info,
> ""));
> +
> config->entry = symtab->addUndefined(args.getLastArgValue(OPT_e,
> "_main"),
> /*file=*/nullptr,
> /*isWeakRef=*/false);
> @@ -1066,6 +1072,8 @@ bool macho::link(ArrayRef<const char *> argsArr,
> bool canExitEarly,
>
> // Write to an output file.
> writeResult();
> +
> + depTracker->write(getLLDVersion(), inputFiles, config->outputFile);
> }
>
> if (config->timeTraceEnabled) {
>
> diff --git a/lld/MachO/Driver.h b/lld/MachO/Driver.h
> index 8176e9828035..89ad82e0c990 100644
> --- a/lld/MachO/Driver.h
> +++ b/lld/MachO/Driver.h
> @@ -11,10 +11,14 @@
>
> #include "lld/Common/LLVM.h"
> #include "llvm/ADT/Optional.h"
> +#include "llvm/ADT/SetVector.h"
> #include "llvm/ADT/StringRef.h"
> #include "llvm/Option/OptTable.h"
> #include "llvm/Support/MemoryBuffer.h"
>
> +#include <set>
> +#include <type_traits>
> +
> namespace llvm {
> namespace MachO {
> class InterfaceFile;
> @@ -61,6 +65,52 @@ uint32_t getModTime(llvm::StringRef path);
>
> void printArchiveMemberLoad(StringRef reason, const InputFile *);
>
> +// Helper class to export dependency info.
> +class DependencyTracker {
> +public:
> + explicit DependencyTracker(llvm::StringRef path);
> +
> + // Adds the given path to the set of not-found files.
> + inline void logFileNotFound(std::string path) {
> + if (active)
> + notFounds.insert(std::move(path));
> + }
> +
> + inline void logFileNotFound(const Twine &path) {
> + if (active)
> + notFounds.insert(path.str());
> + }
> +
> + // Writes the dependencies to specified path.
> + // The content is sorted by its Op Code, then within each section,
> + // alphabetical order.
> + void write(llvm::StringRef version,
> + const llvm::SetVector<InputFile *> &inputs,
> + llvm::StringRef output);
> +
> +private:
> + enum DepOpCode : uint8_t {
> + // Denotes the linker version.
> + Version = 0x00,
> + // Denotes the input files.
> + Input = 0x10,
> + // Denotes the files that do not exist(?)
> + NotFound = 0x11,
> + // Denotes the output files.
> + Output = 0x40,
> + };
> +
> + const llvm::StringRef path;
> + bool active;
> +
> + // The paths need to be alphabetically ordered.
> + // We need to own the paths because some of them are temporarily
> + // constructed.
> + std::set<std::string> notFounds;
> +};
> +
> +extern DependencyTracker *depTracker;
> +
> } // namespace macho
> } // namespace lld
>
>
> diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp
> index faa9b760d904..a12e1c537c16 100644
> --- a/lld/MachO/DriverUtils.cpp
> +++ b/lld/MachO/DriverUtils.cpp
> @@ -23,6 +23,7 @@
> #include "llvm/Option/ArgList.h"
> #include "llvm/Option/Option.h"
> #include "llvm/Support/CommandLine.h"
> +#include "llvm/Support/FileSystem.h"
> #include "llvm/Support/Path.h"
> #include "llvm/TextAPI/MachO/InterfaceFile.h"
> #include "llvm/TextAPI/MachO/TextAPIReader.h"
> @@ -164,12 +165,15 @@ Optional<std::string>
> macho::resolveDylibPath(StringRef path) {
> // they are consistent.
> if (fs::exists(path))
> return std::string(path);
> + else
> + depTracker->logFileNotFound(path);
>
> SmallString<261> location = path;
> path::replace_extension(location, ".tbd");
> if (fs::exists(location))
> return std::string(location);
> -
> + else
> + depTracker->logFileNotFound(location);
> return {};
> }
>
> @@ -240,3 +244,53 @@ void macho::printArchiveMemberLoad(StringRef reason,
> const InputFile *f) {
> if (config->printWhyLoad)
> message(reason + " forced load of " + toString(f));
> }
> +
> +macho::DependencyTracker::DependencyTracker(StringRef path)
> + : path(path), active(!path.empty()) {
> + if (active && fs::exists(path) && !fs::can_write(path)) {
> + warn("Ignoring dependency_info option since specified path is not "
> + "writeable.");
> + active = false;
> + }
> +}
> +
> +void macho::DependencyTracker::write(llvm::StringRef version,
> + const llvm::SetVector<InputFile *>
> &inputs,
> + llvm::StringRef output) {
> + if (!active)
> + return;
> +
> + std::error_code ec;
> + llvm::raw_fd_ostream os(path, ec, llvm::sys::fs::OF_None);
> + if (ec) {
> + warn("Error writing dependency info to file");
> + return;
> + }
> +
> + auto addDep = [&os](DepOpCode opcode, const StringRef &path) {
> + // XXX: Even though DepOpCode's underlying type is uint8_t,
> + // this cast is still needed because Clang older than 10.x has a bug,
> + // where it doesn't know to cast the enum to its underlying type.
> + // Hence `<< DepOpCode` is ambiguous to it.
> + os << static_cast<uint8_t>(opcode);
> + os << path;
> + os << '\0';
> + };
> +
> + addDep(DepOpCode::Version, version);
> +
> + // Sort the input by its names.
> + std::vector<StringRef> inputNames;
> + inputNames.reserve(inputs.size());
> + for (InputFile *f : inputs)
> + inputNames.push_back(f->getName());
> + llvm::sort(inputNames,
> + [](const StringRef &a, const StringRef &b) { return a < b;
> });
> + for (const StringRef &in : inputNames)
> + addDep(DepOpCode::Input, in);
> +
> + for (const std::string &f : notFounds)
> + addDep(DepOpCode::NotFound, f);
> +
> + addDep(DepOpCode::Output, output);
> +}
>
> diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td
> index 0e9f7b8f7390..073cb5b11621 100644
> --- a/lld/MachO/Options.td
> +++ b/lld/MachO/Options.td
> @@ -504,7 +504,6 @@ def map : Separate<["-"], "map">,
> def dependency_info : Separate<["-"], "dependency_info">,
> MetaVarName<"<path>">,
> HelpText<"Dump dependency info">,
> - Flags<[HelpHidden]>,
> Group<grp_introspect>;
> def save_temps : Flag<["-"], "save-temps">,
> HelpText<"Save intermediate LTO compilation results">,
>
> diff --git a/lld/test/MachO/Inputs/DependencyDump.py
> b/lld/test/MachO/Inputs/DependencyDump.py
> new file mode 100644
> index 000000000000..b1c1151d33fa
> --- /dev/null
> +++ b/lld/test/MachO/Inputs/DependencyDump.py
> @@ -0,0 +1,26 @@
> +#
> +# Dump the dependency file (produced with -dependency_info) to text
> +# format for testing purposes.
> +#
> +
> +import sys
> +
> +f = open(sys.argv[1], "rb")
> +byte = f.read(1)
> +while byte != b'':
> + if byte == b'\x00':
> + sys.stdout.write("lld-version: ")
> + elif byte == b'\x10':
> + sys.stdout.write("input-file: ")
> + elif byte == b'\x11':
> + sys.stdout.write("not-found: ")
> + elif byte == b'\x40':
> + sys.stdout.write("output-file: ")
> + byte = f.read(1)
> + while byte != b'\x00':
> + sys.stdout.write(byte.decode("ascii"))
> + byte = f.read(1)
> + sys.stdout.write("\n")
> + byte = f.read(1)
> +
> +f.close()
>
> diff --git a/lld/test/MachO/dependency-info.s
> b/lld/test/MachO/dependency-info.s
> new file mode 100644
> index 000000000000..f76605c35ae8
> --- /dev/null
> +++ b/lld/test/MachO/dependency-info.s
> @@ -0,0 +1,46 @@
> +# REQUIRES: x86
> +## FIXME: Paths on windows have both `\` and '/', as a result, they are
> in a
> diff erent
> +## order when sorted. Maybe create a separate test for that?
> +# UNSUPPORTED: system-windows
> +#
> +# RUN: rm -rf %t
> +# RUN: split-file %s %t
> +
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/foo.o
> %t/foo.s
> +# RUN: %lld -dylib -o %t/libfoo.dylib %t/foo.o
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/bar.o
> %t/bar.s
> +# RUN: llvm-ar csr %t/bar.a %t/bar.o
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/main.o
> %t/main.s
> +
> +# RUN: %lld %t/main.o %t/bar.a %t/libfoo.dylib -lSystem -dependency_info
> %t/deps_info.out
> +# RUN: %python %S/Inputs/DependencyDump.py %t/deps_info.out | FileCheck %s
> +
> +# CHECK: lld-version: LLD {{.*}}
> +# CHECK-DAG: input-file: {{.*}}/bar.a
> +# CHECK-DAG: input-file: {{.*}}/libfoo.dylib
> +# CHECK-DAG: input-file: {{.*}}/libSystem.tbd
> +# CHECK-DAG: input-file: {{.*}}/main.o
> +# CHECK-DAG: bar.o
> +
> +# CHECK-NEXT: not-found: {{.*}}/libdyld.dylib
> +## There could be more not-found here but we are not checking those
> because it's brittle.
> +
> +# CHECK: output-file: a.out
> +
> +#--- foo.s
> +.globl __Z3foo
> +__Z3foo:
> + ret
> +
> +#--- bar.s
> +.globl _bar
> +_bar:
> + callq __Z3foo
> + ret
> +
> +#--- main.s
> +.globl _main
> +_main:
> + callq _bar
> + callq __Z3foo
> + ret
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210330/c84774d5/attachment.html>
More information about the llvm-commits
mailing list