Skip to content
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

Process noreturn functions also for relocations #799

Merged
merged 4 commits into from
Mar 15, 2021
Merged

Conversation

XVilka
Copy link
Member

@XVilka XVilka commented Mar 9, 2021

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the documentation and the rizin book with the relevant information (if needed)

Detailed description

There are some binaries that call noreturn functions e.g. exit only after processing relocations. Disassembly shows these functions just fine, but during the analysis noreturn propagation stage they are not processed properly.

Test plan

[i] ℤ rizin filetime.c-clang-x64-O0.o                                                                                                                                                                                             18:38:51 
Warning: run rizin with -e io.cache=true to fix relocations in disassembly
 -- Use scr.accel to browse the file faster!
[0x08000040]> aaa
[x] Analyze all flags starting with sym. and entry0 (aa)
[x] Analyze function calls (aac)
[x] Analyze len bytes of instructions for references (aar)
[x] Check for vtables
[x] Type matching analysis for all functions (aaft)
[x] Propagate noreturn information
[x] Use -AA or aaaa to perform additional experimental analysis.
[0x08000040]> s sym.showVersion 
[0x08000610]> pdf
            ; CALL XREF from sym.main @ 0x80000b0
╭ sym.showVersion ();
│           0x08000610      push  rbp
│           0x08000611      mov   rbp, rsp
│           0x08000614      mov   edi, 1
│           0x08000619      movabs rsi, 0                              ; RELOC 64 .rodata.str1.1 @ 0x08000636 + 0xd1
│           0x08000623      mov   eax, 0x11d                           ; 285
│           0x08000628      mov   edx, eax
│           0x0800062a      call  sym.Vwrite
│           0x0800062f      xor   edi, edi
╰           0x08000631      call  exit                                 ; RELOC 32 exit
[0x08000610]> tn
0x8000555
0x8000632
_Exit
__assert_fail
__cxa_throw
__libc_init
__libc_start_main
__stack_chk_fail
__uClibc_main
_exit
abort
err
errc
errx
exit

Before the patch it didn't stop at 0x08000631 call exit ; RELOC 32 exit function since the call target before the relocation is 0.

filetime.c-clang-x64-O0.o.zip

librz/core/canalysis.c Outdated Show resolved Hide resolved
librz/core/canalysis.c Outdated Show resolved Hide resolved
librz/core/canalysis.c Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 9, 2021

Codecov Report

Merging #799 (ccc0701) into dev (9c10bfb) will increase coverage by 0.03%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #799      +/-   ##
==========================================
+ Coverage   42.79%   42.82%   +0.03%     
==========================================
  Files         871      871              
  Lines      316903   316936      +33     
==========================================
+ Hits       135631   135741     +110     
+ Misses     181272   181195      -77     
Impacted Files Coverage Δ
librz/include/rz_analysis.h 100.00% <ø> (ø)
librz/core/canalysis.c 64.89% <91.11%> (+0.23%) ⬆️
librz/analysis/fcn.c 77.68% <93.33%> (+0.17%) ⬆️
librz/io/p/io_gdb.c 35.96% <0.00%> (-5.89%) ⬇️
shlr/gdb/src/packet.c 55.35% <0.00%> (-5.36%) ⬇️
librz/bp/bp_io.c 88.46% <0.00%> (-3.85%) ⬇️
shlr/gdb/src/gdbclient/xml.c 62.76% <0.00%> (-1.51%) ⬇️
librz/analysis/class.c 82.97% <0.00%> (-0.56%) ⬇️
librz/util/print_code.c 68.78% <0.00%> (-0.46%) ⬇️
librz/debug/debug.c 54.48% <0.00%> (-0.39%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f770fc...c286630. Read the comment docs.

librz/core/canalysis.c Outdated Show resolved Hide resolved
@XVilka XVilka marked this pull request as ready for review March 10, 2021 06:05
librz/analysis/fcn.c Outdated Show resolved Hide resolved
librz/core/canalysis.c Outdated Show resolved Hide resolved
@XVilka

This comment has been minimized.

@thestr4ng3r
Copy link
Member

Since you are iterating over all xrefs anyway, dict_xrefs should work too I guess. But I would still consider this a bug since if we allocate the memory for the type field anyway, we might as well set it to its actual value when creating an xref.

@XVilka XVilka force-pushed the noreturn-relocs branch 2 times, most recently from c1a23c1 to 1026a28 Compare March 12, 2021 05:16
@XVilka

This comment has been minimized.

@XVilka XVilka force-pushed the noreturn-relocs branch 3 times, most recently from 574de2f to 629f37b Compare March 12, 2021 07:25
@XVilka XVilka added this to the 0.2.0 milestone Mar 12, 2021
@thestr4ng3r thestr4ng3r enabled auto-merge (squash) March 15, 2021 12:27
@thestr4ng3r thestr4ng3r merged commit a39c8e7 into dev Mar 15, 2021
@thestr4ng3r thestr4ng3r deleted the noreturn-relocs branch March 15, 2021 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants