abi violation for R_ARM_THM_ALU_PREL_11_0 in ld

Bug #1421389 reported by Pulin Shah
16
This bug affects 3 people
Affects Status Importance Assigned to Milestone
GNU Arm Embedded Toolchain
Triaged
Undecided
Unassigned

Bug Description

The value for relocation symbol of type R_ARM_THM_ALU_PREL_11_0 isn't being generated correctly according to the AEABI spec, which says it should be calculated with the thumb bit for STT_FUNC Thumb symbols ( ((S + A) | T) – Pa ). When looking at the source for this relocation type in the ld source code, I found that ld isn't checking for whether the symbol is a function and setting the thumb bit accordingly.

This is based on the 4.9.3 release package, ld version 2.24.0.20141128.

Revision history for this message
Thomas Preud'homme (thomas-preudhomme) wrote :

Hi Pulin,

From my understanding the | T is implicit in the code as the symbol will already include the bit set. Indeed, the symbol address is extracted from the symbol table where the bit 0 is already set for thumb function.

Do you have a testcase you could share with us so that we can reproduce the issue?

Best regards.

Changed in gcc-arm-embedded:
status: New → Incomplete
Revision history for this message
Pulin Shah (pulin) wrote :

I don't currently have a testcase, but based on my (admittedly limited) inspection of the linker source, it doesn't seem to be the case that the entry in the symbol table contains the thumb bit.

A number of relocation types handled in the function elf32_arm_final_link_relocate() contain logic such as:

if (branch_type == ST_BRANCH_TO_THUMB)
     value |= 1;

I don't see why this would be necessary if the symbol table already contained the thumb bit in the correct state.

Revision history for this message
Terry Guo (terry.guo) wrote :

Without a real case, I can just guess. I remember I handled a case which branches to an address defined via linker script symbol (an absolute immediate address essentially). The code at that address could be in arm mode or in thumb mode, the linker has no way to figure this out. But one thing the linker can do is to check whether target is thumb-only target, if yes, then we do need "value |= 1" to make sure we stay in thumb mode. In this case, it is possible that the linker script symbol hasn't thumb bit set.

Revision history for this message
Terry Guo (terry.guo) wrote :

In this case, the linker script symbol doesn't behave like the normal symbol. Perhaps they don't even have entries in symbol table.

Revision history for this message
Pulin Shah (pulin) wrote :

Hi Terry,

Thanks for confirming. In my particular case, I am running on a thumb only target (Cortex-M3), so my case would certainly be improved by adding logic to set the thumb bit to function addresses.

Like I said earlier, I don't have a testcase I can provide on this issue, but I can say that there are no linker symbols involved here that could understandably cause an issue like this. In my case, the object files are providing the symbols in such a way that the linker *should* be able to determine how to handle this correctly.

I can provide readelf/objdump output if needed. Let me know what commands you'd like output from and I can provide it tomorrow. Also, If you'd like me to test out a patch of the proposed logic change, I'd be happy to do so.

Revision history for this message
Pulin Shah (pulin) wrote :

I've successfully patched ld to produce the correct output for the scenario described in the original bug report. Details and "patch" follow.

To illustrate the issue, here is some output from readelf of the object file containing the problematic symbol:

ABI header:
Attribute Section: aeabi
File Attributes
  Tag_conformance: "2.06"
  Tag_CPU_name: "Cortex-M3"
  Tag_CPU_arch: v7
  Tag_CPU_arch_profile: Microcontroller
  Tag_THUMB_ISA_use: Thumb-2
  Tag_PCS_config: Bare platform
  Tag_ABI_PCS_GOT_use: direct
  Tag_ABI_PCS_wchar_t: 2
  Tag_ABI_align_needed: 8-byte
  Tag_ABI_align_preserved: 8-byte, except leaf SP
  Tag_ABI_enum_size: forced to int
  Tag_CPU_unaligned_access: v6
  Tag_DIV_use: Not allowed

