[PATCH] [ldd] Support --[no-]as-needed options.

Shankar Kalpathi Easwaran shankarke at gmail.com
Tue Apr 23 12:31:05 PDT 2013


  The title says ldd, which should be renamed to lld as well(A minor nit)


================
Comment at: include/lld/Core/TargetInfo.h:292-297
@@ -290,1 +291,8 @@
 
+  /// Links the given dynamic library unconditionally, even if the resulting
+  /// object will not use any symbols in the library.
+  ///
+  /// \param [libraryName] The name of the library. The meaning of the
+  /// value vary among platforms. SONAME is expected for ELF.
+  virtual void addForcedDynamicLibrary(StringRef libraryName) const;
+
----------------
This should be only in ELFTargetInfo.

================
Comment at: include/lld/Driver/LinkerInput.h:99
@@ -86,2 +98,3 @@
   std::string _file;
+  bool _linkAsNeeded;
 };
----------------
You might want to specify 

bool _linkAsNeeded:1 here.

================
Comment at: include/lld/Driver/LinkerInput.h:92
@@ -83,1 +91,3 @@
 
+  bool linkAsNeeded() const {
+    return _linkAsNeeded;
----------------
Either the function name or the member variable should be changed so that you dont get confused with what is being accessed.

================
Comment at: lib/ReaderWriter/ELF/OutputELFWriter.h:162-167
@@ -153,1 +161,8 @@
+
+  // Libraries specified by command line need to be added to DT_NEEDED too, if
+  // no --no-as-needed option is given.
+  for (const auto &soname : _forcedDynamicLibraries) {
+    _soNeeded.insert(soname.getKey());
+  }
+
   for (const auto &loadName : _soNeeded) {
----------------
This is wrong. This changes the link order for shared libraries that are searched at runtime. 

The libraries should be written in the order that they appear in the command line.

================
Comment at: include/lld/ReaderWriter/Writer.h:39-52
@@ -38,2 +38,16 @@
 
+  // \brief Links the given dynamic library unconditionally, even if the
+  // resulting object will not use any symbols in the library.
+  //
+  // For ELF, linking all the libraries specified by -l is the default
+  // behavior. If you want to link only the libraries containing symbols
+  // actually used, you need to pass --as-needed option.
+  //
+  // For Mach-O, the behavior of --as-needed is the default. There's no
+  // command line option to change the behavior.
+  //
+  // By default, this function does nothing. ELF Writer will override this
+  // function.
+  virtual void addForcedDynamicLibrary(StringRef dynamicLibraryName) {}
+
 protected:
----------------
This should not be in Writer. All the information that you need is already there in ELFTargetInfo.

================
Comment at: include/lld/Driver/LinkerInput.h:48-70
@@ -47,17 +47,25 @@
 public:
-  LinkerInput(StringRef file) : _file(file) {}
+  LinkerInput(StringRef file, bool linkAsNeeded)
+      : _file(file),
+        _linkAsNeeded(linkAsNeeded) {
+  }
 
-  LinkerInput(std::unique_ptr<llvm::MemoryBuffer> buffer)
-      : _buffer(std::move(buffer)), _file(_buffer->getBufferIdentifier()) {
+  LinkerInput(std::unique_ptr<llvm::MemoryBuffer> buffer, bool linkAsNeeded)
+      : _buffer(std::move(buffer)),
+        _file(_buffer->getBufferIdentifier()),
+        _linkAsNeeded(linkAsNeeded) {
   }
 
   LinkerInput(LinkerInput &&other)
-      : _buffer(std::move(other._buffer)), _file(std::move(other._file)) {
+      : _buffer(std::move(other._buffer)),
+        _file(std::move(other._file)),
+        _linkAsNeeded(other._linkAsNeeded) {
   }
 
   LinkerInput &operator=(LinkerInput &&rhs) {
     _buffer = std::move(rhs._buffer);
     _file = std::move(rhs._file);
+    _linkAsNeeded = rhs._linkAsNeeded;
     return *this;
   }
 
----------------
I think LinkerInput was meant to be used by the API's that would essentially use the linker directly in the long run. You might want to add Nick to the review list too.

================
Comment at: lib/Driver/LDOptions.td:29-30
@@ -28,2 +28,4 @@
 def soname : Separate<["-"], "soname">;
+def as_needed : Flag<["--"], "as-needed">;
+def no_as_needed : Flag<["--"], "no-as-needed">;
 
----------------
HelpText ?


http://llvm-reviews.chandlerc.com/D708



More information about the llvm-commits mailing list