Add color support to no-name symbols #4823

[Sl4y3r-07]

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.
• 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
This minor change is done to resolve the issue.

if (value && rz_str_startswith(value, "unknown_")) {
			return ctx->pal.diff_unknown;
		}

...

...

[wargio]

As mentioned, maybe is worth checking all the bin plugins for these usecases in symbols and maybe is worth adding a configurable "prefix" variable to RzBin which can then be reused also here

@notxvilka what do you think?

[wargio]

probably is worth checking what the bin plugins do when these symbols are "empty" and create a function which normalizes this.

[wargio]

(ah, the changes are ok)

[notxvilka]

As mentioned, maybe is worth checking all the bin plugins for these usecases in symbols and maybe is worth adding a configurable "prefix" variable to RzBin which can then be reused also here

@notxvilka what do you think?

What if there is a symbol called unknown_17?
Configurable prefix sounds right, I think. Ideally though, we should have a better method to distinguish those rather than a simple string comparison.

[Sl4y3r-07]

What if there is a symbol called unknown_17?
Configurable prefix sounds right, I think. Ideally though, we should have a better method to distinguish those rather than a simple string comparison.

Just a suggestion, as in some debugger tools like IDA the symbols naming is done like sub_<address> loc_<address>
So, may be like this way we can have these symbols naming

And yes we can come up with a better method instead of just string comparison.

[wargio]

What if there is a symbol called unknown_17?
Configurable prefix sounds right, I think. Ideally though, we should have a better method to distinguish those rather than a simple string comparison.

Just a suggestion, as in some debugger tools like IDA the symbols naming is done like sub_<address> loc_<address> So, may be like this way we can have these symbols naming

And yes we can come up with a better method instead of just string comparison.

true, but if you check the address where that symbol is located, you will notice it might clash with others. the logic should be like: is it a valid vaddr? if not maybe just use an index value

[Sl4y3r-07]

true, but if you check the address where that symbol is located, you will notice it might clash with others. the logic should be like: is it a valid vaddr? if not maybe just use an index value

yeah, I will go through other bin plugins as well for the mentioned usecase and add a function.

[Sl4y3r-07]

Sorry for my inactivity due to some unavoidable circumstances.
I went through all the bin plugins code and checked how they are handling the symbol names when they are "empty".
Some plugins are not handling the "empty" cases as handled in the elf one. For e.g, in smd plugin

static void addsym(RzPVector /*<RzBinSymbol *>*/ *ret, const char *name, ut64 addr) {
	RzBinSymbol *ptr = RZ_NEW0(RzBinSymbol);
	if (!ptr) {
		return;
	}
	ptr->name = rz_str_dup(name ? name : "");
	ptr->paddr = ptr->vaddr = addr;
	ptr->size = 0;
	ptr->ordinal = 0;
	rz_pvector_push(ret, ptr);
}

For mach0 plugin,
ptr->name = rz_str_dup((char *)syms[i].name);

Similar is the case with other plugins too. Shall I proceed with handling the empty cases in these plugins as in elf plugin and then creating a function normalizing it?

[wargio]

create an RzBin function to normalize this and we could allow to have NULL names and since we iterate over them in the RzBin code, we can fix them there.

https://github.com/rizinorg/rizin/blob/dev/librz/bin/bobj_process_symbol.c#L101

[Sl4y3r-07]

@wargio I have made the changes, kindly review them. Also, in elf_relocs_conversion.c
I have removed that unknown_%d string formation from there, as normalising function is handling that, but because of this leak related some test has failed.

And, names are getting generated (as you suggested)

symbol->name = (symbol->vaddr && symbol->vaddr != UT64_MAX) ? rz_str_newf("unknown_0x%" PFMT64x, symbol->vaddr) : rz_str_newf("unknown_%d", symbol->ordinal);
		symbol->is_auto_generated = true;

[Sl4y3r-07]

@wargio according to your convenience, please review the changes.
If the changes are fine, I'll fix the broken testcases

[wargio]

this is not ok

[notxvilka]

@Sl4y3r-07 RzBin should not have dependency on RzCore

[Sl4y3r-07]

Yeah, I have fixed that. Thanks.

[notxvilka]

But the line is still here...

[Sl4y3r-07]

I meant I have fixed it locally, will push it with other changes.

[wargio]

i would prefer to use rz_str_startswith

[Sl4y3r-07]

I used this since we discussed for a better approach instead of direct string comparison. So it fetches the symbol and check whether is_auto_generated true. If rz_str_startswith is to be used, then initial change is good enough ig.

[Sl4y3r-07]

Should I continue with this change?

[wargio]

@notxvilka what do you think?

[notxvilka]

Yes, the idea is good, but I don't like that it searches symbol by name for every entry. Probably it's better to rethink logic of the flow a bit to reduce costly operations.

[notxvilka]

@Sl4y3r-07 RzBin should not have dependency on RzCore

[Sl4y3r-07]

Yes, the idea is good, but I don't like that it searches symbol by name for every entry. Probably it's better to rethink logic of the flow a bit to reduce costly operations.

Okay, I'll go through it and try to optimise it. Thanks.