Defect #663
openSegfault in u8 arithmetics on x86 (works on x86_64)
0%
Description
Meanwhile I was able to track down the segfault to this statemement in glue_imp.c (attached):
glue_imp.c:145: stream->pos.offset += rc;
pos.offset is declared as unsigned long long.
The segfault only happens on x86; when I run the app on x86_64, no segfault happens.
When I change the type of pos.offset to int, then line 145 works (i.e. it segfaults somewhere else).
The generated IR looks correct:
loc "/home/me/Entwicklung/Modules/EiGen/ecc/libc/libc/glue_imp.c", 145, 1
mov ptr $0, ptr [$fp + 8]
mov ptr [$fp - 12], ptr $0 + 20
conv u8 $0, s4 [$fp - 36]
mov ptr $1, ptr [$fp - 12]
add u8 $1, u8 [$1], u8 $0
mov ptr $0, ptr [$fp - 12]
mov u8 [$0], u8 $1
I added all cod files plus the amd32linux.obf required to compile and run the app as an attachment. If you need the source code, the most recent commit is on https://github.com/rochus-keller/Eigen; the test.c application is in the ecs/libc subdirectory with the corresponding BUSY file.
Files
Updated by Florian Negele about 1 year ago
- Status changed from New to Confirmed
Thanks for reporting. The problem is the add instruction which uses the same register to store the result and load an operand from. This is basically the same bug as reported in and needs some time to be resolved.
Updated by Rochus Keller about 1 year ago
Ok, I see. Meanwhile I also tried arma32 and arma64; the latter works without a segfault; arma32 crashes like the x86 version, but unfortunately I don't know where, since GDB on ARM seems to not properly use the debug information. I will do more experiments.
Updated by Florian Negele about 1 year ago
It is the same issue. The 64-bit addition uses two registers on a 32-bit machine to represent the result. Unfortunately, the register for the lower 32 bits is also used to load the upper 32 bits after its modification.
Updated by Rochus Keller about 1 year ago
Can you recommend a work-around compatible with cdemitter for the time being?
Updated by Florian Negele about 1 year ago
The problem is the memory operand in this instruction:
add u8 $1, u8 [$1], u8 $0
You can turn the smart operand into a register using MakeRegister.
Updated by Florian Negele about 1 year ago
You might have to use Move instead to make sure the value is loaded from memory into a different register. This is only necessary if operand is a 64-bit integral memory operand.
Updated by Rochus Keller about 1 year ago
I now changed ND_ADD as follows which worked:
if( targets[target].architecture == Amd32 )
{
Smop tmp = e->Move(e->Convert(lhsT,lhs));
return e->Add(tmp,rhs);
}else
return e->Add(e->Convert(lhsT,lhs),rhs);
This allows me to continue debugging.
Updated by Rochus Keller about 1 year ago
Do I have to generalize the above work-around to all binary operations?
I just found this code which looks correct to me but also causes a segfault:
; source: hash ^= (unsigned char)s[i];
; 53: line hashmap.c:21:29
conv ptr $0, s4 [$fp - 12] ; $0 = i
add ptr $0, ptr [$fp + 8], ptr $0 ; $0 = s + i
conv u1 $0, s1 [$0] ; $0 = (unsigned char)*(s+i)
conv u8 $0, u1 $0 ; ; $0 = (uint64_t)*(s+i)
mov ptr $1, ptr $fp - 8 ; $1 = &hash
xor u8 $1, u8 [$1], u8 $0 ; $1 = hash xor *(s+i)
mov u8 [$fp - 8], u8 $1 ; hash = hash xor *(s+i)
It no longer segfaults if I apply the same work-around to ExclusiveOr, which modifies the xor line like
mov u8 $2, u8 [$1]
xor u8 $2, u8 $2, u8 $0
Is it correct to generalize my work-around to something like
if( target_data[target.target].architecture == Amd32 && lhs2.type.size == 8 &&
( lhs.type.model == Code::Type::Signed || lhs.type.model == Code::Type::Unsigned ) &&
lhs.model == Code::Operand::Memory )
lhs = e->Move(lhs2);
?
Updated by Florian Negele about 1 year ago
Yes, this should cover it. I am working on a proper solution. Sorry for the inconveniences.