Skip to content

Conversation

@AppleMangoOrange
Copy link

@AppleMangoOrange AppleMangoOrange commented Jan 13, 2026

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 documented every RZ_API function and struct this PR changes. (No changes)
  • I've added tests that prove my changes are effective (required for changes to RZ_API).
  • I've updated the Rizin book with the relevant information (if needed).
  • I've used AI tools to generate fully or partially these code changes and I'm sure the changes are not copyrighted by somebody else.

Detailed description

Adds relocation conversion and patching for each relocation mapping for the architecture AVR.

Test plan

  • rz-bin -R test_avr
  • <test for patching>

Closing issues

Relates to #4699

@AppleMangoOrange AppleMangoOrange force-pushed the rizin-4699 branch 2 times, most recently from 0cf6efe to 9937a92 Compare January 13, 2026 11:53
@AppleMangoOrange
Copy link
Author

Should I continue making separate cases for the LDI_NEG relocations or use fall-through? It would look like this:

case R_AVR_HI8_LDI_NEG:
		val = -val;
		/* fall through */
case R_AVR_HI8_LDI:
		rz_buf_read_at(buf_patched, patch_addr, buf, nbytes);
		offset = (val >> 8) & 0xFF;
		opcode = rz_read_ble16(buf, big_endian) | rz_bits_spread(0xF0F, offset);
		rz_write_ble16(buf, opcode, big_endian);
		rz_buf_write_at(buf_patched, patch_addr, buf, nbytes);
		break;

@AppleMangoOrange AppleMangoOrange force-pushed the rizin-4699 branch 2 times, most recently from 5406411 to d69182b Compare January 13, 2026 13:47
@AppleMangoOrange AppleMangoOrange marked this pull request as ready for review January 13, 2026 13:48
@wargio
Copy link
Member

wargio commented Jan 13, 2026

it's missing tests. you can upload binaries in our bin repo (check the tests/README.md)

@wargio
Copy link
Member

wargio commented Jan 13, 2026

Should I continue making separate cases for the LDI_NEG relocations or use fall-through? It would look like this:

case R_AVR_HI8_LDI_NEG:
		val = -val;
		/* fall through */
case R_AVR_HI8_LDI:
		rz_buf_read_at(buf_patched, patch_addr, buf, nbytes);
		offset = (val >> 8) & 0xFF;
		opcode = rz_read_ble16(buf, big_endian) | rz_bits_spread(0xF0F, offset);
		rz_write_ble16(buf, opcode, big_endian);
		rz_buf_write_at(buf_patched, patch_addr, buf, nbytes);
		break;

fall-thru are ok for these cases, just mark them with a comment like you did (also /* fall-thru */ is ok)

@AppleMangoOrange
Copy link
Author

it's missing tests. you can upload binaries in our bin repo (check the tests/README.md)

Does the binary need to test every type of relocation? If so, wouldn't that become tedious?

@Rot127
Copy link
Member

Rot127 commented Jan 14, 2026

Does the binary need to test every type of relocation? If so, wouldn't that become tedious?

Yes please. Otherwise we can't confirm they work.
I'd recommend to search compiler projects for tests.

@Rot127 Rot127 added the Requirements not met The PR doesn't meet the minimum contribution requirements. See CONTRIBUTING.md for details. label Jan 14, 2026
@wargio
Copy link
Member

wargio commented Jan 15, 2026

it's missing tests. you can upload binaries in our bin repo (check the tests/README.md)

Does the binary need to test every type of relocation? If so, wouldn't that become tedious?

in theory yes, but i know it might be problematic, so lets say it should cover more than 50% of them. you can use the Report of codecov to check the coverage.

@AppleMangoOrange AppleMangoOrange force-pushed the rizin-4699 branch 4 times, most recently from fac2a8c to f62e706 Compare January 16, 2026 18:33
@AppleMangoOrange
Copy link
Author

I have made an object file with 28 (out of 36) types of relocations, but it won't be an "executable" binary. Is that fine?

Also, didn't know where to ask this but should I just put the .o and the assembly .s file both in rizin-testbins/elf/?

@wargio
Copy link
Member

wargio commented Jan 17, 2026

yes it is fine. maybe create a folder as rizin-testbins/elf/avr and call the files something like *_reloc.elf

@AppleMangoOrange
Copy link
Author

I assume I need to add a script to test the relocations with the binary, but I can't figure out where it should go...

@AppleMangoOrange
Copy link
Author

rizinorg/rizin-testbins#238 has been approved, can this we go ahead with this PR?

@AppleMangoOrange AppleMangoOrange marked this pull request as ready for review January 27, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ELF Requirements not met The PR doesn't meet the minimum contribution requirements. See CONTRIBUTING.md for details. RzBin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants