1078383 - Update gdbserver

Status: RESOLVED FIXED

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[Gregor Wagner [:gwagner]]

Using ./run-gdb.sh currently fails with debug builds. We need to update gdbserver.

Comment 1

[Gregor Wagner [:gwagner]]

[Blocking Requested - why for this release]: We need gdb for 2.1

Comment 2

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: run-gdb-workaround.patch

Before we upgrade gdbserver, this patch to run-gdb.sh can work the problem around.

Comment 3

[Gregor Wagner [:gwagner]]

Thanks Cervantes.
Michael, can you take care of updating gdbserver?

Comment 4

[Michael Wu [:mwu]]

I recommend finding someone else.

Comment 5

[Gregor Wagner [:gwagner]]

Someone has to own this bug and update gdbserver. Jed, Dave, Cervantes, any takers?

Comment 6

[Cervantes Yu [:cyu] [:cervantes]]

I am interested in this.

Comment 7

[Cervantes Yu [:cyu] [:cervantes]]

So we need the fix included in gdbserver 7.7. gdbserver 7.7 and gdb 7.1 (that we currently use) doesn't work together. What's the best upgrade path to take?

1. Upgrade both gdb and gdbserver to 7.7, but NDK only has 7.6. We need to put the custom-built gdb/gdbserver to prebuilts.
2. Upgrade gdb to 7.6 from NDK, but build gdbserver ourselves. Currently I haven't seen major problems with this combination.
3. Backport the fix to 7.6 or 7.1. Backport to 7.6 should not be a problem, but I am not sure if it's possible to backport to 7.1.

Dave, what do you say?

Comment 8

[Dave Hylands [:dhylands]]

Step 2 sounds like the simplest. Changing which hash we use from prebuilts is possible, but I don't think we can actually change anything in prebuilts (we get it from uptream).

Step 3 also seems feasible (if we can backport in a way that only affects gdbserver). gdbserver is actually quite simple (compared to gdb).

So I think we'll need to add steps to get our updated gdbserver into /system without grabbing it from prebuilts. But I think that just winds up being some Makefile magic.

Getting a gdb/gdbserver that works is the fist step. I can help out with getting it installed.

Comment 9

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Dave Hylands [:dhylands] from comment #8)
> So I think we'll need to add steps to get our updated gdbserver into /system
> without grabbing it from prebuilts. But I think that just winds up being
> some Makefile magic.

In bug 985775, I tried and failed to find the makefile magic in question.  I eventually settled for installing a "gdbserver.new" and making run-gdb.sh detect it; that never landed because we worked around the problem instead.

Speaking of bug 985775, it would be good if we could get rid of the 7.1 gdbservers that don't support POSIX signal features that we're actively using.  It was also be good if we could get rid of the 6.x gdbservers that don't even work (e.g., emulator-x86-kk) and the completely nonexistent gdbservers (e.g., emulator-x86, at least the last time I tried to use it).

Comment 10

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: 0001-Bug-1078373-update-gdbserver-to-7.7.patch

The patch to update prebuilts/misc/android-arm/gdbserver binary to 7.7.

This is built from gdb-7.7 source from GNU using NDK r10b.

Comment 11

[Cervantes Yu [:cyu] [:cervantes]]

For flame-kk the update path is easier: gdb is already upgraded to 7.6, so we only need to upgrade gdbserver to 7.7. It looks like we need to fork prebuilts/misc from upstream before it has upgraded to 7.7.

Comment 12

[Cervantes Yu [:cyu] [:cervantes]]

Dave, what's your suggestion with prebuilts/misc and other configs than flame-kk?

Comment 13

[Dave Hylands [:dhylands]]

I think it depends on what's needed. I guess we need to figure out where else would the gdbserver binary from the flame-kk prebuilt tree work?

For the ones that don't work, we probably need to evaluate the effort required to backport (and maybe it isn't worth backporting anything - I guess it really depends on what platforms are actively being used and having issues.

Comment 14

[Andrew Overholt [:overholt]]

