Reports a function with a mismatching data symbol as matching

[tomsons26]

Seems it doing a diff based on address only discarding the symbol mismatch?

In this case this is an old symbol and the ref to the data itself is correct, but i would think this should report as a mismatch.
Name or address, Data value, Name or address, data value show all the same.

[LagoLunatic]

I'm not familiar with this compiler, is that a compiler-generated literal like a float or something?

In #381 I intentionally changed it so that compiler-generated literals don't show mismatches just because the names/addresses are different (because the names don't mean anything if they're compiler-generated). Since you say the data itself is correct, wouldn't this be desired behavior? Why should it show a mismatch here?

[LagoLunatic]

As far as I can tell it doesn't look like that name matches the patterns that objdiff currently considers to be compiler-generated (doesn't start with @ or _$E). So I'm not sure why it wouldn't be showing a diff for the name. Can you upload the objects?

[tomsons26]

It's an explicit static int in a function, changing } else if symbol_name_addend_matches || address_eq(left_reloc, right_reloc) { to just else if symbol_name_addend_matches makes it show as a mismatch so it's the fact that both symbols are at address 0.

What's the reasoning for diffing addresses?
TBH, this looks deeply flawed to me.
The compiler may place something at address X and i may mismap something in my target obj that happens to be in address 0, don't see any logic of tossing out a symbol mismatch to opt for a much more unreliable entry.

[tomsons26]

objdiffdbg_symissue.zip
data access in Blitter_From_Flags doesn't show as a mismatch due to both flag_3000 and z_read_flags being at address 0, but mismatch in RLEBlitter_From_Flags does as the addresses don't match

[LagoLunatic]

What's the reasoning for diffing addresses?
TBH, this looks deeply flawed to me.

It's because of cases like this, which happen in every function that uses data pooling:

This is 100% matching code, however DTK has incorrectly chosen the l_ship_offset symbol (at address 0) instead of ...rodata.0 pool (also at address 0) for the relocation. Normally objdiff would ignore this diff, but for the screenshot I have applied your suggested change of not ignoring name differences even if the address is the same.

(I think when "name or address" diffing was originally added years ago it was also needed for compiler literals, but nowadays objdiff handles literals much better and it's not needed for that anymore.)

@encounter Do you know if this is something that could be improved on DTK's end? I think this workaround in objdiff wouldn't be necessary anymore if DTK could detect data pools automatically, but I'm not familiar enough with DTK to know if this is fixable or not.

[LagoLunatic]

Alternative idea: Maybe objdiff could keep track of which symbols are pooling symbols (with a new flag named something like SymbolFlag::PoolingSymbol), and then only check address if one side of the comparison is a pool? That might help prevent false positives like the one in this issue.

[encounter]

I never did come up with a good way of representing data pools with dtk except for configuring them yourself (adding the data pool label, etc.). Definitely not generally possible to deduce automatically

[tomsons26]

Well, the option to diff could be split into checkboxes, Name, Address, Data, instead of a strange dropdown
Important that the checkboxes are remembered as checking them every time objdiff is started will get very annoying.
Actually, if checkboxes are added a forth is needed, "Relax externs" or smth like that, a change was made some time ago to treat extern vs symbol diff as name only, that should probably be an option then not a hardcoded hidden thing.