Skip to contentSkip to conversation

linux-user: Fix fault address truncation AArch64

PATCH In review Amanieu d'Antras 3 participants 5 messages Oct 10, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 linux-user: Fix fault address truncation AArch64

On AArch64 the si_addr field of siginfo_t is truncated to 32 bits because the fault address passes through an uint32_t variable. This is fixed by changing the variable to uint64_t.

---
 linux-user/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
linux-user/main.c +1 -1
index 483eb3f..d63e093 100644
@@ -1006,7 +1006,7 @@ void cpu_loop(CPUARMState *env)
     CPUState *cs = CPU(arm_env_get_cpu(env));
     int trapnr, sig;
     target_siginfo_t info;
-    uint32_t addr;
+    uint64_t addr;
 
     for (;;) {
         cpu_exec_start(cs);
Signature · 1 line
2.1.2
Reviewed-by on v1 1/1
#1 lore raw
Quoted 23 lines · Amanieu d'Antras

On AArch64 the si_addr field of siginfo_t is truncated to 32 bits because the fault address passes through an uint32_t variable. This is fixed by changing the variable to uint64_t.

---
 linux-user/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
linux-user/main.c +1 -1
index 483eb3f..d63e093 100644
@@ -1006,7 +1006,7 @@ void cpu_loop(CPUARMState *env)
     CPUState *cs = CPU(arm_env_get_cpu(env));
     int trapnr, sig;
     target_siginfo_t info;
-    uint32_t addr;
+    uint64_t addr;
 
     for (;;) {
         cpu_exec_start(cs);
#2 lore raw
In reply to Amanieu d'Antras
Quoted 14 lines · Amanieu d'Antras

On AArch64 the si_addr field of siginfo_t is truncated to 32 bits because the fault address passes through an uint32_t variable. This is fixed by changing the variable to uint64_t.

---
 linux-user/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
linux-user/main.c +0 -0
index 483eb3f..d63e093 100644

linux-user/main.c void cpu_loop(CPUARMState *env) · line 1012

Quoted 8 lines
linux-user/main.c +1 -1
@@ -1006,7 +1006,7 @@ void cpu_loop(CPUARMState *env)
     CPUState *cs = CPU(arm_env_get_cpu(env));
     int trapnr, sig;
     target_siginfo_t info;
-    uint32_t addr;
+    uint64_t addr;
 
     for (;;) {
         cpu_exec_start(cs);

Thanks for catching this. Better to fix it by dropping the unnecessary local variable completely and just setting
info._sifields._sigfault._addr = env->exception.vaddress;
at the only point where we currently use 'addr', though, I think.

-- PMM

#3 lore raw
Quoted 30 lines · Peter Maydell
Quoted 14 lines · Amanieu d'Antras

On AArch64 the si_addr field of siginfo_t is truncated to 32 bits because the fault address passes through an uint32_t variable. This is fixed by changing the variable to uint64_t.

---
 linux-user/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
linux-user/main.c +0 -0
index 483eb3f..d63e093 100644

linux-user/main.c void cpu_loop(CPUARMState *env) · line 1012

Quoted 8 lines
linux-user/main.c +1 -1
@@ -1006,7 +1006,7 @@ void cpu_loop(CPUARMState *env)
     CPUState *cs = CPU(arm_env_get_cpu(env));
     int trapnr, sig;
     target_siginfo_t info;
-    uint32_t addr;
+    uint64_t addr;
 
     for (;;) {
         cpu_exec_start(cs);

Thanks for catching this. Better to fix it by dropping the unnecessary local variable completely and just setting
info._sifields._sigfault._addr = env->exception.vaddress;
at the only point where we currently use 'addr', though, I think.

Quoted 1 line

-- PMM

We do use the name code pattern in most of the functions of that file, meaning

uint32_t / uint64_t / target_ulong addr;

and then do_something_with_addr(addr);

Ciao,

Claudio

#4 lore raw
Quoted 7 lines · Claudio Fontana
Quoted 5 lines · Peter Maydell

Thanks for catching this. Better to fix it by dropping the unnecessary local variable completely and just setting
info._sifields._sigfault._addr = env->exception.vaddress;
at the only point where we currently use 'addr', though, I think.

Quoted 6 lines

We do use the name code pattern in most of the functions of that file, meaning

uint32_t / uint64_t / target_ulong addr;

and then do_something_with_addr(addr);

In other functions the variable is used more than once, mostly. (In cases where it is not I would also be happy with cleanup patches which removed it.)

-- PMM