Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(22)

Issue 6814002: SH4 support

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years ago by christian.bruel
Modified:
1 year, 11 months ago
CC:
google-breakpad-dev_googlegroups.com
Base URL:
http://google-breakpad.googlecode.com/svn/trunk/
Visibility:
Public.

Description

SH4 support

Patch Set 1 #

Patch Set 2 : SH4 don't need user_fpu_struct in struct ThreadInfo, part of user regs #

Total comments: 14

Patch Set 3 : with sh4_unittest #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M Makefile.am View 7 chunks +8 lines, -0 lines 1 comment Download
M Makefile.in View 1 2 32 chunks +167 lines, -0 lines 0 comments Download
M src/client/linux/dump_writer_common/raw_context_cpu.h View 1 chunk +2 lines, -0 lines 0 comments Download
M src/client/linux/dump_writer_common/thread_info.cc View 1 2 1 chunk +27 lines, -0 lines 0 comments Download
M src/client/linux/dump_writer_common/thread_info.h View 1 1 chunk +2 lines, -1 line 0 comments Download
M src/client/linux/dump_writer_common/ucontext_reader.cc View 1 2 1 chunk +32 lines, -0 lines 0 comments Download
M src/client/linux/handler/exception_handler.cc View 1 2 3 chunks +6 lines, -3 lines 0 comments Download
M src/client/linux/handler/exception_handler.h View 1 2 2 chunks +8 lines, -4 lines 1 comment Download
M src/client/linux/microdump_writer/microdump_writer.cc View 1 2 4 chunks +5 lines, -3 lines 0 comments Download
M src/client/linux/minidump_writer/linux_core_dumper.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M src/client/linux/minidump_writer/linux_dumper.h View 1 chunk +1 line, -1 line 0 comments Download
M src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M src/client/linux/minidump_writer/linux_ptrace_dumper.cc View 1 2 chunks +10 lines, -0 lines 0 comments Download
M src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc View 1 2 2 chunks +4 lines, -1 line 0 comments Download
M src/client/linux/minidump_writer/minidump_writer.cc View 1 2 5 chunks +6 lines, -4 lines 0 comments Download
M src/client/linux/minidump_writer/minidump_writer.h View 1 chunk +1 line, -1 line 1 comment Download
M src/client/linux/minidump_writer/minidump_writer_unittest.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M src/common/dwarf_cfi_to_module.cc View 1 2 1 chunk +19 lines, -0 lines 0 comments Download
M src/common/dwarf_cfi_to_module.h View 1 chunk +3 lines, -0 lines 0 comments Download
M src/common/linux/dump_symbols.cc View 2 chunks +4 lines, -0 lines 0 comments Download
A src/google_breakpad/common/minidump_cpu_sh4.h View 1 2 1 chunk +119 lines, -0 lines 0 comments Download
M src/google_breakpad/common/minidump_format.h View 1 2 3 chunks +1 line, -3 lines 0 comments Download
M src/google_breakpad/processor/dump_context.h View 3 chunks +3 lines, -0 lines 0 comments Download
M src/google_breakpad/processor/stack_frame_cpu.h View 1 chunk +52 lines, -0 lines 0 comments Download
M src/processor/dump_context.cc View 5 chunks +55 lines, -0 lines 0 comments Download
M src/processor/minidump.cc View 1 2 3 chunks +67 lines, -0 lines 0 comments Download
M src/processor/minidump_processor.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M src/processor/stackwalk_common.cc View 1 chunk +42 lines, -0 lines 0 comments Download
M src/processor/stackwalker.cc View 2 chunks +7 lines, -1 line 0 comments Download
M src/processor/stackwalker_selftest.cc View 1 chunk +8 lines, -0 lines 0 comments Download
A src/processor/stackwalker_sh4.cc View 1 2 1 chunk +246 lines, -0 lines 0 comments Download
A src/processor/stackwalker_sh4.h View 1 2 1 chunk +90 lines, -0 lines 0 comments Download
A src/processor/stackwalker_sh4_unittest.cc View 1 2 1 chunk +636 lines, -0 lines 2 comments Download
M src/processor/synth_minidump.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/tools/linux/md2core/minidump-2-core.cc View 1 5 chunks +36 lines, -0 lines 1 comment Download

Messages

