add debugging support for android on x86 phone

add remote debugging support for x86 phone

Committed: http://code.google.com/p/skia/source/detail?r=10511

[yunchao, 2013-07-22 09:29:14]

add debugging support for android on x86 phone

[djsollen, 2013-07-22 13:18:18]

https://codereview.chromium.org/19920003/diff/1/platform_tools/android/bin/an...
File platform_tools/android/bin/android_gdb_exe (right):

https://codereview.chromium.org/19920003/diff/1/platform_tools/android/bin/an...
platform_tools/android/bin/android_gdb_exe:18: 
It would be preferable to do something like...

while (( "$#" )); do

  if [[ $(echo "$1" | grep "^-d$") != "" ]];
  then 
    deviceID="$2"
    shift
  else
    gdbVars=("${gdbVars[@]}" "$1")
  fi

shift
done
# setup the gdbserver
$SCRIPT_DIR/android_gdbserver ${gdbVars[@]}

https://codereview.chromium.org/19920003/diff/1/platform_tools/android/bin/an...
platform_tools/android/bin/android_gdb_exe:33: APP_NAME=$(basename $3)
replace $3 with ${gdbVars[0]}

https://codereview.chromium.org/19920003/diff/1/platform_tools/android/bin/an...
File platform_tools/android/bin/android_gdbserver (right):

https://codereview.chromium.org/19920003/diff/1/platform_tools/android/bin/an...
platform_tools/android/bin/android_gdbserver:14: fi
you should be able to remove this code altogether.

[yunchao, 2013-07-30 07:07:29]

On 2013/07/22 13:18:18, djsollen wrote:
>
https://codereview.chromium.org/19920003/diff/1/platform_tools/android/bin/an...
> File platform_tools/android/bin/android_gdb_exe (right):
> 
>
https://codereview.chromium.org/19920003/diff/1/platform_tools/android/bin/an...
> platform_tools/android/bin/android_gdb_exe:18: 
> It would be preferable to do something like...
> 
> while (( "$#" )); do
> 
>   if [[ $(echo "$1" | grep "^-d$") != "" ]];
>   then 
>     deviceID="$2"
>     shift
>   else
>     gdbVars=("${gdbVars[@]}" "$1")
>   fi
> 
> shift
> done
> # setup the gdbserver
> $SCRIPT_DIR/android_gdbserver ${gdbVars[@]}
> 
>
https://codereview.chromium.org/19920003/diff/1/platform_tools/android/bin/an...
> platform_tools/android/bin/android_gdb_exe:33: APP_NAME=$(basename $3)
> replace $3 with ${gdbVars[0]}
> 
>
https://codereview.chromium.org/19920003/diff/1/platform_tools/android/bin/an...
> File platform_tools/android/bin/android_gdbserver (right):
> 
>
https://codereview.chromium.org/19920003/diff/1/platform_tools/android/bin/an...
> platform_tools/android/bin/android_gdbserver:14: fi
> you should be able to remove this code altogether.

djsollen, thanks for your suggestion. I have revised my patch accordingly and
test it on both x86 phone and arm phone, please help to review it.

[djsollen, 2013-07-30 12:37:50]

https://codereview.chromium.org/19920003/diff/6001/platform_tools/android/bin...
File platform_tools/android/bin/android_gdb_exe (right):

https://codereview.chromium.org/19920003/diff/6001/platform_tools/android/bin...
platform_tools/android/bin/android_gdb_exe:28: source
$SCRIPT_DIR/android_gdbserver ${gdbVars[@]}
I would not source it.  Doing so could cause us to accidentally overwrite
variables that we didn't intend to and doesn't offer any other benefit that I
can see.

[yunchao, 2013-07-31 02:11:53]

https://codereview.chromium.org/19920003/diff/6001/platform_tools/android/bin...
File platform_tools/android/bin/android_gdb_exe (right):

