[llvm-commits] Fix bug 12769 MCAssembler crashes with -mthumb -mcpu=cortex-a9

[Jim Grosbach]

Looks good to me. I'm fine with this going in, assuming Tim agrees. Thanks again for working on this!

-Jim

On May 10, 2012, at 1:47 PM, Weiming Zhao <weimingz at codeaurora.org> wrote:

> Hi Jim,
> 
> Thanks for the info.
> I updated the test case, which checks both the encoding and opcode name as
> well as relocation.
> 
> Please check it.
> 
> Thanks,
> Weiming
> 
> -----Original Message-----
> From: Jim Grosbach [mailto:grosbach at apple.com] 
> Sent: Thursday, May 10, 2012 12:43 PM
> To: Weiming Zhao
> Cc: 'Tim Northover'; llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] Fix bug 12769 MCAssembler crashes with -mthumb
> -mcpu=cortex-a9
> 
> Hi Weiming,
> 
> Creating a temp file for the output is no problem; however, naming it
> explicitly like that won't work because it'll end up in the source directory
> and lit won't know how to clean it up. You can use %t instead to get a
> temporary file name. test/MC/MachO/ARM/nop-thumb2-padding.s has a good
> example in its RUN line:
> @ RUN: llvm-mc -triple armv7-apple-darwin %s -filetype=obj -o %t.obj
> 
> @ RUN: macho-dump --dump-section-data < %t.obj > %t.dump
> 
> @ RUN: FileCheck %s < %t.dump 
> 
> I suggest checking the actual encoded bits of the instruction and the
> relocation data  from the object file as well, not just the opcode. You can
> use separate RUN lines w/ different check prefixes if necessary to have
> llvm-objdump spit out the different pieces you need to look at.
> 
> -Jim
> 
> On May 10, 2012, at 12:19 PM, Weiming Zhao <weimingz at codeaurora.org> wrote:
> 
>> Hi Jim,
>> 
>> Attached is the test case (test/MC/ARM/thumb2-uncond-branch.s)
>> Since llvm-objdump can't be piped, I have to let llvm-mc to generate a 
>> temp file (a.out).
>> And if I just run llvm-mc -show-encoding, it won't cover the polemic code.
>> 
>> If you think this test is OK, I'll update the bugzilla as well.
>> 
>> Thanks,
>> Weiming
>> 
>> 
>> -----Original Message-----
>> From: Jim Grosbach [mailto:grosbach at apple.com]
>> Sent: Wednesday, May 09, 2012 7:25 PM
>> To: Weiming Zhao
>> Cc: 'Tim Northover'; llvm-commits at cs.uiuc.edu
>> Subject: Re: [llvm-commits] Fix bug 12769 MCAssembler crashes with 
>> -mthumb
>> -mcpu=cortex-a9
>> 
>> Can you add a test case? Other than that, this looks good to me.
>> 
>> -Jim
>> 
>> On May 9, 2012, at 6:21 PM, Weiming Zhao wrote:
>> 
>>> Hi,
>>> 
>>> Attached is updated patch based on Tim's suggestion.
>>> I also updated the bugzilla as well.
>>> I run the test suite and unit test. No new fails.
>>> 
>>> Thanks,
>>> Weiming
>>> 
>>> -----Original Message-----
>>> From: Tim Northover [mailto:t.p.northover at gmail.com]
>>> Sent: Wednesday, May 09, 2012 11:55 AM
>>> To: Weiming Zhao
>>> Cc: llvm-commits at cs.uiuc.edu
>>> Subject: Re: [llvm-commits] Fix bug 12769 MCAssembler crashes with 
>>> -mthumb
>>> -mcpu=cortex-a9
>>> 
>>> Hi Weiming,
>>> 
>>> Apologies if this gets through twice. I tried earlier but the list 
>>> seems to be eating e-mails from that address for some reason.
>>> 
>>>> This patch fixes the issue. And it also passes the test suite. For 
>>>> the unit tests, it doesn't create new fails. (details are in
>>>> tests.tar)
>>> 
>>> In this case, I believe we actually need an R_ARM_THM_JUMP24 which 
>>> will put the immediate into the correct bits for a Thumb 
>>> unconditional branch. If you look at the ARM ELF ABI and the 
>>> architecture reference manual you'll see that if a linker encountered 
>>> R_ARM_THM_CALL it would put the immediate into bits 0-11 and 16-26, 
>>> stamping all over your lovingly crafted B.W instruction.
>>> 
>>> We could also do with some tests for this specific issue: a examples 
>>> for a similar case can be found in test/MC/ARM/elf-reloc-condcall.s.
>>> 
>>> Cheers.
>>> 
>>> Tim.
>>> <fix-12769_2.patch>_______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> <thumb2-uncond-branch.s>
> 
> <thumb2-uncond-branch.s>