This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]