[llvm-commits] LLD PATCH: add ELF reader basic skeleton code

[Nick Kledzik]

Hemant,

What is your plan for test cases for this Reader?   Committing code without test cases is a slippery slope.

I think Michael is working on a way to describe COFF object files in YAML, and then have some code to read the COFF-in-YAML text files and produce and in-memory COFF binary file which is then fed to the ReaderCOFF.  With that infrastructure, it is straight forward to add test cases.  



On Wed, Jun 13, 2012 at 5:07 PM, Hemant Kulkarni <khemant at codeaurora.org> wrote:
> Hello Everyone ,
> I am working to get ELF port in LLD linker. I am attaching a patch that
> fills in basic sub-classes in the ReaderELF.  I request feedback for this
> code.  I also want suggestions for ways to test this Reader as I notice
> there is no way right now to test COFF reader as well.
> 
> The ReaderELF will read an object file and instantiate an ELFObjectFile
> object and fill in the atom subclasses that correspond for ELF.
> 
+using namespace lld;

-namespace lld {
+// 	This atom class corresponds to absolute symbol

It looks like this code is not in any namespace.  If it might be used by other code in lib/ReaderWriters/ELF/ then it should be in the lld::elf namespace, otherwise in an anonymous namespace.


+	//	 FIXME What distinguishes a symbol in ELF that can help
+	//	 decide if the symbol is undefined only during build and not
+	//	 runtime? This will make us choose canBeNullAtBuildtime and
+	//	 canBeNullAtRuntime
My understanding is that __attribute__((weak)) in C, leads to canBeNullAtBuildtime.

+
+	//	 FIXME   need to revisit this in future
+
+	virtual Interposable interposable() const {
+		return interposeNo;
+	}
I think ELF is interposeYes by default (for scopeGlobal symbols).  Interposable means that it can be overridden at runtime by having a definition in a shared object loaded earlier.  


+
+	//	FIXME What ways can we determine this in ELF?
+
+	virtual Merge merge() const {
+		return mergeNo;
+	}
You need to recognize C++ weak and C tentative definitions (commons).


+
+	//	 Many non ARM architectures use ELF file format
+	//	 This not really a place to put a architecture
+	//	 specific method in an atom. A better approach is
+	//	 needed.
+	//

Yes.  I'm sure there are other CPU specific attributes.  We need to figure out a generic way to handle these.

-Nick