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