[PATCH] gold: add cast to gold_unreachable to workaround gcc giving invalid "no return statement" warnings

[PATCH] gold: add cast to gold_unreachable to workaround gcc giving invalid "no return statement" warnings

[Mikolaj Zalewski]

[-- Attachment #1: Type: text/plain, Size: 370 bytes --]

  I couldn't compile gold today because of the "no return statement"
warning when using gold_unreachable. At
http://gcc.gnu.org/ml/gcc-bugs/2007-11/msg01605.html I've found a
workaround that should work for gcc >= 3.4. Such a patch that affects
only gold_unreachable is ok?

2009-08-13  Mikolaj Zalewski  <mikolajz@google.com>

	* gold.h (gold_unreachable): Add a cast.

[-- Attachment #2: 0001-gold-add-cast-to-gold_unreachable-to-workaround-gcc.patch --]
[-- Type: text/x-diff, Size: 1041 bytes --]

From 2c4c78358538c7b260a2d23a66add87e28d9a1ad Mon Sep 17 00:00:00 2001
From: Mikolaj Zalewski <mikolajz@google.com>
Date: Thu, 13 Aug 2009 16:56:44 +0200
Subject: [PATCH] gold: add cast to gold_unreachable to workaround gcc giving invalid "no return statement" warnings

---
 gold/gold.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/gold/gold.h b/gold/gold.h
index 1319699..3c9971d 100644
--- a/gold/gold.h
+++ b/gold/gold.h
@@ -255,9 +255,11 @@ gold_nomem() ATTRIBUTE_NORETURN;
 
 // This macro and function are used in cases which can not arise if
 // the code is written correctly.
+// Note: the cast of __FUNCTION__ is needed to workaround gcc bug
+// http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30988 causing invalid warnings.
 
 #define gold_unreachable() \
-  (gold::do_gold_unreachable(__FILE__, __LINE__, __FUNCTION__))
+  (gold::do_gold_unreachable(__FILE__, __LINE__, (const char *)__FUNCTION__))
 
 extern void do_gold_unreachable(const char*, int, const char*)
   ATTRIBUTE_NORETURN;
-- 
1.5.4.3

Re: [PATCH] gold: add cast to gold_unreachable to workaround gcc giving invalid "no return statement" warnings

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

Mikolaj Zalewski <mikolajz@google.com> writes:

>   I couldn't compile gold today because of the "no return statement"
> warning when using gold_unreachable. At
> http://gcc.gnu.org/ml/gcc-bugs/2007-11/msg01605.html I've found a
> workaround that should work for gcc >= 3.4. Such a patch that affects
> only gold_unreachable is ok?
>
> 2009-08-13  Mikolaj Zalewski  <mikolajz@google.com>
>
> 	* gold.h (gold_unreachable): Add a cast.

I would prefer a patch like this one, which gives us a way to clean it
up a few years down the road.  Could you check whether this solves the
problem for you?  If it does, I'll commit it.  Thanks.

Ian


2009-08-14  Ian Lance Taylor  <iant@google.com>

	* gold.h (FUNCTION_NAME): Define.
	(gold_unreachable): Use FUNCTION_NAME.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Avoid gcc bug --]
[-- Type: text/x-patch, Size: 1198 bytes --]

Index: gold.h
===================================================================
RCS file: /cvs/src/src/gold/gold.h,v
retrieving revision 1.39
diff -p -u -r1.39 gold.h
--- gold.h	5 Aug 2009 20:51:56 -0000	1.39
+++ gold.h	14 Aug 2009 08:30:51 -0000
@@ -253,11 +253,22 @@ gold_undefined_symbol_at_location(const 
 extern void
 gold_nomem() ATTRIBUTE_NORETURN;
 
+// In versions of gcc before 4.3, using __FUNCTION__ in a template
+// function can cause gcc to get confused about whether or not the
+// function can return.  See http://gcc.gnu.org/PR30988.  Use a macro
+// to avoid the problem.  This can be removed when we no longer need
+// to care about gcc versions before 4.3.
+#if defined(__GNUC__) && GCC_VERSION < 4003
+#define FUNCTION_NAME static_cast<const char*>(__FUNCTION__)
+#else 
+#define FUNCTION_NAME __FUNCTION__
+#endif
+
 // This macro and function are used in cases which can not arise if
 // the code is written correctly.
 
 #define gold_unreachable() \
-  (gold::do_gold_unreachable(__FILE__, __LINE__, __FUNCTION__))
+  (gold::do_gold_unreachable(__FILE__, __LINE__, FUNCTION_NAME))
 
 extern void do_gold_unreachable(const char*, int, const char*)
   ATTRIBUTE_NORETURN;

Re: [PATCH] gold: add cast to gold_unreachable to workaround gcc giving invalid "no return statement" warnings

[Mikolaj Zalewski]

  Yes, this patch works.

Mikołaj

On Fri, Aug 14, 2009 at 10:34 AM, Ian Lance Taylor<iant@google.com> wrote:
> Mikolaj Zalewski <mikolajz@google.com> writes:
>
>>   I couldn't compile gold today because of the "no return statement"
>> warning when using gold_unreachable. At
>> http://gcc.gnu.org/ml/gcc-bugs/2007-11/msg01605.html I've found a
>> workaround that should work for gcc >= 3.4. Such a patch that affects
>> only gold_unreachable is ok?
>>
>> 2009-08-13  Mikolaj Zalewski  <mikolajz@google.com>
>>
>>       * gold.h (gold_unreachable): Add a cast.
>
> I would prefer a patch like this one, which gives us a way to clean it
> up a few years down the road.  Could you check whether this solves the
> problem for you?  If it does, I'll commit it.  Thanks.
>
> Ian
>
>
> 2009-08-14  Ian Lance Taylor  <iant@google.com>
>
>        * gold.h (FUNCTION_NAME): Define.
>        (gold_unreachable): Use FUNCTION_NAME.
>
>
>

Re: [PATCH] gold: add cast to gold_unreachable to workaround gcc giving invalid "no return statement" warnings

[Ian Lance Taylor]

Mikolaj Zalewski <mikolajz@google.com> writes:

>   Yes, this patch works.

Thanks for checking.

Committed.

Ian

> On Fri, Aug 14, 2009 at 10:34 AM, Ian Lance Taylor<iant@google.com> wrote:
>> Mikolaj Zalewski <mikolajz@google.com> writes:
>>
>>>   I couldn't compile gold today because of the "no return statement"
>>> warning when using gold_unreachable. At
>>> http://gcc.gnu.org/ml/gcc-bugs/2007-11/msg01605.html I've found a
>>> workaround that should work for gcc >= 3.4. Such a patch that affects
>>> only gold_unreachable is ok?
>>>
>>> 2009-08-13  Mikolaj Zalewski  <mikolajz@google.com>
>>>
>>>       * gold.h (gold_unreachable): Add a cast.
>>
>> I would prefer a patch like this one, which gives us a way to clean it
>> up a few years down the road.  Could you check whether this solves the
>> problem for you?  If it does, I'll commit it.  Thanks.
>>
>> Ian
>>
>>
>> 2009-08-14  Ian Lance Taylor  <iant@google.com>
>>
>>        * gold.h (FUNCTION_NAME): Define.
>>        (gold_unreachable): Use FUNCTION_NAME.
>>
>>
>>

[PATCH] Gold: Added R_ARM_ABS8 relocation

[Viktor Kutuzov]

[-- Attachment #1: Type: text/plain, Size: 177 bytes --]

Hello everyone,

Please find attached the patch for the R_ARM_ABS8 relocation.
I'll submit unit tests shortly.

Best regards,
Viktor

* gold/arm.cc: Added R_ARM_ABS8 relocation

[-- Attachment #2: binutil-gold-arm-rel-abs8.patch --]
[-- Type: application/octet-stream, Size: 3170 bytes --]

diff -E -B -b -r -u binutils.orig/src/gold/arm.cc binutils.gold/src/gold/arm.cc
--- binutils.orig/src/gold/arm.cc	2009-08-11 10:09:14.000000000 -0700
+++ binutils.gold/src/gold/arm.cc	2009-08-28 17:17:06.000000000 -0700
@@ -117,6 +117,19 @@
     return as_signed > max || as_signed < min;
   }
 
+  template<int min_no_bits, int max_no_bits>
+  static inline bool has_overflow(uint32_t bits) {
+  	gold_assert(max_no_bits >= 0 && max_no_bits <= 32);
+  	gold_assert(min_no_bits >= 0 && min_no_bits <= 32);
+  	gold_assert(min_no_bits <= max_no_bits);
+  	if (min_no_bits == 32)
+  		return false;
+  	int32_t max = (1 << (max_no_bits - 1)) - 1;
+  	int32_t min = -(1 << (min_no_bits - 1));
+  	int32_t as_signed = static_cast<int32_t> (bits);
+  	return as_signed > max || as_signed < min;
+  }
+
   // Select bits from A and B using bits in MASK.  For each n in [0..31],
   // the n-th bit in the result is chosen from the n-th bits of A and B.
   // A zero selects A and a one selects B.
@@ -555,6 +568,26 @@
   }
 
  public:
+
+  // R_ARM_ABS8: S + A
+  static inline typename This::Status
+  abs8(unsigned char *view,
+	const Sized_relobj<32, big_endian>* object,
+	const Symbol_value<32>* psymval, bool has_thumb_bit)
+  {
+	typedef typename elfcpp::Swap<8, big_endian>::Valtype Valtype;
+	typedef typename elfcpp::Swap<32, big_endian>::Valtype Reltype;
+	Valtype* wv = reinterpret_cast<Valtype*> (view);
+	Valtype val = elfcpp::Swap<8, big_endian>::readval(wv);
+	Reltype addend = utils::sign_extend<8>(val);
+	Reltype x = This::arm_symbol_value(object, psymval, addend, has_thumb_bit);
+	val = utils::bit_select(val, x, 0xffU);
+	elfcpp::Swap<8, big_endian>::writeval(wv, val);
+	return (utils::has_overflow<8, 9>(x)
+		? This::STATUS_OVERFLOW
+		: This::STATUS_OKAY);
+  }
+
   // R_ARM_ABS32: (S + A) | T
   static inline typename This::Status
   abs32(unsigned char *view,
@@ -1074,6 +1107,9 @@
     case elfcpp::R_ARM_NONE:
       break;
 
+	case elfcpp::R_ARM_ABS8:
+	  break;
+
     case elfcpp::R_ARM_ABS32:
       // If building a shared library (or a position-independent
       // executable), we need to create a dynamic relocation for
@@ -1187,6 +1223,10 @@
     case elfcpp::R_ARM_NONE:
       break;
 
+	case elfcpp::R_ARM_ABS8:
+	  //FIXME: This should handles properly the dynamic linking against the shared object.
+	  break;
+
     case elfcpp::R_ARM_ABS32:
       {
 	// Make a dynamic relocation if necessary.
@@ -1595,6 +1635,13 @@
     case elfcpp::R_ARM_NONE:
       break;
 
+	case elfcpp::R_ARM_ABS8:
+	  if (should_apply_static_reloc(gsym, Symbol::ABSOLUTE_REF, false,
+					output_section))
+		  reloc_status = Arm_relocate_functions::abs8(view, object, psymval,
+							has_thumb_bit);
+	  break;
+
     case elfcpp::R_ARM_ABS32:
       if (should_apply_static_reloc(gsym, Symbol::ABSOLUTE_REF, true,
 				    output_section))
@@ -1764,6 +1811,9 @@
     case elfcpp::R_ARM_NONE:
       return 0;
 
+  	case elfcpp::R_ARM_ABS8:
+  		return 1;
+
     case elfcpp::R_ARM_ABS32:
     case elfcpp::R_ARM_REL32:
     case elfcpp::R_ARM_THM_CALL:

Gold: Testsuite

[Viktor Kutuzov]

Hello everyone,

Sorry for a dumb question.
I'm trying to figure out how to use the gold testsuite for platform-specific things.
It doesn't seem like it fits well.

Could anyone point me to any existing and working unit test for any platform-dependent reallocation, please?

Thanks,
Viktor

Re: Gold: Testsuite

[Ian Lance Taylor]

Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> Sorry for a dumb question.
> I'm trying to figure out how to use the gold testsuite for platform-specific things.
> It doesn't seem like it fits well.
>
> Could anyone point me to any existing and working unit test for any platform-dependent reallocation, please?

It's not a dumb question.  There are two ways to approach this.  The
first is to see if there is any to generate the relocation using
ordinary C code.  If so, write a test case in C which verifies that it
does the right thing, or simply extend the existing basic_test.cc.
basic_test.cc tests various different cases, but it does not test
different type sizes.

If the relocation can not be generated using ordinary C code, then you
have to use AM_CONDITIONAL in configure.ac to generate a new Makefile
conditional, and test it in testsuite/Makefile.am to control use of an
assembler test case.

Ian

Re: Gold: Testsuite

[Viktor Kutuzov]

Thanks, Ian,
I'm looking at the configure.ac. The idea is to have a set of asm files and "correct" obj dumps to compare the result with (I'd 
prefer to keep all dependencies inside binutils as much as reasonable).

Is there a "standard" directory structure for the platform specific filees?
Should I create a new "arm" directory under the gold/testsuite and place everything there?

Best regards,
Viktor

----- Original Message ----- 
From: "Ian Lance Taylor" <iant@google.com>
To: "Viktor Kutuzov" <vkutuzov@accesssoftek.com>
Cc: <binutils@sourceware.org>
Sent: Monday, August 31, 2009 5:46 PM
Subject: Re: Gold: Testsuite


Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> Sorry for a dumb question.
> I'm trying to figure out how to use the gold testsuite for platform-specific things.
> It doesn't seem like it fits well.
>
> Could anyone point me to any existing and working unit test for any platform-dependent reallocation, please?

It's not a dumb question.  There are two ways to approach this.  The
first is to see if there is any to generate the relocation using
ordinary C code.  If so, write a test case in C which verifies that it
does the right thing, or simply extend the existing basic_test.cc.
basic_test.cc tests various different cases, but it does not test
different type sizes.

If the relocation can not be generated using ordinary C code, then you
have to use AM_CONDITIONAL in configure.ac to generate a new Makefile
conditional, and test it in testsuite/Makefile.am to control use of an
assembler test case.

Ian

Re: Gold: Testsuite

[Ian Lance Taylor]

Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> I'm looking at the configure.ac. The idea is to have a set of asm
> files and "correct" obj dumps to compare the result with (I'd prefer
> to keep all dependencies inside binutils as much as reasonable).

To be clear, I do not want to follow the pattern of comparing against
precise objdump results.  My experience with the gas testsuite, which
works that way, is that far too often unrelated changes to the tools
require the testsuite to change.  That makes these tests poor ways to
test functionality.  Developers wind up automatically adjusting the
tests to match the tool output, rather than using the test to actually
verify that the tool output is correct.

In my opinion, a test which can be run in a program is by far the best
approach.  That tests that the tool actually generates working output.
That is why basic_test.cc, and indeed much of the linker testsuite, is
written the way it is.  Of course, that approach only works for a native
linker.

For a cross-linker, testing the output with readelf (not objdump!) is
going to be the only reasonable approach.  But it's important to not try
to match the readelf output precisely, but to instead grep the readelf
output for the necessary output.  Ideally the input would be C/C++ code,
so that the same test can be reused for multiple targets.  Of course in
some cases the input must be assembler code.


> Is there a "standard" directory structure for the platform specific
> filees?  Should I create a new "arm" directory under the
> gold/testsuite and place everything there?

For cases where assembler input is required, that seems reasonable.

Ian

Re: Gold: Testsuite

[Viktor Kutuzov]

One day I will learn how to reply to All...

Thanks,
Viktor

----- Original Message ----- 
From: "Viktor Kutuzov" <vkutuzov@accesssoftek.com>
To: "Ian Lance Taylor" <iant@google.com>
Sent: Tuesday, September 01, 2009 6:18 PM
Subject: Re: Gold: Testsuite


Hi Ian,

I have been thinking along the same line.

However, I also think that cross-linker is a very important thing to support nicely.
And C/C++ is not the only codegenerator we link. If the same C/C++ code could be generated differently for different platforms, this
could lead to hard-to-catch issues in the unit tests itself.

Anyway, I'll send the unit test patch with the separate e-mail ina minute.

Best regards,
Viktor

----- Original Message ----- 
From: "Ian Lance Taylor" <iant@google.com>
To: "Viktor Kutuzov" <vkutuzov@accesssoftek.com>
Cc: <binutils@sourceware.org>
Sent: Monday, August 31, 2009 7:10 PM
Subject: Re: Gold: Testsuite


Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> I'm looking at the configure.ac. The idea is to have a set of asm
> files and "correct" obj dumps to compare the result with (I'd prefer
> to keep all dependencies inside binutils as much as reasonable).

To be clear, I do not want to follow the pattern of comparing against
precise objdump results.  My experience with the gas testsuite, which
works that way, is that far too often unrelated changes to the tools
require the testsuite to change.  That makes these tests poor ways to
test functionality.  Developers wind up automatically adjusting the
tests to match the tool output, rather than using the test to actually
verify that the tool output is correct.

In my opinion, a test which can be run in a program is by far the best
approach.  That tests that the tool actually generates working output.
That is why basic_test.cc, and indeed much of the linker testsuite, is
written the way it is.  Of course, that approach only works for a native
linker.

For a cross-linker, testing the output with readelf (not objdump!) is
going to be the only reasonable approach.  But it's important to not try
to match the readelf output precisely, but to instead grep the readelf
output for the necessary output.  Ideally the input would be C/C++ code,
so that the same test can be reused for multiple targets.  Of course in
some cases the input must be assembler code.


> Is there a "standard" directory structure for the platform specific
> filees?  Should I create a new "arm" directory under the
> gold/testsuite and place everything there?

For cases where assembler input is required, that seems reasonable.

Ian

Re: [PATCH] Gold: Added R_ARM_ABS8 relocation

[Ian Lance Taylor]

Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> +  template<int min_no_bits, int max_no_bits>
> +  static inline bool has_overflow(uint32_t bits) {
> +  	gold_assert(max_no_bits >= 0 && max_no_bits <= 32);
> +  	gold_assert(min_no_bits >= 0 && min_no_bits <= 32);
> +  	gold_assert(min_no_bits <= max_no_bits);
> +  	if (min_no_bits == 32)
> +  		return false;
> +  	int32_t max = (1 << (max_no_bits - 1)) - 1;
> +  	int32_t min = -(1 << (min_no_bits - 1));
> +  	int32_t as_signed = static_cast<int32_t> (bits);
> +  	return as_signed > max || as_signed < min;
> +  }

Passing both MIN_NO_BITS and MAX_NO_BITS is not the way to go.  What you
are really testing here is that the value must fit in either a signed or
unsigned value.  You should write it like that.  The function should not
be named has_overflow--that overloads an existing function.  The
function needs a comment.

(Of course this relocation stuff should move to generic code anyhow, but
that is not your problem.)


> +	case elfcpp::R_ARM_ABS8:
> +	  //FIXME: This should handles properly the dynamic linking against the shared object.
> +	  break;

This comment does not make sense as written.  There should be a space
after the "//".  It may be appropriate to warn if the symbol would
normally require a dynamic relocation.  The same may be true in
scan::local.

> +	case elfcpp::R_ARM_ABS8:
> +	  if (should_apply_static_reloc(gsym, Symbol::ABSOLUTE_REF, false,
> +					output_section))
> +		  reloc_status = Arm_relocate_functions::abs8(view, object, psymval,
> +							has_thumb_bit);
> +	  break;

Testing should_apply_static_reloc here only makes sense if we warn in
scan::local and scan::global.  Otherwise we will in effect silently
ignore the relocation.

Thanks for sending the patch.

Ian

[PATCH Take 2] Gold: Added R_ARM_ABS8 relocation

[Viktor Kutuzov]

[-- Attachment #1: Type: text/plain, Size: 2341 bytes --]

Thanks, Ian, for the review.

Please find attached the updated patch.

By the way, I notice that there is a mix of spaces and tabs used in the source code for indents.
I have done my best trying to follow the original stile.
But it might be a formal policy I should follow?

Best regards,
Viktor

* gold/arm.cc: Added R_ARM_ABS8 relocation

----- Original Message ----- 
From: "Ian Lance Taylor" <iant@google.com>
To: "Viktor Kutuzov" <vkutuzov@accesssoftek.com>
Cc: <binutils@sourceware.org>
Sent: Monday, August 31, 2009 5:56 PM
Subject: Re: [PATCH] Gold: Added R_ARM_ABS8 relocation


Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> +  template<int min_no_bits, int max_no_bits>
> +  static inline bool has_overflow(uint32_t bits) {
> +  gold_assert(max_no_bits >= 0 && max_no_bits <= 32);
> +  gold_assert(min_no_bits >= 0 && min_no_bits <= 32);
> +  gold_assert(min_no_bits <= max_no_bits);
> +  if (min_no_bits == 32)
> +  return false;
> +  int32_t max = (1 << (max_no_bits - 1)) - 1;
> +  int32_t min = -(1 << (min_no_bits - 1));
> +  int32_t as_signed = static_cast<int32_t> (bits);
> +  return as_signed > max || as_signed < min;
> +  }

Passing both MIN_NO_BITS and MAX_NO_BITS is not the way to go.  What you
are really testing here is that the value must fit in either a signed or
unsigned value.  You should write it like that.  The function should not
be named has_overflow--that overloads an existing function.  The
function needs a comment.

(Of course this relocation stuff should move to generic code anyhow, but
that is not your problem.)


> + case elfcpp::R_ARM_ABS8:
> +   //FIXME: This should handles properly the dynamic linking against the shared object.
> +   break;

This comment does not make sense as written.  There should be a space
after the "//".  It may be appropriate to warn if the symbol would
normally require a dynamic relocation.  The same may be true in
scan::local.

> + case elfcpp::R_ARM_ABS8:
> +   if (should_apply_static_reloc(gsym, Symbol::ABSOLUTE_REF, false,
> + output_section))
> +   reloc_status = Arm_relocate_functions::abs8(view, object, psymval,
> + has_thumb_bit);
> +   break;

Testing should_apply_static_reloc here only makes sense if we warn in
scan::local and scan::global.  Otherwise we will in effect silently
ignore the relocation.

Thanks for sending the patch.

Ian

[-- Attachment #2: binutil-gold-arm-rel-abs8-02.patch --]
[-- Type: application/octet-stream, Size: 3811 bytes --]

diff -E -B -b -r -u binutils.orig/src/gold/arm.cc binutils.gold/src/gold/arm.cc
--- binutils.orig/src/gold/arm.cc	2009-08-11 10:09:14.000000000 -0700
+++ binutils.gold/src/gold/arm.cc	2009-09-01 12:26:04.000000000 -0700
@@ -117,6 +117,22 @@
     return as_signed > max || as_signed < min;
   }
 
+  // Detects overflow of an NO_BITS integer stored in a uint32_t when it
+  // fits in the given number of bits as either a signed or unsigned value.
+  // For example, has_signed_unsigned_overflow<8> would check
+  // -128 <= bits <= 255
+  template<int no_bits>
+  static inline bool
+  has_signed_unsigned_overflow(uint32_t bits) {
+    gold_assert(no_bits >= 2 && no_bits <= 32);
+  	if (no_bits == 32)
+  		return false;
+  	int32_t max = (1 << no_bits) - 1;
+  	int32_t min = -(1 << (no_bits - 1));
+    int32_t as_signed = static_cast<int32_t>(bits);
+    return as_signed > max || as_signed < min;
+  }
+
   // Select bits from A and B using bits in MASK.  For each n in [0..31],
   // the n-th bit in the result is chosen from the n-th bits of A and B.
   // A zero selects A and a one selects B.
@@ -555,6 +571,26 @@
   }
 
  public:
+
+  // R_ARM_ABS8: S + A
+  static inline typename This::Status
+  abs8(unsigned char *view,
+	const Sized_relobj<32, big_endian>* object,
+	const Symbol_value<32>* psymval, bool has_thumb_bit)
+  {
+	typedef typename elfcpp::Swap<8, big_endian>::Valtype Valtype;
+	typedef typename elfcpp::Swap<32, big_endian>::Valtype Reltype;
+	Valtype* wv = reinterpret_cast<Valtype*> (view);
+	Valtype val = elfcpp::Swap<8, big_endian>::readval(wv);
+	Reltype addend = utils::sign_extend<8>(val);
+	Reltype x = This::arm_symbol_value(object, psymval, addend, has_thumb_bit);
+	val = utils::bit_select(val, x, 0xffU);
+	elfcpp::Swap<8, big_endian>::writeval(wv, val);
+	return (utils::has_signed_unsigned_overflow<8>(x)
+		? This::STATUS_OVERFLOW
+		: This::STATUS_OKAY);
+  }
+
   // R_ARM_ABS32: (S + A) | T
   static inline typename This::Status
   abs32(unsigned char *view,
@@ -1074,6 +1110,15 @@
     case elfcpp::R_ARM_NONE:
       break;
 
+	case elfcpp::R_ARM_ABS8:
+    if (parameters->options().output_is_position_independent())
+    {
+  		// FIXME: Create a dynamic relocation for this location.
+			gold_warning(_("%s: symbol would normally require a dynamic reloc %u."),
+					 object->name().c_str(), r_type);
+    }
+	  break;
+
     case elfcpp::R_ARM_ABS32:
       // If building a shared library (or a position-independent
       // executable), we need to create a dynamic relocation for
@@ -1187,6 +1232,16 @@
     case elfcpp::R_ARM_NONE:
       break;
 
+	case elfcpp::R_ARM_ABS8:
+		// Make a dynamic relocation if necessary.
+		if (gsym->needs_dynamic_reloc(Symbol::ABSOLUTE_REF))
+    {
+  		// FIXME: Create a dynamic relocation for this location.
+			gold_warning(_("%s: %s symbol would normally require a dynamic reloc %u."),
+					 object->name().c_str(), gsym->demangled_name().c_str(), r_type);
+    }
+	  break;
+
     case elfcpp::R_ARM_ABS32:
       {
 	// Make a dynamic relocation if necessary.
@@ -1595,6 +1650,13 @@
     case elfcpp::R_ARM_NONE:
       break;
 
+	case elfcpp::R_ARM_ABS8:
+	  if (should_apply_static_reloc(gsym, Symbol::ABSOLUTE_REF, false,
+					output_section))
+		  reloc_status = Arm_relocate_functions::abs8(view, object, psymval,
+							has_thumb_bit);
+	  break;
+
     case elfcpp::R_ARM_ABS32:
       if (should_apply_static_reloc(gsym, Symbol::ABSOLUTE_REF, true,
 				    output_section))
@@ -1764,6 +1826,9 @@
     case elfcpp::R_ARM_NONE:
       return 0;
 
+  	case elfcpp::R_ARM_ABS8:
+  		return 1;
+
     case elfcpp::R_ARM_ABS32:
     case elfcpp::R_ARM_REL32:
     case elfcpp::R_ARM_THM_CALL:

Re: [PATCH Take 2] Gold: Added R_ARM_ABS8 relocation

[Ian Lance Taylor]

Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> By the way, I notice that there is a mix of spaces and tabs used in the source code for indents.
> I have done my best trying to follow the original stile.
> But it might be a formal policy I should follow?

GNU style is tabs every 8 spaces, and gold should ideally use tabs for
long indents.  However, there are many cases where it uses spaces,
depending upon who wrote the code originally and how their editor was
configured, and that is acceptable.

More generally, there are some pointers to style guides in the README.
gold's style is an attempt to adapt GNU style to C++, or vice-versa.

Ian

[PATCH] Gold: Added R_ARM_ABS8 relocation unit test

[Viktor Kutuzov]

[-- Attachment #1: Type: text/plain, Size: 299 bytes --]

Hello everyone,

Please find attached the patch for the R_ARM_ABS8 relocation unit test.

Best regards,
Viktor

* gold/testsuite/reloc_abs_test.sh: Script to test platform independent and dependent ABS relocations
* gold/testsuite/arm/arm_r_abs8_local.s: Asm for the R_ARM_ABS8 relocation unit test

[-- Attachment #2: binutil-gold-arm-rel-abs8-unittest.diff --]
[-- Type: application/octet-stream, Size: 4807 bytes --]

diff -rupN binutils.orig/src/gold/testsuite/arm/arm_r_abs8_local.s binutils.gold/src/gold/testsuite/arm/arm_r_abs8_local.s
--- binutils.orig/src/gold/testsuite/arm/arm_r_abs8_local.s	1969-12-31 16:00:00.000000000 -0800
+++ binutils.gold/src/gold/testsuite/arm/arm_r_abs8_local.s	2009-09-01 17:49:12.000000000 -0700
@@ -0,0 +1,25 @@
+@FILE: arm_r_abs8_local.s
+@TEST RELOCATION : R_ARM_ABS8
+
+	.cpu arm10tdmi
+
+	.section	.rodata 
+	.align	4
+___T0:
+	.ascii	"123\000"
+LLCT0:
+	.ascii	"Hello_ABS8\000"
+
+	.text
+	.align 0
+
+	.byte	'>'		@ Segment start visual marker
+GCS0:
+	.byte	LLCT0		@ ABS8 relocation (LOCAL)
+
+_start:
+	.global _start
+
+	ldrb	r3, GCS0	@ Load byte offset
+
+	.end
+ 
diff -rupN binutils.orig/src/gold/testsuite/reloc_abs_test.sh binutils.gold/src/gold/testsuite/reloc_abs_test.sh
--- binutils.orig/src/gold/testsuite/reloc_abs_test.sh	1969-12-31 16:00:00.000000000 -0800
+++ binutils.gold/src/gold/testsuite/reloc_abs_test.sh	2009-09-01 17:47:36.000000000 -0700
@@ -0,0 +1,96 @@
+#!/bin/sh
+
+# reloc_abs_test.sh -- a test case for the ABS relocations.
+
+# Copyright 2009 Free Software Foundation, Inc.
+# Written by Viktor Kutuzov <vkutuzov@accesssoftek.com>.
+
+# This file is a part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+check_relocation_abs()
+{
+  # Arguments: 
+  #  $1 - Linked file name. The file with the same name and extention '.o'
+  #       will be looked for objects.
+  #  $2 - Symbol name. The relocation to test is referenced by this unique
+  #       symbol name.
+  #  $3 - Destination segment name the relocation to test is applied to.
+
+  # Retrieve original addend from the object file by using a value
+  # in the second column of the readelf symbol table dump.
+  # The readelf output dump looks like this:
+  # --------vvvvvvvv--------------------------------------------------------
+  # Symbol table '.symtab' contains 13 entries:
+  #    Num:    Value  Size Type    Bind   Vis      Ndx Name
+  #      7: 00000004     0 NOTYPE  LOCAL  DEFAULT    5 LLCT0
+
+  addend=`readelf -s $1.o | grep -w $2 | sed 's/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f3`
+  if test -z "$addend"; then
+    echo "Did not find symbol $2 in $1.o"
+    exit 1
+  fi
+
+  # Retrieve the resolved static relocation from the linked file.
+  # We are looking for a value in the second column of the dump
+  # which looks like the one above.
+
+  linker_resolved=`readelf -s $1 | grep -w $2 | sed 's/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f3`
+  if test -z "$linker_resolved"; then
+    echo "Did not find symbol $2 in $1"
+    exit 1
+  fi
+
+  # Retrieve the requested segment offset from the linked file.
+  # The value is in the  readelf output dump looks something like this:
+  # -------------------------------------vvvvvv-------------------------
+  # Section Headers:
+  #   [Nr] Name      Type       Addr     Off    Size   ES Flg Lk Inf Al
+  #   [ 2] .rodata   PROGBITS   00000080 000080 000010 00   A  0   0 16
+
+  segment_offset=`readelf -S $1 | grep -w $3 | sed 's/\]//;s/\[//;s/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f6`
+  if test -z "$segment_offset"; then
+    echo "Did not find segment $3 in $1"
+    exit 1
+  fi
+
+  linker_result=$((0x$linker_resolved + 0))
+  valid_result=$((0x$addend + 0x$segment_offset))
+
+  # Note: Uncomment the following lines to get additional debug information
+  # printed out.
+  #echo "debug: retrieved addend: $addend"
+  #echo "debug: retrieved $3 segment offset: $segment_offset"
+  #echo "debug: retrieved linker_resolved: $linker_resolved"
+  #echo "debug: linker resolved relocation value: $linker_result"
+  #echo "debug: calculated relocation value: $valid_result"
+
+  if test "$linker_result" != "$valid_result"; then
+    echo "unexpected result for relocation value in $1:"
+    echo "   $valid_result"
+    echo ""
+    echo "Actual value below:"
+    echo "$linker_result"
+    exit 1
+  fi
+}
+
+# Unit tests:
+check_relocation_abs arm/arm_r_abs8_local "LLCT0" ".rodata"
+
+exit 0
+

[PATCH] Gold: Added R_ARM_GOT_PREL relocation and unit tests

[Viktor Kutuzov]

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

Hello everyone,

Please find attached the patch for R_ARM_GOT_PREL relocation along with the unit tests.

Best regards,
Viktor

* gold/arm.cc: Added R_ARM_GOT_PREL relocation, added missing R_ARM_ABS8 in to the list of implemented relocations
* gold/testsuite/arm/Makefile.am: Added unit tests to make
* gold/testsuite/arm/reloc_got_test.sh: Script to test platform-dependent R_ARM_GOT_PREL relocation
* gold/testsuite/arm/arm_r_got_prel.s: Asm for the R_ARM_GOT_PREL relocation unit tests

[-- Attachment #2: binutil-gold-arm-rel-gotprel.diff --]
[-- Type: application/octet-stream, Size: 10833 bytes --]

diff -rupN -x .deps -x autom4te.cache -x configure -x 'config.*' -x Makefile -x Makefile.in -x '*~' binutils.orig/src/gold/arm.cc binutils.gold/src/gold/arm.cc
--- binutils.orig/src/gold/arm.cc	2009-09-15 15:40:09.000000000 -0700
+++ binutils.gold/src/gold/arm.cc	2009-09-15 16:48:49.000000000 -0700
@@ -68,11 +68,13 @@ class Output_data_plt_arm;
 // R_ARM_RELATIVE
 // R_ARM_GOTOFF32
 // R_ARM_GOT_BREL
+// R_ARM_GOT_PREL
 // R_ARM_PLT32
 // R_ARM_CALL
 // R_ARM_JUMP24
 // R_ARM_TARGET1
 // R_ARM_PREL31
+// R_ARM_ABS8
 // 
 // TODOs:
 // - Generate various branch stubs.
@@ -679,6 +681,16 @@ class Arm_relocate_functions : public Re
     return This::STATUS_OKAY;
   }
 
+  // R_ARM_GOT_PREL: GOT(S) + A – P
+  static inline typename This::Status
+  got_prel(unsigned char* view,
+	   typename elfcpp::Swap<32, big_endian>::Valtype got_offset,
+	   elfcpp::Elf_types<32>::Elf_Addr address)
+  {
+    Base::rel32(view, got_offset - address);
+    return This::STATUS_OKAY;
+  }
+
   // R_ARM_PLT32: (S + A) | T - P
   static inline typename This::Status
   plt32(unsigned char *view,
@@ -1156,6 +1168,7 @@ Target_arm<big_endian>::Scan::local(cons
       break;
 
     case elfcpp::R_ARM_GOT_BREL:
+    case elfcpp::R_ARM_GOT_PREL:
       {
 	// The symbol requires a GOT entry.
 	Output_data_got<32, big_endian>* got =
@@ -1351,6 +1364,7 @@ Target_arm<big_endian>::Scan::global(con
       break;
       
     case elfcpp::R_ARM_GOT_BREL:
+    case elfcpp::R_ARM_GOT_PREL:
       {
 	// The symbol requires a GOT entry.
 	Output_data_got<32, big_endian>* got =
@@ -1623,6 +1637,7 @@ Target_arm<big_endian>::Relocate::reloca
   switch (r_type)
     {
     case elfcpp::R_ARM_GOT_BREL:
+    case elfcpp::R_ARM_GOT_PREL:
       if (gsym != NULL)
 	{
 	  gold_assert(gsym->has_got_offset(GOT_TYPE_STANDARD));
@@ -1708,6 +1723,15 @@ Target_arm<big_endian>::Relocate::reloca
       reloc_status = Arm_relocate_functions::got_brel(view, got_offset);
       break;
 
+    case elfcpp::R_ARM_GOT_PREL:
+      gold_assert(have_got_offset);
+      // Get the address origin for GOT PLT, which is allocated right after the GOT section,
+      // to calculate an absolute address of the symbol GOT entry (got_origin + got_offset).
+      elfcpp::Elf_types<32>::Elf_Addr got_origin;
+      got_origin = target->got_plt_section()->address();
+      reloc_status = Arm_relocate_functions::got_prel(view, got_origin + got_offset, address);
+      break;
+
     case elfcpp::R_ARM_PLT32:
       gold_assert(gsym == NULL
 		  || gsym->has_plt_offset()
@@ -1835,6 +1859,7 @@ Target_arm<big_endian>::Relocatable_size
     case elfcpp::R_ARM_GOTOFF32:
     case elfcpp::R_ARM_BASE_PREL:
     case elfcpp::R_ARM_GOT_BREL:
+    case elfcpp::R_ARM_GOT_PREL:
     case elfcpp::R_ARM_PLT32:
     case elfcpp::R_ARM_CALL:
     case elfcpp::R_ARM_JUMP24:
diff -rupN -x .deps -x autom4te.cache -x configure -x 'config.*' -x Makefile -x Makefile.in -x '*~' binutils.orig/src/gold/testsuite/arm/arm_r_got_prel.s binutils.gold/src/gold/testsuite/arm/arm_r_got_prel.s
--- binutils.orig/src/gold/testsuite/arm/arm_r_got_prel.s	1969-12-31 16:00:00.000000000 -0800
+++ binutils.gold/src/gold/testsuite/arm/arm_r_got_prel.s	2009-09-15 16:25:03.000000000 -0700
@@ -0,0 +1,60 @@
+@FILE: arm_r_got_prel.s
+@TEST RELOCATION : R_ARM_GOT_PREL
+@Local static relocation
+
+@COMPILE: arm-none-linux-gnueabi-gcc arm_r_got_prel.s arm_r_got_prel.o
+@LINK:    ../binutils.gold/src/gold/ld-new arm_r_got_prel.o -o arm_r_got_prel
+
+	.cpu arm10tdmi
+	.file	"arm_r_got_prel.s"
+	.text
+	.align	2
+start:
+	.global	start
+.LCFI2:
+	ldr	r4, .L3
+.LPIC0:
+	add	r4, pc, r4		@ r4 will contain the GOT pointer 
+					@ after this.
+
+	@----------------------------------------------------
+	@ printf _global_string_got_prel
+
+	ldr	r3, .LL0		@ Loading statically relocated value 
+					@ -- an offset to GOT entry from 
+					@ the relocation address
+	add	r3, r3, #(.LL0-.LOFF1)  @ Building an absolute address to 
+					@ the GOT entry (pc + off_to_word +
+					@ off_to_GOT)
+	ldr	r3, [pc, r3]		@ Loading a value of the GOT entry
+	ldr	r3, [r3, #0]		@ Loading a pointer to the destination
+					@ string.
+.LOFF1:	mov	r0, r3
+	@bl	printf(PLT)		@ kind of printing
+
+	@----------------------------------------------------
+	.align	2
+.L3:
+	.word	_GLOBAL_OFFSET_TABLE_-(.LPIC0+8)
+REL_PREL_ADDRESS:
+	@ NOTE: put this variable into the symbol table to use in the tests.
+	.global	REL_PREL_ADDRESS
+	@ NOTE: TARGET2 relocation will be interpreted as GOT_PREL by GOLD
+.LL0:	.word   _global_string_got_prel(TARGET2)
+
+	.global	_global_string_got_prel
+
+	@----------------------------------------------------
+	.section	.rodata
+	.align	2
+.LC0:
+	.ascii  "GLOBAL (TARGET2/R_ARM_GOT_PREL).\012\000"
+
+	@----------------------------------------------------
+	.section	.data.rel.local,"aw",%progbits
+	.align	2
+_global_string_got_prel:
+	.word	.LC0
+
+	@----------------------------------------------------
+	.end
diff -rupN -x .deps -x autom4te.cache -x configure -x 'config.*' -x Makefile -x Makefile.in -x '*~' binutils.orig/src/gold/testsuite/arm/Makefile.am binutils.gold/src/gold/testsuite/arm/Makefile.am
--- binutils.orig/src/gold/testsuite/arm/Makefile.am	2009-09-15 15:40:44.000000000 -0700
+++ binutils.gold/src/gold/testsuite/arm/Makefile.am	2009-09-15 14:26:53.000000000 -0700
@@ -55,22 +55,25 @@ if TARGET_ARM
 
 # Test - ABS relocations
 
-MOSTLYCLEANFILES += arm_r_abs8_local arm_r_abs8_localrw arm_r_abs8_external
+MOSTLYCLEANFILES += arm_r_abs8_local arm_r_abs8_localrw arm_r_abs8_external arm_r_got_prel
 
 BUILT_SOURCES += \
 	arm_r_abs8_local.s \
 	arm_r_abs8_localrw.s \
 	arm_r_abs8_global.s \
-	arm_r_abs8_external.s
+	arm_r_abs8_external.s \
+	arm_r_got_prel.s
 
 check_DATA += \
 	arm_r_abs8_local \
 	arm_r_abs8_external \
 	arm_r_abs8_localrw \
-	arm_r_abs8_localoverflow.o
+	arm_r_abs8_localoverflow.o \
+	arm_r_got_prel
 
 check_SCRIPTS += \
-	reloc_abs_test.sh
+	reloc_abs_test.sh \
+	reloc_got_test.sh
 
 arm_r_abs8_local.o: arm_r_abs8_local.s
 	$(TEST_AS) -o $@ $<
@@ -82,6 +85,8 @@ arm_r_abs8_global.o: arm_r_abs8_global.s
 	$(TEST_AS) -o $@ $<
 arm_r_abs8_external.o: arm_r_abs8_external.s
 	$(TEST_AS) -o $@ $<
+arm_r_got_prel.o: arm_r_got_prel.s
+	$(TEST_AS) -o $@ $<
 
 arm_r_abs8_local: arm_r_abs8_local.o
 	$(TEST_LD) -Ttext=0 -o $@ $<
@@ -89,6 +94,8 @@ arm_r_abs8_localrw: arm_r_abs8_localrw.o
 	$(TEST_LD) -Ttext=0 -Tdata=0x90 -o $@ $<
 arm_r_abs8_external: arm_r_abs8_external.o arm_r_abs8_global.o
 	$(TEST_LD) -Ttext=0 -o $@ $< arm_r_abs8_global.o
+arm_r_got_prel: arm_r_got_prel.o
+	$(TEST_LD) -o $@ $<
 
 endif TARGET_ARM
 
diff -rupN -x .deps -x autom4te.cache -x configure -x 'config.*' -x Makefile -x Makefile.in -x '*~' binutils.orig/src/gold/testsuite/arm/reloc_got_test.sh binutils.gold/src/gold/testsuite/arm/reloc_got_test.sh
--- binutils.orig/src/gold/testsuite/arm/reloc_got_test.sh	1969-12-31 16:00:00.000000000 -0800
+++ binutils.gold/src/gold/testsuite/arm/reloc_got_test.sh	2009-09-15 16:27:25.000000000 -0700
@@ -0,0 +1,88 @@
+#!/bin/sh
+
+# reloc_got_test.sh -- a test case for the GOT relocations.
+
+# Copyright 2009 Free Software Foundation, Inc.
+# Written by Viktor Kutuzov <vkutuzov@accesssoftek.com>.
+
+# This file is a part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+check_relocation_got_prel_value()
+{
+  # Arguments: 
+  #  $1 - Linked file name. The file with the same name and extention '.o'
+  #       will be looked for objects.
+  file_name=$1
+  #  $2 - Symbol name. The relocation to test is referenced by this unique
+  #       symbol name.
+  symbol_name=$2
+  #  $3 - Symbol name. The relocation reference within the .text section
+  symbol_reloc_name=$3
+
+
+  # Retrieve the requested segment address from the linked file.
+  # The value is in the readelf output dump looks something like this:
+  # ----------------------------vvvvvvvv--------------------------------
+  # Section Headers:
+  #   [Nr] Name      Type       Addr     Off    Size   ES Flg Lk Inf Al
+  #   [ 4] .got      PROGBITS   000090c0 0000c0 000010 00  WA  0   0  4
+  valid_result=`readelf -S $file_name | grep -w ".got" | sed 's/\]//;s/\[//;s/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f5`
+  if test -z "$valid_result"; then
+    echo "Did not find segment .got in $file_name"
+    exit 1
+  fi
+
+  symbol_reloc_address=`readelf -s $file_name | grep -w $symbol_reloc_name | sed 's/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f3 | sed 's/^0*//g'` 
+  if test -z "$symbol_reloc_address"; then
+    echo "Did not find symbol $symbol_reloc_name in $file_name"
+    exit 1
+  fi
+
+  # Result transformation from string like "76:	90          	.byte	0x90" 
+  # to "76:90:.byte:0x90"
+  linker_result=`../../../binutils/objdump -ds $file_name | sed -e 's/^\s*//g' -e 's/^0*//g' | grep "^$symbol_reloc_address:" | sed -e 's/://' -e 's/\t/:/g' -e 's/\s//g' | cut -d: -f2`
+
+  # Add the symbol relocation address to the relocated value. We will get 
+  # a pointer (address) to the GOT entry as result.
+  # This entry has been placed at beginning of the table, because we have 
+  # only one entry in GOT. In that way an address of GOT must be equal with 
+  # an address of the entry (see $valid_result)
+  linker_result=$((0x$linker_result + 0x$symbol_reloc_address))
+  valid_result=$((0x$valid_result + 0))
+
+  #echo "debug: valid_result=$valid_result"
+  #echo "debug: symbol_reloc_address=$symbol_reloc_address"
+  #echo "debug: linked_result=$linker_result"
+
+  if test $linker_result != $valid_result; then
+    echo "check_relocation_abs_global: unexpected result for relocation value in $file_name:"
+    echo "   $valid_result"
+    echo ""
+    echo "Actual value below:"
+    echo "$linker_result"
+    exit 1
+  fi
+}
+
+
+# Unit tests:
+
+check_relocation_got_prel_value arm_r_got_prel "_global_string_got_prel" "REL_PREL_ADDRESS"
+
+exit 0
+

[Gold] Heads up. Working on interworking

[Viktor Kutuzov]

I'm about to start working on ARM interworking.
Is anybody else working on this?

Thatnks,
Viktor

[GOLD] Heads up. Gold for mingw.

[Viktor Kutuzov]

Hello everyone,

I have started working to make gold build and work for mingw (Windows).
There are 4 areas need to get changed:

1.	The plug-in shared libraries will be plug-in DLLs. This one is easy and straight forward.

2.	Windows memory mapping doesn't fit well the used memory mapping. Mingw does not support mmap/munmap. I think of extracting memory mapping to a separate set of classes (one multi-platform abstract base class and 2 platform-specific implementation classes).

3.	Gathering read is not supported on Windows (unless we want to use overlapped I/O). I think of implementing a Windows-specific readv based on a regular read for now. Later we can make it better if we will want to.

4.	Make files to add a new mingw configuration. This one is also easy and straight forward.

Does anybody else work on this?
Does anybody else interesting in gold for Windows?
Is there a better idea how to do this?

Cheers,
Viktor

Re: [GOLD] Heads up. Gold for mingw.

[Vincent R.]

On Tue, 6 Oct 2009 14:45:40 -0700, Viktor Kutuzov
<vkutuzov@accesssoftek.com> wrote:
> Hello everyone,
> 
> I have started working to make gold build and work for mingw (Windows).
> There are 4 areas need to get changed:
> 
> 1.	The plug-in shared libraries will be plug-in DLLs. This one is easy
and
> straight forward.
> 
> 2.	Windows memory mapping doesn't fit well the used memory mapping.
Mingw
> does not support mmap/munmap. I think of extracting memory mapping to a
> separate set of classes (one multi-platform abstract base class and 2
> platform-specific implementation classes).
> 
> 3.	Gathering read is not supported on Windows (unless we want to use
> overlapped I/O). I think of implementing a Windows-specific readv based
on
> a regular read for now. Later we can make it better if we will want to.
> 
> 4.	Make files to add a new mingw configuration. This one is also easy
and
> straight forward.
> 
> Does anybody else work on this?
> Does anybody else interesting in gold for Windows?
> Is there a better idea how to do this?
> 
> Cheers,
> Viktor

Last time I asked I was answered that GOLD was very specific to elf and
when I read announcment :

"gold only supports ELF targets such as GNU/Linux and other free
operating systems.  gold does not support Windows.  There is no
expectation that gold will ever support anything other than ELF
targets."

But I like this quote from Marc Twain : They did not know it was
impossible, so they did it!

Re: [GOLD] Heads up. Gold for mingw.

[Viktor Kutuzov]

> "gold only supports ELF targets such as GNU/Linux and other free
> operating systems.  gold does not support Windows.  There is no
> expectation that gold will ever support anything other than ELF
> targets."

That's fine.
I'm targeting the cross-build scenario. Like gold itself is running on Windows to make an elf file for some other platform.

-Viktor
 
----- Original Message ----- 
From: "Vincent R." <forumer@smartmobili.com>
To: "Viktor Kutuzov" <vkutuzov@accesssoftek.com>
Cc: <binutils@sourceware.org>
Sent: Tuesday, October 06, 2009 3:02 PM
Subject: Re: [GOLD] Heads up. Gold for mingw.


> On Tue, 6 Oct 2009 14:45:40 -0700, Viktor Kutuzov
> <vkutuzov@accesssoftek.com> wrote:
>> Hello everyone,
>> 
>> I have started working to make gold build and work for mingw (Windows).
>> There are 4 areas need to get changed:
>> 
>> 1. The plug-in shared libraries will be plug-in DLLs. This one is easy
> and
>> straight forward.
>> 
>> 2. Windows memory mapping doesn't fit well the used memory mapping.
> Mingw
>> does not support mmap/munmap. I think of extracting memory mapping to a
>> separate set of classes (one multi-platform abstract base class and 2
>> platform-specific implementation classes).
>> 
>> 3. Gathering read is not supported on Windows (unless we want to use
>> overlapped I/O). I think of implementing a Windows-specific readv based
> on
>> a regular read for now. Later we can make it better if we will want to.
>> 
>> 4. Make files to add a new mingw configuration. This one is also easy
> and
>> straight forward.
>> 
>> Does anybody else work on this?
>> Does anybody else interesting in gold for Windows?
>> Is there a better idea how to do this?
>> 
>> Cheers,
>> Viktor
> 
> Last time I asked I was answered that GOLD was very specific to elf and
> when I read announcment :
> 
> "gold only supports ELF targets such as GNU/Linux and other free
> operating systems.  gold does not support Windows.  There is no
> expectation that gold will ever support anything other than ELF
> targets."
> 
> But I like this quote from Marc Twain : They did not know it was
> impossible, so they did it!
> 
> 
>

Re: [GOLD] Heads up. Gold for mingw.

[Matt Rice]

On Tue, Oct 6, 2009 at 2:45 PM, Viktor Kutuzov
<vkutuzov@accesssoftek.com> wrote:

> Does anybody else work on this?

Andrew Pinski was working on this i believe

Re: [PATCH] Gold: Added R_ARM_GOT_PREL relocation and unit tests

[Ian Lance Taylor]

Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> Please find attached the patch for R_ARM_GOT_PREL relocation along with the unit tests.
>
> Best regards,
> Viktor
>
> * gold/arm.cc: Added R_ARM_GOT_PREL relocation, added missing R_ARM_ABS8 in to the list of implemented relocations
> * gold/testsuite/arm/Makefile.am: Added unit tests to make
> * gold/testsuite/arm/reloc_got_test.sh: Script to test platform-dependent R_ARM_GOT_PREL relocation
> * gold/testsuite/arm/arm_r_got_prel.s: Asm for the R_ARM_GOT_PREL relocation unit tests


I approved and committed your patch to arm.cc with the following
ChangeLog entry.  I did not commit the patch to the testsuite, pending
further work on the testsuite.

Thanks, and sorry for the long delay.

Ian

2009-10-07  Viktor Kutuzov  <vkutuzov@accesssoftek.com>

	* arm.cc (Arm_relocate_functions::got_prel) New function.
	(Scan::local, Scan::global): Handle R_ARM_GOT_PREL.
	(Relocate::relocate): Likewise.
	(Relocatable_size_for_reloc::get_size_for_reloc): Likewise.

[PATCH, Take 2] Gold: Added R_ARM_ABS8 relocation unit test

[Viktor Kutuzov]

[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]

Hello everyone,

Please find attached the updated patch for the R_ARM_ABS8 relocation unit test.

The patch contains changes for automake system to include testsuite/arm directory to the build as well as a shell unit test script 
for R_ARM_ABS8 relocation along with the needed asm file. The unit tests are testing R_ARM_ABS8 relocation for the follwoing cases:

* static local relocation in a read-only segment;
* static global relocation in a read-only segment;
* static local relocation in a read-write segment;
* static global relocation in a read-write segment;
* static local relocation overflow in a read-only segment;

Hope the patch is not too big.

Best regards,
Viktor

* gold/configure.ac: Added target-dependent (for ARM) testsuite/arm/Makefile
* gold/testsuite/Makefile.am: Included testsuite/arm directory for automake
* gold/testsuite/arm/Makefile.am: Included testsuite/arm directory for automake

* gold/testsuite/arm/reloc_abs_test.sh: Unit test script for R_ARM_ABS*

* gold/testsuite/arm/arm_r_abs8_local.s: Asm for the R_ARM_ABS8 relocation unit test (local reloc)
* gold/testsuite/arm/arm_r_abs8_global.s: Asm for the R_ARM_ABS8 relocation unit test (global reloc)
* gold/testsuite/arm/arm_r_abs8_external.s: Asm for the R_ARM_ABS8 relocation unit test (external symbol)
* gold/testsuite/arm/arm_r_abs8_localrw.s: Asm for the R_ARM_ABS8 relocation unit test (local reloc in a read/write segment)

----- Original Message ----- 
From: "Viktor Kutuzov" <vkutuzov@accesssoftek.com>
To: <binutils@sourceware.org>
Sent: Tuesday, September 01, 2009 6:30 PM
Subject: [PATCH] Gold: Added R_ARM_ABS8 relocation unit test


Hello everyone,

Please find attached the patch for the R_ARM_ABS8 relocation unit test.

Best regards,
Viktor

* gold/testsuite/reloc_abs_test.sh: Script to test platform independent and dependent ABS relocations
* gold/testsuite/arm/arm_r_abs8_local.s: Asm for the R_ARM_ABS8 relocation unit test

[-- Attachment #2: binutil-gold-arm-rel-abs8-unittest-02.diff --]
[-- Type: application/octet-stream, Size: 16622 bytes --]

diff -rupN binutils.orig/src/gold/configure.ac binutils.gold/src/gold/configure.ac
--- binutils.orig/src/gold/configure.ac	2009-06-23 11:10:41.000000000 -0700
+++ binutils.gold/src/gold/configure.ac	2009-09-02 12:41:45.000000000 -0700
@@ -224,6 +224,8 @@ AM_CONDITIONAL(FN_PTRS_IN_SO_WITHOUT_PIC
 dnl Whether we can test -mcmodel=medium.
 AM_CONDITIONAL(MCMODEL_MEDIUM, [test "$target_cpu" = "x86_64"])
 
+AM_CONDITIONAL(TARGET_ARM, [test "$target_cpu" = "arm"])
+
 dnl Test for __thread support.
 AC_CACHE_CHECK([for thread support], [gold_cv_c_thread],
 [AC_COMPILE_IFELSE([__thread int i = 1;],
@@ -368,4 +370,4 @@ AC_LANG_POP(C++)
 
 AM_MAINTAINER_MODE
 
-AC_OUTPUT(Makefile testsuite/Makefile po/Makefile.in:po/Make-in)
+AC_OUTPUT(Makefile testsuite/Makefile po/Makefile.in:po/Make-in testsuite/arm/Makefile)
diff -rupN binutils.orig/src/gold/testsuite/arm/arm_r_abs8_external.s binutils.gold/src/gold/testsuite/arm/arm_r_abs8_external.s
--- binutils.orig/src/gold/testsuite/arm/arm_r_abs8_external.s	1969-12-31 16:00:00.000000000 -0800
+++ binutils.gold/src/gold/testsuite/arm/arm_r_abs8_external.s	2009-09-02 16:58:58.000000000 -0700
@@ -0,0 +1,26 @@
+@FILE: arm_r_abs8_external.s
+@TEST RELOCATION : R_ARM_ABS8
+
+	.cpu arm10tdmi
+
+	.section	.rodata 
+	.align	4
+LLCT0:
+	.ascii	"Hello_ABS8\000"
+
+	.text
+	.align 0
+
+	.byte	'>'		@ Segment start visual marker
+GCS0:
+	.byte	LLCT0		@ ABS8 relocation (LOCAL)
+GCS0_GL:
+	.byte	GLCT0		@ ABS8 relocation (GLOBAL)
+
+_start:
+	.global _start
+
+	ldrb	r3, GCS0	@ Load byte offset
+	ldrb	r3, GCS0_GL	@ Load byte offset
+
+	.end
diff -rupN binutils.orig/src/gold/testsuite/arm/arm_r_abs8_global.s binutils.gold/src/gold/testsuite/arm/arm_r_abs8_global.s
--- binutils.orig/src/gold/testsuite/arm/arm_r_abs8_global.s	1969-12-31 16:00:00.000000000 -0800
+++ binutils.gold/src/gold/testsuite/arm/arm_r_abs8_global.s	2009-09-03 13:47:03.000000000 -0700
@@ -0,0 +1,11 @@
+@FILE: arm_r_abs8_global.s
+@TEST RELOCATION : R_ARM_ABS8
+
+	.cpu arm10tdmi
+
+	.section	.rodata 
+	.align	4
+GLCT0:	.global GLCT0
+	@ Unique 128-bit signature
+	.octa    0xD78E074738E649789A7D5ED45F7EFF74
+	.end
diff -rupN binutils.orig/src/gold/testsuite/arm/arm_r_abs8_localrw.s binutils.gold/src/gold/testsuite/arm/arm_r_abs8_localrw.s
--- binutils.orig/src/gold/testsuite/arm/arm_r_abs8_localrw.s	1969-12-31 16:00:00.000000000 -0800
+++ binutils.gold/src/gold/testsuite/arm/arm_r_abs8_localrw.s	2009-09-02 15:28:49.000000000 -0700
@@ -0,0 +1,41 @@
+@FILE: arm_r_abs8_localrw.s
+@TEST RELOCATION : R_ARM_ABS8
+@Local static relocation for RO/RW data.
+
+	.cpu arm10tdmi
+	.org 0
+
+	@----------------------------------------------------
+	.section	.rodata 
+	.align	4
+___T0:
+	.ascii	"123\000"	@ Make some shift for the source data.
+LLCT0:
+	.ascii	"Hello_ABS8\000"
+
+	@----------------------------------------------------
+	.section	.data
+___D0:
+	.ascii	"456\000"
+DLCT0:
+	.ascii	"Hello_ABS8_RW\000"
+
+	@----------------------------------------------------
+	.text
+	.align 0
+
+	.byte	'>'		@ Segment start visual marker
+LCS0:
+	.byte	LLCT0		@ ABS8 relocation (LOCAL)
+LCS1:
+	.byte	DLCT0
+
+	@----------------------------------------------------
+_start:
+	.global _start
+
+	ldrb	r3, LCS0	@ Load byte offset
+	ldrb	r3, LCS1
+
+	@----------------------------------------------------
+	.end
diff -rupN binutils.orig/src/gold/testsuite/arm/arm_r_abs8_local.s binutils.gold/src/gold/testsuite/arm/arm_r_abs8_local.s
--- binutils.orig/src/gold/testsuite/arm/arm_r_abs8_local.s	1969-12-31 16:00:00.000000000 -0800
+++ binutils.gold/src/gold/testsuite/arm/arm_r_abs8_local.s	2009-09-02 15:43:46.000000000 -0700
@@ -0,0 +1,30 @@
+@FILE: arm_r_abs8_local.s
+@TEST RELOCATION : R_ARM_ABS8
+@Local static relocation for RO data.
+
+	.cpu arm10tdmi
+
+	@----------------------------------------------------
+	.section	.rodata 
+	.align	4
+___T0:
+	.ascii	"123\000"	@ Make some shift for the source data.
+LLCT0:
+	.ascii	"Hello_ABS8\000"
+
+	@----------------------------------------------------
+	.text
+	.align 0
+
+	.byte	'>'		@ Segment start visual marker
+LCS0:
+	.byte	LLCT0		@ ABS8 relocation (LOCAL)
+
+	@----------------------------------------------------	
+_start:
+	.global _start
+
+	ldrb	r3, LCS0	@ Load byte offset
+
+	@----------------------------------------------------
+	.end
diff -rupN binutils.orig/src/gold/testsuite/arm/Makefile.am binutils.gold/src/gold/testsuite/arm/Makefile.am
--- binutils.orig/src/gold/testsuite/arm/Makefile.am	1969-12-31 16:00:00.000000000 -0800
+++ binutils.gold/src/gold/testsuite/arm/Makefile.am	2009-09-02 15:59:15.000000000 -0700
@@ -0,0 +1,94 @@
+# Process this file with automake to generate Makefile.in
+
+# Platform depended tests - ARM
+#
+# As far as I can tell automake testing support assumes that the build
+# system and the host system are the same.  So these tests will not
+# work when building with a cross-compiler.
+
+# Ignore warning about AM_PROG_CC_C_O due to large_CFLAGS
+AUTOMAKE_OPTIONS = foreign -Wno-portability
+
+# The two_file_test tests -fmerge-constants, so we simply always turn
+# it on.  This may need to be controlled by a configure option
+# eventually.
+AM_CFLAGS = $(WARN_CFLAGS) $(LFS_CFLAGS) -fmerge-constants
+AM_CXXFLAGS = $(WARN_CXXFLAGS) $(LFS_CFLAGS) -fmerge-constants
+
+AM_CPPFLAGS = \
+	-I$(srcdir) -I$(srcdir)/.. -I$(srcdir)/../../include \
+	-I$(srcdir)/../../elfcpp -I.. \
+	-DLOCALEDIR="\"$(datadir)/locale\"" \
+	@INCINTL@
+
+TEST_READELF = $(top_builddir)/../binutils/readelf
+TEST_OBJDUMP = $(top_builddir)/../binutils/objdump
+TEST_CXXFILT = $(top_builddir)/../binutils/cxxfilt
+TEST_STRIP = $(top_builddir)/../binutils/strip-new
+TEST_AR = $(top_builddir)/../binutils/ar
+TEST_NM = $(top_builddir)/../binutils/nm-new
+TEST_AS = $(top_builddir)/../gas/as-new
+TEST_LD = $(top_builddir)/../gold/ld-new
+
+# 'make clean' is good about deleting some intermediate files (such as
+# .o's), but not all of them (such as .so's and .err files).  We
+# improve on that here.  automake-1.9 info docs say "mostlyclean" is
+# the right choice for files 'make' builds that people rebuild.
+MOSTLYCLEANFILES = *.so *.o
+
+
+# We will add to these later, for each individual test.  Note
+# that we add each test under check_SCRIPTS or check_PROGRAMS;
+# the TESTS variable is automatically populated from these.
+check_SCRIPTS =
+check_DATA =
+check_PROGRAMS =
+check_LIBRARIES = 
+BUILT_SOURCES =
+
+TESTS = $(check_SCRIPTS) $(check_PROGRAMS)
+
+# ---------------------------------------------------------------------
+# The unittests themselves
+
+if TARGET_ARM
+
+# Test - ABS relocations
+
+MOSTLYCLEANFILES += arm_r_abs8_local arm_r_abs8_localrw arm_r_abs8_external
+
+BUILT_SOURCES += \
+	arm_r_abs8_local.s \
+	arm_r_abs8_localrw.s \
+	arm_r_abs8_global.s \
+	arm_r_abs8_external.s
+
+check_DATA += \
+	arm_r_abs8_local \
+	arm_r_abs8_external \
+	arm_r_abs8_localrw \
+	arm_r_abs8_localoverflow.o
+
+check_SCRIPTS += \
+	reloc_abs_test.sh
+
+arm_r_abs8_local.o: arm_r_abs8_local.s
+	$(TEST_AS) -o $@ $<
+arm_r_abs8_localrw.o: arm_r_abs8_localrw.s
+	$(TEST_AS) -o $@ $<
+arm_r_abs8_localoverflow.o: arm_r_abs8_localrw.s
+	$(TEST_AS) -o $@ $<
+arm_r_abs8_global.o: arm_r_abs8_global.s
+	$(TEST_AS) -o $@ $<
+arm_r_abs8_external.o: arm_r_abs8_external.s
+	$(TEST_AS) -o $@ $<
+
+arm_r_abs8_local: arm_r_abs8_local.o
+	$(TEST_LD) -Ttext=0 -o $@ $<
+arm_r_abs8_localrw: arm_r_abs8_localrw.o
+	$(TEST_LD) -Ttext=0 -Tdata=0x90 -o $@ $<
+arm_r_abs8_external: arm_r_abs8_external.o arm_r_abs8_global.o
+	$(TEST_LD) -Ttext=0 -o $@ $< arm_r_abs8_global.o
+
+endif TARGET_ARM
+
diff -rupN binutils.orig/src/gold/testsuite/arm/reloc_abs_test.sh binutils.gold/src/gold/testsuite/arm/reloc_abs_test.sh
--- binutils.orig/src/gold/testsuite/arm/reloc_abs_test.sh	1969-12-31 16:00:00.000000000 -0800
+++ binutils.gold/src/gold/testsuite/arm/reloc_abs_test.sh	2009-09-03 14:28:31.000000000 -0700
@@ -0,0 +1,211 @@
+#!/bin/sh
+
+# reloc_abs_test.sh -- a test case for the ABS relocations.
+
+# Copyright 2009 Free Software Foundation, Inc.
+# Written by Viktor Kutuzov <vkutuzov@accesssoftek.com>.
+
+# This file is a part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+check_relocation_abs()
+{
+  # Arguments: 
+  #  $1 - Linked file name. The file with the same name and extention '.o'
+  #       will be looked for objects.
+  #  $2 - Symbol name. The relocation to test is referenced by this unique
+  #       symbol name.
+  #  $3 - Destination segment name the relocation to test is applied to.
+
+  # Retrieve original addend from the object file by using a value
+  # in the second column of the readelf symbol table dump.
+  # The readelf output dump looks like this:
+  # --------vvvvvvvv--------------------------------------------------------
+  # Symbol table '.symtab' contains 13 entries:
+  #    Num:    Value  Size Type    Bind   Vis      Ndx Name
+  #      7: 00000004     0 NOTYPE  LOCAL  DEFAULT    5 LLCT0
+
+  addend=`readelf -s $1.o | grep -w $2 | sed 's/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f3`
+  if test -z "$addend"; then
+    echo "Did not find symbol $2 in $1.o"
+    exit 1
+  fi
+
+  # Retrieve the resolved static relocation from the linked file.
+  # We are looking for a value in the second column of the dump
+  # which looks like the one above.
+
+  linker_resolved=`readelf -s $1 | grep -w $2 | sed 's/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f3`
+  if test -z "$linker_resolved"; then
+    echo "Did not find symbol $2 in $1"
+    exit 1
+  fi
+
+  # Retrieve the requested segment offset from the linked file.
+  # The value is in the  readelf output dump looks something like this:
+  # -------------------------------------vvvvvv-------------------------
+  # Section Headers:
+  #   [Nr] Name      Type       Addr     Off    Size   ES Flg Lk Inf Al
+  #   [ 2] .rodata   PROGBITS   00000080 000080 000010 00   A  0   0 16
+
+  segment_offset=`readelf -S $1 | grep -w $3 | sed 's/\]//;s/\[//;s/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f6`
+  if test -z "$segment_offset"; then
+    echo "Did not find segment $3 in $1"
+    exit 1
+  fi
+
+  linker_result=$((0x$linker_resolved + 0))
+  valid_result=$((0x$addend + 0x$segment_offset))
+
+  # Note: Uncomment the following lines to get additional debug information
+  # printed out.
+  #echo "debug: retrieved addend: $addend"
+  #echo "debug: retrieved $3 segment offset: $segment_offset"
+  #echo "debug: retrieved linker_resolved: $linker_resolved"
+  #echo "debug: linker resolved relocation value: $linker_result"
+  #echo "debug: calculated relocation value: $valid_result"
+
+  if test "$linker_result" != "$valid_result"; then
+    echo "check_relocation_abs: unexpected result for relocation value in $1:"
+    echo "   $valid_result"
+    echo ""
+    echo "Actual value below:"
+    echo "$linker_result"
+    exit 1
+  fi
+}
+
+check_relocation_abs_global()
+{
+  # Arguments: 
+  #  $1 - Linked file name. The file with the same name and extention '.o'
+  #       will be looked for objects.
+  file_name=$1
+  #  $2 - Symbol name. The relocation to test is referenced by this unique
+  #       symbol name.
+  symbol_name=$2
+  #  $3 - Destination segment name the relocation to test is applied to.
+  segment_name=$3
+  #  $4 - Unique 128-bit signature to match.
+  signature=$4
+  
+
+  linker_result=`readelf -s $file_name | grep -w $symbol_name | sed 's/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f3`
+  if test -z "$linker_result"; then
+    echo "Did not find symbol $symbol_name in $file_name"
+    exit 1
+  fi
+
+  # readelf -x ".rodata" <executable_object>
+  #
+  # Hex dump of section '.rodata':
+  #   0x00000080 48656c6c 6f5f4142 53380000 00000000 Hello_ABS8......
+  #   0x00000090 74ff7e5f d45e7d9a 7849e638 47078ed7 t.~_.^}.xI.8G...
+
+  valid_result=`readelf -x $segment_name $file_name | grep -iw "$signature" | sed 's/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f2`
+
+  linker_result=$((0x$linker_result + 0))
+  valid_result=$(($valid_result + 0))
+
+  #echo "debug: linked_result=$linker_result"
+  #echo "debug: valid_result=$valid_result"
+
+  if test $linker_result != $valid_result; then
+    echo "check_relocation_abs_global: unexpected result for relocation value in $file_name:"
+    echo "   $valid_result"
+    echo ""
+    echo "Actual value below:"
+    echo "$linker_result"
+    exit 1
+  fi
+}
+
+check_relocation_abs_value()
+{
+  # Arguments: 
+  #  $1 - Linked file name. The file with the same name and extention '.o'
+  #       will be looked for objects.
+  file_name=$1
+  #  $2 - Symbol name. The relocation to test is referenced by this unique
+  #       symbol name.
+  symbol_name=$2
+  #  $3 - Symbol name. The relocation reference within the .text section
+  symbol_reloc_name=$3
+
+  valid_result=`readelf -s $file_name | grep -w $symbol_name | sed 's/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f3`
+  if test -z "$valid_result"; then
+    echo "Did not find symbol $symbol_name in $file_name"
+    exit 1
+  fi
+
+  symbol_reloc_address=`readelf -s $file_name | grep -w $symbol_reloc_name | sed 's/^/:/;s/$/:/;s/[: ][: ]*/:/g' | cut -d: -f3 | sed 's/^0*//g'` 
+  if test -z "$symbol_reloc_address"; then
+    echo "Did not find symbol $symbol_reloc_name in $file_name"
+    exit 1
+  fi
+
+  # Result transformation from string like "76:	90          	.byte	0x90" 
+  # to "76:90:.byte:0x90"
+  linker_result=`../../../binutils/objdump -ds $file_name | sed -e 's/^\s*//g' -e 's/^0*//g' | grep "^$symbol_reloc_address:" | sed -e 's/://' -e 's/\t/:/g' -e 's/\s//g' | cut -d: -f2`
+
+  linker_result=$((0x$linker_result + 0))
+  valid_result=$((0x$valid_result + 0))
+
+  #echo "debug: valid_result=$valid_result"
+  #echo "debug: symbol_reloc_address=$symbol_reloc_address"
+  #echo "debug: linked_result=$linker_result"
+
+  if test $linker_result != $valid_result; then
+    echo "check_relocation_abs_global: unexpected result for relocation value in $file_name:"
+    echo "   $valid_result"
+    echo ""
+    echo "Actual value below:"
+    echo "$linker_result"
+    exit 1
+  fi
+}
+
+check_relocation_abs_overflow()
+{
+  valid_result="relocation overflow in relocation"
+  linker_result=`../../ld-new -Ttext=$2 -Tdata=$3 -o $1 $1.o 2>&1 | grep "$valid_result"`
+
+  if test "$linker_result" = ""; then
+    echo "check_relocation_abs_overflow: unexpected result in $1.o:"
+    echo "   $valid_result"
+    echo ""
+    echo "Actual value below:"
+    echo "$linker_result"
+    exit 1
+  fi
+}
+
+
+# Unit tests:
+
+check_relocation_abs arm_r_abs8_local "LLCT0" ".rodata"
+check_relocation_abs arm_r_abs8_external "LLCT0" ".rodata"
+check_relocation_abs arm_r_abs8_localrw "LLCT0" ".rodata"
+check_relocation_abs arm_r_abs8_localrw "DLCT0" ".data"
+
+check_relocation_abs_overflow arm_r_abs8_localoverflow 0 0x100
+
+check_relocation_abs_global arm_r_abs8_external "GLCT0" ".rodata" "74ff7e5f d45e7d9a 7849e638 47078ed7"
+check_relocation_abs_value arm_r_abs8_external "GLCT0" "GCS0_GL"
+
+exit 0
+
diff -rupN binutils.orig/src/gold/testsuite/Makefile.am binutils.gold/src/gold/testsuite/Makefile.am
--- binutils.orig/src/gold/testsuite/Makefile.am	2009-08-22 12:02:57.000000000 -0700
+++ binutils.gold/src/gold/testsuite/Makefile.am	2009-09-02 13:09:21.000000000 -0700
@@ -26,6 +26,12 @@ TEST_STRIP = $(top_builddir)/../binutils
 TEST_AR = $(top_builddir)/../binutils/ar
 TEST_NM = $(top_builddir)/../binutils/nm-new
 
+SUBDIRS =
+
+if TARGET_ARM
+SUBDIRS += arm
+endif
+
 if PLUGINS
 LIBDL = -ldl
 endif

Re: [PATCH, Take 2] Gold: Added R_ARM_ABS8 relocation unit test

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 804 bytes --]

Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> Please find attached the updated patch for the R_ARM_ABS8 relocation unit test.

I appreciate all the hard work you've done here, but this doesn't seem
like the right approach to me.

For standard relocations like 8-bit absolute we can write a reasonably
standard test.  We don't need to write a target dependent test.

I think we can make these tests more robust by using special section
names.

I don't understand your check_relocation_abs test.  As far as I can
tell, what it tests is that the final value of a symbol is the value
in the object file plus the offset of the section in the executable.
It doesn't seem to test the relocation value.

What I'm thinking is something along the lines of the appended,
although that is incomplete.

Ian



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: reloc test --]
[-- Type: text/x-diff, Size: 6055 bytes --]

Index: configure.ac
===================================================================
RCS file: /cvs/src/src/gold/configure.ac,v
retrieving revision 1.46
diff -u -r1.46 configure.ac
--- configure.ac	23 Jun 2009 18:10:41 -0000	1.46
+++ configure.ac	27 Sep 2009 07:34:02 -0000
@@ -180,6 +180,11 @@
 fi
 AC_SUBST(TARGETOBJS)
 
+DEFAULT_ELF_MACHINE="$default_machine"
+AC_SUBST(DEFAULT_ELF_MACHINE)
+DEFAULT_ELF_SIZE="$default_size"
+AC_SUBST(DEFAULT_ELF_SIZE)
+
 AC_DEFINE_UNQUOTED(GOLD_DEFAULT_MACHINE, $default_machine,
 		   [Default machine code])
 AC_DEFINE_UNQUOTED(GOLD_DEFAULT_SIZE, $default_size,
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.103
diff -u -r1.103 Makefile.am
--- testsuite/Makefile.am	18 Sep 2009 20:02:21 -0000	1.103
+++ testsuite/Makefile.am	27 Sep 2009 07:34:03 -0000
@@ -25,6 +25,7 @@
 TEST_STRIP = $(top_builddir)/../binutils/strip-new
 TEST_AR = $(top_builddir)/../binutils/ar
 TEST_NM = $(top_builddir)/../binutils/nm-new
+TEST_AS = $(top_builddir)/../gas/as-new
 
 if PLUGINS
 LIBDL = -ldl
@@ -76,6 +77,20 @@
 binary_unittest_SOURCES = binary_unittest.cc
 
 
+# Test simple relocations
+check_SCRIPTS += relocs_test.sh
+check_DATA += relocs-syms.stdout relocs-secs.stdout relocs-size.stdout
+relocs.o: relocs.s
+	$(TEST_AS) --defsym CPU=$(DEFAULT_ELF_MACHINE) --defsym SIZE=$(DEFAULT_ELF_SIZE) -o $@ $<
+relocs: relocs.o gcctestdir/ld
+	gcctestdir/ld -N -o $@ --defsym global_sym=0x10 -Ttext 0 -e entry relocs.o
+relocs-syms.stdout: relocs
+	$(TEST_NM) relocs > $@
+relocs-secs.stdout: relocs
+	$(TEST_OBJDUMP) -s relocs > $@
+relocs-size.stdout: Makefile
+	echo $(DEFAULT_ELF_SIZE) > $@
+
 # ---------------------------------------------------------------------
 # These tests test the output of gold (end-to-end tests).  In
 # particular, they make sure that gold can link "difficult" object
@@ -670,7 +685,6 @@
 
 endif
 
-
 # Test --detect-odr-violations
 check_SCRIPTS += debug_msg.sh
 
Index: testsuite/relocs.s
===================================================================
RCS file: testsuite/relocs.s
diff -N testsuite/relocs.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/relocs.s	27 Sep 2009 07:34:03 -0000
@@ -0,0 +1,69 @@
+	.text
+entry:
+	.globl	entry
+	.8byte	0
+
+	.data
+	.8byte	0
+local_sym:
+	.8byte	0
+
+.macro	section	name
+.ifeqs	"CPU", "EM_ARM"
+	.section	\name,"aw",%progbits
+.else
+	.section	\name,"aw",@progbits
+.endif
+.endm
+
+	section	test_1_l
+	.byte	local_sym
+
+	section	test_1_g
+	.byte	global_sym
+
+	section	test_1_lp
+	.byte	local_sym - .
+
+	section	test_1_gp
+	.byte	global_sym - .
+
+	section	test_2_l
+	.2byte	local_sym
+
+	section	test_2_g
+	.2byte	global_sym
+
+	section	test_2_lp
+	.2byte	local_sym - .
+
+	section	test_2_gp
+	.2byte	global_sym - .
+
+	section	test_4_l
+	.4byte	local_sym
+
+	section	test_4_g
+	.4byte	global_sym
+
+	section	test_4_lp
+	.4byte	local_sym - .
+
+	section	test_4_gp
+	.4byte	global_sym - .
+
+.if	SIZE != 32
+
+	section	test_8_l
+	.8byte	local_sym
+
+	section	test_8_g
+	.8byte	global_sym
+
+	section	test_8_lp
+	.8byte	local_sym - .
+
+	section	test_8_gp
+	.8byte	global_sym - .
+
+.endif
Index: testsuite/relocs_test.sh
===================================================================
RCS file: testsuite/relocs_test.sh
diff -N testsuite/relocs_test.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/relocs_test.sh	27 Sep 2009 07:34:03 -0000
@@ -0,0 +1,76 @@
+#!/bin/sh
+
+# Copyright 2009 Free Software Foundation, Inc.
+# Written by Ian Lance Taylor <iant@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# Test that basic relocations produce the right value.
+
+# Get the value of the symbol "local_sym".
+locval=`grep local_sym relocs-syms.stdout | sed -e 's/ .*//' -e 's/^0*//'`
+if ! expr "$locval" : '[0-9a-f][0-9a-f]' >/dev/null 2>&1; then
+  echo 1>&2 "local_sym value is ${locval}; expected two hex digits"
+  exit 1
+fi
+
+# Get the value of the symbol "global_sym", which should be 0x10.
+globval=`grep global_sym relocs-syms.stdout | sed -e 's/ .*//' -e 's/^0*//'`
+if test "$globval" != "10"; then
+  echo 1>&2 "global_sym value is ${globval}; expected two hex digits"
+  exit 1
+fi
+
+# Set the variable contents to the contents of section $1.
+get_contents()
+{
+  contents=`sed -ne "
+    /section $1:/ {
+      n
+      s/^ *[0-9a-fA-F]* //
+      s/  .*$//
+      s/ //g
+      p
+    }
+  " < relocs-secs.stdout`
+}
+
+# Verify that the contents of section $1 is either $2 or $3.
+check_contents()
+{
+  get_contents $1
+  if test "$contents" != "$2" -a "$contents" != "$3"; then
+    echo 1>&2 "Section $1 is '$contents'; expected '$2' or '$3'"
+    exit 1
+  fi
+}
+
+check_contents "test_1_l" "${locval}" "${locval}"
+check_contents "test_1_g" "${globval}" "${globval}"
+check_contents "test_2_l" "00${locval}" "${locval}00"
+check_contents "test_2_g" "00${globval}" "${globval}00"
+check_contents "test_4_l" "000000${locval}" "${locval}000000"
+check_contents "test_4_g" "000000${globval}" "${globval}000000"
+
+size=`cat relocs-size.stdout`
+if test "$size" != "32"; then
+  check_contents "test_8_l" "00000000000000${locval}" "${locval}00000000000000"
+  check_contents "test_8_g" "00000000000000${globval}" "${globval}00000000000000"
+fi
+
+exit 0

Re: [PATCH, Take 2] Gold: Added R_ARM_ABS8 relocation unit test

[Viktor Kutuzov]

Hello Ian,

Thank you for the feedback.

I wasn't able to build that assembler code for ARM with the TOT binutils.
Errors about BFD_RELOC_8_PCREL representation in that object file format.

However, I think I see your point and it looks like it is simpler than my approach.
I'll add the R_ARM_ABS16 relocation and will extend the unit tests for it.
I would also like to have overflows checked as well. So, if you would not mind I'll add this cases to the script.

How does the R_ARM_GOT_PREL look? I didn't find a nice generic way to test it as target independent.
Is it Ok to have it tested  as platform dependent?

>  I don't understand your check_relocation_abs test.

It checked the relocation to be in comply with the spec. The script actually calculates the relocation value using symbol address, 
addend and such from the object file and compares the result with the one produced by the gold.

Best regards,
Viktor

----- Original Message ----- 
From: "Ian Lance Taylor" <iant@google.com>
To: "Viktor Kutuzov" <vkutuzov@accesssoftek.com>
Cc: <binutils@sourceware.org>
Sent: Sunday, September 27, 2009 12:34 AM
Subject: Re: [PATCH, Take 2] Gold: Added R_ARM_ABS8 relocation unit test


> Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:
>
>> Please find attached the updated patch for the R_ARM_ABS8 relocation unit test.
>
> I appreciate all the hard work you've done here, but this doesn't seem
> like the right approach to me.
>
> For standard relocations like 8-bit absolute we can write a reasonably
> standard test.  We don't need to write a target dependent test.
>
> I think we can make these tests more robust by using special section
> names.
>
> I don't understand your check_relocation_abs test.  As far as I can
> tell, what it tests is that the final value of a symbol is the value
> in the object file plus the offset of the section in the executable.
> It doesn't seem to test the relocation value.
>
> What I'm thinking is something along the lines of the appended,
> although that is incomplete.
>
> Ian
>
>
> 

Re: [PATCH, Take 2] Gold: Added R_ARM_ABS8 relocation unit test

[Ian Lance Taylor]

Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> How does the R_ARM_GOT_PREL look? I didn't find a nice generic way to test it as target independent.
> Is it Ok to have it tested  as platform dependent?

Yes.  I didn't get that far, but I think that it would be nice to have
something table driven here.  It would be something like the (target
dependent) code to generate the reloc and then what the section
contents should look like.  For the section contents it would have to
be possible to write a little expression of some sort.  This isn't
really fleshed out but I think it ought to be possible.

Ian