diff mbox

[2/5] ARM: extract_arm_insn function need to read instrs correctly in be8 case

Message ID 1413853021-4393-3-git-send-email-victor.kamensky@linaro.org
State New
Headers show

Commit Message

vkamensky Oct. 21, 2014, 12:56 a.m. UTC 
extract_arm_insn function needs to read instructions in
gdbarch_byte_order_for_code byte order, because in case armv7b,
even data is big endian, instructions are still little endian.
Currently function uses gdbarch_byte_order which would be
big endian in armv7b case.

Because of this issue pretty much all gdb.reverse/ tests are
failing with 'Process record does not support instruction' message.

Fix is to change gdbarch_byte_order to gdbarch_byte_order_for_code,
when passed to extract_unsigned_integer that reads instruction.
---
 gdb/ChangeLog  | 5 +++++
 gdb/arm-tdep.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Yao Qi Oct. 21, 2014, 7:54 a.m. UTC | #1 
Victor Kamensky <victor.kamensky@linaro.org> writes:

> Fix is to change gdbarch_byte_order to gdbarch_byte_order_for_code,
> when passed to extract_unsigned_integer that reads instruction.
> ---
>  gdb/ChangeLog  | 5 +++++
>  gdb/arm-tdep.c | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index c967a93..2aef5dc 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2014-10-13  Victor Kamensky  <victor.kamensky@linaro.org>
> +
> +	* arm-tdep.c (extract_arm_insn): use dbarch_byte_order_for_code
> +	to read arm instruction.
> +
>  2014-09-30  Don Breazeal  <donb@codesourcery.com>
>  
>  	* inf-ptrace.c (inf_ptrace_follow_fork): Remove target-independent

Looks good to me.

We don't include the ChangeLog changes in the patch, because that will
cause conflicts when applying your patch locally in the review.
Instead, we include ChangeLog entries in the commit messages, see
https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_formatted_commit_messages
Yao Qi Oct. 21, 2014, 8 a.m. UTC | #2 
Yao Qi <yao@codesourcery.com> writes:

>> +2014-10-13  Victor Kamensky  <victor.kamensky@linaro.org>
>> +
>> +	* arm-tdep.c (extract_arm_insn): use dbarch_byte_order_for_code

s/use dbarch/Use gdbarch/

>> +	to read arm instruction.
vkamensky Oct. 21, 2014, 2:44 p.m. UTC | #3 
On 21 October 2014 00:54, Yao Qi <yao@codesourcery.com> wrote:
> Victor Kamensky <victor.kamensky@linaro.org> writes:
>
>> Fix is to change gdbarch_byte_order to gdbarch_byte_order_for_code,
>> when passed to extract_unsigned_integer that reads instruction.
>> ---
>>  gdb/ChangeLog  | 5 +++++
>>  gdb/arm-tdep.c | 2 +-
>>  2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index c967a93..2aef5dc 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,8 @@
>> +2014-10-13  Victor Kamensky  <victor.kamensky@linaro.org>
>> +
>> +     * arm-tdep.c (extract_arm_insn): use dbarch_byte_order_for_code
>> +     to read arm instruction.
>> +
>>  2014-09-30  Don Breazeal  <donb@codesourcery.com>
>>
>>       * inf-ptrace.c (inf_ptrace_follow_fork): Remove target-independent
>
> Looks good to me.
>
> We don't include the ChangeLog changes in the patch, because that will
> cause conflicts when applying your patch locally in the review.
> Instead, we include ChangeLog entries in the commit messages, see
> https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_formatted_commit_messages

Thanks! It is good pointer. I have not seen it before.
Maybe gdb/CONTRIBUTE could mention this wiki page.

I will move all proposed commit ChangeLogs as per wiki,
will incorporate review comments and repost updated series.

Thanks,
Victor

> --
> Yao (齐尧)
Pedro Alves Oct. 24, 2014, 12:20 p.m. UTC | #4 
Hi Victor,

On 10/21/2014 03:44 PM, Victor Kamensky wrote:
>> >
>> > We don't include the ChangeLog changes in the patch, because that will
>> > cause conflicts when applying your patch locally in the review.
>> > Instead, we include ChangeLog entries in the commit messages, see
>> > https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_formatted_commit_messages
> Thanks! It is good pointer. I have not seen it before.
> Maybe gdb/CONTRIBUTE could mention this wiki page.

OOC, how did you find gdb/CONTRIBUTE?  Was that through
https://sourceware.org/gdb/contribute/ or some other means?

IMO we should just move/merge the gross of the contents in the
file to the web/wiki, and either delete the file, or leave just
the preamble.

If it stays, I think it should link to:

  http://www.gnu.org/software/gdb/contribute/

as that's more stable than the wiki, and then that page should
link to relevant wiki pages.

Thanks,
Pedro Alves
vkamensky Oct. 24, 2014, 5:36 p.m. UTC | #5 
On 24 October 2014 05:20, Pedro Alves <palves@redhat.com> wrote:
> Hi Victor,
>
> On 10/21/2014 03:44 PM, Victor Kamensky wrote:
>>> >
>>> > We don't include the ChangeLog changes in the patch, because that will
>>> > cause conflicts when applying your patch locally in the review.
>>> > Instead, we include ChangeLog entries in the commit messages, see
>>> > https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_formatted_commit_messages
>> Thanks! It is good pointer. I have not seen it before.
>> Maybe gdb/CONTRIBUTE could mention this wiki page.
>
> OOC, how did you find gdb/CONTRIBUTE?  Was that through
> https://sourceware.org/gdb/contribute/ or some other means?

Yes, through above link. I got to it from
https://sourceware.org/gdb/
following 'contributing' (3rd) tab on top under title.

Thanks,
Victor

> IMO we should just move/merge the gross of the contents in the
> file to the web/wiki, and either delete the file, or leave just
> the preamble.
>
> If it stays, I think it should link to:
>
>   http://www.gnu.org/software/gdb/contribute/
>
> as that's more stable than the wiki, and then that page should
> link to relevant wiki pages.
>
> Thanks,
> Pedro Alves
>
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c967a93..2aef5dc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2014-10-13  Victor Kamensky  <victor.kamensky@linaro.org>
+
+	* arm-tdep.c (extract_arm_insn): use dbarch_byte_order_for_code
+	to read arm instruction.
+
 2014-09-30  Don Breazeal  <donb@codesourcery.com>
 
 	* inf-ptrace.c (inf_ptrace_follow_fork): Remove target-independent
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index e2559ec..e7a1ec5 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -13860,7 +13860,7 @@  extract_arm_insn (insn_decode_record *insn_record, uint32_t insn_size)
     return 1;
   insn_record->arm_insn = (uint32_t) extract_unsigned_integer (&buf[0],
                            insn_size, 
-                           gdbarch_byte_order (insn_record->gdbarch));
+			   gdbarch_byte_order_for_code (insn_record->gdbarch));
   return 0;
 }