Symbol table entry:
   Num: Value Size Type Bind Vis Ndx Name
    61: 000000a9 74 FUNC LOCAL DEFAULT 12 ThumbFunc1

Relocation entry:
 Offset Info Type Sym. Value Symbol's Name
0000008a 00003d35 R_ARM_THM_ALU_PREL_11_ 000000a9 ThumbFunc1

As you can see from the symbol table entry, the symbol "ThumbFunc1" is listed as a "FUNC" type with the LSB set for its value. This should be enough to indicate to the linker that the symbol is a thumb function.

According to the ABI,
 - a relocation entry of type R_ARM_THM_ALU_PREL_11_0 should calculate the relocation value via "((S + A) | T) – Pa"
 - "T is 1 if the target symbol S has type STT_FUNC and the symbol addresses a Thumb instruction"
 - ld is clearly not checking the type of the symbol for whether it is a function and additionally if it is a Thumb function

In this case, the symbol is provided by an object with appropriate information for the linker to determine it is a Thumb function and should be setting the LSB accordingly.

In order to correct this behavior, I applied the following patch:

diff a/binutils/bfd/elf32-arm.c b/binutils/bfd/elf32-arm.c
8663a8664,8667
> if (st_type == STT_FUNC &&
> (using_thumb_only(globals) || branch_type == ST_BRANCH_TO_THUMB))
> value |= 1;
>

Adding the check above, the linker now sets the LSB correctly for relocation entries of this type.

Any feedback you have would be appreciated.

Revision history for this message
Launchpad Janitor (janitor) wrote :

[Expired for GCC ARM Embedded because there has been no activity for 60 days.]

Changed in gcc-arm-embedded:
status: Incomplete → Expired
Revision history for this message
Thomas Preud'homme (thomas-preudhomme) wrote :

Hi Pulin,

Sorry for the slow response. You patch looks right indeed and I therefore suggest you to submit it for review to the binutils community. I have no doubt that this should be accepted. Have you sign a FSF copyright assignment form? If not, you might need to (not it's needed because the patch is really small, you'll see what they'll tell you).

Best regards,

Thomas

Changed in gcc-arm-embedded:
status: Expired → Triaged
Revision history for this message
rew (r-e-wolff) wrote :

I'm being "affected" by this bug. I'm running an ubuntu-provided compiler tagged with "may 2015", but it still has this bug.

My problem is that the patch does not contain enough context for me to be sure where to add the three line fix.

I am tempted to add it near line 8823,
```
       insn = (insn & 0xfb0f8f00) | (value & 0xff)
             | ((value & 0x700) << 4)
             | ((value & 0x800) << 15);
        if (relocation < 0)
          insn |= 0xa00000;

       // PATCH GOES HERE

        bfd_put_16 (input_bfd, insn >> 16, hit_data);
        bfd_put_16 (input_bfd, insn & 0xffff, hit_data + 2);
```all this in the "case: R_ARM_THM_ALU_PREL_11_0:", which starts on line 8790 (in my source... I should say: 33 lines higher up).

Correct?

Revision history for this message
rew (r-e-wolff) wrote :

Update1: git markup seems to not-work here. Duh!

Update2: after struggling to get my patch into the binary, (dpkg-buildpackage apparently unpacks the source again), it now works. As in: the objdump of the created executable looks good to me. I don't have access to the hardware at this point in time.

Revision history for this message
Francisco Giraldez (fgiraldez) wrote :

Hi,

I also have the same problem. I am using AC6 for STM32, which includes GNU ARM GCC 5.4-2016q2. Since the GNU ARM GCC source is not provided with Ac6, I cannot directly find elf23-arm.c

Could I clone the source, patch it, compile it and just feed that into AC6? If so, then I would like to know where/how exactly is the patch to be applied. Thanks a lot in advance.

Hopefully this bug will be solved in a future version of GCC for ARM. I really need this to work since it is for a company project.

Best,

Francisco

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.