Fix #17523 - Resolve internal symbols referenced from the PLT ##bin

[trufae]

• Mark this if you consider it ready to merge
• I've added tests (optional)
• I wrote some lines in the book (optional)

Description

[grgalex]

@trufae The build ('sys/install.sh') fails with:

In file included from /home/grg/radare2/libr/..//libr/bin/p/bin_cgc.c:4:
/home/grg/radare2/libr/..//libr/bin/p/bin_elf.inc.c: In function ‘imports’:
/home/grg/radare2/libr/..//libr/bin/p/bin_elf.inc.c:434:21: error: ‘import_symbol’ undeclared (first use in this function)
  434 |                 if (import_symbol->is_internal) {
      |                     ^~~~~~~~~~~~~
/home/grg/radare2/libr/..//libr/bin/p/bin_elf.inc.c:434:21: note: each undeclared identifier is reported only once for each function it appears in
gmake[4]: *** [/home/grg/radare2/libr/../global.mk:45: /home/grg/radare2/libr/..//libr/bin/p/bin_cgc.o] Error 1
gmake[4]: *** Waiting for unfinished jobs....
gmake[3]: *** [/home/grg/radare2/libr/..//libr/rules.mk:87: prelib-build] Error 2
gmake[2]: *** [Makefile:178: bin] Error 2

[grgalex]

I think the fix for the compile error is to replace import_symbol with is:

diff --git a/libr/bin/p/bin_elf.inc.c b/libr/bin/p/bin_elf.inc.c
index 6ec451a462..607a9777bf 100644
--- a/libr/bin/p/bin_elf.inc.c
+++ b/libr/bin/p/bin_elf.inc.c
@@ -431,7 +431,7 @@ static RList* imports(RBinFile *bf) {
 
        RBinElfSymbol *is;
        R_VEC_FOREACH (imports, is) {
-               if (import_symbol->is_internal) {
+               if (is->is_internal) {
                        continue;
                }
                RBinImport *ptr = R_NEW0 (RBinImport);

[trufae]

doesnt seems to break anything. but .. @grgalex it is improving anything? maybe we should have a test for this before merging :?

[grgalex]

@trufae I think there are still commits missing from this PR.

In the issue (#17523), GitHub shows multiple commits referencing the issue.

Some of them may be duplicates, while others implement a distinct part of the fix.

This PR only contains (the updated equivalent) of d98edd0.

All commits in the list from the issue are telling are part of the same story, namely:

1. dd62a56
2. b0dc879
3. 625dc4e
4. bbaf7db
5. c1d2f34
6. 53cab17
7. [PROBABLY NOT NEEDED] luc-tielen@0b4b671
8. d736136
9. dd62a56
10. f00bfbf
11. [THIS IS THE ONLY ONE WE INCLUDED IN THIS PR] d98edd0

Right now, with the commit that this PR has, we don't set the value of the is_internal field somehow, yet we check for its value, because the changes from the previous commits are missing (for example we set it in b0dc879).

So, we need to apply all those commits in order to get the final proposed change.

As it is right now, with the missing content, the PR can't fix the problem :(

[grgalex]

What do you think is the smoothest way to fix this?

(Btw, I think this happened because the PR branch was deleted and we forgot to merge, so now all we have of it are a bunch of lonely commits)

[trufae]

I would suggest you to create a PR with the patches you mention and squash/rebase them to make it work, testing them in local and waiting for the CI, adding more tests will be good too.

Which is the PR that wasn't merged? I guess the PR was closed because of conflicts and not clearly fixing the problem, but if you can fix #21117

[grgalex]

FOUND IT!

#21708

This is your original PR that was never merged, and whose branch was deleted.

It's a single commmit too!

So, in essence, the present PR (#23805) should have picked the commit from #21708 instead of the one it has now :)

I think it should be very straightforward for you to do this and force-push to this branch.