Developer blocking = 2.1+ (despite it not being a user-visible thing and me not being entirely clear why we can't just uplift).

Comment 15

[Cervantes Yu [:cyu] [:cervantes]]

Dave

Would you please clone https://github.com/CervantesYu/platform_prebuilts_misc_2 to mozilla-b2g? This contains 'gdbserver-7-7' branch for flame-kk. Then I will update the manifest for flame-kk.

For other JB-based devices we might need to use arm-linux-android-eabi-4.8, but only for its gdb-7.6 if we don't want to also upgrade the toolchain.

Comment 16

[Dave Hylands [:dhylands]]

I don't have permission to do that. Perhaps mwu does?

Comment 17

[Michael Wu [:mwu]]

Fork created at https://github.com/mozilla-b2g/platform_prebuilts_misc . Use b2g-4.4.2_r1 as the branch for kk aosp based devices. For kk caf based devices, I recommend attempting to upstream it first (ask :m1 for review) since it is a generally useful fix/upgrade for all users. If it's not accepted upstream, we can point our manifest at our fork.

Why is your gdbserver binary twice the size of the old binary? Do you need to strip it?

Comment 18

[Cervantes Yu [:cyu] [:cervantes]]

It's not stripped. I will strip it and send out the update.

Comment 19

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: GitHub Pull Request

Comment 20

[Michael Wu [:mwu]]

Comment on attachment 8509428 [details] [review]
GitHub Pull Request

The PR is on the wrong branch.

Comment 21

[Cervantes Yu [:cyu] [:cervantes]]

Michael, would you please open the target branch on mozilla-b2g/platform_prebuilts_misc? Thanks.

Comment 22

[Michael Wu [:mwu]]

I created the branch when I created the mirror. It's b2g-4.4.2_r1, as mentioned in comment 17.

Comment 23

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: GitHub Pull Request

Pull request to mozilla-b2g/platform_prebuilts_misc/b2g-4.4.2_r1.

Comment 24

[Michael Wu [:mwu]]

Did you want me to review that?

BTW, you'll also need to open a bug to mirror the new repo so emulator/nexus kk builds can pick it up. https://wiki.mozilla.org/ReleaseEngineering/RepositoryCreationRequest#Requester.27s_Actions:

Comment 25

[Cervantes Yu [:cyu] [:cervantes]]

(In reply to Michael Wu [:mwu] from comment #24)
> Did you want me to review that?
> 
Sorry for forgetting to set the review flag.

> BTW, you'll also need to open a bug to mirror the new repo so emulator/nexus
> kk builds can pick it up.
> https://wiki.mozilla.org/ReleaseEngineering/
> RepositoryCreationRequest#Requester.27s_Actions:

So we are using git.mozilla.org instead of the this one hosted on github.com/mozilla-b2g for the .repo manifests? I am getting confused.

Comment 26

[Michael Wu [:mwu]]

(In reply to Cervantes Yu from comment #25)
> > BTW, you'll also need to open a bug to mirror the new repo so emulator/nexus
> > kk builds can pick it up.
> > https://wiki.mozilla.org/ReleaseEngineering/
> > RepositoryCreationRequest#Requester.27s_Actions:
> 
> So we are using git.mozilla.org instead of the this one hosted on
> github.com/mozilla-b2g for the .repo manifests? I am getting confused.

The manifests are automatically rewritten to pull from git.mozilla.org on the build machines.

Comment 27

[Michael Wu [:mwu]]

The gdbserver binary works for me - thanks.

Can you update your commit message so it includes all the necessary details of how to reproduce the binary? Include the exact NDK used, additional packages (if any), and the commands you used to build.

Comment 28

[Cervantes Yu [:cyu] [:cervantes]]

I'll collect the build steps and update the commit messages.

Comment 29

[Cervantes Yu [:cyu] [:cervantes]]

How I build gdbserver (the hacky way)

With ndk-r10b under /opt/build/android-ndk-r10b/
and gdb-7.7 source under /opt/build/gdb-7.7/:

> cd /opt/build/gdb-7.7/gdb/gdbserver
> export CC="/opt/build/android-ndk-r10b/toolchains/arm-linux-androideabi-4.8/prebuilt/linux-x86_64/arm-linux-androideabi/bin/gcc --sysroot=/opt/build/android-ndk-r10b/platforms/android-19/arch-arm"
> ./configure --host=arm-linux-androideabi
> make
This fails on my machine for Elf64_auxv_t and Elf32_auxv_t not found:
linux-low.c:5227:15: error: 'Elf64_auxv_t' undeclared (first use in this function)
     ? sizeof (Elf64_auxv_t) : sizeof (Elf32_auxv_t);
               ^
I have to manually comment out
#define HAVE_ELF32_AUXV_T 1
#define HAVE_ELF64_AUXV_T 1
in config.h to make it build.
> /opt/build/android-ndk-r10b/toolchains/arm-linux-androideabi-4.8/prebuilt/linux-x86_64/arm-linux-androideabi/bin/strip gdbserver

The struct definition is in elf.h, there seems to be inconsistency between configure and build time. I need to find out how to build without this hacky modification.

Comment 30

[Michael Wu [:mwu]]

Wasn't there scripts in the ndk to rebuild parts of the ndk?

Comment 31

[Cervantes Yu [:cyu] [:cervantes]]

You mean make-standalone-toolchain.sh in NDK? Yes, but we don't really have to go through that step. According to https://developer.android.com/tools/sdk/ndk/index.html we just need to invoke gcc with correct --sysroot argument.

android-19 sysroot has the above error. I used the combination of ndk8/android14 and gdbserver builds without the above error.

Comment 32

[Michael Wu [:mwu]]

Ok. Can you update the commit message?

Comment 33

[Cervantes Yu [:cyu] [:cervantes]]

(In reply to Michael Wu [:mwu] from comment #32)
> Ok. Can you update the commit message?

The commit message is updated.

Comment 34

[Michael Wu [:mwu]]

We'll need manifest updates too to switch to this new repo and branch.

Comment 35

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Can we get this landed soon?

Comment 36

[Michael Wu [:mwu]]

Not without manifest updates and bug 1093597.

Comment 37

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: Pull request to merge manifest on master

Comment 38

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: Pull request to merge manifest on v2.1

Comment 39

[Michael Wu [:mwu]]

Comment on attachment 8522890 [details] [review]
Pull request to merge manifest on v2.1

There's a typo in this patch. Please test your manifest changes before submitting them.

Comment 40

[Michael Wu [:mwu]]

Comment on attachment 8522888 [details]
Pull request to merge manifest on master

>https://github.com/mozilla-b2g/b2g-manifest/pull/243/files

FWIW, bugzilla didn't think this was a pull request attachment. The extra /files at the end probably confused it.

Comment 41

[Cervantes Yu [:cyu] [:cervantes]]

Comment on attachment 8522890 [details] [review]
Pull request to merge manifest on v2.1

The typo is fixed.

Comment 42

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: CPU usage when the bug is reproduced

Comment 43

[Cervantes Yu [:cyu] [:cervantes]]

Comment on attachment 8523639 [details]
CPU usage when the bug is reproduced

Sorry Wrong bug

Comment 44

[Ken Chang[:kenkai]]

Can we land this patch? Or we are still waiting for other fixes.

Comment 45

[Carsten Book [:Tomcat]]

master: https://github.com/mozilla-b2g/b2g-manifest/commit/c7ab57c51f634cece213031e179d207876e0239a

Comment 46

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/platform_prebuilts_misc/commit/0e7c060db684b409616fe67ea433ef19f5634c60

Comment 47

[bhavana bajaj [:bajaj]]

NI :cyu to help seek approval on this given on master.

Comment 48

[Cervantes Yu [:cyu] [:cervantes]]

NI bhavana for approval since we don't have approval flags for gonk pull requests.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): https://sourceware.org/bugzilla/show_bug.cgi?id=15604 (it's a bug in gdbserver)
User impact if declined: No. It only affects developers debugging b2g using gdb.
Testing completed: Manually.
Risk to taking this patch (and alternatives if risky): No. It's invisible to end users.

Comment 49

[bhavana bajaj [:bajaj]]

(In reply to Cervantes Yu from comment #48)
> NI bhavana for approval since we don't have approval flags for gonk pull
> requests.
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #):
> https://sourceware.org/bugzilla/show_bug.cgi?id=15604 (it's a bug in
> gdbserver)
> User impact if declined: No. It only affects developers debugging b2g using
> gdb.
> Testing completed: Manually.
> Risk to taking this patch (and alternatives if risky): No. It's invisible to
> end users.
Thanks, please go ahead and land this with a=bajaj if you can, else NI :Tomcat for help checking-in.

Comment 50

[Kai-Zhen Li [:seinlin][:kli]]

https://github.com/mozilla-b2g/b2g-manifest/commit/b38229220fa1b5ef6a5a1024f387172ae6d290b3