ubuntu-phone team mailing list archive

[Petr Bláha <pb@xxxxxxxx>]

Hi Alex,

thank you for your reply and for your time you have spent with my problem.


2013/4/28 Alexander Antimonov <alexander.antimonov.od.ua@xxxxxxxxx>

> Hi, Petr.
>
> There is, probably, nothing wrong with ANDROID_MEMORY_BARRIER.
> The code of android_atomic_acquire_store and android_atomic_release_load
> is not much different from CM (http://get.cm/?device=tenderloin).
> If the code was wrong, it would affect many Touchpad users.
>

This is right. During past few days I have checked as many as possible
sources to discover what wrong with atomic functions and decided same. If
there were any issue it must affect somebody before me.


> From the backtrace it's obvious the crash happened at the
> android_atomic_add function.
> The crash log you posted in the letter is not full, so I looked at
> http://pastebin.com/ir3Gdk1K.
> From that one I can tell, the crash is caused by ldrex instruction, ldrex
> r0, [r4].
> It is a load from memory and the address was wrong at that time, r4 ==
> 00000105.
>

Absolutely good observation. Changed my focus from atomic functions some
day ago. I have discovered that something went wrong in direct assembler
code. After this I have tried to detect if I haven't missed some compiler
switch to support LDREX and SDREX ( now I know they are exclusive load and
store functions Please keep in mind I have been using assembler code 25
years ago on Zilog 8bit cpu ;-) )


> The function prototype is the following:
> int32_t android_atomic_add(int32_t increment, volatile int32_t *ptr),
> passed ptr is wrong memory address.
>

Looking in source ./system/core/include/cutils/atomic-arm.h I can see

extern inline int32_t android_atomic_add(int32_t increment, volatile
int32_t *ptr)
{
    int32_t prev, tmp, status;
    android_memory_barrier();
    do {
        __asm__ __volatile__ ("ldrex %0, [%4]\n"
                              "add %1, %0, %5\n"
                              "strex %2, %1, [%4]"
                              : "=&r" (prev), "=&r" (tmp),
                                "=&r" (status), "+m" (*ptr)
                              : "r" (ptr), "Ir" (increment)
                              : "cc");
    } while (__builtin_expect(status != 0, 0));
    return prev;
}

Maybe this is a goog point to add some debug code. I expect that %0 stands
for 1st, %1 for 2nd parameter and so, or I am wrong? In that case will
first line of assembler code be ldrex r0, [r4] ... I need to investigate
how to add some debug function here. But this will not explain where from
this get bad pointer.



> Also, in the logcat in stack "section" there was
> android::RefBase::incStrong(void const*) call.
> It must be related to memory management by reference counting.
>

There are few debug options disabled ( in file
./frameworks/native/libs/utils/RefBase.cpp ) by default. Enabling debug
options leads in various logs with no detailed info about what happened.


>
> Let sum up.
> The problematic peace of software (almost obvious) is ubuntuappmanager.
> Seems like it is in development and may crash.
> Why it crashes is another question. Maybe ubuntuappmanager developers can
> say why it crashes most often?
> Just in case, re-check that software you build is up to date.
>

Yes I've checked it and have fresh sources and developer tools as well.

I am glad you can confirm that problematic part is ubuntuappmanager. I have
written few emails to this  mailinglist about my problems with this part of
code, but you are very first that can confirm my hypothesis.


> So, you should stop blame yourself and (maybe) post a bug report on
> ubuntuappmanager (platform-api?), if you didn't already.
> I suppose, those are the "child issues" of ubuntuappmanager and will be
> resolved with time.
>
> Cheers.
>
>

Regards Petr