https://codereview.chromium.org/19920003/diff/6001/platform_tools/android/bin...
platform_tools/android/bin/android_gdb_exe:28: source
$SCRIPT_DIR/android_gdbserver ${gdbVars[@]}
On 2013/07/30 12:37:51, djsollen wrote:
> I would not source it.  Doing so could cause us to accidentally overwrite
> variables that we didn't intend to and doesn't offer any other benefit that I
> can see.

If we don't source it, ANDROID_ARCH env parameter above will not transfer into
android_gdbserver, when the later execute "source android_setup.sh", no
ANDROID_ARCH  transfer into android_setup.sh, so it would still think it is an
arm-based machine(although "-d x86" are added into command line parameter) and
use arm tool_chains to run gdbserver. 
Well, we can pass all command line parameters to android_gdbserver and analyze
it again if you still think we should not source it to pass the context from
android_gdb_exe.

[djsollen, 2013-07-31 13:45:06]

https://codereview.chromium.org/19920003/diff/6001/platform_tools/android/bin...
File platform_tools/android/bin/android_gdb_exe (right):

https://codereview.chromium.org/19920003/diff/6001/platform_tools/android/bin...
platform_tools/android/bin/android_gdb_exe:28: source
$SCRIPT_DIR/android_gdbserver ${gdbVars[@]}
I see what your saying, but you can also just export ANDROID_ARCH (on line 22)
and get that behavior without all the side effects of sourcing this file.

[yunchao, 2013-08-01 02:10:52]

https://codereview.chromium.org/19920003/diff/6001/platform_tools/android/bin...
File platform_tools/android/bin/android_gdb_exe (right):

https://codereview.chromium.org/19920003/diff/6001/platform_tools/android/bin...
platform_tools/android/bin/android_gdb_exe:28: source
$SCRIPT_DIR/android_gdbserver ${gdbVars[@]}
On 2013/07/31 13:45:06, djsollen wrote:
> I see what your saying, but you can also just export ANDROID_ARCH (on line 22)
> and get that behavior without all the side effects of sourcing this file.
Yes, very good suggestion, djsollen. I have update the patch accordingly. Also,
the same work is added into andorid_gdb to support remote debugging for x86
phone. please help to review it.

[djsollen, 2013-08-01 12:17:31]

lgtm if you make the one change.

https://codereview.chromium.org/19920003/diff/16001/platform_tools/android/bi...
File platform_tools/android/bin/android_gdb_exe (right):

https://codereview.chromium.org/19920003/diff/16001/platform_tools/android/bi...
platform_tools/android/bin/android_gdb_exe:13: gdbVars="$gdbVars $1"
this just keeps adding the args to a string to keep them in a bash array your
going to want to use...

gdbVars=("${gdbVars[@]}" "$1")

[yunchao, 2013-08-02 02:19:26]

djsollen, please review it. If it is OK, please help to land it. BTW, could you
add this(-d x86 to support remote debug for IA-based machine) into skia wiki in
the section of debugging on
andorid(https://sites.google.com/site/skiadocs/user-documentation/quick-start-guides/android#TOC-Debugging-on-Android)?
Thanks!

https://codereview.chromium.org/19920003/diff/16001/platform_tools/android/bi...
File platform_tools/android/bin/android_gdb_exe (right):

https://codereview.chromium.org/19920003/diff/16001/platform_tools/android/bi...
platform_tools/android/bin/android_gdb_exe:13: gdbVars="$gdbVars $1"
On 2013/08/01 12:17:31, djsollen wrote:
> this just keeps adding the args to a string to keep them in a bash array your
> going to want to use...
> 
> gdbVars=("${gdbVars[@]}" "$1")
It's OK.

[djsollen, 2013-08-02 11:47:32]

lgtm

[commit-bot: I haz the power, 2013-08-02 11:47:42]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/yunchao.he@intel.com/19920003/22001

[commit-bot: I haz the power, 2013-08-02 15:14:28]

Change committed as 10511