Fluorohydride / ygopro-core

ygopro script engine.
MIT License
327 stars 134 forks source link

Unaligned access on smart phone #299

Open ytlDL opened 4 years ago

ytlDL commented 4 years ago

Build Environment

Android NDK r21 (armv7)

Description

**::Refresh** pushes 3 byte in qbuf with BufferIO::WriteInt8, which is later used by query_field_card, and causing SIGBUS pcard->get_infos since qbuf is casted to int* and it is not dword aligned. For example,


void SingleDuel::RefreshExtra(int player, int flag, int use_cache) {
    char query_buffer[0x2000]; // Stack : 0xcd0fbd28
    char* qbuf = query_buffer;
    BufferIO::WriteInt8(qbuf, MSG_UPDATE_DATA);
    BufferIO::WriteInt8(qbuf, player);
    BufferIO::WriteInt8(qbuf, LOCATION_EXTRA); // qbuf = 0xcd0fbd2b
    int len = query_field_card(pduel, player, LOCATION_EXTRA, flag, (unsigned char*)qbuf, use_cache); // <- Unalined acccess (int*)(0xcd0fbd2b) in this line
    NetServer::SendBufferToPlayer(players[player], STOC_GAME_MSG, query_buffer, len + 3);
}

Adding 1 to qbuf may be a workaround but it requires modification on code.

I am wondering how YGOMobile apps get this work?

mercury233 commented 4 years ago

actually YGOMobile is using r15b...

ytlDL commented 4 years ago

So it would work without any issue with r15b?

mercury233 commented 4 years ago

I didn't compile ygomobile recently, but I think it will work

DailyShana commented 4 years ago

try -fno-strict-aliasing

but in fact we should avoid such type punning: https://github.com/Fluorohydride/ygopro-core/blob/f39885a8dbfbfb7a6ca2503398445472836a94d4/card.cpp#L102

ytlDL commented 4 years ago

@mercury233 Thank for the reply, I will give it a try after I get older NDK to work.

@DailyShana It appears that -fno-strict-aliasing did not help; the problems still exist:


uint32 card::get_infos(byte* buf, int32 query_flag, int32 use_cache) {
        // Thumb mode
        // p = 0xcd1fbd23
    int32* p = (int32*)buf; 
    int32 tdata = 0;
        // p = 0xcd1fbd23 + 0x8 = 0xcd1fbd2b
    p += 2; 
         // SIGBUS (BUS_ADRALN)
        //  Unaligned write to 0xcd1fbd2b
    if(query_flag & QUERY_CODE) *p++ = data.code;
    if(query_flag & QUERY_POSITION) *p++ = get_info_location();
        // ...

Do you mean that lower 2 bits of buf being masked out automatically during int* cast by compiler?

DailyShana commented 4 years ago

Cast to a pointer of different type and then write data via this pointer is undefined behavior in most case. This is why this code doesn't always work on ARM. We should rewrite this code to avoid type punning.

ytlDL commented 4 years ago

Recent ARM architecture (after v6) should support unaligned access to my knowledge; it is just very inefficient. I do not know why SIGBUS in raised by a STR with unaligned operand. Maybe it is some problem with newer NDK or processor (I am using Sony Xperia Z5P for debugging, which have MSM8994 processors)

However, unaligned access should indeed be prevented in any cases.

ytlDL commented 4 years ago

Tried compile with NDK r15c, but still get SIGBUS.

It appears that android disables unaligned memory access by default, and there is no way to enable it in user mode, i.e., without modification (e.g., adding system call) on system kernel. I still have no idea how this code works on android variants of ygopro ......