-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
scripts/symbolize.py: try to resolve abort address to symbol/section[+offset] #1767
Conversation
jforissier
commented
Aug 24, 2017
•
edited
Loading
edited
5778064
to
38e47d7
Compare
Update: also print ELF section name [+offset] and added |
scripts/symbolize.py
Outdated
return '???' | ||
reladdr = '0x{:x}'.format(int(addr, 0) - int(offs, 0)) | ||
return '' | ||
return '0x{:x}'.format(int(addr, 0) - int(offs, 0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not super important and not really part of this PR, but I guess you always expect to get the numbers in hex? In that case the int(xyz, 0)
can be changed to int(xyz, 16)
which makes it a bit more clear what kind of string we are dealing with.
scripts/symbolize.py
Outdated
idx, name, size, vma, lma, offs, algn = line.split() | ||
except: | ||
continue; | ||
ivma = int(vma, 16) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub fooling me? Or is here an indent that isn't correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RIght, there's a tab. Will fix.
scripts/symbolize.py
Outdated
continue; | ||
ivma = int(vma, 16) | ||
isize = int(size, 16) | ||
if ivma == iaddr: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this check superfluous? Wouldn't you get the same in the next if check but with offset 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure but I'd rather have symbol
than symbol+0
in the tool output.
scripts/symbolize.py
Outdated
break | ||
prevsize = size | ||
prevname = name | ||
if ret is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplication of the same checkif ret is None
. I suggest to adda function like
def xstr(s):
if s is None:
return ''
return str(s)
and then wrap , for example on lines 220, 221 with this function call:
if sym:
ret += ' ' + xstr(sym)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll fix as above (initialize ret = ''
).
scripts/symbolize.py
Outdated
@@ -128,13 +137,101 @@ def resolve(self, addr): | |||
ret = '!!!' | |||
return ret | |||
|
|||
def symbol_plus_offset(self, addr): | |||
ret = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not to define ret = ''
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No particular reason. Will fix.
38e47d7
to
cc3f47c
Compare
Comments addressed. Sorry for the non-fast-forward push, I have inserted the |
|
|
When converting a hex string formatted as '0x<hex>', a value of 0 may be given for base and Python will automatically assume a base-16 literal. However, since we're always dealing with hex strings (with or without a 0x prefix), it is more convenient to specify base-16 everywhere. Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> Reviewed-by: Joakim Bech <joakim.bech@linaro.org> Reviewed-by: Igor Opaniuk <igor.opaniuk@linaro.org>
…+offset] Use nm and objdump to find the symbol and ELF section that match the address reported in the the abort line. This can help debug writes to read-only data or unaligned accesses to global data, for example. If the address can be resolved to a symbol plus some offset and/or a section plus some offset, the abort line is printed again just before the call stack with the symbolic information added. Not that the translation cannot be done immediately when the abort line is seen because at this point we don't know the architecture, and we don't have the load address of the TA. Here is an example (the line added by this patch is marked with >>): User TA data-abort at address 0x1314d0 (write permission fault) fsr 0x0000080f ttbr0 0x0e07a06a ttbr1 0x0e07406a cidr 0x1 cpu #0 cpsr 0x60000030 r0 0x00000001 r4 0x00102780 r8 0x00000000 r12 0xb736e358 r1 0x00102724 r5 0x00121e4f r9 0x00000000 sp 0x001026e0 r2 0x00000001 r6 0x001026dc r10 0x00000000 lr 0x00105cf1 r3 0x001314d0 r7 0x001026e0 r11 0x00000000 pc 0x00105790 Status of TA 5b9e0e40-2636-11e1-ad9e-0002a5d5c51b (0xe073b70) (active) arch: arm load address: 0x103000 ctx-idr: 1 stack: 0x100000 10240 region 0: va 0x100000 pa 0xe21e000 size 0x3000 region 1: va 0x103000 pa 0xe100000 size 0x2e000 region 2: va 0x131000 pa 0xe12e000 size 0xa000 region 3: va 0x13b000 pa 0xe138000 size 0xe6000 region 4: va 0 pa 0 size 0 region 5: va 0 pa 0 size 0 region 6: va 0 pa 0 size 0 region 7: va 0 pa 0 size 0 >> User TA data-abort at address 0x1314d0 const_val+4 .rodata+4452 (write permission fault) Call stack: 0x00105790 ta_entry_bad_mem_access at optee_test/ta/os_test/os_test.c:917 0x00105cf1 TA_InvokeCommandEntryPoint at optee_test/ta/os_test/ta_entry.c:101 0x00121e33 entry_invoke_command at optee_os/lib/libutee/arch/arm/user_ta_entry.c:207 0x00121e8f __utee_entry at optee_os/lib/libutee/arch/arm/user_ta_entry.c:235 The test TA does the following: const int const_val[3] = { 1, }; /* ... */ ((int *)const_val)[1] = 2; Suggested-by: Zeng Tao <prime.zeng@hisilicon.com> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> Reviewed-by: Joakim Bech <joakim.bech@linaro.org> Reviewed-by: Igor Opaniuk <igor.opaniuk@linaro.org>
cc3f47c
to
d9f57d7
Compare
Great Jerome! Somewhere in the future it would be nice to have it written down somewhere telling people that this actually exists and then some example of how to use it. It could be a blog post and/or adding it to our debug.md file. |
@jbech-linaro good idea(s). I'll definitely propose something to add to debug.md. |