Total messages: 9
Ted Mielczarek
For as invasive as new-architecture patches are they're surprisingly not that hard to read. This ...
2 years ago #1
Ben Chan
https://breakpad.appspot.com/6814002/diff/20001/src/google_breakpad/common/minidump_cpu_sh4.h File src/google_breakpad/common/minidump_cpu_sh4.h (right): https://breakpad.appspot.com/6814002/diff/20001/src/google_breakpad/common/minidump_cpu_sh4.h#newcode1 src/google_breakpad/common/minidump_cpu_sh4.h:1: /* Copyright (c) 2015, Google Inc. this is a ...
2 years ago #2
christian.bruel
Hi, On Thursday, April 2, 2015 at 2:24:35 PM UTC+2, Ted Mielczarek wrote: > > ...
2 years ago #3
christian.bruel
with sh4_unittest
2 years ago #4
christian.bruel
Hi, I've posted a new patch set version with the sh4 unittests thanks, Le jeudi ...
2 years ago #5
christian.bruel
with sh4_unittest
2 years ago #6
christian.bruel
could you please review this version. It should fix the aforementioned comments. Mostly: - the ...
2 years ago #7
Ted Mielczarek
Just one thing that needs to be fixed before landing, you missed adding stackwalker_sh4_unittest to ...
1 year, 11 months ago #8
christian.bruel
1 year, 11 months ago #9
Hi Ted,

On 05/04/2015 01:39 PM, ted.mielczarek@gmail.com wrote:
> Just one thing that needs to be fixed before landing, you missed adding
> stackwalker_sh4_unittest to Makefile.am.

Interestingly, I saw PASS: src/processor/stackwalker_sh4_unittest
with make check, I wonder what consequence had this missing dependency

do I need to resend the patch or is it just "as obvious" ?

> 
> Have you run the unit tests on a SH4 device?
> 
> 

yes I did, they were ran on a SH4-STLinux distro:

machine		: hdk7108
processor	: 0
cpu family	: sh4
cpu variant	: st40-300
cpu type	: STx7108
cut		: 2.x
cpu flags	: fpu icbi synco fpchg
cache type	: split (harvard)
icache size	: 32KiB (2-way)
dcache size	: 32KiB (2-way)
address sizes	: 32 bits physical
bogomips	: 500.00

I also ran the build+tests on a x86_64-linux-gnu host.

thanks

Christian

> https://breakpad.appspot.com/6814002/diff/250002/Makefile.am
> File Makefile.am (right):
> 
> https://breakpad.appspot.com/6814002/diff/250002/Makefile.am#newcode354
> Makefile.am:354: src/processor/stackwalker_x86_unittest \
> You need to add stackwalker_sh4_unittest to this list as well.
> 
>
https://breakpad.appspot.com/6814002/diff/250002/src/client/linux/handler/exc...
> File src/client/linux/handler/exception_handler.h (right):
> 
>
https://breakpad.appspot.com/6814002/diff/250002/src/client/linux/handler/exc...
> src/client/linux/handler/exception_handler.h:51: #define
> CONTEXT_FLOAT_STATE
> Thanks, this is nicer.
> 
>
https://breakpad.appspot.com/6814002/diff/250002/src/client/linux/minidump_wr...
> File src/client/linux/minidump_writer/minidump_writer.h (right):
> 
>
https://breakpad.appspot.com/6814002/diff/250002/src/client/linux/minidump_wr...
> src/client/linux/minidump_writer/minidump_writer.h:50: #elif
> !defined(__ARM_EABI__) && !defined(__mips__) && !defined(__SH4__)
> Oh hum, this gets included before the #define you added in
> exception_handler.h? You could move the #define here, but I'm not that
> hung up on it.
> 
>
https://breakpad.appspot.com/6814002/diff/250002/src/processor/stackwalker_sh...
> File src/processor/stackwalker_sh4_unittest.cc (right):
> 
>
https://breakpad.appspot.com/6814002/diff/250002/src/processor/stackwalker_sh...
> src/processor/stackwalker_sh4_unittest.cc:2: // All rights reserved.
> Thanks for adding tests. I know these are a very copy-paste affair, I
> wish it was easier to write generic tests for this stuff.
> 
>
https://breakpad.appspot.com/6814002/diff/250002/src/processor/stackwalker_sh...
> src/processor/stackwalker_sh4_unittest.cc:70: class
> StackwalkerSH4Fixture {
> We should probably move this out into a common header for all the tests
> to subclass at some point.
> 
>
https://breakpad.appspot.com/6814002/diff/250002/src/tools/linux/md2core/mini...
> File src/tools/linux/md2core/minidump-2-core.cc (right):
> 
>
https://breakpad.appspot.com/6814002/diff/250002/src/tools/linux/md2core/mini...
> src/tools/linux/md2core/minidump-2-core.cc:78: #define ELF_ARCH  EM_SH
> These changes are going to conflict with
> https://breakpad.appspot.com/10734002/ depending on who lands first.
> 
> https://breakpad.appspot.com/6814002/
> 
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted