This is an archive of the discontinued LLVM Phabricator instance.

[LLDB][LoongArch] Delete the s9 register alias definition
ClosedPublic

Authored by lh03061238 on Dec 23 2022, 1:39 AM.

Details

Summary

In RegisterInfos_loongarch64.h, r22 is defined twice. Having an extra array
member causes problems reading and writing registers defined after r22. So,
for r22, keep the alias fp, delete the s9 alias.

The PC register is incorrectly accessed when the step command is executed.
The step command behavior is incorrect.

This test reflects this problem:

loongson@linux:~$ cat test.c

 #include <stdio.h>

int func(int a) {
  return a + 1;
}

int main(int argc, char const *argv[]) {
  func(10);
  return 0;
}

loongson@linux:~$ clang -g test.c  -o test

Without this patch:

loongson@linux:~$ llvm-project/llvm/build/bin/lldb test
(lldb) target create "test"
Current executable set to '/home/loongson/test' (loongarch64).
(lldb) b main
Breakpoint 1: where = test`main + 40 at test.c:8:3, address = 0x0000000120000668
(lldb) r
Process 278049 launched: '/home/loongson/test' (loongarch64)
Process 278049 stopped
* thread #1, name = 'test', stop reason = breakpoint 1.1
    frame #0: 0x0000000120000668 test`main(argc=1, argv=0x00007fffffff72a8) at test.c:8:3
   5   	}
   6   	
   7   	int main(int argc, char const *argv[]) {
-> 8   	  func(10);
   9   	  return 0;
   10  	}
   11  	
(lldb) s
Process 278049 stopped
* thread #1, name = 'test', stop reason = step in
    frame #0: 0x0000000120000670 test`main(argc=1, argv=0x00007fffffff72a8) at test.c:9:3
   6   	
   7   	int main(int argc, char const *argv[]) {
   8   	  func(10);
-> 9   	  return 0;
   10  	}

With this patch:

loongson@linux:~$ llvm-project/llvm/build/bin/lldb test
(lldb) target create "test"
Current executable set to '/home/loongson/test' (loongarch64).
(lldb) b main
Breakpoint 1: where = test`main + 40 at test.c:8:3, address = 0x0000000120000668
(lldb) r
Process 278632 launched: '/home/loongson/test' (loongarch64)
Process 278632 stopped
* thread #1, name = 'test', stop reason = breakpoint 1.1
    frame #0: 0x0000000120000668 test`main(argc=1, argv=0x00007fffffff72a8) at test.c:8:3
   5   	}
   6   	
   7   	int main(int argc, char const *argv[]) {
-> 8   	  func(10);
   9   	  return 0;
   10  	}
   11  	
(lldb) s
Process 278632 stopped
* thread #1, name = 'test', stop reason = step in
    frame #0: 0x0000000120000624 test`func(a=10) at test.c:4:10
   1   	 #include <stdio.h>
   2   	
   3   	int func(int a) {
-> 4   	  return a + 1;
   5   	}

Diff Detail

Event Timeline

lh03061238 created this revision.Dec 23 2022, 1:39 AM
lh03061238 requested review of this revision.Dec 23 2022, 1:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 23 2022, 1:39 AM
lh03061238 edited the summary of this revision. (Show Details)Dec 23 2022, 5:34 PM
This revision is now accepted and ready to land.Dec 29 2022, 4:42 AM
xen0n added a comment.Jan 12 2023, 4:12 AM

BTW do we have a way of fixing the bug while preserving the alias?

This comment was removed by lh03061238.

BTW do we have a way of fixing the bug while preserving the alias?

The register information defined in registerinfos_longarch64.h contains only one alias.
So far, no other way has been found, Therefore, We first fixed this bug by removing the
less used alias s9

lh03061238 added a comment.EditedJan 12 2023, 6:42 PM

BTW do we have a way of fixing the bug while preserving the alias?

The register information defined in registerinfos_longarch64.h contains only one alias.
So far, no other way has been found, Therefore, We first fixed this bug by removing the
less used alias s9

At this stage, we want to ensure that register number mappings are correct in all relevant register definition files.
So make this change to ensure that the register access is correct.
is this ok ? @xen0n

This revision was automatically updated to reflect the committed changes.